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

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 19 13:37:02 PDT 2020


asbirlea added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1943
+        // between them can modify the memory location.
+        auto *ClobberDef = MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(
+            Def->getDefiningAccess(), DefLoc);
----------------
One small note here: this query will not cache any result, because it's checking access to the DefLoc. It seems like the scenario of having: store value 0 + calloc as underlying object may be rare enough that it will not kick in often without benefit, but it's also possible to see a compile-time regression if the following pattern is found often.
```
calloc 
store val, loc
store 0, loc
```

In contrast, doing getClobbering(Def) will cache and the info could be used by this or subsequent passes.  Would it make sense to make this check here instead?


================
Comment at: llvm/test/Transforms/DeadStoreElimination/MSSA/calloc-store.ll:113
+  store i8 0, i8* %p.2, align 1
+  ret i8* %p
+}
----------------
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
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82204





More information about the llvm-commits mailing list