[PATCH] D79391: [DSE] Remove noop stores in MSSA.

Thu May 28 10:23:47 PDT 2020

fhahn added a comment.

In D79391#2056342 <https://reviews.llvm.org/D79391#2056342>, @zoecarver wrote:

> Also, it looks like the test suit does not have any noop stores (either that or I implemented the `NumNoopStores` statistic incorrectly).

I think it may be that currently most of those stores are eliminated earlier by EarlyCSE (see earlier comment). Might be interesting to see what happens with MSSA disabled for EarlyCSE. (just to double check: MSSA based DSE is not enabled by default, so for your measurements you have to make sure it is turned on via `-mllvm -enable-dse-memoryssa=true` or by setting `EnableMemorySSA`)

I think it is almost ready, just a question about some of the test changes remaining.

================
Comment at: llvm/test/Transforms/DeadStoreElimination/MSSA/simple.ll:34
 ;
-  %g_value = load i32, i32* %g_addr, align 4
   store i32 -1, i32* @g, align 4
----------------
Is there a reason for this change in the source (and some of the ones below)? I think we should keep the existing tests as is and add additional cases as required.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79391