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

Fri Apr 21 05:27:57 PDT 2023

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 reduced around:

  ==609193== Conditional jump or move depends on uninitialised value(s)
  ==609193==    at 0x5653F8B: simplifyICmpInst(unsigned int, llvm::Value*, llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in /fast/fs/llvm-main/build/bin/opt)
  ==609193==    by 0x5654F22: simplifyICmpInst(unsigned int, llvm::Value*, llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in /fast/fs/llvm-main/build/bin/opt)
  ==609193==    by 0x566107B: simplifyInstructionWithOperands(llvm::Instruction*, llvm::ArrayRef<llvm::Value*>, llvm::SimplifyQuery const&, unsigned int) (in /fast/fs/llvm-main/build/bin/opt)
  ==609193==    by 0x566181E: llvm::simplifyInstruction(llvm::Instruction*, llvm::SimplifyQuery const&) (in /fast/fs/llvm-main/build/bin/opt)

I've been building rG61967bbc7d4e9 <https://reviews.llvm.org/rG61967bbc7d4e9f72fb1fa082fa2235b99e36b698> using ubuntu's packaged clang-9. I can't be completely certain, but seemingly a user of PatternMatch::BinaryOp_match doesn't check that the pattern matched before examining the pattern? It doesn't reproduce in a stage2 RelWithDebInfo build, which is highly suspicious and feels like it's my environment. Could you confirm it's definitely assignment-tracking at fault by using `-Xclang -fexperimental-assignment-tracking=forced` to enable and `-Xclang -fexperimental-assignment-tracking=disabled` to disable, which should control the behaviour if it's AT at fault.

@dmgreen Aaahh, yes, we hadn't considered SVE at all. I think there's a natural solution, which is to treat such stores as indicating a variable is memory-homed at that point, although determining the size could be troublesome. I'll leave that to @Orlando .

@srj I'm unable to reproduce that assertion on linux with rG61967bbc7d4e9 <https://reviews.llvm.org/rG61967bbc7d4e9f72fb1fa082fa2235b99e36b698>, however it looks like that assertion doesn't expect to have two identical objects compared with each other. Some light googling suggests that it's permissible for std::sort to do that, and some people complain about it, so I suppose in some environments that assertion is ill formed. We should be able to fix that with a small refinement.

Many thanks for reporting the problems all.

[0] https://gist.github.com/jmorse/55da51f56d9c756fa77b42e705bdc674

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146987/new/

https://reviews.llvm.org/D146987