[PATCH] D24480: [InstCombine] remove fold: zext(bool) + C -> bool ? C + 1 : C
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 12 16:28:49 PDT 2016
spatel created this revision.
spatel added reviewers: efriedma, majnemer, hfinkel.
spatel added a subscriber: llvm-commits.
Herald added subscribers: mcrosier, aemerson.
This is a small patch with a long backstory.
First, I'm proposing to remove a fold of (zext i1) + C to select of constants.
A pair of sibling folds for this was added at:
https://reviews.llvm.org/rL75531
The commit was made with a request for feedback in case selects might not be the best form. 3 years later, we lost half of the transform with:
https://reviews.llvm.org/rL159230
Sadly, that commit just removed code and didn't bother to canonicalize the other way (from select to ext+add). That left us in a state where logically equivalent code could be present in multiple forms - PR30327:
https://llvm.org/bugs/show_bug.cgi?id=30327
So in addition to removing the remaining piece of r75531, I'm proposing to canonicalize selects to ext+add and filling in whatever folds may be missing as follow-up patches (see the motivating case based on the source code for InstCombine itself in PR30273).
There are a few reasons to prefer ext+add to select:
1. Adds with constants are easier to combine with other adds and subs.
2. We are already canonicalizing selects with {-1,0,1} to extends; doing this for all constants is more consistent.
3. As noted, we already lost half of the original patch, so we're not even consistent about C+1 vs. C-1.
4. The stated reason for killing the C-1 half of the patch was poor codegen. This is not a valid reason by itself to change a canonicalization (just fix the backend). However, as a practical matter, it is 7 years later and we still have not fixed the backend! See the codegen examples for AArch64 and PowerPC here:
https://llvm.org/bugs/show_bug.cgi?id=30327#c2
In order to avoid a regression on the tests in test/Transforms/InstCombine/icmp-div-constant.ll (note that those are phantom diffs), I'm adding a fold that converts icmp (add (zext), -1) to icmp' (sext) in this patch. I'm not sure how to expose this difference independently of removing the select fold.
https://reviews.llvm.org/D24480
Files:
lib/Transforms/InstCombine/InstCombineAddSub.cpp
test/Transforms/InstCombine/icmp-div-constant.ll
test/Transforms/InstCombine/zext-bool-add-sub.ll
Index: test/Transforms/InstCombine/zext-bool-add-sub.ll
===================================================================
--- test/Transforms/InstCombine/zext-bool-add-sub.ll
+++ test/Transforms/InstCombine/zext-bool-add-sub.ll
@@ -5,8 +5,9 @@
define i32 @a(i1 zeroext %x, i1 zeroext %y) {
; CHECK-LABEL: @a(
+; CHECK-NEXT: [[CONV:%.*]] = zext i1 %x to i32
; CHECK-NEXT: [[CONV3_NEG:%.*]] = sext i1 %y to i32
-; CHECK-NEXT: [[SUB:%.*]] = select i1 %x, i32 2, i32 1
+; CHECK-NEXT: [[SUB:%.*]] = add nuw nsw i32 [[CONV]], 1
; CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[SUB]], [[CONV3_NEG]]
; CHECK-NEXT: ret i32 [[ADD]]
;
@@ -47,8 +48,8 @@
define i32 @PR30273_three_bools(i1 %x, i1 %y, i1 %z) {
; CHECK-LABEL: @PR30273_three_bools(
; CHECK-NEXT: [[FROMBOOL:%.*]] = zext i1 %x to i32
-; CHECK-NEXT: [[ADD1:%.*]] = select i1 %x, i32 2, i32 1
-; CHECK-NEXT: [[SEL1:%.*]] = select i1 %y, i32 [[ADD1]], i32 [[FROMBOOL]]
+; CHECK-NEXT: [[ADD1:%.*]] = zext i1 %y to i32
+; CHECK-NEXT: [[SEL1:%.*]] = add nuw nsw i32 [[FROMBOOL]], [[ADD1]]
; CHECK-NEXT: [[ADD2:%.*]] = zext i1 %z to i32
; CHECK-NEXT: [[SEL2:%.*]] = add nuw nsw i32 [[SEL1]], [[ADD2]]
; CHECK-NEXT: ret i32 [[SEL2]]
Index: test/Transforms/InstCombine/icmp-div-constant.ll
===================================================================
--- test/Transforms/InstCombine/icmp-div-constant.ll
+++ test/Transforms/InstCombine/icmp-div-constant.ll
@@ -12,8 +12,8 @@
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i16 %a, 0
; CHECK-NEXT: br i1 [[TOBOOL]], label %then, label %exit
; CHECK: then:
-; CHECK-NEXT: [[NOT_CMP:%.*]] = icmp eq i16 %c, 0
-; CHECK-NEXT: [[PHITMP1:%.*]] = sext i1 [[NOT_CMP]] to i32
+; CHECK-NEXT: [[TMP0:%.*]] = icmp eq i16 %c, 0
+; CHECK-NEXT: [[PHITMP1:%.*]] = sext i1 [[TMP0]] to i32
; CHECK-NEXT: br label %exit
; CHECK: exit:
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ -1, %entry ], [ [[PHITMP1]], %then ]
@@ -68,8 +68,8 @@
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i16 %a, 0
; CHECK-NEXT: br i1 [[TOBOOL]], label %then, label %exit
; CHECK: then:
-; CHECK-NEXT: [[NOT_CMP:%.*]] = icmp eq i16 %c, 0
-; CHECK-NEXT: [[PHITMP1:%.*]] = sext i1 [[NOT_CMP]] to i32
+; CHECK-NEXT: [[TMP0:%.*]] = icmp eq i16 %c, 0
+; CHECK-NEXT: [[PHITMP1:%.*]] = sext i1 [[TMP0]] to i32
; CHECK-NEXT: br label %exit
; CHECK: exit:
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ -1, %entry ], [ [[PHITMP1]], %then ]
Index: lib/Transforms/InstCombine/InstCombineAddSub.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1067,10 +1067,19 @@
if (SimplifyDemandedInstructionBits(I))
return &I;
- // zext(bool) + C -> bool ? C + 1 : C
- if (ZExtInst *ZI = dyn_cast<ZExtInst>(LHS))
- if (ZI->getSrcTy()->isIntegerTy(1))
- return SelectInst::Create(ZI->getOperand(0), AddOne(CI), CI);
+ // add (zext (icmp Pred X, Y)), -1 --> sext (icmp Pred' X, Y)
+ Instruction *ZextOp;
+ ICmpInst::Predicate Pred;
+ Value *X, *Y;
+ if (CI->getSExtValue() == -1 && match(LHS, m_ZExt(m_Instruction(ZextOp))) &&
+ match(ZextOp, m_OneUse(m_ICmp(Pred, m_Value(X), m_Value(Y))))) {
+ // We have this pattern: if the compare is true, return 0;
+ // if the compare is false, return -1.
+ // Transform it to: if the inverse compare is true, return -1;
+ // if the inverse compare is false, return 0.
+ auto *Cmp = Builder->CreateICmp(CmpInst::getInversePredicate(Pred), X, Y);
+ return CastInst::Create(Instruction::SExt, Cmp, I.getType());
+ }
Value *XorLHS = nullptr; ConstantInt *XorRHS = nullptr;
if (match(LHS, m_Xor(m_Value(XorLHS), m_ConstantInt(XorRHS)))) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24480.71070.patch
Type: text/x-patch
Size: 3878 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160912/d1a9458c/attachment.bin>
More information about the llvm-commits
mailing list