Page MenuHomePhabricator

[AArch64][SVE2] Add CPU and arch directive tests
ClosedPublic

Authored by c-rhodes on May 29 2019, 8:42 AM.

Details

Summary

This patch adds tests for directives .arch, .arch_extension and .cpu for
all features defined in Arm SVE2 architecture extension.

Diff Detail

Repository
rL LLVM

Event Timeline

c-rhodes created this revision.May 29 2019, 8:42 AM
chill added inline comments.May 30 2019, 7:24 AM
test/MC/AArch64/SVE2/directive-arch-negative.s
3 ↗(On Diff #201937)

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.

test/MC/AArch64/SVE2/directive-arch_extension-negative.s
3 ↗(On Diff #201937)

Likewise here, removing .arch_extension nosve2 (resp. adding it) has no effect on the test outcome.

c-rhodes marked an inline comment as done.May 31 2019, 1:42 AM
c-rhodes added inline comments.
test/MC/AArch64/SVE2/directive-arch-negative.s
3 ↗(On Diff #201937)

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
test/MC/AArch64/SVE2/directive-arch_extension-negative.s
3 ↗(On Diff #201937)

I can take the same approach as above if you think that makes sense

c-rhodes marked an inline comment as not done.May 31 2019, 1:42 AM
chill added inline comments.May 31 2019, 3:25 AM
test/MC/AArch64/SVE2/directive-arch-negative.s
3 ↗(On Diff #201937)

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.

c-rhodes updated this revision to Diff 202683.Jun 3 2019, 3:12 AM

Addressed Momchil's comments: explicitly enable/disable arch features in negative directive tests.

chill accepted this revision.Jun 3 2019, 3:21 AM
This revision is now accepted and ready to land.Jun 3 2019, 3:21 AM
This revision was automatically updated to reflect the committed changes.