aaron.ballman added a comment. Clang bits generally LGTM aside from a minor request; I'll leave the final sign-off to an LLVM person so those changes can also be reviewed.
================ Comment at: clang/lib/Sema/SemaDecl.cpp:12092 + diag::err_sme_definition_using_sm_in_non_sme_target); + if (UsesZA) + Diag(NewFD->getLocation(), ---------------- rsandifo-arm wrote: > Suggest making this an `else if`. We only need to give one error for a call > to a streaming shared-ZA function, since the underlying error is the same. Agreed; I think we need only one diagnostic in that case. ================ Comment at: clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp:20 + +// No code is generated for declarations, so it should be fine to declare using the attribute. +void streaming_compatible_decl() __arm_streaming_compatible; // OK ---------------- rsandifo-arm wrote: > aaron.ballman wrote: > > Is this a requirement of the specification? I guess I was surprised we're > > going to these lengths rather than diagnosing the attribute in > > SemaDeclAttr.cpp when sme is not enabled. > Yeah. The problem is that it's possible (and reasonable!) to enable SME for > only part of a translation unit. E.g. an ifunc might have SME and non-SME > implementations, with those implementations being in the same translation > unit. Things like the `target` and `target_version` attributes exist to > allow this. > > A function that's compiled with SME enabled might want to call streaming > functions. There therefore needs to be a way of declaring streaming > functions without assuming that the whole TU is SME. Ah, thank you for the explanation, then this extra effort makes a lot more sense. 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
