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

Cullen Rhodes via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 31 01:42:35 PDT 2019


c-rhodes marked an inline comment as done.
c-rhodes 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
----------------
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
```


================
Comment at: test/MC/AArch64/SVE2/directive-arch_extension-negative.s:3
+
+.arch_extension nosve2
+tbx z0.b, z1.b, z2.b
----------------
chill wrote:
> Likewise here, removing `.arch_extension nosve2` (resp. adding it)  has no effect on the test outcome.
I can take the same approach as above if you think that makes sense


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