[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244

jmorse created this revision. jmorse added reviewers: rsmith, doug.gregor, majnemer. Herald added a subscriber: cfe-commits. Hi, This patch tries to fix a problem in clangs implementation of C++11's [basic.lookup.qual]p6 as demonstrated in PR12350, by: - Re-enabling looking in name-specifier

[PATCH] D47628: Detect an incompatible VLA pointer assignment

jmorse updated this revision to Diff 149688. jmorse added a comment. Avoid un-necessary call, use APInt::isSameValue rather than operator!= https://reviews.llvm.org/D47628 Files: lib/AST/ASTContext.cpp test/Sema/vla.c Index: test/Sema/vla.c

[PATCH] D47628: Detect an incompatible VLA pointer assignment

jmorse marked 2 inline comments as done. jmorse added inline comments. Comment at: lib/AST/ASTContext.cpp:8588 + Expr *E = VAT->getSizeExpr(); + if (E && VAT->getSizeExpr()->isIntegerConstantExpr(TheInt, *this)) +return std::make_pair(true, TheInt);

[PATCH] D47628: Detect an incompatible VLA pointer assignment

jmorse created this revision. jmorse added reviewers: eli.friedman, majnemer. Herald added a subscriber: cfe-commits. For pointer assignments of VLA types, Clang currently detects when array dimensions _lower_ than a variable dimension differ, and reports a warning. However it does not do the

[PATCH] D47628: Detect an incompatible VLA pointer assignment

This revision was automatically updated to reflect the committed changes. jmorse marked 2 inline comments as done. Closed by commit rL333989: Detect an incompatible VLA pointer assignment (authored by jmorse, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244

jmorse added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D48072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244

jmorse added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D48072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244

jmorse added a comment. ping Repository: rC Clang https://reviews.llvm.org/D48072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244

jmorse added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D48072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67723: [CodeView] Add option to disable inline line tables.

jmorse added a comment. Looks good, although I'm not familiar enough with frontend things to approve IMO. Using a function attribute seems fine and appropriate too -- although CU flags is another thing I'm unfamiliar with, so I can't really offer an opinion. Repository: rG LLVM Github

[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

jmorse added a comment. In D88363#2317241 , @Mordante wrote: > Can you explain the kind of issues you're having? At the shallowest level, our -O1 produces different IR and fails the test, which is more or less our problem; however my understanding is

[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

jmorse added a comment. Hi -- We (Sony) are running into a bit of difficulty with the test for this change, as it relies on the configuration of the -O1 optimisation pipeline. Would it be possible to reduce down to a frontend test, and then tests for whatever passes are to interpret the IR

[PATCH] D89204: Make likelihood lit test less brittle

jmorse accepted this revision. jmorse added a comment. This revision is now accepted and ready to land. This fixes things for us, and by limiting to one optimisation pass it's much less brittle for the future -- much appreciated! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83048: [LiveDebugValues] 3/4 Add Xclang and CodeGen options for using instr-ref variable locations

This revision was automatically updated to reflect the committed changes. Closed by commit rG121a49d839d7: [LiveDebugValues] Add switches for using instr-ref variable locations (authored by jmorse). Changed prior to commit: https://reviews.llvm.org/D83048?vs=286814=287655#toc Repository: rG

[PATCH] D83048: [LiveDebugValues] 3/4 Add Xclang and CodeGen options for using instr-ref variable locations

jmorse updated this revision to Diff 286814. jmorse added a comment. Herald added a subscriber: dang. Add a test for the driver -Xclang option. As far as I understand this, we're just checking that the switch is accepted by the driver, not that it does anything in particular (please correct me

[PATCH] D83048: [LiveDebugValues] 3/4 Add Xclang and CodeGen options for using instr-ref variable locations

jmorse created this revision. jmorse added reviewers: aprantl, vsk, probinson, Orlando, StephenTozer, TWeaver, djtodoro. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. jmorse added a parent revision: D83047: [LiveDebugValues] 2/4 Add

[PATCH] D83048: [LiveDebugValues] 3/4 Add Xclang and CodeGen options for using instr-ref variable locations

jmorse updated this revision to Diff 276040. jmorse added a comment. (Rebase, only affecting LiveDebugValues.cpp) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83048/new/ https://reviews.llvm.org/D83048 Files: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D100298: [Clang][Coroutine][DebugInfo] Follow-up: reduce a tests ordering requirements

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3c9bcf0e3549: [Clang][Coroutine][DebugInfo] Relax test ordering requirement (authored by jmorse). Changed prior to commit:

[PATCH] D82547: [Debugify] Expose original debug info preservation check as CC1 option

jmorse added a comment. FTR, the patch LGTM. I haven't really been following the development of debugify in detail so I have no feeling for whether this is good from a high level; if you're confident the mailing list discussion finished with this being the right direction, I'd say this is

[PATCH] D100298: [Clang][Coroutine][DebugInfo] Follow-up: reduce a tests ordering requirements

jmorse updated this revision to Diff 337793. jmorse added a comment. Right you are -- another thing I didn't clock was that this test was running all of the LLVM passes (generating many instances of the coroutine function). Latest revision disables those passes so that there are only

[PATCH] D97533: [Clang][Coroutine][DebugInfo] remove useless debug info for coroutine parameters

jmorse added a comment. NB, while I flagged that CHECK-NEXT added too much ordering, I forgot that CHECK still requires some ordering. It causes us some difficulties downstream; there's a follow-up patch here D100298 for consideration. Repository: rG LLVM

[PATCH] D100298: [Clang][Coroutine][DebugInfo] Follow-up: reduce a tests ordering requirements

jmorse created this revision. jmorse added a reviewer: dongAxis1944. Herald added subscribers: ChuanqiXu, lxfind. jmorse requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Hi, The test added in D97533 (and

[PATCH] D114631: [DebugInfo][InstrRef] Turn instruction referencing on by default for x86

jmorse added a comment. Hmmm. I thought this was quiet after landing -- and it turns out the reason why is there's still a sneek-circuit in clang that's turning instruction referencing off by default. Woe is me; should have done more end-to-end testing :|. On the upside, in the last few weeks

[PATCH] D114631: [DebugInfo][InstrRef] Turn instruction referencing on by default for x86

jmorse created this revision. jmorse added reviewers: StephenTozer, Orlando, TWeaver, djtodoro. Herald added subscribers: dang, pengfei, hiraditya. jmorse requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This patch makes

[PATCH] D114631: [DebugInfo][InstrRef] Turn instruction referencing on by default for x86

jmorse updated this revision to Diff 390003. jmorse added a comment. (Don't test for cc1 flag that's being deleted) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114631/new/ https://reviews.llvm.org/D114631 Files: clang/include/clang/Driver/Options.td

[PATCH] D114631: [DebugInfo][InstrRef] Turn instruction referencing on by default for x86

jmorse added inline comments. Comment at: llvm/lib/Target/X86/X86TargetMachine.cpp:29 #include "llvm/Analysis/TargetTransformInfo.h" +#include "llvm/CodeGen/CommandFlags.h" #include "llvm/CodeGen/ExecutionDomainFix.h" Orlando wrote: > nit: Is this change

[PATCH] D114631: [DebugInfo][InstrRef] Turn instruction referencing on by default for x86

jmorse added a comment. Cheers all, especially for the latest reviews; we'll now see whether there are extra edge cases I haven't caught! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114631/new/ https://reviews.llvm.org/D114631

[PATCH] D114631: [DebugInfo][InstrRef] Turn instruction referencing on by default for x86

This revision was automatically updated to reflect the committed changes. Closed by commit rG3c045070882f: [DebugInfo] Turn instruction referencing on by default for x86 (authored by jmorse). Changed prior to commit: https://reviews.llvm.org/D114631?vs=390003=390673#toc Repository: rG LLVM

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

jmorse accepted this revision. jmorse added a comment. In D106084#2887702 , @dblaikie wrote: > It'd be preferable not to split these two cases (current "limited" versus > "ctor" homing) - because they rely on the same assumption, that the whole >

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

jmorse added a comment. In D106084#2890541 , @probinson wrote: > @jmorse am I remembering correctly, that we require dllimport-style > annotations, so "limited" actually includes these types even if they aren't > constructed locally? I am vague on the

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

jmorse added a comment. This is going to be excellent for linux targets and similar, In D106084#2882970 , @probinson wrote: > + @jmorse who is better placed than I am to say whether this is what Sony > would prefer. Slightly trickier -- our debugger

[PATCH] D106084: [DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled

jmorse added a comment. David wrote: > think what I'm missing here: If -fno-standalone-debug is already in use/the > default and is causing missing types because parts of the program are bulit > without debug info, then I'm not sure what the rationale is for slicing > -fstandalone-debug into

[PATCH] D157613: [Clang][ExtendLifetimes][3/4] Add -fextend-lifetimes flag to Clang to extend the lifetime of local variables

jmorse added a comment. The code changes here look good, but it's time to be annoying about the tests -- most of them appear to run the optimisation pipelines, which is testing all of LLVM as well as clang in these tests. Possibly a simple fix to that is to put these in cross-project-tests.

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

jmorse added subscribers: StephenTozer, jmorse. jmorse added a comment. Hi, Just to note that we've been seeing LTO crashes as a result of rG3b449bd46a11a (now reverted on trunk), which @StephenTozer has kindly reduced down

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

jmorse added a comment. Hi, In D141381#4056661 , @fdeazeve wrote: > While SROA will not touch this: > > define @foo(ptr %arg) { > call void @llvm.dbg.declare(%arg, [...], metadata !DIExpression()) > > It completely destroys the debug information

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. Many thanks for all the feedback, In D146987#4263139 , @aeubanks wrote: > I'm seeing > > llc: ../../llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6214: void > llvm::SelectionDAGBuilder::visitIntrinsicCall(const

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. Seeing how this has been in and out quite a bit, I figure it's worth explaning my understanding of what's failing and why. Just so it doesn't look like we're needlessly fuzzing other peoples CI. Three kinds of failures so far: - The usual edge cases for things we hadn't

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. In D146987#4285275 , @srj wrote: > For the record, this also breaks (broke?) Halide: > > Assertion failed: (!(Elmt.getFragmentOrDefault() == > Next.getFragmentOrDefault())), function operator(), file >

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. In D146987#4287794 , @srj wrote: > (But more seriously, could we please revert all of this unless/until a fix is > imminent? Our testing is dead in the water at the moment.) Should have been reverted in rG0ba922f60046

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. Ah, you're right, it's actually the same assertion message that @srj reported which I totally skipped over. Currently I suspect that's due to different std::sort implementations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. Given our repeated back-and-forth, it's probably better to do some pre-testing: @MaskRay @paulkirth we've got a command line switch for enabling this, are there any fuchsia / chromium facilities for pull-request-builds that we'd be able to use to pre-check this; or if

[PATCH] D146987: [Assignment Tracking] Enable by default

jmorse added a comment. /me grumbles about all the world being a VAX, @mstorsjo I can't replicate the crash, but can replicate the valgrind jump-on-uninitialized-value with a small reproducer [0] that doesn't feature any debug-info, using `opt --passes=early-cse reduced.ll`. The trace I've