[InstCombine] canonicalize abs pattern

[shchenz]

Summary

two more abs pattern and canonicalize abs pattern.
This patch handle two more pattern:
1:

define i32 @abs_canonical_6(i32 %a, i32 %b) {
  %tmp1 = sub i32 %a, %b
  %cmp = icmp sgt i32 %tmp1, -1
  %tmp2 = sub i32 %b, %a
  %abs = select i1 %cmp, i32 %tmp1, i32 %tmp2
  ret i32 %abs
}

this should be |a-b|.

2:

define <2 x i8> @abs_canonical_7(<2 x i8> %a, <2 x i8 > %b) {
  %tmp1 = sub <2 x i8> %a, %b
  %cmp = icmp sgt <2 x i8> %tmp1, <i8 -1, i8 -1>
  %tmp2 = sub <2 x i8> %b, %a
  %abs = select <2 x i1> %cmp, <2 x i8> %tmp1, <2 x i8> %tmp2
  ret <2 x i8> %abs
}

this should be |a - b| for vector type.

3:

define i32 @abs_canonical_8(i32 %a) {
; CHECK-LABEL: @abs_canonical_8(
; CHECK-NEXT:    [[TMP:%.*]] = sub i32 0, [[A:%.*]]
; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[TMP]], 0
; CHECK-NEXT:    [[ABS:%.*]] = select i1 [[CMP]], i32 [[A]], i32 [[TMP]]
; CHECK-NEXT:    ret i32 [[ABS]]
;
  %tmp = sub i32 0, %a
  %cmp = icmp slt i32 %tmp, 0
  %abs = select i1 %cmp, i32 %a, i32 %tmp
  ret i32 %abs
}

this should be |-a| -> |a|
Also add test cases for -|a-b|(i32 and vector), -|-a|(-|a|)

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

17528 ↗

(On Diff #153437)

Just declare this local to the 2 blocks that need since it doesn't live outside of them.

17530 ↗

(On Diff #153437)

You probably don't need the GE/LE checks. We should be canonicalizing those away by altering the constant.

[shchenz]

fix Craig's comments

[spatel]

For min/max, we made value tracking (matchSelectPattern) recognize more patterns:
https://reviews.llvm.org/rL286776

And then we canonicalized the IR to the most recognizable form (where the cmp operands match the select operands):
https://reviews.llvm.org/rL295758

I think that's a better solution than adding more pattern matching to the DAG. For example, this isn't going to help vector code?

[shchenz]

@spatel Hi Sanjay, thanks for your comments. I will look into it.

[spatel]

In D48754#1149226, @shchenz wrote:

@spatel Hi Sanjay, thanks for your comments. I will look into it.

Thanks - here are recent patches for abs IR canonicalization:
D47076
rL334137

...so you want modify/add the pattern that has a 'sub' with something similar. We haven't made ValueTracking changes corresponding to those (but probably should). There's also an open question about creating ISD::ABS nodes directly in SelectionDAGBuilder. I think it's just an oversight that we use matchSelectPattern to create min/max there, but not abs.

[RKSimon]

In D48754#1149389, @spatel wrote:

There's also an open question about creating ISD::ABS nodes directly in SelectionDAGBuilder. I think it's just an oversight that we use matchSelectPattern to create min/max there, but not abs.

For reference, this was raised in PR37743

[shchenz]

@spatel Hi Sanjay, thanks for your comments. After investigation, I split this task into three parts:
1: find more abs pattern in matchSelectPattern and do canonicalization for abs/nabs pattern. (cmp + select). This is what this patch doing.
2: recognize ISD::ABS in selectionDAGBuilder to leverage platform abs instruction. (https://bugs.llvm.org/show_bug.cgi?id=36036). This will be done in next patch.
3: if no platform abs instructions, recognize canonicalized abs pattern (cmp + select) to shift + add + xor in DAGCombiner. This will be done in third patch.

What do you think about above proposal? And could you please help to do another review for this patch? Thanks.

[spatel]

In D48754#1151712, @shchenz wrote:

@spatel Hi Sanjay, thanks for your comments. After investigation, I split this task into three parts:
1: find more abs pattern in matchSelectPattern and do canonicalization for abs/nabs pattern. (cmp + select). This is what this patch doing.
2: recognize ISD::ABS in selectionDAGBuilder to leverage platform abs instruction. (https://bugs.llvm.org/show_bug.cgi?id=36036). This will be done in next patch.
3: if no platform abs instructions, recognize canonicalized abs pattern (cmp + select) to shift + add + xor in DAGCombiner. This will be done in third patch.

What do you think about above proposal? And could you please help to do another review for this patch? Thanks.

Yes, this sounds like a good plan to me.

[spatel]

llvm/include/llvm/Analysis/ValueTracking.h

104–105

Doesn't "complement" mean bitwise-not? I think this should be called "isKnownNegation" unless I'm mixing up the terminology.

Nit: add a period at end of comment.

llvm/lib/Analysis/ValueTracking.cpp

4506

Don't repeat the comment from the header file.

4508–4509

Make that an assert? Other functions in this file just assume that the parameters are valid. Ideally, we'd change everything to references instead of pointers...

4515

The comments don't match the code. The parameters to this function should be named X and Y, and these local values should be named A and B?

4516

Don't initialize these to nullptr. They should not be read if they are uninitialized, so explicitly initializing them removes a chance to produce a warning for that situation.

4521

Remove the TODO or specify a pattern that you know should be added later.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

815–817

We should check the negated value in the select here. If it is not m_Neg(), then we should create a new sub (neg) in the canonical form.

If you would prefer to make that a separate patch, then please add a 'TODO' comment here.

826

Formatting: goes over 80-columns.

llvm/test/Transforms/InstCombine/abs-1.ll

119–123

Please commit all new tests to trunk with the current output. Then rebase this patch so we see only the diffs.

[shchenz]

fix Sanjay's comments

[shchenz]

@spatel Hi Sanjay, I have made changes accordingly. Could you help to have another review. Thanks.

[spatel]

In D48754#1158271, @shchenz wrote:

@spatel Hi Sanjay, I have made changes accordingly. Could you help to have another review. Thanks.

Thanks for the changes. This looks mostly right, but I want to make sure we're testing everything properly. This is really several independent steps in 1 patch, so I'd like to reduce the risk of bugs by breaking it up.

1. Please separate the creation of isKnownNegation() into its own patch and use it from InstSimplify -> SimplifyAddInst(). I have added tests at rL336822, so that should be a very small patch. Improving InstSimplify helps several other passes, so that's a good patch independent of abs().
2. The changes in matchSelectPattern can be separated and tested using patterns that are optimized by InstCombiner::foldSPFofSPF(). We'll need to add some tests for that part. Let me know if it is not clear.
3. The canonicalization changes will be the last step and should be covered by the tests shown in this patch, but I think we're missing some cases where the code may have bugs.

llvm/lib/Analysis/ValueTracking.cpp

4624

cmpEqNegated -> CmpUsesNegatedOp ?

4650–4651

Would it be better to return {N}ABS(X) here rather than {N}ABS(-X)?

Why is it useful to return the negated operand as the input rather than the original operand X?

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

829

What happens if the subtract has other uses? We should have tests for that possibility.

[shchenz]

In D48754#1159009, @spatel wrote:

In D48754#1158271, @shchenz wrote:

@spatel Hi Sanjay, I have made changes accordingly. Could you help to have another review. Thanks.

Thanks for the changes. This looks mostly right, but I want to make sure we're testing everything properly. This is really several independent steps in 1 patch, so I'd like to reduce the risk of bugs by breaking it up.

1. Please separate the creation of isKnownNegation() into its own patch and use it from InstSimplify -> SimplifyAddInst(). I have added tests at rL336822, so that should be a very small patch. Improving InstSimplify helps several other passes, so that's a good patch independent of abs().
2. The changes in matchSelectPattern can be separated and tested using patterns that are optimized by InstCombiner::foldSPFofSPF(). We'll need to add some tests for that part. Let me know if it is not clear.
3. The canonicalization changes will be the last step and should be covered by the tests shown in this patch, but I think we're missing some cases where the code may have bugs.

@spatel Hi Sanjay, I have made patches for 1 & 2. But I am not very clear about 3. You mentioned the code may have bugs, could you give me some details? I saw your inline comment "What happens if the subtract has other uses", do you mean this one? In my opinion, RHS from matchSelectPattern() currently must be sub(0, X) or sub(A, B) (LHS = sub(B, A)). If I change RHS(TVal or FVal) to sub(0, LHS), I think there will be no impact for RHS(TVal or FVal) other users. yes, you are right, there should be some test cases for this scenario. Thanks.

[shchenz]

part 1 & 2 are done.
part 3: canonicalize abs pattern and fix code review comments

[spatel]

Please add a test like this (it will fail with the current patch):

define i32 @abs_canonical_9(i32 %a, i32 %b) {
  %tmp2 = sub i32 %b, %a
  %tmp1 = sub i32 %a, %b
  %cmp = icmp sgt i32 %tmp1, -1
  %abs = select i1 %cmp, i32 %tmp1, i32 %tmp2
  %add = add i32 %abs, %tmp2 ; increase use count for %tmp2.
  ret i32 %add
}

[spatel]

Actually, it will fail even without the extra use, so this is a minimal test:

define i32 @abs_canonical_9(i32 %a, i32 %b) {
  %tmp2 = sub i32 %b, %a
  %tmp1 = sub i32 %a, %b
  %cmp = icmp sgt i32 %tmp1, -1
  %abs = select i1 %cmp, i32 %tmp1, i32 %tmp2
  ret i32 %abs
}

We can not just replace operands without considering the order of the instructions. It would be better to create a new instruction with the IRBuilder, so we don't have to worry about this problem.

[spatel]

llvm/lib/Analysis/ValueTracking.cpp

4505–4525

There is some white space difference in this file. It should not be included in this patch now.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

825

typo: canoical

[shchenz]

In D48754#1163459, @spatel wrote:

Actually, it will fail even without the extra use, so this is a minimal test:

define i32 @abs_canonical_9(i32 %a, i32 %b) {
  %tmp2 = sub i32 %b, %a
  %tmp1 = sub i32 %a, %b
  %cmp = icmp sgt i32 %tmp1, -1
  %abs = select i1 %cmp, i32 %tmp1, i32 %tmp2
  ret i32 %abs
}

We can not just replace operands without considering the order of the instructions. It would be better to create a new instruction with the IRBuilder, so we don't have to worry about this problem.

@spatel Hi Sanjay, Sorry about this. Seems this is a big mistake. I will do some study about IRBuilder and upload a new patch later. Thanks for your help.

llvm/lib/Analysis/ValueTracking.cpp

4505–4525

Yes, I deleted two white spaces added in line 4629 by mistake in rL337143.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

825

I'll fix that.

[spatel]

In D48754#1163584, @shchenz wrote:

We can not just replace operands without considering the order of the instructions. It would be better to create a new instruction with the IRBuilder, so we don't have to worry about this problem.

@spatel Hi Sanjay, Sorry about this. Seems this is a big mistake. I will do some study about IRBuilder and upload a new patch later. Thanks for your help.

It's not too bad - see the code just above here in canonicalizeMinMaxWithConstant() for an example.

[shchenz]

fix Sanjay's comments

[shchenz]

rebase the patch

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

797–799

This comment is not accurate now. It should mention the possibility of a compare that uses the negated op. I'm not sure how many variants we can match now, but you can include that number if you know the answer.

818–819

Rephrase this as:
The compare may use the negated abs()/nabs() operand, or it may use negation in non-canonical form such as: sub A, B.

820–821

Indentation is not as expected.

827

This assert can be more specific? The sub must be with the specific operands of LHS reversed?

829

Use Builder.CreateNeg().

llvm/test/Transforms/InstCombine/abs-1.ll

129–133

This is the extra-uses scenario that I was worried about. We have more instructions than we started with. We should not canonicalize if we can not have equal or less-than instructions from the transform.

225–230

Similar to above - we should not ever have more instructions than we started with.

[shchenz]

fix Sanjay's comments.

[shchenz]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

797–799

Maybe we want to add new pattern in isKnownNegation(), to avoid change the comments every time we change isKnownNegetion(), I don't use a accurate number. Hope this is ok.

827

The assert is added in previous patch which is directly changing operands of operator Sub. Since that is a wrong patch and we create new node for RHS, I delete this assert.

[spatel]

Thanks for all of the IR improvements - LGTM.

Note that the 2nd step of the larger proposal for abs() improvements is implemented in D49837.

llvm/include/llvm/Analysis/ValueTracking.h

class Value;
   /// specified, perform context-sensitive analysis and return true if the
   /// pointer couldn't possibly be null at the specified instruction.
   /// Supports values with integer or pointer type and vectors of integers.
   bool isKnownNonZero(const Value *V, const DataLayout &DL, unsigned Depth = 0,
                       AssumptionCache *AC = nullptr,
                       const Instruction *CxtI = nullptr,
                       const DominatorTree *DT = nullptr);
 
+  /// Return true if the two given values are complement
+  bool isKnownComplement(const Value *LHS, const Value *RHS);
+
   /// Returns true if the give value is known to be non-negative.
   bool isKnownNonNegative(const Value *V, const DataLayout &DL,
                           unsigned Depth = 0,
                           AssumptionCache *AC = nullptr,
                           const Instruction *CxtI = nullptr,
                           const DominatorTree *DT = nullptr);
 
   /// Returns true if the given value is known be positive (i.e. non-negative

llvm/lib/Analysis/ValueTracking.cpp

static SelectPatternResult matchMinMax(CmpInst::Predicate Pred,
   // (X >s C) ? ~C : ~X ==> (~X <s ~C) ? ~C : ~X ==> SMAX(~C, ~X)
   // (X <s C) ? ~C : ~X ==> (~X >s ~C) ? ~C : ~X ==> SMIN(~C, ~X)
   if (match(FalseVal, m_Not(m_Specific(CmpLHS))) &&
       match(TrueVal, m_APInt(C2)) && ~(*C1) == *C2)
     return {Pred == CmpInst::ICMP_SGT ? SPF_SMAX : SPF_SMIN, SPNB_NA, false};
 
   return {SPF_UNKNOWN, SPNB_NA, false};
 }
 
+/// Return true if LHS and RHS are complement, otherwise return false
+bool llvm::isKnownComplement(const Value *LHS, const Value *RHS) {
+  if (!LHS || !RHS)
+    return false;
+
+  // Y = sub (0, X)
+  if (match(LHS, m_Neg(m_Specific(RHS))) || match(RHS, m_Neg(m_Specific(LHS))))
+    return true;
+
+  // X = sub (a, b), Y = sub (b, a)
+  Value *X = nullptr, *Y = nullptr;
+  if (match(LHS, m_Sub(m_Value(X), m_Value(Y))) &&
+      match(RHS, m_Sub(m_Specific(Y), m_Specific(X))))
+    return true;
+
+  //TODO: add more pattern here
+  return false;
+}
+
 static SelectPatternResult matchSelectPattern(CmpInst::Predicate Pred,
                                               FastMathFlags FMF,
                                               Value *CmpLHS, Value *CmpRHS,
                                               Value *TrueVal, Value *FalseVal,
                                               Value *&LHS, Value *&RHS,
                                               unsigned Depth) {
   LHS = CmpLHS;
   RHS = CmpRHS;
 
if (TrueVal == CmpLHS && FalseVal == CmpRHS) {
     case FCmpInst::FCMP_OGE: return {SPF_FMAXNUM, NaNBehavior, Ordered};
     case FCmpInst::FCMP_ULT:
     case FCmpInst::FCMP_ULE:
     case FCmpInst::FCMP_OLT:
     case FCmpInst::FCMP_OLE: return {SPF_FMINNUM, NaNBehavior, Ordered};
     }
   }
 
   const APInt *C1;
   if (match(CmpRHS, m_APInt(C1))) {
     // Sign-extending LHS does not change its sign, so TrueVal/FalseVal can
     // match against either LHS or sext(LHS).
     auto MaybeSExtLHS = m_CombineOr(m_Specific(CmpLHS),
                                     m_SExt(m_Specific(CmpLHS)));
-    if ((match(TrueVal, MaybeSExtLHS) &&
-         match(FalseVal, m_Neg(m_Specific(TrueVal)))) ||
-        (match(FalseVal, MaybeSExtLHS) &&
-         match(TrueVal, m_Neg(m_Specific(FalseVal))))) {
+    if ((match(TrueVal, MaybeSExtLHS) || match(FalseVal, MaybeSExtLHS)) &&
+        isKnownComplement(TrueVal, FalseVal)) {
       // Set LHS and RHS so that RHS is the negated operand of the select
       if (match(TrueVal, MaybeSExtLHS)) {
         LHS = TrueVal;
         RHS = FalseVal;
       } else {
         LHS = FalseVal;
         RHS = TrueVal;
       }
 
       // ABS(X) ==> (X >s 0) ? X : -X and (X >s -1) ? X : -X
       // NABS(X) ==> (X >s 0) ? -X : X and (X >s -1) ? -X : X
       if (Pred == ICmpInst::ICMP_SGT &&
           (C1->isNullValue() || C1->isAllOnesValue())) {
         return {(LHS == TrueVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};
       }
 
       // ABS(X) ==> (X <s 0) ? -X : X and (X <s 1) ? -X : X
       // NABS(X) ==> (X <s 0) ? X : -X and (X <s 1) ? X : -X
       if (Pred == ICmpInst::ICMP_SLT &&
           (C1->isNullValue() || C1->isOneValue())) {
         return {(LHS == FalseVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};
       }
     }
   }
 
   if (CmpInst::isIntPredicate(Pred))
     return matchMinMax(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal, LHS, RHS, Depth);
 

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

canonicalizeMinMaxWithConstant(SelectInst &Sel, ICmpInst &Cmp,
   assert(Sel.getTrueValue() == RHS && Sel.getFalseValue() == LHS &&
          "Unexpected results from matchSelectPattern");
   Sel.setTrueValue(LHS);
   Sel.setFalseValue(RHS);
   Sel.swapProfMetadata();
   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(Cmp.getOperand(0)->getType()));
 
   // If the select operands do not change, we're done.
   Value *TVal = Sel.getTrueValue();
   Value *FVal = Sel.getFalseValue();
+  assert(isKnownComplement(TVal, FVal) && "Unexpected result from matchSelectPattern");
   if (SPF == SelectPatternFlavor::SPF_NABS) {
-    if (TVal == LHS && match(FVal, m_Neg(m_Specific(TVal))))
+    if (TVal == LHS)
       return &Sel;
-    assert(FVal == LHS && match(TVal, m_Neg(m_Specific(FVal))) &&
-           "Unexpected results from matchSelectPattern");
+    assert(FVal == LHS && "Unexpected results from matchSelectPattern");
   } else {
-    if (FVal == LHS && match(TVal, m_Neg(m_Specific(FVal))))
+    if (FVal == LHS)
       return &Sel;
-    assert(TVal == LHS && match(FVal, m_Neg(m_Specific(TVal))) &&
-           "Unexpected results from matchSelectPattern");
+    assert(TVal == LHS && "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;
 }

llvm/test/Transforms/InstCombine/abs-1.ll

 ;
   %cmp = icmp sgt i8 %x, 0
   %conv = sext i8 %x to i32
   %neg = sub i32 0, %conv
   %abs = select i1 %cmp, i32 %conv, i32 %neg
   ret i32 %abs
 }
 
+define i32 @abs_canonical_6(i32 %a, i32 %b) {
+; CHECK-LABEL: @abs_canonical_6(
+; CHECK-NEXT:    [[TMP:%.*]] = sub i32 [[A:%.*]], [[B:%.*]] 
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[TMP]], 0
+; CHECK-NEXT:    [[NEG:%.*]] = sub i32 [[B]], [[A]]
+; CHECK-NEXT:    [[ABS:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[TMP]]
+; CHECK-NEXT:    ret i32 [[ABS]]
+;
+  %tmp1 = sub i32 %a, %b
+  %cmp = icmp sgt i32 %tmp1, -1
+  %tmp2 = sub i32 %b, %a
+  %abs = select i1 %cmp, i32 %tmp1, i32 %tmp2
+  ret i32 %abs
+}
+
+define <2 x i8> @abs_canonical_7(<2 x i8> %a, <2 x i8 > %b) {
+; CHECK-LABEL: @abs_canonical_7(
+; CHECK-NEXT:    [[TMP:%.*]] = sub <2 x i8> [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt <2 x i8> [[TMP]], zeroinitializer
+; CHECK-NEXT:    [[NEG:%.*]] = sub <2 x i8> [[B]], [[A]]
+; CHECK-NEXT:    [[ABS:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[NEG]], <2 x i8> [[TMP]]
+; CHECK-NEXT:    ret <2 x i8> [[ABS]]
+;
+  %tmp = sub <2 x i8> %a, %b
+  %cmp = icmp sgt <2 x i8> %tmp, <i8 -1, i8 -1>
+  %neg = sub <2 x i8> %b, %a
+  %abs = select <2 x i1> %cmp, <2 x i8> %tmp, <2 x i8> %neg
+  ret <2 x i8> %abs
+}
+
+
 ; 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 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]]

 ;
   %cmp = icmp sgt i8 %x, 0
   %conv = sext i8 %x to i32
   %neg = sub i32 0, %conv
   %abs = select i1 %cmp, i32 %neg, i32 %conv
   ret i32 %abs
 }
 
+define i32 @nabs_canonical_6(i32 %a, i32 %b) {
+; CHECK-LABEL: @nabs_canonical_6(
+; CHECK-NEXT:    [[TMP:%.*]] = sub i32 [[A:%.*]], [[B:%.*]] 
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[TMP]], 0
+; CHECK-NEXT:    [[NEG:%.*]] = sub i32 [[B]], [[A]]
+; CHECK-NEXT:    [[ABS:%.*]] = select i1 [[CMP]], i32 [[TMP]], i32 [[NEG]]
+; CHECK-NEXT:    ret i32 [[ABS]]
+;
+  %tmp1 = sub i32 %a, %b
+  %cmp = icmp sgt i32 %tmp1, -1
+  %tmp2 = sub i32 %b, %a
+  %abs = select i1 %cmp, i32 %tmp2, i32 %tmp1
+  ret i32 %abs
+}
+
+define <2 x i8> @nabs_canonical_7(<2 x i8> %a, <2 x i8 > %b) {
+; CHECK-LABEL: @nabs_canonical_7(
+; CHECK-NEXT:    [[TMP:%.*]] = sub <2 x i8> [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt <2 x i8> [[TMP]], zeroinitializer
+; CHECK-NEXT:    [[NEG:%.*]] = sub <2 x i8> [[B]], [[A]]
+; CHECK-NEXT:    [[ABS:%.*]] = select <2 x i1> [[CMP]], <2 x i8> [[TMP]], <2 x i8> [[NEG]]
+; CHECK-NEXT:    ret <2 x i8> [[ABS]]
+;
+  %tmp = sub <2 x i8> %a, %b
+  %cmp = icmp sgt <2 x i8> %tmp, <i8 -1, i8 -1>
+  %neg = sub <2 x i8> %b, %a
+  %abs = select <2 x i1> %cmp, <2 x i8> %neg, <2 x i8> %tmp
+  ret <2 x i8> %abs
+}
+
 ; The following 5 tests use a shift+add+xor to implement abs():
 ; B = ashr i8 A, 7  -- smear the sign bit.
 ; xor (add A, B), B -- add -1 and flip bits if negative
 
 define i8 @shifty_abs_commute0(i8 %x) {
 ; CHECK-LABEL: @shifty_abs_commute0(
 ; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i8 [[X:%.*]], 0
 ; CHECK-NEXT:    [[TMP2:%.*]] = sub i8 0, [[X]]