[llvm] r331061 - [GuardWidening] Add some clarifying comments about heuristics [NFC]

[Philip Reames via llvm-commits]

Author: reames
Date: Fri Apr 27 10:41:37 2018
New Revision: 331061

URL: http://llvm.org/viewvc/llvm-project?rev=331061&view=rev
Log:
[GuardWidening] Add some clarifying comments about heuristics [NFC]


Modified:
    llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp?rev=331061&r1=331060&r2=331061&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp Fri Apr 27 10:41:37 2018
@@ -335,6 +335,8 @@ GuardWideningImpl::WideningScore GuardWi
   bool HoistingOutOfLoop = false;
 
   if (DominatingGuardLoop != DominatedGuardLoop) {
+    // Be conservative and don't widen into a sibling loop.  TODO: If the
+    // sibling is colder, we should consider allowing this.
     if (DominatingGuardLoop &&
         !DominatingGuardLoop->contains(DominatedGuardLoop))
       return WS_IllegalOrNegative;
@@ -345,6 +347,12 @@ GuardWideningImpl::WideningScore GuardWi
   if (!isAvailableAt(DominatedGuard->getArgOperand(0), DominatingGuard))
     return WS_IllegalOrNegative;
 
+  // If the guard was conditional executed, it may never be reached
+  // dynamically.  There are two potential downsides to hoisting it out of the
+  // conditionally executed region: 1) we may spuriously deopt without need and
+  // 2) we have the extra cost of computing the guard condition in the common
+  // case.  At the moment, we really only consider the second in our heuristic
+  // here.  TODO: evaluate cost model for spurious deopt
   bool HoistingOutOfIf =
       !PDT.dominates(DominatedGuard->getParent(), DominatingGuard->getParent());