[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 a subscriber: laytonio.
RKSimon 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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127115