[PATCH] D157269: [Clang][AArch64] Diagnostics for SME attributes when target doesn't have 'sme'

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp:13-17
+__attribute__((target("sme"))) void streaming_compatible_def_sme_attr() __arm_streaming_compatible {} // OK
+__attribute__((target("sme"))) void streaming_def_sme_attr() __arm_streaming { } // OK
+__attribute__((target("sme"))) void shared_za_def_sme_attr() __arm_shared_za { } // OK
+__arm_new_za __attribute__((target("sme"))) void new_za_def_sme_attr() {} // OK
+__arm_locally_streaming __attribute__((target("sme"))) void locally_streaming_def_sme_attr() {} // OK
----------------
paulwalker-arm wrote:
> Is it worth including tests where "sme2" is used? or are we already comfortable feature inheritance is well tested?
I'm not sure how well this is tested, but I guess there's no harm in adding an extra test for it.


================
Comment at: llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td:144-163
 // It's tricky to using the existing pstate operand defined in
 // AArch64SystemOperands.td since it only encodes 5 bits including op1;op2,
 // when these fields are also encoded in CRm[3:1].
 def MSRpstatesvcrImm1
   : PstateWriteSimple<(ins svcr_op:$pstatefield, timm0_1:$imm), "msr",
                       "\t$pstatefield, $imm">,
     Sched<[WriteSys]> {
----------------
paulwalker-arm wrote:
> Doesn't this class belong in SMEInstrFormats.td, then you'll not need to override `Predicates`?
Good point, I've moved the class over and the InstAliases as well (which I guess also shouldn't depend on SME being available)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157269