[PATCH] D55030: [AMDGPU] Fold brcond (setcc zext(i1 x), 1, ne) -> brcond x

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked 2 inline comments as done.
rampitec added a comment.

In D55030#1312124 <https://reviews.llvm.org/D55030#1312124>, @arsenm wrote:

> Should this be a generic combine?


I tried to make it generic but got real regression in ARM (tests 2007-05-09-tailmerge-2.ll and 2007-05-22-tailmerge-3.ll). The tail merge did not happen when expected. Turns out that is not unconditionally good regardless of the target.



================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:8643
+  if (CC.getOpcode() != ISD::SETCC ||
+      CC.getOperand(0)->getOpcode() != ISD::ZERO_EXTEND ||
+      !isa<ConstantSDNode>(CC.getOperand(1)))
----------------
arsenm wrote:
> sext should also be OK
sext would mean comparison with -1, not with 1 and does not really happen.
I can add it, but it will complicate the code.


================
Comment at: test/CodeGen/AMDGPU/dag-combine-brcond.ll:10
+; GCN:   s_cbranch_vccz BB0_4
+define amdgpu_kernel void @test(float addrspace(1)* %arg1) {
+bb:
----------------
arsenm wrote:
> Can't this be reduced further?
I do not think so. That is the final code produced:


```
        s_and_b64 vcc, exec, s[12:13]
        s_cbranch_vccz BB0_4
; %bb.3:                                ; %bb132
                                        ;   in Loop: Header=BB0_2 Depth=2
        s_add_u32 s4, s8, s14
        s_addc_u32 s5, s9, s15
        s_branch BB0_5
```

As you may see we do not change exec here, just test it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55030/new/

https://reviews.llvm.org/D55030