Hi Aaron, The additional tests tell me that every composite type is either Trivial or NonTrivial, which does not answer the question: Why do you need two flags? DIFlags are not a plentiful resource and we should be reluctant to use them up. Thanks, --paulr
From: Aaron Smith [mailto:aaron.sm...@microsoft.com] Sent: Wednesday, March 13, 2019 4:05 PM To: Robinson, Paul; dblai...@gmail.com; r...@google.com; apra...@apple.com; jdevliegh...@apple.com Cc: cfe-commits@lists.llvm.org Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial Hi Paul, There are additional tests here that may answer your questions, https://reviews.llvm.org/rGe8475f78e2634d5d348d7ad746efc1e6526e72f5 Aaron From: "paul.robin...@sony.com" <paul.robin...@sony.com> Date: Wednesday, March 13, 2019 at 12:49 PM To: Aaron Smith <aaron.sm...@microsoft.com>, "dblai...@gmail.com" <dblai...@gmail.com>, "r...@google.com" <r...@google.com>, "apra...@apple.com" <apra...@apple.com>, "jdevliegh...@apple.com" <jdevliegh...@apple.com> Cc: "cfe-commits@lists.llvm.org" <cfe-commits@lists.llvm.org> Subject: RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial Hi Aaron, I think I am less clever than David, and it's not clear what the difference is between the two flags. In the review, Zach asked the question but I did not see a straight answer. What do these flags actually mean? What is the third state, with both flags off, implying not Trivial and not NonTrivial? (At the very least it seems that the names could be improved.) Thanks, --paulr From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of Aaron Smith via cfe-commits Sent: Monday, March 04, 2019 6:22 PM To: David Blaikie; Reid Kleckner; Adrian Prantl; Jonas Devlieghere Cc: cfe-commits@lists.llvm.org Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial Hi David, I can take a look at adding another test. Please read the code review which answers your question. A new flag is required. Thanks. ________________________________ From: David Blaikie <dblai...@gmail.com> Sent: Tuesday, March 5, 2019 12:51:27 AM To: Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere Cc: cfe-commits@lists.llvm.org Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial Hi Aaron, I don't see any mention of this in D44406 - so it might have been good to have a separate review for this (or included this in the review of D44406, which I think is possible with the monorepo). Specifically - this change is missing test coverage (there should be a clang test that goes from C++ source code to LLVM IR & demonstrates the flag being emitted into the IR, etc). Also - what's the reason the non-triviality can't be implied by the absence of the trivial flag? (or the other way around) - the flags seem redundant with one another. On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: Author: asmith Date: Mon Feb 25 19:49:05 2019 New Revision: 354843 URL: http://llvm.org/viewvc/llvm-project?rev=354843&view=rev<https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D354843%26view%3Drev&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496912406&sdata=lSb5rDLYvfn3Fx25%2FISjPJ1z6sUyvNHnlPBIUFM22aQ%3D&reserved=0> Log: [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial This goes with https://reviews.llvm.org/D44406<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD44406&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496912406&sdata=mJz7kzeqyH%2B5mAld8TA%2BELQ4ouBkVyr2opJyICfEM60%3D&reserved=0> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843&r1=354842&r2=354843&view=diff<https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FCodeGen%2FCGDebugInfo.cpp%3Frev%3D354843%26r1%3D354842%26r2%3D354843%26view%3Ddiff&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496922411&sdata=Y%2BVdpDTdlUcyZzPpcs4TMB3DQT7eAK%2F5ASLmWM%2Fg73A%3D&reserved=0> ============================================================================== --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019 @@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea // Record if a C++ record is trivial type. if (CXXRD->isTrivial()) Flags |= llvm::DINode::FlagTrivial; + else + Flags |= llvm::DINode::FlagNonTrivial; } llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits&data=02%7C01%7Caaron.smith%40microsoft.com%7Ca81fbd6197e04e329a7708d6a7ecf4ce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636881033496932416&sdata=X%2B6QGG3ySr1r0lyyP7tx2rzxIGfl4giQllnoX8pG6kM%3D&reserved=0>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
