[PATCH] D46031: [DAGCombiner] DON'T unfold scalar masked merge if 'B' is constant
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 25 06:46:54 PDT 2018
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
More information about the llvm-commits
mailing list