[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 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
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127115