[PATCH] D82204: [DSE,MSSA] Treat `store 0` after calloc as noop stores.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: llvm/test/Transforms/DeadStoreElimination/MSSA/calloc-store.ll:113
+  store i8 0, i8* %p.2, align 1
+  ret i8* %p
+}
----------------
fhahn wrote:
> asbirlea wrote:
> > Negative test: add before ret and check it does not get eliminated:
> > ```
> > %p.3.2 = getelementptr i8, i8* %p, i32 3
> >  store i8 0, i8* %p.3.2, align 1
> > ```
> Done, I've added it as a separate test. Currently only the last store is left over, but as it is a store of 0, it could also be eliminated. We currently don't do that, because of the order in which we perform the eliminations. 
I was suggesting a test where the store would intentionally not be eliminated (negative test), and this seems correct below. There is a store of 5, then the store of 0 should not be eliminated because it would overwrite the 5.
Add a check that the store of value 5 is also there?

If you mean the store is the last before ret, and it should be eliminated because of *that*, then add another method call afterwards to defeat it. This way, we'll have a unit test to check that DSE works correctly on the particular feature added in this patch, the pattern:
```
calloc
store non-zero value
store zero is not eliminated (i.e. clobbering correctly find the non-zero store, not the calloc)
```

Does this make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82204