[PATCH] D55870: [X86] Don't match TESTrr from (cmp (and X, Y), 0) during isel. Defer to post processing
Andrea Di Biagio via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 19 06:06:12 PST 2018
andreadb accepted this revision.
andreadb added a comment.
This revision is now accepted and ready to land.
The change LGTM.
However, tests look a bit too fragile. If the goal is to verify the selection of BMI/TBM instructions, then tests should be made a bit more robust.
Future improvemenets may break some of those patterns; the code from some of those tests can be aggressively simplified...
I suggest to improve the tests first, and then commit this patch.
I think that we should probably raise a couple of bugs for missing simplifications. Most of those missing simplifications can be probably catched at IR level.
================
Comment at: test/CodeGen/X86/bmi.ll:624-635
; X64-LABEL: blsi64_z2:
; X64: # %bb.0:
; X64-NEXT: movq %rsi, %rax
-; X64-NEXT: movq %rdi, %rcx
-; X64-NEXT: negq %rcx
-; X64-NEXT: testq %rdi, %rcx
+; X64-NEXT: blsiq %rdi, %rcx
; X64-NEXT: cmovneq %rdx, %rax
; X64-NEXT: retq
%t0 = sub i64 0, %a
----------------
Again. Here we may prefer POPCNT to BLSI. It tends to have better latency/throughput overall. I think it is worthy to raise a bug for this.
Speaking about these tests in general:
I think that we should make these more robust (maybe in a separate patch).
We can probably make this test more robust by changing how we check the result. For example, rather than comparing against zero, we can compare against a specific power-of-2. That would force the selection of BLSI, since we would need to know the position of that bit.
We can probably do something similar to improve the other test.
================
Comment at: test/CodeGen/X86/bmi.ll:880-882
; X64-NEXT: movl %esi, %eax
-; X64-NEXT: # kill: def $edi killed $edi def $rdi
-; X64-NEXT: leal -1(%rdi), %ecx
-; X64-NEXT: testl %edi, %ecx
+; X64-NEXT: blsrl %edi, %ecx
; X64-NEXT: cmovnel %edx, %eax
----------------
Same.
Could be a simple `test+cmov`. But - again - I take that the purpose of this test is not to check if we are smart enough to fold away that sequence...
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55870/new/
https://reviews.llvm.org/D55870
More information about the llvm-commits
mailing list