[PATCH] D24812: Lit C++11 Compatibility Patch #11

2017-02-02 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: rjmccall. probinson added a comment. +rjmccall as CodeGen owner. https://reviews.llvm.org/D24812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

2017-02-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. These are all Objective-C++ tests, and AFAIK we don't intend to change the default Objective-C++ dialect when we finally do change the default C++ dialect. So I think these tests do not need to be modified. https://reviews.llvm.org/D29739

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

2017-02-10 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D29739#673971, @rjmccall wrote: > In https://reviews.llvm.org/D29739#673933, @tigerleapgorge wrote: > > > Hi John, > > > > Here is the most recent discussion I can find on cfe-dev. > > “I'm guessing that Objective-C/C++ is kind of passe, so

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

2017-02-13 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D29739#674297, @rjmccall wrote: > In https://reviews.llvm.org/D29739#674288, @probinson wrote: > > > I really think Apple would need to step up here if the default > > Objective-C++ dialect is going to change. > > > I don't mind stepping up

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-02-15 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#675687, @chandlerc wrote: > In https://reviews.llvm.org/D28404#675616, @mehdi_amini wrote: > > > We're still waiting for @rsmith to comment whether it'd be better to `have > > a LangOpts flag that basically means "pragma clang

[PATCH] D28620: Guard __gnuc_va_list typedef

2017-01-23 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL292819: Guard __gnuc_va_list typedef. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D28620?vs=84151=85430#toc Repository: rL LLVM https://reviews.llvm.org/D28620

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

2016-12-13 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. As dblaikie said in email, probably better to make this X86-specific; if long-double varies by OS you can put in a specific triple. FTR we don't rely on Perl being available everywhere, anything that does this kind of scripty stuff uses Python.

[PATCH] D27641: DebugInfo: Added support for Checksum debug info feature (Clang part)

2016-12-13 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:350 + std::string Checksum; + SM.getChecksumMD5(SM.getFileID(Loc), Checksum); + rnk wrote: > We should only do this if `CGM.getCodeGenOpts().EmitCodeView`, or we will > regress compile

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

2016-12-13 Thread Paul Robinson via Phabricator via cfe-commits
probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D27597#621618, @dgross wrote: > So would a Python equivalent of the Perl be acceptable? I think this is an > academic question -- better to explicitly test

[PATCH] D27794: Make some diagnostic tests C++11 clean

2016-12-15 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: ABataev. probinson added a comment. +abataev for OpenMP. https://reviews.llvm.org/D27794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27794: Make some diagnostic tests C++11 clean

2016-12-14 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. probinson added a reviewer: rsmith. probinson added subscribers: cfe-commits, tigerleapgorge. Another half-dozen test revisions in the ongoing campaign to make things ready for C++11 as Clangs's default dialect. Most of these are straightforward, but I am not

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

2016-12-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson added subscribers: cfe-commits, probinson. probinson added a comment. Hi David! As this is a Clang patch, you should subscribe cfe-commits rather than llvm-commits. I've done that for you. See also inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3765 +

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

2016-12-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson accepted this revision. probinson added a reviewer: probinson. probinson added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/CodeGen/CGDebugInfo.cpp:3765 +InitExpr = +

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-10 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I guess I'm getting irritated because people are trying to tell me what optnone means. I know what it means; I spent probably a whole year pushing to get it adopted. Optnone means: When you are running optimizations, try not to optimize this part, if you can.

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Basically, I don't see why having clang always emit a real .o at -O0 would be a problem. I haven't gotten through the other-CFI documentation yet though. https://reviews.llvm.org/D28404 ___ cfe-commits mailing list

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640182, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > > > Also, that's not practicable: what if I have an LTO static library for > > which I don't have the source, now if I build my own file

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > I don't follow: IMO if I generate a module with optnone and pipe it to `opt > -O3` I expect no function IR to be touched. If it is not the case it is a bug. Your opinion and expectation are not

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640362, @probinson wrote: > In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > > > I don't follow: IMO if I generate a module with optnone and pipe it to `opt > > -O3` I expect no function IR to be touched. If it is not

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640588, @mehdi_amini wrote: > Actually, as mentioned before, I could be fine with making `O0` incompatible > with LTO, however security features like CFI (or other sort of whole-program > analyses/instrumentations) requires LTO.

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640170, @probinson wrote: > In my experience, modifying source Note that the source modification consists of adding `#pragma clang optimize off` to the top of the file. It is not a complicated thing.

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640284, @probinson wrote: > > > In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D28404#640170, @probinson wrote: > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > You just wrote above that " mixing -O0 and LTO " is wrong, *if* I were to > agree with you at some point, then I'd make it a hard error. Yes, I was not clear that I meant that `-O0 -flto` on the same

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#639887, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#639874, @probinson wrote: > > > Over the weekend I had a thought: Why is -O0 so special here? That is, > > after going to all this trouble to propagate -O0 to LTO,

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640178, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640170, @probinson wrote: > > > In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D28404#640046, @probinson wrote: > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640314, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640284, @probinson wrote: > > > Upfront, it seemed peculiar to handle only one optimization level. After > > more thought, the whole idea of mixing -O0 and LTO seems

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640090, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#640046, @probinson wrote: > > > "I don't care" doesn't seem like much of a principle. > > > Long version is: "There is no use-case, no users, so I don't have much >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#640682, @mehdi_amini wrote: > > I'm now thinking along the lines of a `-foptimize-off` flag (bikesheds > > welcome) which would set the default for the pragma to 'off'. How is that > > different than what you wanted for `-O0`? It

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-11 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. @rsmith could you say whether it seems reasonable to have a LangOpts flag that basically means "`pragma clang optimize off` is always in effect." I think it would make the other optnone-related logic simpler. It would not be the only sort-of-codegen-related flag in

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-10 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#641078, @chandlerc wrote: > For me, the arguments you're raising against -O0 and -flto don't hold up on > closer inspection: > > - O0 != optnone: correct. But this is only visible in LTO. And in LTO, Os != > optsize, and Oz !=

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-10 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#641557, @mehdi_amini wrote: > As I stand right now, there hasn't been any correction. > I still consider the fact that `optnone` wouldn't produce the "same" result > (modulo corner cases around `merging global variables` for

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-10 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#641606, @mehdi_amini wrote: > If we want to support `-O0 -flto` and `optnone` it the way to convey this to > the optimizer, I don't see the alternative. optsize != -Os (according to Chandler) minsize != -Oz (according to Chandler)

[PATCH] D28503: Documentation for the newly added x86 intrinsics.

2017-01-11 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: emmintrin.h:1607 +/// +/// This intrinsic corresponds to the VMOVSD / MOVSD instruction. +/// should this be VMOVQ/MOVQ instead? Repository: rL LLVM https://reviews.llvm.org/D28503

[PATCH] D28620: Guard __gnuc_va_list typedef

2017-01-12 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. probinson added a reviewer: yaron.keren. probinson added a subscriber: cfe-commits. https://reviews.llvm.org/D28620 Files: lib/Headers/stdarg.h test/Headers/stdarg-gnuc_va_list.c Index: test/Headers/stdarg-gnuc_va_list.c

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Maybe instead, pass a flag to enable setting optnone on everything when the driver sees `-O0 -flto`? The patch as-is obviously has a massive testing cost, and it's easy to imagine people being tripped up by this in the future. https://reviews.llvm.org/D28404

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#638217, @probinson wrote: > > > The patch as-is obviously has a massive testing cost, and it's easy to > > imagine people being tripped up by this in the future. > > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D28404#638350, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#638299, @probinson wrote: > > > In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D28404#638217, @probinson wrote: > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912 // OptimizeNone wins over OptimizeForSize and MinSize. F->removeFnAttr(llvm::Attribute::OptimizeForSize); F->removeFnAttr(llvm::Attribute::MinSize);

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:962 + ShouldAddOptNone &= !D->hasAttr(); + ShouldAddOptNone &= !D->hasAttr(); + ShouldAddOptNone &= !F->hasFnAttribute(llvm::Attribute::AlwaysInline); chandlerc wrote: > why is

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-06 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:896 + !CodeGenOpts.DisableO0ImplyOptNone && CodeGenOpts.OptimizationLevel == 0; + // We can't add optnone in the following cases, it won't pass the verifier + ShouldAddOptNone &=

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-01-09 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Over the weekend I had a thought: Why is -O0 so special here? That is, after going to all this trouble to propagate -O0 to LTO, how does this generalize to propagating -O1 or any other specific -O option? (Maybe this question would be better dealt with on the dev

[PATCH] D27994: Make two vtable tests tolerate C++11

2016-12-20 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. probinson added a reviewer: rjmccall. probinson added a subscriber: cfe-commits. In C++11, we don't emit vtables as eagerly as we do for C++03, so fiddle the tests to emit them when the test expects them. In the C++11 test cleanup project, we're commonly making

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

2016-12-20 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: rjmccall. probinson added a comment. +rjmccall as IR Gen owner. https://reviews.llvm.org/D27936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

2016-12-20 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290207: C++11 test cleanup: nonthrowing destructors (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27936?vs=81982=82157#toc Repository: rL LLVM

[PATCH] D27994: Make two vtable tests tolerate C++11

2016-12-20 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290205: Make two vtable tests tolerate C++11. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27994?vs=82126=82156#toc Repository: rL LLVM

[PATCH] D27956: Make CodeGenCXX/stack-reuse-miscompile.cpp tolerate C++11

2016-12-20 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290208: Make a test use a specific C++ dialect (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27956?vs=82020=82160#toc Repository: rL LLVM

[PATCH] D27794: Make some diagnostic tests C++11 clean

2016-12-21 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290262: Make some diagnostic tests C++11 clean. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27794?vs=81977=82248#toc Repository: rL LLVM

[PATCH] D27794: Make some diagnostic tests C++11 clean

2016-12-21 Thread Paul Robinson via Phabricator via cfe-commits
probinson marked an inline comment as done. probinson added a comment. FIXME added. https://reviews.llvm.org/D27794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27955: Make CodeGenCXX/arm-swiftcall.cpp tolerate C++11

2016-12-19 Thread Paul Robinson via Phabricator via cfe-commits
probinson updated this revision to Diff 82021. probinson added a comment. Force C++03 on this test, to make it insensitive to future changes in the default dialect. https://reviews.llvm.org/D27955 Files: test/CodeGenCXX/arm-swiftcall.cpp Index: test/CodeGenCXX/arm-swiftcall.cpp

[PATCH] D27955: Make CodeGenCXX/arm-swiftcall.cpp tolerate C++11

2016-12-19 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. probinson added a reviewer: rjmccall. probinson added a subscriber: cfe-commits. Herald added subscribers: rengolin, aemerson. The test conjures up and returns a temp which has a struct type, and the struct has some empty/padding bytes in the middle. In C++03

[PATCH] D27956: Make CodeGenCXX/stack-reuse-miscompile.cpp tolerate C++11

2016-12-19 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. probinson added a reviewer: lenykholodov. probinson added a subscriber: cfe-commits. In this test, the allocas for the temps come out in a different order depending on whether the dialect is C++03 or C++11. To avoid depending on the default dialect, I forced it

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

2016-12-19 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. probinson added a reviewer: rsmith. probinson added a subscriber: cfe-commits. If a dtor has no interesting members, then it ends up being nothrow, which affects the generated IR. Modify some tests to tolerate this difference between C++03 and C++11. In C++11, a

[PATCH] D27794: Make some diagnostic tests C++11 clean

2016-12-19 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: rnk. probinson updated this revision to Diff 81977. probinson added a comment. Remove the OpenMP tests from this review (committed in r290128). +rnk who added test/Parser/backtrack-off-by-one.cpp originally. https://reviews.llvm.org/D27794 Files:

[PATCH] D27955: Make CodeGenCXX/arm-swiftcall.cpp tolerate C++11

2016-12-19 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290145: Make another test insensitive to the default C++ dialect. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27955?vs=82021=82028#toc Repository: rL LLVM

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-07-11 Thread Paul Robinson via Phabricator via cfe-commits
probinson updated this revision to Diff 106063. probinson added a comment. Refresh to current TOT, and ping. Funny what you can find in a year-old to-do list https://reviews.llvm.org/D14358 Files: lib/CodeGen/CGDebugInfo.cpp test/CodeGenCXX/debug-info-template-fwd-param.cpp

[PATCH] D35583: Debug Info: Add a file: field to DIImportedEntity

2017-07-18 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Sweet. I don't have the chops to understand the Objective-C test but otherwise LGTM. Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1684 + HasFile ? getMDOrNull(Record[6]) : nullptr, + HasFile ?

[PATCH] D35715: Preserve typedef names in debug info for template type parameters

2017-07-20 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. If a template instantiation uses a typedef'd name as a template parameter, this is usually resolved to its underlying type when we generate the name of the instance in the debug info. PS4 prefers to see the original parameter as in the source. Define an option

[PATCH] D36411: Restore previous structure ABI for bitfields with 'packed' attribute for PS4 targets

2017-08-08 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. FTR, from a PS4 perspective this is all good, but we'd like somebody from outside our team to take a look. https://reviews.llvm.org/D36411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-22 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I mentioned this in the PR but I should have restated it here, sorry... Wolfgang authored this patch. He is away for a month so I volunteered to post this, in case it was okay for resolving the PR as the crash is present in the 5.0 branch. I do know Wolfgang was not

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-22 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. Make sure all temporary MD nodes have been replaced with uniqued or distinct nodes before we clone a function. Fixes PR33930. https://reviews.llvm.org/D37038 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h lib/CodeGen/CGVTables.cpp

[PATCH] D37604: Disable debuginfo-tests for non-native configurations

2017-09-08 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D37604#864187, @aprantl wrote: > This seems reasonable to me, thanks! > When you commit this, could you please double-check that the tests are still > running on the green dragon builders? I'll also keep an eye on them. I was able to

[PATCH] D37604: Disable debuginfo-tests for non-native configurations

2017-09-08 Thread Paul Robinson via Phabricator via cfe-commits
probinson closed this revision. probinson added a comment. r312803. Forgot to put the tag in the commit message https://reviews.llvm.org/D37604 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-29 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D37038#89, @aprantl wrote: > This may have gotten lost earlier: Would it be possible to instruct > CloneFunction to not clone any temporary MDNodes via one of the flags that > are passed to the ValueMapper? I did look at that, sorry

[PATCH] D37604: Disable debuginfo-tests for non-native configurations

2017-09-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision. This requires the host triple to match the default target triple, in order to run debuginfo-tests. It's possible this is too restrictive, but offhand it seems like a reasonable starting point. https://reviews.llvm.org/D37604 Files: lit.local.cfg Index:

[PATCH] D37604: Disable debuginfo-tests for non-native configurations

2017-09-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. The new file goes in the debuginfo-tests directory. It's a separate project so that's probably not obvious from the diff. https://reviews.llvm.org/D37604 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-09-26 Thread Paul Robinson via Phabricator via cfe-commits
probinson updated this revision to Diff 116711. probinson added a comment. Add a command-line flag to control emitting the template parameter children. Default to on for SCE debugger tuning. I am not super excited about my choice of option name or the help text; alternate suggestions welcome.

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-09-28 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL31: [DWARF] Allow forward declarations of a class template instantiation (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D14358?vs=116862=117030#toc Repository: rL

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-09-28 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I think I will go ahead and commit this; it doesn't change the status quo for CodeView and if something is warranted we can do that in a follow-up. https://reviews.llvm.org/D14358 ___ cfe-commits mailing list

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-09-27 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D14358#882445, @dblaikie wrote: > > I would prefer to eliminate the `` from the instance name as well, > > because our debugger reconstructs a name more to its liking from the > > parameter children. However, IIUC the name with params is

[PATCH] D35715: Preserve typedef names in debug info for template type parameters

2017-09-29 Thread Paul Robinson via Phabricator via cfe-commits
probinson abandoned this revision. probinson added a comment. Abandoning. This change is irrelevant to the SCE debugger, and while I believe it could be made more complete and better reflect the original source (which is what the DWARF spec says names should be), I do not have time to pursue

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-09-27 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D14358#881666, @aprantl wrote: > Does this have to be exposed through the driver or could this be a cc1 option > only? My thinking was to make it easier for LLDB to play with it and decide whether DWARF conformance on this point is a

[PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-09-27 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: rnk. probinson added a comment. +rnk for the CodeView question. Comment at: include/clang/Frontend/CodeGenOptions.def:222 ///< of inline stack frames without .dwo files. +CODEGENOPT(DebugFwdTemplateParams, 1, 0)

[PATCH] D36501: add flag to undo ABI change in r310401

2017-08-24 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Locally we have a couple different tactics for dealing with changes that we can't support. A more coherent approach would be great. For example we defined a new TargetCXXABI::Kind value that is mostly GenericItaniumABI except where it isn't. I personally did not do

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:2934 +ABICompatArg->render(Args, CmdArgs); + // Add runtime flag for PS4 when PGO or Coverage are enabled. ``` else if (getToolChain().getTriple().isPS4())

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:2934 +ABICompatArg->render(Args, CmdArgs); + // Add runtime flag for PS4 when PGO or Coverage are enabled. rsmith wrote: > probinson wrote: > > ``` > > else if

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: lib/CodeGen/CGVTables.cpp:157 + if (DebugInfo) +DebugInfo->replaceTemporaryNodes(); + aprantl wrote: > Have you looked at what it would take to only finalize the nodes reachable > via the function? > Otherwise —

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Sorry it took so long to wrap my head around this, but it has been about 20 years since I've had to accommodate an intentional ABI breakage, and that's actually what you want to do. Remembering that case, I have a model for this one, and I understand what you are

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-28 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Trying to understand the broader context here, I looked back through the list of revisions mentioned in PR33930 to see if that helped. When called on a method, CodeGenModule::EmitGlobalDefinition() calls CodeGenModule::EmitGlobalFunctionDefinition(), which in turn

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-28 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D37038#854722, @probinson wrote: > finalizeSubprogram() retrieves the variables from the subprogram and handles > them; what is it missing? For temp-md-nodes2.cpp, the assertion in mapTopLevelUniquedNode() trips on a DICompositeType for

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-28 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Dumping the whole module, the temporary DICompositeType node for CBdVfsImpl is used both as the scope node for the DISubprogram, and also as the baseType of a DIDerivedType which is a pointer type in the type list for the subprogram. Given that the DICompositeType is

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

2017-08-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I would prefer a diagnostic if PS4 && >3.2. Whether you map "latest" to 3.2 or diagnose that also, up to you, I'm okay either way. Repository: rL LLVM https://reviews.llvm.org/D36501 ___ cfe-commits mailing list

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-29 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D37038#89, @aprantl wrote: > This may have gotten lost earlier: Would it be possible to instruct > CloneFunction to not clone any temporary MDNodes via one of the flags that > are passed to the ValueMapper? I tried that. Basically,

[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning

2017-08-31 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I now think the tactic to pursue is making sure the containing DICompositeType(s) for the method have complete descriptions, and RAUW the temporary nodes, prior to calling CloneFunction. Actually wading around in the MDNodes, Types, and Decls to accomplish that

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Anytime the code between saveIP() and restoreIP() could set the current debug location, it needs to be saved/restored along with the insertion point. I have to say the problem is not obvious to me here, so maybe saveIP/restoreIP should be changed (or eliminated in

[PATCH] D39622: Fix type name generation in DWARF for template instantiations with enum types and template specializations

2017-12-13 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Philosophically, mangled names and DWARF information serve different purposes, and I don't think you will find one true solution where both of them can yield the same name that everyone will be happy with. Mangled names exist to provide unique and reproducible

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-11-01 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Have you tried the GDB suite yet? If it has no problems, and we can make LLDB happy, I'm okay with it. https://reviews.llvm.org/D39239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40948: Switch Clang's default C++ language target to C++14

2017-12-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: rjmccall. probinson added a comment. +rjmccall for the codegen bits. Comment at: clang/test/SemaCXX/new-array-size-conv.cpp:1 -// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s +// RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=gnu++98 %s

[PATCH] D40712: Add cc1 flag enabling the function stack size section that was added in r319430

2017-12-08 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. The title says "Add cc1 option..." which to me implies a "cc1 only" option. What you're actually doing is adding a driver option. Please update the title. Comment at: test/Driver/stack-size-section.c:2 +// RUN: %clang -target x86_64-unknown %s -S

[PATCH] D41044: Implementation of -fextend-lifetimes and -fextend-this-ptr to aid with debugging of optimized code

2017-12-11 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. I should note we've had at least one request to make this specifiable per-function, which would mean defining an attribute to control the emission of the fake-use intrinsics. Doing the feature that way would be consistent with 'optnone'.

[PATCH] D40948: Switch Clang's default C++ language target to C++14

2017-12-08 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733 // The PS4 uses C++11 as the default C++ standard. - if (T.isPS4()) -LangStd = LangStandard::lang_gnucxx11; - else -LangStd = LangStandard::lang_gnucxx98;

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-24 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Have you tried this change against the GDB and LLDB test suites? If they have failures then we should think about whether those tests are over-specific or whether we should put this under a tuning option. https://reviews.llvm.org/D39239

[PATCH] D46767: Force the PS4 clang ABI version to 6, and add a warning if this is attempted to be overridden

2018-05-14 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D46767#1096746, @rsmith wrote: > Everything old is new again. If only that were true about my brain. :-P > This was discussed when `-fclang-abi-compat` was introduced; see > https://reviews.llvm.org/D36501 for the argument why this patch

[PATCH] D46767: Force the PS4 clang ABI version to 6, and add a warning if this is attempted to be overridden

2018-05-11 Thread Paul Robinson via Phabricator via cfe-commits
probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D46767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

2018-05-21 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. This wouldn't be another layout/ABI breakage, would it? Repository: rC Clang https://reviews.llvm.org/D46439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. @trixirt do you mind if I commandeer this review? I think I have a patch. Repository: rC Clang https://reviews.llvm.org/D47260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson updated this revision to Diff 148663. probinson added a comment. Upload patch to suppress checksums when we see a preprocessed file. https://reviews.llvm.org/D47260 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h clang/test/CodeGen/md5-checksum-crash.c

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. probinson marked an inline comment as done. Closed by commit rL11: [DebugInfo] Dont bother with MD5 checksums of preprocessed files. (authored by probinson, committed by ). Herald added a subscriber: llvm-commits.

[PATCH] D47375: [Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. LGTM with the indicated test tweak, but best if @filcab also takes a look. Comment at: lib/Driver/ToolChains/PS4CPU.cpp:87 +CmdArgs.push_back("--dependent-lib=libSceDbgAddressSanitizer_stub_weak.a"); + } +} Don't bother with

[PATCH] D47291: Proposal to make rtti errors more generic.

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; filcab wrote: > I'd

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. probinson marked an inline comment as done. Closed by commit rC11: [DebugInfo] Dont bother with MD5 checksums of preprocessed files. (authored by probinson, committed by ). Changed prior to commit:

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson marked an inline comment as done. probinson added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:378 return None; + if (Entry.getFile().hasLineDirectives()) { +EmitFileChecksums = false; aprantl wrote: > Can you add a comment

  1   2   3   4   5   6   >