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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/CodeGen/X86/dagcombine-select.ll:202
+; CHECK-NEXT:    # kill: def $cl killed $cl killed $ecx
+; CHECK-NEXT:    shll %cl, %eax
 ; CHECK-NEXT:    retq
----------------
RKSimon wrote:
> deadalnix wrote:
> > Looking into this guy, it is not obvious if the backend is the right place to fix it. Previously, the DAG looked like:
> > 
> > ```
> > SelectionDAG has 17 nodes:
> >   t0: ch = EntryToken
> >                 t2: i32,ch = CopyFromReg t0, Register:i32 %0
> >               t32: i8 = truncate t2
> >             t38: i8 = xor t32, Constant:i8<-1>
> >           t34: i32 = any_extend t38
> >         t36: i32 = and t34, Constant:i32<1>
> >       t30: i32 = shl t36, Constant:i8<2>
> >     t25: i32 = add t30, Constant:i32<4>
> >   t13: ch,glue = CopyToReg t0, Register:i32 $eax, t25
> >   t14: ch = X86ISD::RET_FLAG t13, TargetConstant:i32<0>, Register:i32 $eax, t13:1
> > ```
> > 
> > Now it does look like:
> > ```
> > SelectionDAG has 14 nodes:
> >   t0: ch = EntryToken
> >             t2: i32,ch = CopyFromReg t0, Register:i32 %0
> >           t22: i8 = truncate t2
> >         t24: i8 = and t22, Constant:i8<1>
> >       t21: i8 = sub Constant:i8<3>, t24
> >     t10: i32 = shl Constant:i32<1>, t21
> >   t13: ch,glue = CopyToReg t0, Register:i32 $eax, t10
> >   t14: ch = X86ISD::RET_FLAG t13, TargetConstant:i32<0>, Register:i32 $eax, t13:1
> > ```
> > 
> > It must be noted that opt will transform this IR into:
> > ```
> > define i32 @shl_constant_sel_constants(i1 %cond) local_unnamed_addr #0 {
> >   %bo = select i1 %cond, i32 4, i32 8
> >   ret i32 %bo
> > }
> > ```
> > 
> > Which, regardless of this patch, compiles to:
> > ```
> > shl_constant_sel_constants:             # @shl_constant_sel_constants
> > # %bb.0:
> >         notb    %dil
> >         movzbl  %dil, %eax
> >         andl    $1, %eax
> >         leal    4(,%rax,4), %eax
> >         retq
> > ```
> > 
> > So it seems that this is fine. how do we proceed in such a case? Simply add a second version of that test case with the optimized IR?
> Last person to touch this was @laytonio in D90349 - it looks like this is the same codegen as BEFORE that patch.
D128080 might help this example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127115