[PATCH] D21007: DSE: Don't remove stores made live by a call which unwinds.

[Hal Finkel via llvm-commits]

hfinkel added inline comments.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:860
@@ +859,3 @@
+      // the store.
+      // FIXME: Check whether a throwing call is actually between the stores, instead
+      // of just in the same block.  Maybe using OrderedBasicBlock?
----------------
Not clear that OrderedBasicBlock will help here directly. I think you really want two maps:

 1. Instruction* -> unsigned (for ordering)
 2. Instruction* -> Instruction* (or unsigned) (for the next call that might throw)

Why not just collect the maps above (where you currently set MayThrow), and then answer the question precisely? I think it is likely I'll see performance regressions from this if we over-approximate.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:867
@@ +866,3 @@
+          IsStoreDeadOnUnwind = isAllocLikeFn(Underlying, TLI) &&
+              !PointerMayBeCapturedBefore(Underlying, true, true, Inst, DT, false);
+        }
----------------
`Inst` here should really be the first call that might throw, right?


http://reviews.llvm.org/D21007