[PATCH] D46031: [DAGCombiner] DON'T unfold scalar masked merge if 'B' is constant

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

I have stared at the test change a bit more, and unless there are some other patterns i did not analyze, i do think this is the way we want to handle this.



================
Comment at: test/CodeGen/X86/unfold-masked-merge-scalar-variablemask.ll:573-574
 ; CHECK-BMI:       # %bb.0:
-; CHECK-BMI-NEXT:    andl %edx, %edi
-; CHECK-BMI-NEXT:    notl %edx
-; CHECK-BMI-NEXT:    orl %edi, %edx
-; CHECK-BMI-NEXT:    movl %edx, %eax
+; CHECK-BMI-NEXT:    andnl %edx, %edi, %eax
+; CHECK-BMI-NEXT:    notl %eax
 ; CHECK-BMI-NEXT:    retq
----------------
This improves.


================
Comment at: test/CodeGen/X86/unfold-masked-merge-scalar-variablemask.ll:613-615
+; CHECK-BMI-NEXT:    notl %edx
+; CHECK-BMI-NEXT:    andnl %edx, %edi, %eax
+; CHECK-BMI-NEXT:    notl %eax
----------------
This degrades, but instcombine will canonicalize it to `@in_constant_mone_vary`, and that one is ok.
So this is ok too.


================
Comment at: test/CodeGen/X86/unfold-masked-merge-scalar-variablemask.ll:658
 ; CHECK-BMI:       # %bb.0:
+; CHECK-BMI-NEXT:    xorl $42, %edi
 ; CHECK-BMI-NEXT:    andl %edx, %edi
----------------
Looked at this in llvm-mca, no clear winner.
The change decreases instruction count and IPC, but the cycle count does not change.
So i guess it's ok?


================
Comment at: test/CodeGen/X86/unfold-masked-merge-scalar-variablemask.ll:704
+; CHECK-BMI-NEXT:    xorl $42, %edi
 ; CHECK-BMI-NEXT:    andnl %edi, %edx, %eax
+; CHECK-BMI-NEXT:    xorl $42, %eax
----------------
As per mca this is an unimportant change, but again, instcombine will canonicalize it to `@in_constant_42_vary`, which is ok.
So this one appears ok too.


Repository:
  rL LLVM

https://reviews.llvm.org/D46031