[PATCH] D120259: [AArch64][AsmParser] Arch directives should set implied features.

Cullen Rhodes via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 21 13:15:49 PST 2022


c-rhodes added inline comments.


================
Comment at: llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s:12
+
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
----------------
SVE is never enabled so I don't think this is doing what you intended, plus this instruction will be in the output for both pass/fail? I'm not sure it makes sense to mix positive/negative tests like this. As an aside, I'm not sure what my original intention was with this test but it would make sense to enable the feature before disabling it to verify nosve is doing what it's supposed to.


================
Comment at: llvm/test/MC/AArch64/SVE/directive-arch_extension.s:7-12
+
+// Test that the implied +sve feature is also set from +sve2.
+.arch_extension sve2
+
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
----------------
unlike `.arch`, `.arch_extension` doesn't clear any previously selected architecture extensions, probably best to disable SVE with `.arch_extension nosve` first.


================
Comment at: llvm/test/MC/AArch64/SVE/directive-cpu-negative.s:3
 
-.arch_extension nosve
+.cpu generic+nosve
 
----------------
enable SVE before disabling it?


================
Comment at: llvm/test/MC/AArch64/SVE/directive-cpu-negative.s:9-13
+// Check that setting +nosve2 does not imply +nosve
+.cpu generic+sve+nosve2
+
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
----------------
I think this belongs in the positive test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120259



More information about the llvm-commits mailing list