[PATCH] D90328: Eliminates dead store of an exisiting value

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks for sharing the patch! Some initial comments inline. It would also be good to explain the approach in a comment and also in the patch description.

It would be interesting to know how often this triggers. For testing, you could try using a build of clang with the patch with https://www.llvm.org/docs/TestSuiteGuide.html

There's a `TEST_SUITE_COLLECT_STATS` which tells clang to collect statistics in the result.json file. The interesting one would probably be `dse.NumFastStores`



================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2385
+      PushMemUses(Def);
+      for (unsigned I = 0; I < WorkList.size(); I++) {
+        if (WorkList.size() >= MemorySSAScanLimit) {
----------------
Initially it would probably be best to start with just looking at the direct uses of the def and then go from there. One thing to watch out for is compile-time.


================
Comment at: llvm/test/Transforms/DeadStoreElimination/MSSA/pr16520.ll:1
+; ModuleID = 'pr16520.cc'
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
----------------
this test needs `RUN` & `CHECK` lines. You should be able to get some inspiration from other files in the directory.


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

https://reviews.llvm.org/D90328