[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:312
     switch (II->getIntrinsicID()) {
     default: llvm_unreachable("doesn't pass 'hasAnalyzableMemoryWrite' predicate");
     case Intrinsic::lifetime_end:
----------------
asbirlea wrote:
> The crash is hitting this unreachable for the intrinsic "llvm.memcpy.inline".
> Simple repro: opt -dse test1.ll
> ```
> define void @test(i8* noalias nocapture %0, i8* noalias nocapture readonly %1) {
>   tail call void @llvm.memcpy.inline(i8* align 1 %0, i8* align 1 %1, i64 3, i1 false)
>   ret void
> }
> 
> declare void @llvm.memcpy.inline(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64 immarg, i1 immarg)
> ```
Thanks! The problem was that MemorySSA-based DSE only relies on whether we can find read/write memory locations. For `memcpy.inline`, MemoryLocation knows how to extract the arguments, but the DSE functions where not updated to account for that.

In the legacy DSE, we silently failed to optimized/eliminate `memcpy.inline` calls, but with the MemorySSA version we got a crash highlighting that the code needs updating.

That should be fixed by a5ec99da6ea7


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163