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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/test/CodeGen/ARM/dsp-mlal.ll:39
+; NODSP-NEXT:    bx lr
 entry:
   %conv = sext i32 %b1 to i64
----------------
craig.topper wrote:
> RKSimon wrote:
> > RKSimon wrote:
> > > BEFORE:
> > > ```
> > > SelectionDAG has 21 nodes:
> > >   t0: ch = EntryToken
> > >         t45: i32,i32 = uaddo t44, Constant:i32<-2147483648>
> > >       t46: i32,i32 = addcarry t44:1, Constant:i32<0>, t45:1
> > >       t49: i32,i32 = uaddo t41, Constant:i32<-2147483648>
> > >     t52: i32,i32 = addcarry t41:1, t46, t49:1
> > >   t29: ch,glue = CopyToReg t0, Register:i32 $r0, t52
> > >     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
> > > ```
> > > AFTER:
> > > ```
> > > SelectionDAG has 21 nodes:
> > >   t0: ch = EntryToken
> > >         t45: i32,i32 = uaddo t44, Constant:i32<-2147483648>
> > >       t53: i32,i32 = addcarry t41:1, t44:1, t45:1
> > >       t49: i32,i32 = uaddo t41, Constant:i32<-2147483648>
> > >     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
> > > ```
> > DAGTypeLegalizer::ExpandIntRes_ADDSUB is creating UADDO(X, 0) nodes and it takes too long to get rid of them, even if I perform the fold in getNode it takes too long as we need a MERGE_VALUES node to bind the two results together.
> > 
> > 2 possible hacks might work- (1) we just handle the no-overflow case in ExpandIntRes_ADDSUB, or (2) we replace Lo.getValue(1) calls that access the overflow flag result with a DAG.getValue() wrapper that attempts to peek through MERGE_VALUES nodes. I don't really like either of these.....
> I haven’t been following this fully. Can you use CombineTo to avoid the MERGE_VALUES?
This is inside SelectionDAG called from the legalizer, we're not combining an existing node.

There's already getBuildPairElt in DAGCombiner.cpp to help peek through MERGE_VALUES nodes so I guess there's precedent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127115