[PATCH] D82261: [ValueTracking, BasicAA] Don't simplify instructions

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/test/CodeGen/AMDGPU/promote-alloca-to-lds-select.ll:78
   ret void
 }
 
----------------
nikic wrote:
> Let me explain what is going on here: We have selects on undefs involved here, which means that instruction simplification will simplify to the first select operand. If a real condition is used, then that of course doesn't happen. Because of that, this test is not testing anything useful right now.
> 
> In fact, I think this indicates that the current usage of SimplifyInstruction inside GetUnderlyingObject may be subtly unsound. GetUnderlyingObject will declare that the first select operand is the underlying object, but other code is permitted to later simplify that select to the second operand instead. In this case optimizations may have been performed based on an incorrect assumption. (While undef can be true or false, it cannot be both at the same time.)
Makes sense.  This is called out in https://github.com/llvm/llvm-project/blob/0e3faab6f0fa00668f97747a6a4afa1bc5647ef9/llvm/include/llvm/Analysis/InstructionSimplify.h#L16 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82261