[PATCH] D60485: [AArch64] Add support for MTE intrinsics

Tim Northover via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 15 03:02:26 PDT 2019


t.p.northover added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9566
+def err_memtag_arg_null_or_pointer : Error<
+  "%0 argument must be a null or a pointer (%1 invalid)">;
+def err_memtag_any2arg_pointer : Error<
----------------
I think these diagnostics could do with a bit more context for consistency. They seem to take "MTE builtin" for granted, whereas most Clang messages mention what they're talking about.

I'm not saying "MTE builtin" is the best we can come up with, BTW, just that something more would be nice.


================
Comment at: lib/Headers/arm_acle.h:610-615
+#define __arm_mte_create_random_tag(__ptr, __mask)  __builtin_arm_irg(__ptr, __mask)
+#define __arm_mte_increment_tag(__ptr, __tag_offset)  __builtin_arm_addg(__ptr, __tag_offset)
+#define __arm_mte_exclude_tag(__ptr, __excluded)  __builtin_arm_gmi(__ptr, __excluded)
+#define __arm_mte_get_tag(__ptr) __builtin_arm_ldg(__ptr)
+#define __arm_mte_set_tag(__ptr) __builtin_arm_stg(__ptr)
+#define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)
----------------
Why are the builtin names so different from the ones exposed. GCC compatibility? LLVM?


================
Comment at: lib/Sema/SemaChecking.cpp:6113
+bool Sema::SemaBuiltinARMMemoryTaggingCall( unsigned BuiltinID, CallExpr *TheCall) {
+  bool IsMTEBuiltin = BuiltinID == AArch64::BI__builtin_arm_irg ||
+                      BuiltinID == AArch64::BI__builtin_arm_addg ||
----------------
I think this will generate an unused variable warning in a non-asserts build.


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

https://reviews.llvm.org/D60485





More information about the cfe-commits mailing list