[PATCH] D127910: [Clang][AArch64] Add SME C intrinsics for load and store

Thu Jun 16 11:39:31 PDT 2022

sagarkulkarni19 added inline comments.

================
Comment at: clang/include/clang/Basic/arm_sve.td:209
 def IsTupleSet                : FlagType<0x400000000>;
+def IsSME                     : FlagType<0x800000000>;
+def IsSMELoadStore            : FlagType<0x1000000000>;
----------------
sdesmalen wrote:
> Is there value in having both `IsSME` and `IsSMELoadStore`?
`IsSME` flag is checked in the SveEmitter.cpp : createSMEHeader(...) to generate arm_sme.h with only the SME intrinsics, whereas `IsSMELoadStore` is used to correctly CodeGen (CGBuiltins.cpp) load and store intrinsics. 

================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:342
+  if (HasSME)
+    Builder.defineMacro("__ARM_FEATURE_SME", "1");
+
----------------
sdesmalen wrote:
> When this macro is non-zero, it suggests that the compiler implements the full SME ACLE. That is currently not yet the case, so until then we should leave this macro undefined.
Okay makes sense. But until all SME ACLE is implemented, do we need some sort of guard in the meantime while the SME intrinsics that are implemented incrementally? 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127910