Skip to content

Commit

Permalink
[InstCombine] choose 1 form of abs and nabs as canonical
Browse files Browse the repository at this point in the history 
We already do this for min/max (see the blob above the diff), 
so we should do the same for abs/nabs.
A sign-bit check (<s 0) is used as a predicate for other IR 
transforms and it's likely the best for codegen.

This might solve the motivating cases for D47037 and D47041, 
but I think those patches still make sense. We can't guarantee 
this canonicalization if the icmp has more than one use.

Differential Revision: https://reviews.llvm.org/D47076

llvm-svn: 332819
  • Loading branch information
@rotateright
rotateright committed May 20, 2018
1 parent 4a227e5 commit a003c72
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 119 deletions.
51 changes: 51 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
View file
Expand Up @@ -794,6 +794,54 @@ canonicalizeMinMaxWithConstant(SelectInst &Sel, ICmpInst &Cmp,
return &Sel;
}

/// There are 4 select variants for each of ABS/NABS (different compare
/// constants, compare predicates, select operands). Canonicalize to 1 pattern.
/// This makes CSE more likely.
static Instruction *canonicalizeAbsNabs(SelectInst &Sel, ICmpInst &Cmp,
InstCombiner::BuilderTy &Builder) {
if (!Cmp.hasOneUse() || !isa<Constant>(Cmp.getOperand(1)))
return nullptr;

// Choose a sign-bit check for the compare (likely simpler for codegen).
// ABS: (X <s 0) ? -X : X
// NABS: (X <s 0) ? X : -X
Value *LHS, *RHS;
SelectPatternFlavor SPF = matchSelectPattern(&Sel, LHS, RHS).Flavor;
if (SPF != SelectPatternFlavor::SPF_ABS &&
SPF != SelectPatternFlavor::SPF_NABS)
return nullptr;

// Is this already canonical?
if (match(Cmp.getOperand(1), m_ZeroInt()) &&
Cmp.getPredicate() == ICmpInst::ICMP_SLT)
return nullptr;

// Create the canonical compare.
Cmp.setPredicate(ICmpInst::ICMP_SLT);
Cmp.setOperand(1, ConstantInt::getNullValue(LHS->getType()));

// If the select operands do not change, we're done.
Value *TVal = Sel.getTrueValue();
Value *FVal = Sel.getFalseValue();
if (SPF == SelectPatternFlavor::SPF_NABS) {
if (TVal == LHS && match(FVal, m_Neg(m_Specific(TVal))))
return &Sel;
assert(FVal == LHS && match(TVal, m_Neg(m_Specific(FVal))) &&
"Unexpected results from matchSelectPattern");
} else {
if (FVal == LHS && match(TVal, m_Neg(m_Specific(FVal))))
return &Sel;
assert(TVal == LHS && match(FVal, m_Neg(m_Specific(TVal))) &&
"Unexpected results from matchSelectPattern");
}

// We are swapping the select operands, so swap the metadata too.
Sel.setTrueValue(FVal);
Sel.setFalseValue(TVal);
Sel.swapProfMetadata();
return &Sel;
}

/// Visit a SelectInst that has an ICmpInst as its first operand.
Instruction *InstCombiner::foldSelectInstWithICmp(SelectInst &SI,
ICmpInst *ICI) {
Expand All @@ -803,6 +851,9 @@ Instruction *InstCombiner::foldSelectInstWithICmp(SelectInst &SI,
if (Instruction *NewSel = canonicalizeMinMaxWithConstant(SI, *ICI, Builder))
return NewSel;

if (Instruction *NewAbs = canonicalizeAbsNabs(SI, *ICI, Builder))
return NewAbs;

bool Changed = adjustMinMax(SI, *ICI);

if (Value *V = foldSelectICmpAnd(SI, ICI, Builder))
Expand Down
42 changes: 21 additions & 21 deletions llvm/test/Transforms/InstCombine/abs-1.ll
View file
Expand Up @@ -8,13 +8,13 @@ declare i64 @labs(i64)
declare i64 @llabs(i64)

; Test that the abs library call simplifier works correctly.
; abs(x) -> x >s -1 ? x : -x.
; abs(x) -> x <s 0 ? -x : x.

define i32 @test_abs(i32 %x) {
; CHECK-LABEL: @test_abs(
; CHECK-NEXT: [[ISPOS:%.*]] = icmp sgt i32 [[X:%.*]], -1
; CHECK-NEXT: [[ISPOS:%.*]] = icmp slt i32 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[X]]
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISPOS]], i32 [[X]], i32 [[NEG]]
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISPOS]], i32 [[NEG]], i32 [[X]]
; CHECK-NEXT: ret i32 [[TMP1]]
;
%ret = call i32 @abs(i32 %x)
Expand All @@ -23,9 +23,9 @@ define i32 @test_abs(i32 %x) {

define i64 @test_labs(i64 %x) {
; CHECK-LABEL: @test_labs(
; CHECK-NEXT: [[ISPOS:%.*]] = icmp sgt i64 [[X:%.*]], -1
; CHECK-NEXT: [[ISPOS:%.*]] = icmp slt i64 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[X]]
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISPOS]], i64 [[X]], i64 [[NEG]]
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISPOS]], i64 [[NEG]], i64 [[X]]
; CHECK-NEXT: ret i64 [[TMP1]]
;
%ret = call i64 @labs(i64 %x)
Expand All @@ -34,22 +34,22 @@ define i64 @test_labs(i64 %x) {

define i64 @test_llabs(i64 %x) {
; CHECK-LABEL: @test_llabs(
; CHECK-NEXT: [[ISPOS:%.*]] = icmp sgt i64 [[X:%.*]], -1
; CHECK-NEXT: [[ISPOS:%.*]] = icmp slt i64 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i64 0, [[X]]
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISPOS]], i64 [[X]], i64 [[NEG]]
; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[ISPOS]], i64 [[NEG]], i64 [[X]]
; CHECK-NEXT: ret i64 [[TMP1]]
;
%ret = call i64 @llabs(i64 %x)
ret i64 %ret
}

; FIXME: We should have a canonical form of abs to make CSE easier.
; We have a canonical form of abs to make CSE easier.

define i8 @abs_canonical_1(i8 %x) {
; CHECK-LABEL: @abs_canonical_1(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i8 0, [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select i1 [[CMP]], i8 [[X]], i8 [[NEG]]
; CHECK-NEXT: [[ABS:%.*]] = select i1 [[CMP]], i8 [[NEG]], i8 [[X]]
; CHECK-NEXT: ret i8 [[ABS]]
;
%cmp = icmp sgt i8 %x, 0
Expand All @@ -58,13 +58,13 @@ define i8 @abs_canonical_1(i8 %x) {
ret i8 %abs
}

; FIXME: Vectors should work too.
; Vectors should work too.

define <2 x i8> @abs_canonical_2(<2 x i8> %x) {
; CHECK-LABEL: @abs_canonical_2(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: [[NEG:%.*]] = sub <2 x i8> zeroinitializer, [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[X]], <2 x i8> [[NEG]]
; CHECK-NEXT: [[ABS:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[NEG]], <2 x i8> [[X]]
; CHECK-NEXT: ret <2 x i8> [[ABS]]
;
%cmp = icmp sgt <2 x i8> %x, <i8 -1, i8 -1>
Expand All @@ -90,7 +90,7 @@ define i8 @abs_canonical_3(i8 %x) {

define i8 @abs_canonical_4(i8 %x) {
; CHECK-LABEL: @abs_canonical_4(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i8 0, [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select i1 [[CMP]], i8 [[NEG]], i8 [[X]]
; CHECK-NEXT: ret i8 [[ABS]]
Expand All @@ -101,13 +101,13 @@ define i8 @abs_canonical_4(i8 %x) {
ret i8 %abs
}

; FIXME: We should have a canonical form of nabs to make CSE easier.
; We have a canonical form of nabs to make CSE easier.

define i8 @nabs_canonical_1(i8 %x) {
; CHECK-LABEL: @nabs_canonical_1(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], 0
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i8 0, [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select i1 [[CMP]], i8 [[NEG]], i8 [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select i1 [[CMP]], i8 [[X]], i8 [[NEG]]
; CHECK-NEXT: ret i8 [[ABS]]
;
%cmp = icmp sgt i8 %x, 0
Expand All @@ -116,13 +116,13 @@ define i8 @nabs_canonical_1(i8 %x) {
ret i8 %abs
}

; FIXME: Vectors should work too.
; Vectors should work too.

define <2 x i8> @nabs_canonical_2(<2 x i8> %x) {
; CHECK-LABEL: @nabs_canonical_2(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
; CHECK-NEXT: [[CMP:%.*]] = icmp slt <2 x i8> [[X:%.*]], zeroinitializer
; CHECK-NEXT: [[NEG:%.*]] = sub <2 x i8> zeroinitializer, [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[NEG]], <2 x i8> [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[X]], <2 x i8> [[NEG]]
; CHECK-NEXT: ret <2 x i8> [[ABS]]
;
%cmp = icmp sgt <2 x i8> %x, <i8 -1, i8 -1>
Expand All @@ -148,7 +148,7 @@ define i8 @nabs_canonical_3(i8 %x) {

define i8 @nabs_canonical_4(i8 %x) {
; CHECK-LABEL: @nabs_canonical_4(
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub i8 0, [[X]]
; CHECK-NEXT: [[ABS:%.*]] = select i1 [[CMP]], i8 [[X]], i8 [[NEG]]
; CHECK-NEXT: ret i8 [[ABS]]
Expand Down

0 comments on commit a003c72

Please sign in to comment.