Skip to content

Commit

Permalink
[LVI] Don't push both binop operands at once
Browse files Browse the repository at this point in the history
If one of the binop operands depends on the other, this may end
up evaluating them in the wrong order, producing sub-optimal
results.

Make sure that only one unevaluated operand gets pushed per
iteration.

Fixes #76705.
  • Loading branch information
nikic committed Jan 2, 2024
1 parent aa6bb16 commit d5db2cd
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
6 changes: 4 additions & 2 deletions llvm/lib/Analysis/LazyValueInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -976,9 +976,11 @@ LazyValueInfoImpl::solveBlockValueBinaryOpImpl(
// lets us pick up facts from expressions like "and i32 (call i32
// @foo()), 32"
std::optional<ConstantRange> LHSRes = getRangeFor(I->getOperand(0), I, BB);
if (!LHSRes)
return std::nullopt;

std::optional<ConstantRange> RHSRes = getRangeFor(I->getOperand(1), I, BB);
if (!LHSRes || !RHSRes)
// More work to do before applying this transfer rule.
if (!RHSRes)
return std::nullopt;

const ConstantRange &LHSRange = *LHSRes;
Expand Down
3 changes: 1 addition & 2 deletions llvm/test/Transforms/CorrelatedValuePropagation/basic.ll
Original file line number Diff line number Diff line change
Expand Up @@ -1915,8 +1915,7 @@ define i1 @binop_eval_order(i32 %x) {
; CHECK-NEXT: [[A:%.*]] = add nuw nsw i32 [[X:%.*]], 1
; CHECK-NEXT: [[B:%.*]] = add nuw nsw i32 [[A]], 1
; CHECK-NEXT: [[C:%.*]] = add nuw nsw i32 [[A]], [[B]]
; CHECK-NEXT: [[D:%.*]] = icmp ugt i32 [[C]], 2
; CHECK-NEXT: ret i1 [[D]]
; CHECK-NEXT: ret i1 true
;
%a = add nuw nsw i32 %x, 1
%b = add nuw nsw i32 %a, 1
Expand Down

0 comments on commit d5db2cd

Please sign in to comment.