[PATCH] D133307: [Assignment Tracking][14/*] Account for assignment tracking in instcombine

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando marked an inline comment as not done.
Orlando added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:168-169
   New->setUsedWithInAlloca(AI.isUsedWithInAlloca());
+  New->setMetadata(LLVMContext::MD_DIAssignID,
+                   AI.getMetadata(LLVMContext::MD_DIAssignID));
 
----------------
Orlando wrote:
> jmorse wrote:
> > Note that D133118 fiddles with this area, as the call below to replaceInstUsesWith doesn't update debug users. With D133118 landed, I think this code is fine.
> I'll double-check it looks right when rebasing before landing? I think it'd be okay either way (see related reply in test `alloca-bitcast.ll`).
The test that exercises this particular modification, `alloca-bitcast.ll`, doesn't work when updated to use opaque pointers because the bitcast is noop and gets removed. I spent a little time seeing if the typed pointer codepath could be removed, but unfortunately a bunch of tests that haven't migrated to opaque pointers yet hit it.

https://llvm.org/docs/OpaquePointers.html#version-support claims that in LLVM 15.0 "Typed pointers are still available, but only supported on a best-effort basis and may be untested", so I feel that deleting the test (rather than adding a typed pointer test) is reasonable. Especially given it's a one-liner and quite "obvious". Are you okay with that @jmorse?


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

https://reviews.llvm.org/D133307