[PATCH] D62602: [AArch64][SVE2] Add CPU and arch directive tests

Momchil Velikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 31 03:26:40 PDT 2019


chill added inline comments.


================
Comment at: test/MC/AArch64/SVE2/directive-arch-negative.s:3
+
+.arch armv8-a+nosve2
+tbx z0.b, z1.b, z2.b
----------------
c-rhodes wrote:
> chill wrote:
> > Just removing the `+nosve2` part would still make the test pass. It looks to me that `-mattr=+sve2` should be added to the command lime and the test split in five separate files, or otherwise reset arch between test cases.
> Fair point, I don't think separate tests in general for `.arch/.arch_extension/.cpu` etc scales very well and `-mattr=+sve2` doesn't enable all SVE2 extensions, so I think enabling/disabling arch between tests cases makes most sense, e.g.:
> 
> ```
> .arch armv8-a+sve2
> .arch armv8-a+nosve2
> tbx z0.b, z1.b, z2.b
> // CHECK: error: instruction requires: sve2
> // CHECK-NEXT: tbx z0.b, z1.b, z2.b
> 
> .arch armv8-a+sve2-aes
> .arch armv8-a+nosve2-aes
> aesd z23.b, z23.b, z13.b
> // CHECK: error: instruction requires: sve2-aes
> // CHECK-NEXT: aesd z23.b, z23.b, z13.b
> ```
IMHO,  removing an arch extension can only be tested if the architecture included the extension by default.

The approaches above work due to a bug (https://bugs.llvm.org/show_bug.cgi?id=32873), but I guess that's
as good as it gets, so let's go with your variant.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D62602





More information about the llvm-commits mailing list