[PATCH] D21007: DSE: Don't remove stores made live by a call which unwinds.
Eli Friedman via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 5 16:52:28 PDT 2016
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
More information about the llvm-commits
mailing list