[PATCH] D15630: [WinEH] Update LCSSA to handle catchswitch with handlers inside and outside a loop

Andy Kaylor via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 18 12:00:22 PST 2015


andrew.w.kaylor added a comment.

In http://reviews.llvm.org/D15630#313904, @reames wrote:

> I'm a bit concerned by this change.  Specifically, I'm worried that various optimization passes have built in assumptions about what LCSSA means.  If an optimization pass expects to have to only update PHIs in loop exists, this change could lead to crashes or potentially silent miscompiles.  Have you reviewed the loop transform passes to understand the implications of the changed invariant?


David tells me that CodeMetrics::analyzeBasicBlock already disables loop optimizations involving live-out token type SSA values.

I just reviewed all the passes that are calling either LoopInfo::isLCSSAForm() or LoopInfo::isRecursivelyLCSSAForm(), and it does appear to me that they will be safe with this change.  Loop unroll and loop unswitch both call CodeMetrics::analyzeBasicBlock and I verified experimentally that the sort of code involved in this patch won't be unrolled.  LCIM is slightly more involved, but I believe that it won't move code involving tokens.  Finally, IndVarSimplify does exactly what you are suggesting (i.e. uses the existence of PHI nodes to determine whether a value is used on an exit path) but again I don't think there is a risk of it operating on token values.


Repository:
  rL LLVM

http://reviews.llvm.org/D15630





More information about the llvm-commits mailing list