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

[Eli Friedman via llvm-commits]

eli.friedman 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?
----------------
hfinkel wrote:
> 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.
Yes, I can do something like that. Not sure the second map is necessary: I think we just need the most recently seen unwinding call in the block?

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


http://reviews.llvm.org/D21007