[PATCH] D69830: [LoopPred/WC] Use a dominating widenable condition to remove analyze loop exits

Evgeniy via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 12 04:37:30 PST 2019


ebrevnov added a comment.

In general the code looks fine to me. What is not clear how it is intended to work on real case scenarios. On the one hand we rely on WC having explicit form in the IR (ExitBB->getTerminatingDeoptimizeCall()) on the other hand we skip optimizing exits over WC since they are not analyzable. Are you going to introduce that support in the next patch or I'm missing something here?



================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:1025
+/// widenable branch *outside* the loop to allow us to simplify loop exits in a
+/// following loop.  This is closer is spirit to the IndVarSimplify transform
+/// of the same name, but is materially different widening loosens legality
----------------
is closer is->is closer in


================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:1083
+  bool Changed = false;
+  Value *MinECV = nullptr; //lazy generated if needed
+  for (BasicBlock *ExitingBB : ExitingBlocks) {
----------------
Ill formatted comment...not trivial to understand either...I would suggest to remove it....or improve...


================
Comment at: llvm/test/Transforms/LoopPredication/predicate-exits.ll:684
+
+; Non-latch exits can still be predicated
+define i32 @unconditional_latch(i32* %array, i32 %length, i1 %cond_0) {
----------------
I believe you meant this is something to handle in future, right?


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

https://reviews.llvm.org/D69830





More information about the llvm-commits mailing list