paulwalker-arm added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:6762
+ Context.getFunctionFeatureMap(CallerFeatureMap, CallerFD);
+ if (!CallerFeatureMap.count("sme"))
+ Diag(Loc, diag::err_sme_call_in_non_sme_target);
----------------
`contains("sme")` seems more appropriate here?
================
Comment at: clang/lib/Sema/SemaDecl.cpp:12159
+ Context.getFunctionFeatureMap(FeatureMap, NewFD);
+ if (!FeatureMap.count("sme")) {
+ if (UsesSM)
----------------
As above.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:12163
+ diag::err_sme_definition_using_sm_in_non_sme_target);
+ else if (UsesZA)
+ Diag(NewFD->getLocation(),
----------------
Can this be just `else` given by this point I believe you know `UsesZA` has to
be true.
================
Comment at: clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve
-fsyntax-only -verify %s
+
----------------
Does the test require SVE to be enabled?
================
Comment at: clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp:3
+
+// This test is testing the diagnostic that Clang emits when compiling without
'+sme'.
+
----------------
diagnostics
================
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
----------------
Is it worth including tests where "sme2" is used? or are we already comfortable
feature inheritance is well tested?
================
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]> {
----------------
Doesn't this class belong in SMEInstrFormats.td, then you'll not need to
override `Predicates`?
================
Comment at:
llvm/test/CodeGen/AArch64/sme-call-streaming-compatible-to-normal-fn-wihout-sme-attr.ll:4
+
+; This that the following code can be compiled without +sme, because if the
+; call is not entered in streaming-SVE mode at runtime, the codepath leading
----------------
Verify the...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157269/new/
https://reviews.llvm.org/D157269
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
