Skip to content

[ConstraintElim] Teach checkAndReplaceCondition about samesign #128168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 24, 2025
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
11 changes: 10 additions & 1 deletion llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1427,7 +1427,7 @@ static std::optional<bool> checkCondition(CmpInst::Predicate Pred, Value *A,
}

static bool checkAndReplaceCondition(
CmpInst *Cmp, ConstraintInfo &Info, unsigned NumIn, unsigned NumOut,
ICmpInst *Cmp, ConstraintInfo &Info, unsigned NumIn, unsigned NumOut,
Instruction *ContextInst, Module *ReproducerModule,
ArrayRef<ReproducerEntry> ReproducerCondStack, DominatorTree &DT,
SmallVectorImpl<Instruction *> &ToRemove) {
Expand Down Expand Up @@ -1460,6 +1460,15 @@ static bool checkAndReplaceCondition(
checkCondition(Cmp->getPredicate(), Cmp->getOperand(0),
Cmp->getOperand(1), Cmp, Info))
return ReplaceCmpWithConstant(Cmp, *ImpliedCondition);

// When the predicate is samesign and unsigned, we can also make use of the
// signed predicate information.
if (Cmp->hasSameSign() && Cmp->isUnsigned())
if (auto ImpliedCondition =
checkCondition(Cmp->getSignedPredicate(), Cmp->getOperand(0),
Cmp->getOperand(1), Cmp, Info))
return ReplaceCmpWithConstant(Cmp, *ImpliedCondition);

return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,3 +303,64 @@ define i1 @ugt_assumed_positive_values(i8 %a, i8 %b) {

ret i1 %result
}

define i1 @implied_condition_sgt_ugt(i8 %a, i8 %b) {
; CHECK-LABEL: @implied_condition_sgt_ugt(
; CHECK-NEXT: [[CMP_SGT:%.*]] = icmp sgt i8 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: br i1 [[CMP_SGT]], label [[GREATER:%.*]], label [[EXIT:%.*]]
; CHECK: greater:
; CHECK-NEXT: ret i1 true
; CHECK: exit:
; CHECK-NEXT: ret i1 false
;
%cmp_sgt = icmp sgt i8 %a, %b
br i1 %cmp_sgt, label %greater, label %exit

greater:
%cmp_ugt = icmp samesign ugt i8 %a, %b
ret i1 %cmp_ugt

exit:
ret i1 false
}

define i1 @implied_condition_sle_ule(i8 %a) {
; CHECK-LABEL: @implied_condition_sle_ule(
; CHECK-NEXT: [[CMP_SLE:%.*]] = icmp sle i8 [[A:%.*]], 42
; CHECK-NEXT: br i1 [[CMP_SLE]], label [[LESS_OR_EQUAL:%.*]], label [[EXIT:%.*]]
; CHECK: less_or_equal:
; CHECK-NEXT: ret i1 true
; CHECK: exit:
; CHECK-NEXT: ret i1 false
;
%cmp_sle = icmp sle i8 %a, 42
br i1 %cmp_sle, label %less_or_equal, label %exit

less_or_equal:
%cmp_ule = icmp samesign ule i8 %a, 42
Copy link
Contributor

Choose a reason for hiding this comment

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

could you also add a test with samesign that cannot be simplified to also guard the negative case?

And maybe a variant with samesign on the signed known condition to also cover the case where we have samesign but a single predicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And maybe a variant with samesign on the signed known condition

Based on comments above, I thought samesign mustn't go on signed predicates?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I misunderstood something, @fhahn: samesign is always canonicalized with unsigned predicates, and we generally avoid writing tests with samesign and signed predicates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, AFAIK you can have samesign with signed predicates in general, it looks like ConstraintElimination already handles those case (https://llvm.godbolt.org/z/fhs3r5M56), so this may already be covered by tests

ret i1 %cmp_ule

exit:
ret i1 false
}

define i1 @implied_condition_cannot_simplify(i8 %a, i8 %b) {
; CHECK-LABEL: @implied_condition_cannot_simplify(
; CHECK-NEXT: [[CMP_SGT:%.*]] = icmp ne i8 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: br i1 [[CMP_SGT]], label [[GREATER:%.*]], label [[EXIT:%.*]]
; CHECK: not_equal:
; CHECK-NEXT: [[CMP_UGT:%.*]] = icmp samesign ugt i8 [[A]], [[B]]
; CHECK-NEXT: ret i1 [[CMP_UGT]]
; CHECK: exit:
; CHECK-NEXT: ret i1 false
;
%cmp_ne = icmp ne i8 %a, %b
br i1 %cmp_ne, label %not_equal, label %exit

not_equal:
%cmp_ugt = icmp samesign ugt i8 %a, %b
ret i1 %cmp_ugt

exit:
ret i1 false
}
Loading