[PATCH] D34515: [ARM] Materialise some boolean values to avoid a branch

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:10015-10025
+  if (N->getOpcode() == ARMISD::SUBC) {
+    // (SUBC (ADDE 0, 0, C), 1) -> C
+    SDValue LHS = N->getOperand(0);
+    SDValue RHS = N->getOperand(1);
+    if (LHS->getOpcode() == ARMISD::ADDE &&
+        isNullConstant(LHS->getOperand(0)) &&
+        isNullConstant(LHS->getOperand(1)) && isOneConstant(RHS)) {
----------------
efriedma wrote:
> rogfer01 wrote:
> > I think this may make the `ADDC` combiner above redundant as `ADDC x, -1` will usually become `SUBC x, 1`. I'll try to see if I can remove the `ADDC` one.
> Did you end up checking this?
Yep, neither is redundant apparently, but I will check with more detail why it happens.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:7443
+  if (isNullConstant(Carry))
+    return SDValue();
+
----------------
efriedma wrote:
> When do we hit this case?  I would expect that normally DAGCombiner::visitADDCARRY would combine this away.
In a testcase like

```
define i32 @test1a(i32 %a, i32 %b) {
entry:
  %cmp = icmp ne i32 %a, %b
  %cond = zext i1 %cmp to i32
  ret i32 %cond
}
```
without this we generate

```
	subs	r0, r0, r1
	movs	r1, #1
	subs	r2, r1, #1
	mov	r2, r0
	sbcs	r2, r1
	sbcs	r0, r2
	bx	lr
```

with it we can generate

```
	subs	r0, r0, r1
	subs	r1, r0, #1
	sbcs	r0, r1
	bx	lr
```

Alternatively I could queue a combine to the newly created `ISD::SUBCARRY` in line 12270 below. I think `DCI.CombineTo` should be able to do that. Does this sound right?


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:12245
 
+  if (!VT.isInteger())
+      return SDValue();
----------------
efriedma wrote:
> Do we generate ARMISD::CMOV for values which aren't integers?
In cases like this 
```lang=c
float a;
int b;
a = (b & 0x1) != 0;
```
there is a moment where the DAG contains this node
```
 t19: f32 = ARMISD::CMOV ConstantFP:f32<0.000000e+00>, ConstantFP:f32<1.000000e+00>, Constant:i32<1>, Register:i32 %cpsr, t18
```


https://reviews.llvm.org/D34515