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