Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions llvm/lib/Transforms/Utils/SCCPSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
#include "llvm/Analysis/ValueLattice.h"
#include "llvm/Analysis/ValueLatticeUtils.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/InstVisitor.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/NoFolder.h"
#include "llvm/IR/PatternMatch.h"
#include "llvm/Support/Casting.h"
Expand Down Expand Up @@ -284,6 +286,58 @@ static Value *simplifyInstruction(SCCPSolver &Solver,
return Sub;
}

// Relax range checks.
if (auto *ICmp = dyn_cast<ICmpInst>(&Inst)) {
Value *X;
auto MatchTwoInstructionExactRangeCheck =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost the same as MatchRangeCheck in #158498 is it possible to have this common somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to add a common helper in ValueTracking. But I finally decided to duplicate the code because they have different one-use constraints from each other.

[&]() -> std::optional<ConstantRange> {
const APInt *RHSC;
if (!match(ICmp->getOperand(1), m_APInt(RHSC)))
return std::nullopt;

Value *LHS = ICmp->getOperand(0);
ICmpInst::Predicate Pred = ICmp->getPredicate();
const APInt *Offset;
if (match(LHS, m_OneUse(m_AddLike(m_Value(X), m_APInt(Offset)))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it not good to remove the dependency to the "add" even if there is muli use?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is tuned based on the feedback from opt-bench. See the ir diff in previous runs.

return ConstantRange::makeExactICmpRegion(Pred, *RHSC).sub(*Offset);
// Match icmp eq/ne X & NegPow2, C
if (ICmp->isEquality()) {
const APInt *Mask;
if (match(LHS, m_OneUse(m_And(m_Value(X), m_NegatedPower2(Mask)))) &&
RHSC->countr_zero() >= Mask->countr_zero()) {
ConstantRange CR(*RHSC, *RHSC - *Mask);
return Pred == ICmpInst::ICMP_EQ ? CR : CR.inverse();
}
}
return std::nullopt;
};

if (auto CR = MatchTwoInstructionExactRangeCheck()) {
ConstantRange LRange = GetRange(X);
// Early exit if we know nothing about X.
if (LRange.isFullSet())
return nullptr;
// We are allowed to refine the comparison to either true or false for out
// of range inputs. Here we refine the comparison to true, i.e. we relax
// the range check.
auto NewCR = CR->exactUnionWith(LRange.inverse());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe good to have a comment here that describe why union of the inverse relax the range

// TODO: Check if we can narrow the range check to an equality test.
// E.g, for X in [0, 4), X - 3 u< 2 -> X == 3
if (!NewCR)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also want to check the intersection with LRange as if there only is two values it is possible to have EQ/NE predicate instead? eg if relax_range_check had the argument range [0,4) instead of [0,5)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add a todo here.

return nullptr;
ICmpInst::Predicate Pred;
APInt RHS;
// Check if we can represent NewCR as an icmp predicate.
if (NewCR->getEquivalentICmp(Pred, RHS)) {
IRBuilder<NoFolder> Builder(&Inst);
Value *NewICmp =
Builder.CreateICmp(Pred, X, ConstantInt::get(X->getType(), RHS));
InsertedValues.insert(NewICmp);
return NewICmp;
}
}
}

return nullptr;
}

Expand Down
92 changes: 92 additions & 0 deletions llvm/test/Transforms/SCCP/relax-range-checks.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
; RUN: opt < %s -passes=sccp -S | FileCheck %s

define i1 @relax_range_check(i8 range(i8 0, 5) %x) {
; CHECK-LABEL: define i1 @relax_range_check(
; CHECK-SAME: i8 range(i8 0, 5) [[X:%.*]]) {
; CHECK-NEXT: [[ADD:%.*]] = add nsw i8 [[X]], -3
; CHECK-NEXT: [[RET:%.*]] = icmp uge i8 [[X]], 3
; CHECK-NEXT: ret i1 [[RET]]
;
%add = add i8 %x, -3
%ret = icmp ult i8 %add, 2
ret i1 %ret
}

define i1 @relax_range_check_highbits_check(i8 range(i8 2, 0) %x) {
; CHECK-LABEL: define i1 @relax_range_check_highbits_check(
; CHECK-SAME: i8 range(i8 2, 0) [[X:%.*]]) {
; CHECK-NEXT: [[AND:%.*]] = and i8 [[X]], -2
; CHECK-NEXT: [[RET:%.*]] = icmp ult i8 [[X]], 4
; CHECK-NEXT: ret i1 [[RET]]
;
%and = and i8 %x, -2
%ret = icmp eq i8 %and, 2
ret i1 %ret
}

; Negative tests.

define i1 @relax_range_check_one_instruction(i8 range(i8 0, 5) %x) {
; CHECK-LABEL: define i1 @relax_range_check_one_instruction(
; CHECK-SAME: i8 range(i8 0, 5) [[X:%.*]]) {
; CHECK-NEXT: [[RET:%.*]] = icmp ult i8 [[X]], 2
; CHECK-NEXT: ret i1 [[RET]]
;
%ret = icmp ult i8 %x, 2
ret i1 %ret
}

define i1 @relax_range_check_not_profitable(i8 range(i8 0, 6) %x) {
; CHECK-LABEL: define i1 @relax_range_check_not_profitable(
; CHECK-SAME: i8 range(i8 0, 6) [[X:%.*]]) {
; CHECK-NEXT: [[ADD:%.*]] = add nsw i8 [[X]], -3
; CHECK-NEXT: [[RET:%.*]] = icmp ult i8 [[ADD]], 2
; CHECK-NEXT: ret i1 [[RET]]
;
%add = add i8 %x, -3
%ret = icmp ult i8 %add, 2
ret i1 %ret
}

define i1 @relax_range_check_unknown_range(i64 %x) {
; CHECK-LABEL: define i1 @relax_range_check_unknown_range(
; CHECK-SAME: i64 [[X:%.*]]) {
; CHECK-NEXT: [[AND:%.*]] = and i64 [[X]], -67108864
; CHECK-NEXT: [[TMP1:%.*]] = icmp eq i64 [[AND]], 0
; CHECK-NEXT: ret i1 [[TMP1]]
;
%and = and i64 %x, -67108864
%test = icmp eq i64 %and, 0
ret i1 %test
}

define i1 @relax_range_check_highbits_check_multiuse(i8 range(i8 2, 0) %x) {
; CHECK-LABEL: define i1 @relax_range_check_highbits_check_multiuse(
; CHECK-SAME: i8 range(i8 2, 0) [[X:%.*]]) {
; CHECK-NEXT: [[AND:%.*]] = and i8 [[X]], -2
; CHECK-NEXT: call void @use(i8 [[AND]])
; CHECK-NEXT: [[RET:%.*]] = icmp eq i8 [[AND]], 2
; CHECK-NEXT: ret i1 [[RET]]
;
%and = and i8 %x, -2
call void @use(i8 %and)
%ret = icmp eq i8 %and, 2
ret i1 %ret
}

define i1 @relax_range_check_multiuse(i8 range(i8 0, 5) %x) {
; CHECK-LABEL: define i1 @relax_range_check_multiuse(
; CHECK-SAME: i8 range(i8 0, 5) [[X:%.*]]) {
; CHECK-NEXT: [[ADD:%.*]] = add nsw i8 [[X]], -3
; CHECK-NEXT: call void @use(i8 [[ADD]])
; CHECK-NEXT: [[RET:%.*]] = icmp ult i8 [[ADD]], 2
; CHECK-NEXT: ret i1 [[RET]]
;
%add = add i8 %x, -3
call void @use(i8 %add)
%ret = icmp ult i8 %add, 2
ret i1 %ret
}

declare void @use(i8)