[PATCH] D15679: [FIX] Schedule generation PR25879

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 03:07:30 PST 2015


jdoerfert added inline comments.

================
Comment at: lib/Analysis/ScopInfo.cpp:3472
@@ -3439,1 +3471,3 @@
+      LoopStack.push_back(L);
+    }
 
----------------
grosser wrote:
> This piece of code looks a little complex. At the very least it requires some in-source comments.
> 
> From my point of view, this code would possibly be easier to understand if we find a way to somehow (more explicitly) walk down the loop tree and just do the ReversePostOrderTraversal to process the statements within the loop bodies. We had such a tree walk already earlier, but it was removed when you generalized the schedule construction. Would adding back such a tree walk be difficult due to the generality of the CFGs we allow?
> 
> One of the reasons this might be difficult is if we aim to support irreducible control flow (http://llvm.org/PR25909). Due to LoopInfo working with natural loops which can not be formed from irregular control flow, we do not correctly model irregular control flow before and after this patch and I believe we should also not try to do so.  Do you have any intentions to support such control flow with your patch?
I could outline it and/or add a comment,  is that OK? I am unsure how your want to achieve a reverse post order traversal of all blocks while staying in loops first more explicitly. What we had used the Region::element_XXX() iterator. I was not aware of any guarantees on the traversal order when you use this iterator (it does not even have a comment). If __you know__ of any tree traversal that will
  1) traverse blocks in reverse post order
  2) traverse loops (even if they are not covered by a sub-region) first once the header was visisted.
we probably use it.

I am unsure why you talk about irreducible control flow now. The patches comes with an reducible CFG test case for which we generated a __plain wrong__ schedule. The irreducible case will be broken as long as LoopInfo does not offer information about a irreducible loop, there is nothing I can do about that in the ScopInfo anyway (except write an own loop info ...).




http://reviews.llvm.org/D15679





More information about the llvm-commits mailing list