[PATCH] D127115: [RFC][DAGCombine] Make sure combined nodes are added back to the worklist in topological order.

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix added a comment.

In D127115#3674124 <https://reviews.llvm.org/D127115#3674124>, @RKSimon wrote:

> Please have a go if you're interested - I've wasted ages trying to get it to work in a better way.

So exploring the situation, it appears to me that rG2421a5af72e7da7c29fd153dcee0981deaaeba64 <https://reviews.llvm.org/rG2421a5af72e7da7c29fd153dcee0981deaaeba64> has oen effect: change the order in which node are processed. The optimization happens anyway in the combiner later on, but the factorization is somewhat different as a result:

main:

  SelectionDAG has 21 nodes:
    t0: ch = EntryToken
    t49: i32,i32 = uaddo t41, Constant:i32<-2147483648>
    t45: i32,i32 = uaddo t44, Constant:i32<-2147483648>
  
    t46: i32,i32 = addcarry t44:1, Constant:i32<0>, t45:1
    t51: i32,i32 = addcarry t41:1, t46, t49:1
  
    t29: ch,glue = CopyToReg t0, Register:i32 $r0, t51
      t8: i32,ch = CopyFromReg t0, Register:i32 %3
      t4: i32,ch = CopyFromReg t0, Register:i32 %1
    t41: i32,i32 = smul_lohi t8, t4
      t42: i32,ch = load<(load (s32) from %fixed-stack.0, align 8)> t0, FrameIndex:i32<-1>, undef:i32
      t6: i32,ch = CopyFromReg t0, Register:i32 %2
    t44: i32,i32 = smul_lohi t42, t6
    t30: ch = ARMISD::RET_FLAG t29, Register:i32 $r0, t29:1

vs

This patch:

  SelectionDAG has 21 nodes:
    t0: ch = EntryToken
    t49: i32,i32 = uaddo t41, Constant:i32<-2147483648>
    t45: i32,i32 = uaddo t44, Constant:i32<-2147483648>
  
    t53: i32,i32 = addcarry t41:1, t44:1, t45:1
    t50: i32,i32 = addcarry t53, Constant:i32<0>, t49:1
  
    t29: ch,glue = CopyToReg t0, Register:i32 $r0, t50
      t8: i32,ch = CopyFromReg t0, Register:i32 %3
      t4: i32,ch = CopyFromReg t0, Register:i32 %1
    t41: i32,i32 = smul_lohi t8, t4
      t42: i32,ch = load<(load (s32) from %fixed-stack.0, align 8)> t0, FrameIndex:i32<-1>, undef:i32
      t6: i32,ch = CopyFromReg t0, Register:i32 %2
    t44: i32,i32 = smul_lohi t42, t6
    t30: ch = ARMISD::RET_FLAG t29, Register:i32 $r0, t29:1

New lines added by myself to isolate the meaningful difference: the two addcarry. It looks like to me that, as the ordering become more and more set, we'll see that problem return. Should we try to push zeros or constants in general through ADDCARRYs? What's the policy for reassociation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127115