[PATCH] InstCombine: Transform select with and/or cond into select sequence

[Matthias Braun]

Hi hfinkel,

Putting this up for early review to get some feedback:
- In my early tests the transformation was benficial on arm, aarch64, x86
  but of course not on powerpc. Either the powerpc target should normalize
  into the other direction or we add another TargetTransformInfo flag that
  tells InstCombine which is the preferred form. I am not sure which to pick.
- Is there any reason why this would not be beneficial? ON x86, arm, aarch64
  this seemed to always reduce the number of instructions and saves a register.
  I still have to run some proper benchmarks.

This transforms:
select((C0 & C1), a, b) -> select(C0, select(C1, a, b), b)
select((C0 | C1), a, b) -> select(C0, a, select(C1, a, b))

The result is better on most targets as C0 and C1 do not need to be
materialized into an integer register but can stay flags.

http://reviews.llvm.org/D7399

Files:
  lib/Transforms/InstCombine/InstCombineSelect.cpp
  test/Transforms/InstCombine/select.ll
  test/Transforms/Reassociate/basictest.ll

Index: lib/Transforms/InstCombine/InstCombineSelect.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -911,6 +911,16 @@
   return V;
 }
 
+/// Returns true if all users of @p Val are Select instructions.
+static bool onlySelectAsUser(const Value &Val) {
+  for (const Use &U : Val.uses()) {
+    Instruction *User = cast<Instruction>(U.getUser());
+    if (User->getOpcode() != Instruction::Select)
+      return false;
+  }
+  return true;
+}
+
 Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
   Value *CondVal = SI.getCondition();
   Value *TrueVal = SI.getTrueValue();
@@ -1214,6 +1224,29 @@
     if (isa<ConstantAggregateZero>(CondVal)) {
       return ReplaceInstUsesWith(SI, FalseVal);
     }
+  } else {
+    // Avoid arithmetic on i1 types if we can express it with a series of
+    // selects this usually results in better target code as C0 and C1
+    // don't need to be materialized in an integer register.
+
+    // select((C0 & C1), a, b) -> select(C0, select(C1, a, b), b)
+    if (const Instruction *And = dyn_cast<Instruction>(CondVal))
+      if (And->getOpcode() == Instruction::And && onlySelectAsUser(*CondVal)) {
+        Value *InnerSelect = Builder->CreateSelect(And->getOperand(1), TrueVal,
+                                                   FalseVal, SI.getName()+".0");
+        SI.setOperand(0, And->getOperand(0));
+        SI.setOperand(1, InnerSelect);
+        return &SI;
+      }
+    // select((C0 | C1), a, b) -> select(C0, a, select(C1, a, b))
+    if (const Instruction *Or = dyn_cast<Instruction>(CondVal))
+      if (Or->getOpcode() == Instruction::Or && onlySelectAsUser(*CondVal)) {
+        Value *InnerSelect = Builder->CreateSelect(Or->getOperand(1), TrueVal,
+                                                   FalseVal, SI.getName()+".0");
+        SI.setOperand(0, Or->getOperand(0));
+        SI.setOperand(2, InnerSelect);
+        return &SI;
+      }
   }
 
   return nullptr;
Index: test/Transforms/InstCombine/select.ll
===================================================================
--- test/Transforms/InstCombine/select.ll
+++ test/Transforms/InstCombine/select.ll
@@ -1492,3 +1492,31 @@
   %v = load i128* %p
   ret i128 %v
 }
+
+define i32 @test_select_and(i32 %a, i32 %r0, i32 %r1, i32 %v1, i32 %v2) {
+  ; CHECK-LABEL: @test_select_and(
+  ; CHECK: %[[C0:.*]] = icmp sge i32 %a, %v1
+  ; CHECK-NEXT: %[[C1:.*]] = icmp slt i32 %a, %v2
+  ; CHECK-NEXT: %[[SEL0:.*]] = select i1 %[[C1]], i32 %r0, i32 %r1
+  ; CHECK-NEXT: %[[SEL1:.*]] = select i1 %[[C0]], i32 %[[SEL0]], i32 %r1
+  ; CHECK-NEXT: ret i32 %[[SEL1]]
+  %c0 = icmp sge i32 %a, %v1
+  %c1 = icmp slt i32 %a, %v2
+  %and = and i1 %c0, %c1
+  %res = select i1 %and, i32 %r0, i32 %r1
+  ret i32 %res
+}
+
+define i32 @test_select_or(i32 %a, i32 %r0, i32 %r1, i32 %v1, i32 %v2) {
+  ; CHECK-LABEL: @test_select_or(
+  ; CHECK: %[[C0:.*]] = icmp sge i32 %a, %v1
+  ; CHECK-NEXT: %[[C1:.*]] = icmp slt i32 %a, %v2
+  ; CHECK-NEXT: %[[SEL0:.*]] = select i1 %[[C1]], i32 %r0, i32 %r1
+  ; CHECK-NEXT: %[[SEL1:.*]] = select i1 %[[C0]], i32 %r0, i32 %[[SEL0]]
+  ; CHECK-NEXT: ret i32 %[[SEL1]]
+  %c0 = icmp sge i32 %a, %v1
+  %c1 = icmp slt i32 %a, %v2
+  %and = or i1 %c0, %c1
+  %res = select i1 %and, i32 %r0, i32 %r1
+  ret i32 %res
+}
Index: test/Transforms/Reassociate/basictest.ll
===================================================================
--- test/Transforms/Reassociate/basictest.ll
+++ test/Transforms/Reassociate/basictest.ll
@@ -212,7 +212,7 @@
   %A = icmp ne i32 %X1, 0
   %B = icmp slt i32 %X2, %X3
   %C = and i1 %A, %B
-  %D = select i1 %C, i32 %X1, i32 0
+  %D = zext i1 %C to i32
   ret i32 %D
 ; CHECK-LABEL: @test15
 ; CHECK: and i1 %A, %B

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7399.19299.patch
Type: text/x-patch
Size: 3833 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150204/d03427e4/attachment.bin>