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<
----------------
javed.absar wrote:
> t.p.northover wrote:
> > 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.
> I thought of doing that too , so can prefix these warnings with 'mte builtin' 
> if that's what you meant. But the intrinsic called kind of names same thing 
> (__arm_mte_..).
"Builtin" or even "function" (fixed up to be grammatical) would suffice if you 
don't like the repetition.


================
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)
----------------
javed.absar wrote:
> t.p.northover wrote:
> > Why are the builtin names so different from the ones exposed. GCC 
> > compatibility? LLVM?
> The builtin name (e.g. _mte_irg) is reflecting the instruction that 
> implements the otherwise meaningful ACLE names  (mte_create_tag). Its just 
> that the instruction names are sometimes cryptic (e.g. stg, ldg). I could 
> change the names to __builtin_arm_create_tag etc and push the meaningful name 
> -> insn level name to intrinsic level or further down but that would mean 
> lots of name changes to current patch and tests. 
OK, sounds reasonable to leave it as is then.


================
Comment at: lib/Sema/SemaChecking.cpp:6112
+/// SemaBuiltinARMMemoryTaggingCall - Handle calls of memory tagging extensions
+bool Sema::SemaBuiltinARMMemoryTaggingCall( unsigned BuiltinID, CallExpr 
*TheCall) {
+  bool IsMTEBuiltin = BuiltinID == AArch64::BI__builtin_arm_irg ||
----------------
Stray space here between '(' and 'unsigned', BTW.


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

https://reviews.llvm.org/D60485



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to