[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