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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.


================
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
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > 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>
> > ```
> > 
> Or more specifically, https://rise4fun.com/Alive/slGX
> This would be an ok transform here because the second argument of `addcarry` is 0 here anyway.
Or without hardcoded constants: https://rise4fun.com/Alive/B2k
Not sure if it's worthwhile doing this if it requires creating whole new 'add',
but here we should be able to do it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62266