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

Orlando added a comment. > Looks like this is causing a crash on current main: > https://github.com/llvm/llvm-project/issues/62838 That was fixed in D151326 . I think we should be okay to re-enable this now. The first assertion was a bit of an edge case; I had

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

Orlando added a comment. In D146987#4375881 , @hctim wrote: > Hey, found another error that occurs when building Android that looks to be > different from @maskray's revert. > ... Thanks for the report and the tiny reproducer! I've written D151795

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

hctim added a comment. Hey, found another error that looks to be different from @maskray's revert. $ bin/clang -c -O2 -g -ftrivial-auto-var-init=zero /tmp/repro.c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org

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

fhahn added a comment. Looks like this is causing a crash on current `main`: https://github.com/llvm/llvm-project/issues/62838 Please take a look and revert the commit if it requires longer to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146

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

Orlando added a comment. That should be fixed with D149959 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 ___ cfe-commits ma

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

Orlando added a comment. Thanks for the reproducer and report! I'll have a patch up for this shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 ___ cfe-co

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

kito-cheng added a comment. Got crash for RISC-V on top of trunk: [kitoc@hsinchu02 build]$ cat x.c typedef __rvv_uint32m4_t a; void b() { a c; } [kitoc@hsinchu02 build]$ bin/clang -target riscv64-elf x.c -O -g clang-14: /home/kitoc/llvm-workspace/llvm-project/llvm/lib/IR/DebugInfo.cpp:

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

Orlando added a comment. Great, thanks @paulkirth! I've just landed this again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 ___ cfe-commits mailing list cf

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

paulkirth added a comment. In D146987#4299353 , @Orlando wrote: > In D146987#4299081 , @paulkirth > wrote: > >> @jmorse Sorry it took me a bit to set up, but I ran an experiment in our CI >> that reverted the ch

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

Orlando added a comment. In D146987#4299081 , @paulkirth wrote: > @jmorse Sorry it took me a bit to set up, but I ran an experiment in our CI > that reverted the change that disabled this as default and tried it for x86. > It looks like this patch is wo

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

paulkirth added a comment. @jmorse Sorry it took me a bit to set up, but I ran an experiment in our CI that reverted the change that disabled this as default and tried it for x86. It looks like this patch is working OK for us now: https://ci.chromium.org/raw/build/logs.chromium.org/fuchsia/led/

[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 th

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

Orlando added a comment. Thanks everyone for your reports and help, it's really appreciated. In D146987#4286318 , @dmgreen wrote: > Hello, I also noticed this potentially causing problems for scalable vectors: > https://godbolt.org/z/qdr8P86aW > That pro

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

srj added a comment. In D146987#4287847 , @srj wrote: > In D146987#4287819 , @jmorse wrote: > >> Should have been reverted in rG0ba922f60046 >>

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

srj added a comment. In D146987#4287819 , @jmorse wrote: > Should have been reverted in rG0ba922f60046 > earlier > today, are you still experiencing the same behaviour with that patch

[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

srj added a comment. In D146987#4287137 , @jmorse wrote: > 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. ...`std::s

[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 https://reviews.llvm.org

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

mstorsjo added a comment. In D146987#4286797 , @jmorse wrote: > /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

[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 red

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

mstorsjo added a comment. I'm also running into issues due to this commit. Unfortunately, the issues don't seem to be entirely deterministic. They can be reproduced with https://martin.st/temp/python-preproc.c: $ clang -target i686-w64-mingw32 -c -g -O3 python-preproc.c Assertion failed: (!

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

dmgreen added a comment. Hello, I also noticed this potentially causing problems for scalable vectors: https://godbolt.org/z/qdr8P86aW That probably counts as one of the "edge cases for things we hadn't accounted for". Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

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

srj added a comment. >> That's a new one -- would you be able to give some context and a reproducer? >> Thanks for reporting! > > Yep -- as of LLVM commit 3c9083f6757cbaf6f8d6c601586d99a11faf642e > , Halide > is still broken.

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

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

[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 > AssignmentTrackingAnalysis.

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

srj added a comment. For the record, this also breaks (broke?) Halide: Assertion failed: (!(Elmt.getFragmentOrDefault() == Next.getFragmentOrDefault())), function operator(), file AssignmentTrackingAnalysis.cpp, line 2020. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[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

Orlando added a comment. In D146987#4281825 , @paulkirth wrote: > Here are reduced cases. I didn't bother bisecting flags, but the test case is > quite small > F27199288: clang-crashreports.zip > > struct a {}; >

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

paulkirth added a comment. Here are reduced cases. I didn't bother bisecting flags, but the test case is quite small F27199288: clang-crashreports.zip struct a {}; struct b { struct b *c; struct b *d; }; _Bool e; inline void g(struct b *p1)

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

MaskRay added a comment. In D146987#4281068 , @Orlando wrote: >> I think we may have an issue due to this. I'm basing that on the blamelist, >> but I'm still bisecting our build, which may take some time to confirm. Can >> you take a look, and revert if

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

Orlando added a comment. > I think we may have an issue due to this. I'm basing that on the blamelist, > but I'm still bisecting our build, which may take some time to confirm. Can > you take a look, and revert if it's not a quick fix? That looks related to assignment tracking to me. I'm unlike

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

paulkirth added a comment. Here are the reproducers, though I haven't reduced them yet. F27193433: clang-crashreports.zip Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D1469

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

paulkirth added a comment. I think we may have an issue due to this. I'm basing that on the blamelist, but I'm still bisecting our build, which may take some time to confirm. Can you take a look, and revert if it's not a quick fix? FAILED: obj/third_party/mesa/src/compiler/nir/nir.nir_opt_uni

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

Orlando added a comment. Thanks everyone for begin patient with this one. I've just pushed again after landing D148536 . I am hoping that's the last issue... I am watching the buildbots and chromium build page. Repository: rG LLVM Github Monorepo CHANGES SI

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

aeubanks added a comment. IR that passes the verifier generally shouldn't crash llvm, so an alternative would be to make a verifier check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 _

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

Orlando added a comment. In D146987#4264491 , @jmorse wrote: > ... > H, that feels like a legitimate IR input where the configuration of > debug-info / intrinsics is just unexpected -- we probably shouldn't assert in > that situation but gracefully

[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 CallInst

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

Orlando added a comment. In D146987#4263263 , @rupprecht wrote: > In D146987#4263048 , @MaskRay wrote: > >> The latest reland 3820e9a2b29a2e268319ed6635da0d59e18d736d >>

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

Orlando added a comment. In D146987#4263139 , @aeubanks wrote: > I'm seeing > > llc: ../../llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6214: void > llvm::SelectionDAGBuilder::visitIntrinsicCall(const CallInst &, unsigned > int): Assertion `A

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

Orlando added a comment. Thank you for the reports and revert, I will dig into it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 ___ cfe-commits mailing list

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

aeubanks added a comment. F27125556: a.tgz this is the original repro that crashed but maybe differently (you'll need to remove the plugin arguments since our clang has some custom plugins) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

rupprecht added a comment. In D146987#4263048 , @MaskRay wrote: > The latest reland 3820e9a2b29a2e268319ed6635da0d59e18d736d > still > caused some issues to our internal workloads. St

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

aeubanks added a comment. I'm seeing llc: ../../llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6214: void llvm::SelectionDAGBuilder::visitIntrinsicCall(const CallInst &, unsigned int): Assertion `AssignmentTrackingEnabled && "expected assignment tracking to be enabled"' failed.

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

MaskRay added a comment. The latest reland 3820e9a2b29a2e268319ed6635da0d59e18d736d still caused some issues to our internal workloads. Stay tuned... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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

Orlando added a comment. In D146987#4237124 , @haowei wrote: > This patch (which enables assignment tracking) and D147312 > breaks llvm runtime build for > `runtimes-armv7-unknown-linux-gnueabihf` > > ... > > Example o

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

Orlando added a comment. Thank you @uabelho for the reproducer- I've added a fix in D147435 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 _

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

uabelho added a comment. We've also seen crashes with this patch, e.g. clang -cc1 -triple x86_64-unknown-linux -emit-obj -debug-info-kind=constructor -O1 bbi-80938.c crashes with clang: ../include/llvm/ADT/IntervalMap.h:1187: bool llvm::IntervalMap >::overlaps(KeyT, KeyT) const [KeyT =

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

Orlando added a comment. Hi @haowei, thanks for the report and revert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146987/new/ https://reviews.llvm.org/D146987 ___ cfe-commits mailing list cfe-commits@

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

haowei added a comment. This patch (which enables assignment tracking) and D147312 breaks llvm runtime build for `runtimes-armv7-unknown-linux-gnueabihf` Error message: FAILED: libcxx/src/CMakeFiles/cxx_static.dir/charconv.cpp.o /b/s/w/ir/cache/goma/clie

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

Orlando added a comment. > The expected compile-time regression on ReleaseLTO-g is roughly 1.2% right, > in exchange for greater variable location coverage? The compile time tracker is currently reporting around 1.5% for an LTO build. However, it's disabled for LTO at the moment until that can

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaa32490bfe0b: [Assignment Tracking] Enable by default (authored by Orlando). Herald added a project: clang. Herald added a subscriber: cfe-commits.