[PATCH] D47452: [NFC][X86][AArch64] Reorganize/cleanup BZHI test patterns

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 3 06:33:17 PDT 2018


lebedev.ri added inline comments.


================
Comment at: test/CodeGen/X86/bmi.ll:3
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+bmi | FileCheck %s --check-prefix=CHECK --check-prefix=BMI1
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+bmi,+bmi2 | FileCheck %s --check-prefix=CHECK --check-prefix=BMI2
 
----------------
lebedev.ri wrote:
> RKSimon wrote:
> > Should we add i686 test coverage for bmi?
> Just duplicate every run line with 32-bit triple?
> I'll defer to @craig.topper, since i don't know whether that would be useful.
This is a very good idea, in retrospect.
Right now the llc completely crashes for any i686 runline here (e.g. can not select `@llvm.x86.bmi.bextr.32`),
so i did not add the 32-bit coverage.


================
Comment at: test/CodeGen/X86/extract-lowbits.ll:12
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi,-tbm,+bmi2 < %s | FileCheck %s --check-prefixes=CHECK,X64,BMI1,X64-BMI1,BMI1BMI2,X64-BMI1BMI2,BMI1NOTBMBMI2,X64-BMI1NOTBMBMI2
+
+; *Please* keep in sync with test/CodeGen/AArch64/extract-lowbits.ll
----------------
RKSimon wrote:
> Given bzhi is a bmi2 op I see very little benefit from testing it on bmi1/tbm only targets, what does it look like if you limit this to 'plain' X86/X64 and BMI2 X86/X64 test?
> 
> Also, you don't appear to have left in all the generated code from the update script.
> 
> 
> what does it look like if you limit this to 'plain' X86/X64 and BMI2 X86/X64 test?
No.
The bmi1/tbm test coverage is needed.
The next step is to express this pattern using `bextr %val (%numlowbits << 8)`,
when there is no bmi2.
(caveat: i did not yet check that it is profitable as per mca, but i'm pretty sure it should be.)

> Also, you don't appear to have left in all the generated code from the update script.
These check prefixes are quite a mystery to me.
I did not manually delete anything here.



Repository:
  rL LLVM

https://reviews.llvm.org/D47452





More information about the llvm-commits mailing list