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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon 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
----------------
deadalnix wrote:
> 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?
Adding a select(c2, select(c1, x, y), y) fold is possible.

Another option is to just alter the test - InstCombine will now optimize this code to a reduction pattern: https://simd.godbolt.org/z/hfv3YW4Y6
```
define i32 @PR39665_c_ray(<2 x double> %x, <2 x double> %y) {
  %cmp = fcmp ogt <2 x double> %x, %y
  %shift = shufflevector <2 x i1> %cmp, <2 x i1> poison, <2 x i32> <i32 1, i32 undef>
  %1 = and <2 x i1> %cmp, %shift
  %u = extractelement <2 x i1> %1, i64 0
  %r = select i1 %u, i32 42, i32 99
  ret i32 %r
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124743