[PATCH] D124743: [DAGCombine] Add node in the worklist in topological order in CombineTo

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix added inline comments.


================
Comment at: llvm/test/CodeGen/X86/movmsk-cmp.ll:4564
+; AVX1OR2-NEXT:    testb $1, %cl
+; AVX1OR2-NEXT:    cmovel %edx, %eax
 ; AVX1OR2-NEXT:    retq
----------------
I investigated this regression.

before this patch, the DAG after type legalization and optimization looks like:
```SelectionDAG has 25 nodes:
  t0: ch = EntryToken
            t62: i8 = xor t52, Constant:i8<-1>
          t55: i8 = and t62, Constant:i8<1>
          t53: i8 = and t52, Constant:i8<2>
        t56: i8 = or t55, t53
      t57: i8 = setcc t56, Constant:i8<2>, seteq:ch
    t40: i32 = select t57, Constant:i32<42>, Constant:i32<99>
  t19: ch,glue = CopyToReg t0, Register:i32 $eax, t40
        t2: v2f64,ch = CopyFromReg t0, Register:v2f64 %0
        t4: v2f64,ch = CopyFromReg t0, Register:v2f64 %1
      t41: v2i64 = setcc t2, t4, setogt:ch
    t24: i32 = X86ISD::MOVMSK t41
  t52: i8 = truncate t24
  t20: ch = X86ISD::RET_FLAG t19, TargetConstant:i32<0>, Register:i32 $eax, t19:1
```

After, it looks like:
```
SelectionDAG has 20 nodes:
  t0: ch = EntryToken
      t55: i8 = and t46, Constant:i8<1>
        t56: i8 = srl t46, Constant:i8<1>
      t21: i32 = select t56, Constant:i32<42>, Constant:i32<99>
    t22: i32 = select t55, t21, Constant:i32<99>
  t19: ch,glue = CopyToReg t0, Register:i32 $eax, t22
        t2: v2f64,ch = CopyFromReg t0, Register:v2f64 %0
        t4: v2f64,ch = CopyFromReg t0, Register:v2f64 %1
      t40: v2i64 = setcc t2, t4, setogt:ch
    t24: i32 = X86ISD::MOVMSK t40
  t46: i8 = truncate t24
  t20: ch = X86ISD::RET_FLAG t19, TargetConstant:i32<0>, Register:i32 $eax, t19:1
```

Arguably, the DAG after this patch looks simpler/better, so it is hard to argue this does the wrong thing here. However, the rest of the backend doesn't seems to be able to do as good of a job on this DAG. Is there something that can be done here? It seems like there is a legitimate blind spot in the X86 backend rather than a problem with this patch. @RKSimon maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124743