[llvm] ec0f678 - [GuardWidening] Fix widening possibility check (#66064)

[via llvm-commits]

Author: Aleksandr Popov
Date: 2023-09-12T14:49:14+02:00
New Revision: ec0f67874406e7f9d85b0a5da674565296ad7698

URL: https://github.com/llvm/llvm-project/commit/ec0f67874406e7f9d85b0a5da674565296ad7698
DIFF: https://github.com/llvm/llvm-project/commit/ec0f67874406e7f9d85b0a5da674565296ad7698.diff

LOG: [GuardWidening] Fix widening possibility check (#66064)

In the 0e0ff8573de69286536e4f49098226eda0c4c7f5 was introduced
inconsistency between condition widening and checking if it's possible
to widen. We check the possibility to hoist checks parsed from the
condition, but hoist entire condition.

This patch returns testing that a condition can be hoisted rather than
the checks parsed from that condition.

Co-authored-by: Aleksander Popov <apopov at azul.com>

Added: 
    llvm/test/Transforms/GuardWidening/hoist-checks.ll

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/GuardWidening.cpp b/llvm/lib/Transforms/Scalar/GuardWidening.cpp
index 701cd1b90950d55..fb1db11e85a0b8f 100644
--- a/llvm/lib/Transforms/Scalar/GuardWidening.cpp
+++ b/llvm/lib/Transforms/Scalar/GuardWidening.cpp
@@ -184,6 +184,7 @@ class GuardWideningImpl {
   /// Compute the score for widening the condition in \p DominatedInstr
   /// into \p WideningPoint.
   WideningScore computeWideningScore(Instruction *DominatedInstr,
+                                     Instruction *ToWiden,
                                      Instruction *WideningPoint,
                                      SmallVectorImpl<Value *> &ChecksToHoist,
                                      SmallVectorImpl<Value *> &ChecksToWiden);
@@ -423,8 +424,8 @@ bool GuardWideningImpl::eliminateInstrViaWidening(
         continue;
       SmallVector<Value *> CandidateChecks;
       parseWidenableGuard(Candidate, CandidateChecks);
-      auto Score = computeWideningScore(Instr, WideningPoint, ChecksToHoist,
-                                        CandidateChecks);
+      auto Score = computeWideningScore(Instr, Candidate, WideningPoint,
+                                        ChecksToHoist, CandidateChecks);
       LLVM_DEBUG(dbgs() << "Score between " << *Instr << " and " << *Candidate
                         << " is " << scoreTypeToString(Score) << "\n");
       if (Score > BestScoreSoFar) {
@@ -456,8 +457,8 @@ bool GuardWideningImpl::eliminateInstrViaWidening(
 }
 
 GuardWideningImpl::WideningScore GuardWideningImpl::computeWideningScore(
-    Instruction *DominatedInstr, Instruction *WideningPoint,
-    SmallVectorImpl<Value *> &ChecksToHoist,
+    Instruction *DominatedInstr, Instruction *ToWiden,
+    Instruction *WideningPoint, SmallVectorImpl<Value *> &ChecksToHoist,
     SmallVectorImpl<Value *> &ChecksToWiden) {
   Loop *DominatedInstrLoop = LI.getLoopFor(DominatedInstr->getParent());
   Loop *DominatingGuardLoop = LI.getLoopFor(WideningPoint->getParent());
@@ -475,7 +476,10 @@ GuardWideningImpl::WideningScore GuardWideningImpl::computeWideningScore(
 
   if (!canBeHoistedTo(ChecksToHoist, WideningPoint))
     return WS_IllegalOrNegative;
-  if (!canBeHoistedTo(ChecksToWiden, WideningPoint))
+  // Further in the GuardWideningImpl::hoistChecks the entire condition might be
+  // widened, not the parsed list of checks. So we need to check the possibility
+  // of that condition hoisting.
+  if (!canBeHoistedTo(getCondition(ToWiden), WideningPoint))
     return WS_IllegalOrNegative;
 
   // If the guard was conditional executed, it may never be reached

diff  --git a/llvm/test/Transforms/GuardWidening/hoist-checks.ll b/llvm/test/Transforms/GuardWidening/hoist-checks.ll
new file mode 100644
index 000000000000000..7220b3d7a4a99cd
--- /dev/null
+++ b/llvm/test/Transforms/GuardWidening/hoist-checks.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=guard-widening < %s | FileCheck %s
+
+declare void @llvm.experimental.deoptimize.isVoid(...)
+declare i1 @llvm.experimental.widenable.condition()
+
+; In the current scheme we widen `br i1 %and1`, so we try to build
+; `and i1 poison, %and1` using `%call0` as insertion point.
+; That's not possible, so widening will not occur in the case.
+; TODO: Widen `%call0` in the test case, not a branch.
+define void @test() {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  bb0:
+; CHECK-NEXT:    [[CALL0:%.*]] = call i1 @llvm.experimental.widenable.condition()
+; CHECK-NEXT:    [[AND0:%.*]] = and i1 false, [[CALL0]]
+; CHECK-NEXT:    [[AND1:%.*]] = and i1 false, [[AND0]]
+; CHECK-NEXT:    br i1 [[AND1]], label [[BB1:%.*]], label [[DEOPT:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[CALL1:%.*]] = call i1 @llvm.experimental.widenable.condition()
+; CHECK-NEXT:    [[AND2:%.*]] = and i1 poison, [[CALL1]]
+; CHECK-NEXT:    br i1 [[AND2]], label [[UNREACH:%.*]], label [[DEOPT]]
+; CHECK:       unreach:
+; CHECK-NEXT:    unreachable
+; CHECK:       deopt:
+; CHECK-NEXT:    call void (...) @llvm.experimental.deoptimize.isVoid(i32 0) [ "deopt"(i32 0) ]
+; CHECK-NEXT:    ret void
+;
+bb0:
+  %call0 = call i1 @llvm.experimental.widenable.condition()
+  %and0 = and i1 false, %call0
+  %and1 = and i1 false, %and0
+  br i1 %and1, label %bb1, label %deopt
+
+bb1:
+  %call1 = call i1 @llvm.experimental.widenable.condition()
+  %and2 = and i1 poison, %call1
+  br i1 %and2, label %unreach, label %deopt
+
+unreach:
+  unreachable
+
+deopt:
+  call void (...) @llvm.experimental.deoptimize.isVoid(i32 0) [ "deopt"(i32 0) ]
+  ret void
+}