[PATCH] D62266: [DAGCombine][X86][AArch64][ARM] (C - x) + y -> (y - x) + C fold

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 2 inline comments as done.
lebedev.ri added inline comments.


================
Comment at: test/CodeGen/ARM/addsubcarry-promotion.ll:10
 
+; FIXME: this could be optimized better.
+
----------------
efriedma wrote:
> It looks like this test didn't get rebased correctly?
No, D62392 is a child revision of this.


================
Comment at: test/CodeGen/ARM/addsubcarry-promotion.ll:35
+; THUMBV6M-NEXT:    ldr r1, .LCPI0_0
+; THUMBV6M-NEXT:    cmp r0, r1
+; THUMBV6M-NEXT:    beq .LBB0_2
----------------
efriedma wrote:
> The big difference that's making the new code worse, now, is that somehow the compare `x==0` is getting transformed to something more like `x==-1`... and I guess something isn't handling that well. That's probably something the ARM backend should be handling in target-specific code, though; feel free to just file a bug for the missed optimization on `void a(int s, void f()) { if ((short)s==-1)f(); }`
I understand how we get here. In IR terms, this is just two folds:
https://rise4fun.com/Alive/fTpN
https://godbolt.org/z/6RLVTQ

And thus we end with:
```
Optimized type-legalized selection DAG: %bb.0 'fn1:entry'
SelectionDAG has 20 nodes:
  t0: ch = EntryToken
              t6: i32,ch = CopyFromReg t0, Register:i32 %2
            t37: i32 = sub Constant:i32<0>, t6
          t39: i32 = sign_extend_inreg t37, ValueType:ch:i16
            t4: i32,ch = CopyFromReg t0, Register:i32 %1
            t2: i32,ch = CopyFromReg t0, Register:i32 %0
          t36: i32,i32 = uaddo t4, t2
        t47: i32,i32 = addcarry t39, Constant:i32<0>, t36:1
      t46: i32 = and t47, Constant:i32<65535>
    t50: ch = br_cc t0, seteq:ch, t46, Constant:i32<65535>, BasicBlock:ch<if.end 0x5618ef707f18>
  t19: ch = br t50, BasicBlock:ch<for.cond.preheader 0x5618ef707d88>
```

I wonder, maybe this can still be undone in DAGCombine.
The important bits are:
```
        t47: i32,i32 = addcarry t39, Constant:i32<0>, t36:1
      t46: i32 = and t47, Constant:i32<65535>
    t50: ch = br_cc t0, seteq:ch, t46, Constant:i32<65535>, BasicBlock:ch<if.end 0x5618ef707f18>
```
I wonder if the problem can be fixed by transforming this back into:
```
      t47: i32,i32 = addcarry t39, Constant:i32<65535>, t36:1
    t50: ch = br_cc t0, seteq:ch, t47, Constant:i32<0>, BasicBlock:ch<if.end 0x5618ef707f18>
```



Repository:
  rL LLVM

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

https://reviews.llvm.org/D62266