Skip to content

Commit

Permalink
[LVI] Disable at-use reasoning across phi nodes (PR60629)
Browse files Browse the repository at this point in the history
For phi nodes, while we can make use of the edge condition for the
incoming value, we shouldn't look past the phi node to look for
further conditions, because we might be reasoning about values
from two different cycle iterations (which will have the same
SSA value).

To handle this more specifically we would have to detect cycles,
and there doesn't seem to be any motivating case for that at this
point.
  • Loading branch information
d-makogon authored and tru committed Feb 21, 2023
1 parent b98d959 commit 7eac876
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 6 deletions.
13 changes: 7 additions & 6 deletions llvm/lib/Analysis/LazyValueInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1673,19 +1673,20 @@ ConstantRange LazyValueInfo::getConstantRangeAtUse(const Use &U,
// TODO: Use non-local query?
CondVal =
getEdgeValueLocal(V, PHI->getIncomingBlock(*CurrU), PHI->getParent());
} else if (!isSafeToSpeculativelyExecute(CurrI)) {
// Stop walking if we hit a non-speculatable instruction. Even if the
// result is only used under a specific condition, executing the
// instruction itself may cause side effects or UB already.
break;
}
if (CondVal && CondVal->isConstantRange())
CR = CR.intersectWith(CondVal->getConstantRange());

// Only follow one-use chain, to allow direct intersection of conditions.
// If there are multiple uses, we would have to intersect with the union of
// all conditions at different uses.
if (!CurrI->hasOneUse())
// Stop walking if we hit a non-speculatable instruction. Even if the
// result is only used under a specific condition, executing the
// instruction itself may cause side effects or UB already.
// This also disallows looking through phi nodes: If the phi node is part
// of a cycle, we might end up reasoning about values from different cycle
// iterations (PR60629).
if (!CurrI->hasOneUse() || !isSafeToSpeculativelyExecute(CurrI))
break;
CurrU = &*CurrI->use_begin();
}
Expand Down
46 changes: 46 additions & 0 deletions llvm/test/Analysis/LazyValueAnalysis/pr60629.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=correlated-propagation -S %s | FileCheck %s

; Check that shift is not removed by CVP because of incorrect range returned by LVI::getConstantRangeAtUse.
; https://github.com/llvm/llvm-project/issues/60629

define i32 @test(i32 %arg) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 127, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[DO_SHIFT:%.*]] ]
; CHECK-NEXT: [[SHIFTED_IV:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[SHIFT:%.*]], [[DO_SHIFT]] ]
; CHECK-NEXT: [[IV_NEXT]] = add i32 [[IV]], 1
; CHECK-NEXT: [[LOOP_COND:%.*]] = icmp sgt i32 [[IV]], 127
; CHECK-NEXT: br i1 [[LOOP_COND]], label [[EXIT:%.*]], label [[DO_SHIFT]]
; CHECK: do.shift:
; CHECK-NEXT: [[SHIFT]] = ashr i32 [[IV]], [[ARG:%.*]]
; CHECK-NEXT: br label [[LOOP]]
; CHECK: bb1:
; CHECK-NEXT: br label [[EXIT]]
; CHECK: exit:
; CHECK-NEXT: [[RETVAL:%.*]] = phi i32 [ 0, [[BB1:%.*]] ], [ [[SHIFTED_IV]], [[LOOP]] ]
; CHECK-NEXT: ret i32 [[RETVAL]]
;
entry:
br label %loop

loop: ; preds = %do.shift, %entry
%iv = phi i32 [ 127, %entry ], [ %iv.next, %do.shift ]
%shifted.iv = phi i32 [ 0, %entry ], [ %shift, %do.shift ]
%iv.next = add i32 %iv, 1
%loop_cond = icmp sgt i32 %iv, 127
br i1 %loop_cond, label %exit, label %do.shift

do.shift: ; preds = %loop
%shift = ashr i32 %iv, %arg
br label %loop

bb1: ; No predecessors!
br label %exit

exit: ; preds = %loop, %bb1
%retval = phi i32 [ 0, %bb1 ], [ %shifted.iv, %loop ]
ret i32 %retval
}

0 comments on commit 7eac876

Please sign in to comment.