Skip to content

Commit

Permalink
[InstCombine] Fold logical and/or of range icmps with nowrap flags
Browse files Browse the repository at this point in the history
This is an edge-case where we don't convert to bitwise and/or based
on implies poison reasoning, so explicitly try to perform the fold
in logical form. The transform itself is poison-safe, as both icmps
are based on the same value and any nowrap flags are discarded as
part of the fold (https://alive2.llvm.org/ce/z/aCwC8b for the used
example).
  • Loading branch information
nikic committed Apr 29, 2022
1 parent 3c2a74a commit 982cbed
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 15 deletions.
8 changes: 5 additions & 3 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1128,8 +1128,10 @@ static Value *foldAndOrOfICmpsWithConstEq(ICmpInst *Cmp0, ICmpInst *Cmp1,
/// Fold (icmp Pred1 V1, C1) & (icmp Pred2 V2, C2)
/// or (icmp Pred1 V1, C1) | (icmp Pred2 V2, C2)
/// into a single comparison using range-based reasoning.
static Value *foldAndOrOfICmpsUsingRanges(ICmpInst *ICmp1, ICmpInst *ICmp2,
IRBuilderBase &Builder, bool IsAnd) {
/// NOTE: This is also used for logical and/or, must be poison-safe!
Value *InstCombinerImpl::foldAndOrOfICmpsUsingRanges(ICmpInst *ICmp1,
ICmpInst *ICmp2,
bool IsAnd) {
ICmpInst::Predicate Pred1, Pred2;
Value *V1, *V2;
const APInt *C1, *C2;
Expand Down Expand Up @@ -2513,7 +2515,7 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
}
}

return foldAndOrOfICmpsUsingRanges(LHS, RHS, Builder, IsAnd);
return foldAndOrOfICmpsUsingRanges(LHS, RHS, IsAnd);
}

// FIXME: We use commutative matchers (m_c_*) for some, but not all, matches
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final

Value *foldEqOfParts(ICmpInst *Cmp0, ICmpInst *Cmp1, bool IsAnd);

Value *foldAndOrOfICmpsUsingRanges(ICmpInst *ICmp1, ICmpInst *ICmp2,
bool IsAnd);

/// Optimize (fcmp)&(fcmp) or (fcmp)|(fcmp).
/// NOTE: Unlike most of instcombine, this returns a Value which should
/// already be inserted into the function.
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2805,6 +2805,12 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {

if (auto *V = foldEqOfParts(ICmp0, ICmp1, IsAnd))
return replaceInstUsesWith(SI, V);

// This pattern would usually be converted into a bitwise and/or based
// on "implies poison" reasoning. However, this may fail if adds with
// nowrap flags are involved.
if (auto *V = foldAndOrOfICmpsUsingRanges(ICmp0, ICmp1, IsAnd))
return replaceInstUsesWith(SI, V);
}
}
}
Expand Down
20 changes: 8 additions & 12 deletions llvm/test/Transforms/InstCombine/and-or-icmps.ll
Original file line number Diff line number Diff line change
Expand Up @@ -1249,12 +1249,10 @@ define i1 @and_two_ranges_to_mask_and_range_no_add_on_one_range(i16 %x) {
; converted into a bitwise or.
define i1 @is_ascii_alphabetic(i32 %char) {
; CHECK-LABEL: @is_ascii_alphabetic(
; CHECK-NEXT: [[ADD1:%.*]] = add nsw i32 [[CHAR:%.*]], -65
; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i32 [[ADD1]], 26
; CHECK-NEXT: [[ADD2:%.*]] = add nsw i32 [[CHAR]], -97
; CHECK-NEXT: [[CMP2:%.*]] = icmp ult i32 [[ADD2]], 26
; CHECK-NEXT: [[LOGICAL:%.*]] = select i1 [[CMP1]], i1 true, i1 [[CMP2]]
; CHECK-NEXT: ret i1 [[LOGICAL]]
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[CHAR:%.*]], -33
; CHECK-NEXT: [[TMP2:%.*]] = add i32 [[TMP1]], -65
; CHECK-NEXT: [[TMP3:%.*]] = icmp ult i32 [[TMP2]], 26
; CHECK-NEXT: ret i1 [[TMP3]]
;
%add1 = add nsw i32 %char, -65
%cmp1 = icmp ult i32 %add1, 26
Expand All @@ -1266,12 +1264,10 @@ define i1 @is_ascii_alphabetic(i32 %char) {

define i1 @is_ascii_alphabetic_inverted(i32 %char) {
; CHECK-LABEL: @is_ascii_alphabetic_inverted(
; CHECK-NEXT: [[ADD1:%.*]] = add nsw i32 [[CHAR:%.*]], -91
; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i32 [[ADD1]], -26
; CHECK-NEXT: [[ADD2:%.*]] = add nsw i32 [[CHAR]], -123
; CHECK-NEXT: [[CMP2:%.*]] = icmp ult i32 [[ADD2]], -26
; CHECK-NEXT: [[LOGICAL:%.*]] = select i1 [[CMP1]], i1 [[CMP2]], i1 false
; CHECK-NEXT: ret i1 [[LOGICAL]]
; CHECK-NEXT: [[TMP1:%.*]] = and i32 [[CHAR:%.*]], -33
; CHECK-NEXT: [[TMP2:%.*]] = add i32 [[TMP1]], -91
; CHECK-NEXT: [[TMP3:%.*]] = icmp ult i32 [[TMP2]], -26
; CHECK-NEXT: ret i1 [[TMP3]]
;
%add1 = add nsw i32 %char, -91
%cmp1 = icmp ult i32 %add1, -26
Expand Down

0 comments on commit 982cbed

Please sign in to comment.