[PATCH] D88725: [GVN LoadPRE] Extend the scope of optimization by using context to prove safety of speculation

[Serguei Katkov via Phabricator via llvm-commits]

skatkov updated this revision to Diff 295786.
skatkov added a comment.

test is precomitted.


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

https://reviews.llvm.org/D88725

Files:
  llvm/lib/Transforms/Scalar/GVN.cpp
  llvm/test/Transforms/GVN/loadpre-context.ll


Index: llvm/test/Transforms/GVN/loadpre-context.ll
===================================================================
--- llvm/test/Transforms/GVN/loadpre-context.ll
+++ llvm/test/Transforms/GVN/loadpre-context.ll
@@ -7,12 +7,15 @@
 ; CHECK-LABEL: @loadpre_critical_edge(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32* [[ARG:%.*]], null
-; CHECK-NEXT:    br i1 [[CMP]], label [[NULL_EXIT:%.*]], label [[HEADER:%.*]]
+; CHECK-NEXT:    br i1 [[CMP]], label [[NULL_EXIT:%.*]], label [[ENTRY_HEADER_CRIT_EDGE:%.*]]
+; CHECK:       entry.header_crit_edge:
+; CHECK-NEXT:    [[V_PRE:%.*]] = load i32, i32* [[ARG]], align 4
+; CHECK-NEXT:    br label [[HEADER:%.*]]
 ; CHECK:       header:
-; CHECK-NEXT:    [[IV:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[HEADER]] ]
+; CHECK-NEXT:    [[V:%.*]] = phi i32 [ [[V_PRE]], [[ENTRY_HEADER_CRIT_EDGE]] ], [ [[SUM:%.*]], [[HEADER]] ]
+; CHECK-NEXT:    [[IV:%.*]] = phi i32 [ 0, [[ENTRY_HEADER_CRIT_EDGE]] ], [ [[IV_NEXT:%.*]], [[HEADER]] ]
 ; CHECK-NEXT:    [[NEW_V:%.*]] = call i32 @foo(i32 [[IV]])
-; CHECK-NEXT:    [[V:%.*]] = load i32, i32* [[ARG]], align 4
-; CHECK-NEXT:    [[SUM:%.*]] = add i32 [[NEW_V]], [[V]]
+; CHECK-NEXT:    [[SUM]] = add i32 [[NEW_V]], [[V]]
 ; CHECK-NEXT:    store i32 [[SUM]], i32* [[ARG]], align 4
 ; CHECK-NEXT:    [[IV_NEXT]] = add i32 [[IV]], 1
 ; CHECK-NEXT:    [[COND:%.*]] = icmp eq i32 [[IV_NEXT]], [[N:%.*]]
@@ -50,12 +53,13 @@
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32* [[ARG:%.*]], null
 ; CHECK-NEXT:    br i1 [[CMP]], label [[NULL_EXIT:%.*]], label [[PREHEADER:%.*]]
 ; CHECK:       preheader:
+; CHECK-NEXT:    [[V_PRE:%.*]] = load i32, i32* [[ARG]], align 4
 ; CHECK-NEXT:    br label [[HEADER:%.*]]
 ; CHECK:       header:
+; CHECK-NEXT:    [[V:%.*]] = phi i32 [ [[V_PRE]], [[PREHEADER]] ], [ [[SUM:%.*]], [[HEADER]] ]
 ; CHECK-NEXT:    [[IV:%.*]] = phi i32 [ 0, [[PREHEADER]] ], [ [[IV_NEXT:%.*]], [[HEADER]] ]
 ; CHECK-NEXT:    [[NEW_V:%.*]] = call i32 @foo(i32 [[IV]])
-; CHECK-NEXT:    [[V:%.*]] = load i32, i32* [[ARG]], align 4
-; CHECK-NEXT:    [[SUM:%.*]] = add i32 [[NEW_V]], [[V]]
+; CHECK-NEXT:    [[SUM]] = add i32 [[NEW_V]], [[V]]
 ; CHECK-NEXT:    store i32 [[SUM]], i32* [[ARG]], align 4
 ; CHECK-NEXT:    [[IV_NEXT]] = add i32 [[IV]], 1
 ; CHECK-NEXT:    [[COND:%.*]] = icmp eq i32 [[IV_NEXT]], [[N:%.*]]
Index: llvm/lib/Transforms/Scalar/GVN.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/GVN.cpp
+++ llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1133,7 +1133,6 @@
   // backwards through predecessors if needed.
   BasicBlock *LoadBB = LI->getParent();
   BasicBlock *TmpBB = LoadBB;
-  bool IsSafeToSpeculativelyExecute = isSafeToSpeculativelyExecute(LI);
 
   // Check that there is no implicit control flow instructions above our load in
   // its block. If there is an instruction that doesn't always pass the
@@ -1150,8 +1149,8 @@
   // because if the index is out of bounds we should deoptimize rather than
   // access the array.
   // Check that there is no guard in this block above our instruction.
-  if (!IsSafeToSpeculativelyExecute && ICF->isDominatedByICFIFromSameBlock(LI))
-    return false;
+  bool NeedSafeToSpeculativelyExecute = ICF->isDominatedByICFIFromSameBlock(LI);
+
   while (TmpBB->getSinglePredecessor()) {
     TmpBB = TmpBB->getSinglePredecessor();
     if (TmpBB == LoadBB) // Infinite (unreachable) loop.
@@ -1168,8 +1167,8 @@
       return false;
 
     // Check that there is no implicit control flow in a block above.
-    if (!IsSafeToSpeculativelyExecute && ICF->hasICF(TmpBB))
-      return false;
+    NeedSafeToSpeculativelyExecute =
+        NeedSafeToSpeculativelyExecute || ICF->hasICF(TmpBB);
   }
 
   assert(TmpBB);
@@ -1241,6 +1240,17 @@
   if (NumUnavailablePreds != 1)
       return false;
 
+  // Now we know where we will insert load. We must ensure that it is safe
+  // to speculatively execute the load at that points.
+  if (NeedSafeToSpeculativelyExecute) {
+    if (CriticalEdgePred.size())
+      if (!isSafeToSpeculativelyExecute(LI, LoadBB->getFirstNonPHI(), DT))
+        return false;
+    for (auto &PL : PredLoads)
+      if (!isSafeToSpeculativelyExecute(LI, PL.first->getTerminator(), DT))
+        return false;
+  }
+
   // Split critical edges, and update the unavailable predecessors accordingly.
   for (BasicBlock *OrigPred : CriticalEdgePred) {
     BasicBlock *NewPred = splitCriticalEdges(OrigPred, LoadBB);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88725.295786.patch
Type: text/x-patch
Size: 4474 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201002/bbb0235b/attachment.bin>