This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add support for MTE intrinsics
ClosedPublic

Authored by javed.absar on Apr 9 2019, 1:46 PM.

Details

Summary

This patch provides intrinsics support for Memory Tagging Extension (MTE),
which was introduced with the Armv8.5-a architecture.
This is clang patch. Corresponding llvm-patch is at: https://reviews.llvm.org/D60486.

These intrinsics are available when __ARM_FEATURE_MEMORY_TAGGING is defined.
Each intrinsic is described in detail in the latest ACLE Q1 2019 documentation:

https://developer.arm.com/docs/101028/latest

However, below we also list the intrinsics:

  1. T* __arm_mte_create_random_tag(T* src, uint64_t mask); This intrinsic returns a pointer containing a randomly created logical address tag.
  2. T* __arm_mte_increment_tag(T* src, unsigned offset); This intrinsic returns a pointer which is a copy of the input pointer src but with the logical address tag part offset by a specified offset value.
  3. uint64_t __arm_mte_exclude_tag(T* src, uint64_t excluded); This intrinsic adds a logical tag to the set of excluded logical tags.
  4. void __arm_mte_set_tag(T* tag_address); This intrinsic stores an allocation tag, computed from the logical tag, to the tag memory thereby setting the allocation tag for the 16-byte granule of memory.
  5. T* __arm_mte_get_tag(T* address); This intrinsic loads the allocation tag from tag memory and returns the corresponding logical tag as part of the returned pointer value.
  6. ptrdiff_t __arm_mte_ptrdiff(T* a, T* b); The intrinsic calculates the difference between the address parts of the two pointers, ignoring the tags.

Diff Detail

Repository
rC Clang

Event Timeline

javed.absar created this revision.Apr 9 2019, 1:46 PM
javed.absar edited the summary of this revision. (Show Details)Apr 9 2019, 1:47 PM
javed.absar edited the summary of this revision. (Show Details)Apr 9 2019, 2:00 PM
DavidSpickett added a comment.EditedApr 10 2019, 3:27 AM

Do we need a separate file for that one "arm_mte_ptrdiff" test? Wouldn't it be easier to wrap the whole function in "__cplusplus" and put it in the same file.

Tests merged as suggested.

t.p.northover added inline comments.
include/clang/Basic/DiagnosticSemaKinds.td
9626

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.

lib/Headers/arm_acle.h
610–615

Why are the builtin names so different from the ones exposed. GCC compatibility? LLVM?

lib/Sema/SemaChecking.cpp
6117

I think this will generate an unused variable warning in a non-asserts build.

javed.absar marked 2 inline comments as done.Apr 15 2019, 3:41 AM
javed.absar added inline comments.
include/clang/Basic/DiagnosticSemaKinds.td
9626

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_..).

lib/Headers/arm_acle.h
610–615

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.

t.p.northover added inline comments.Apr 15 2019, 5:36 AM
include/clang/Basic/DiagnosticSemaKinds.td
9626

"Builtin" or even "function" (fixed up to be grammatical) would suffice if you don't like the repetition.

lib/Headers/arm_acle.h
610–615

OK, sounds reasonable to leave it as is then.

lib/Sema/SemaChecking.cpp
6116

Stray space here between '(' and 'unsigned', BTW.

javed.absar marked an inline comment as done.

Hi Tim:
Have made the changes as suggested. Please have a look.

t.p.northover added inline comments.Apr 24 2019, 3:13 AM
include/clang/Sema/Sema.h
10762

Slightly misaligned.

lib/CodeGen/CGBuiltin.cpp
7129–7131

I think this should be fixed now. It looks like technical debt from the fact that the instructions only fairly recently gained that feature after the intrinsics were implemented internally. There's no good way to justify the current semantics to someone unaware of that history.

javed.absar marked an inline comment as done.Apr 25 2019, 3:52 AM
javed.absar added inline comments.
lib/CodeGen/CGBuiltin.cpp
7129–7131

Not quite that really. So the instruction did gain the feature recently like you mentioned. But the ACLE/intrinsics were designed and agreed upon after it and it was decided in ACLE discussions that the exta feature added complexity that need not be exposed at ACLE level yet. No big use case to justify complicating the ACLE MTE spec yet. Directly assembly can use that instruction though.

t.p.northover accepted this revision.Apr 25 2019, 11:31 AM
t.p.northover added inline comments.
lib/CodeGen/CGBuiltin.cpp
7129–7131

I think the ACLE decision was a mistake, but since it happened I withdraw this request. I expect (and hope) far more people will use these through ACLE than as compiler-specific builtins.

This revision is now accepted and ready to land.Apr 25 2019, 11:31 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2019, 2:07 PM

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/12035

-- Testing: 14692 tests, 96 threads --
Testing: 
FAIL: Clang :: AST/float16.cpp (132 of 14692)
******************** TEST 'Clang :: AST/float16.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/lib/clang/9.0.0/include -nostdsysteminc -std=c++11 -ast-dump -triple aarch64-linux-gnu /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/test/AST/float16.cpp | /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/test/AST/float16.cpp --strict-whitespace
: 'RUN: at line 2';   /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/lib/clang/9.0.0/include -nostdsysteminc -std=c++11 -ast-dump -triple aarch64-linux-gnu -fnative-half-type /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/test/AST/float16.cpp | /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/test/AST/float16.cpp --check-prefix=CHECK-NATIVE --strict-whitespace
--
Exit Code: 2

Command Output (stderr):
--
==12261==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0xd4212a8 in clang::targets::AArch64TargetInfo::getTargetDefines(clang::LangOptions const&, clang::MacroBuilder&) const /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Basic/Targets/AArch64.cpp:197:7
    #1 0xd422b5b in clang::targets::AArch64leTargetInfo::getTargetDefines(clang::LangOptions const&, clang::MacroBuilder&) const /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Basic/Targets/AArch64.cpp:463:22
    #2 0xd3ee87e in clang::targets::OSTargetInfo<clang::targets::AArch64leTargetInfo>::getTargetDefines(clang::LangOptions const&, clang::MacroBuilder&) const /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Basic/Targets/OSTargets.h:33:14
    #3 0x784d4c7 in InitializePredefinedMacros(clang::TargetInfo const&, clang::LangOptions const&, clang::FrontendOptions const&, clang::MacroBuilder&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Frontend/InitPreprocessor.cpp:1087:6
    #4 0x7830b1f in clang::InitializePreprocessor(clang::Preprocessor&, clang::PreprocessorOptions const&, clang::PCHContainerReader const&, clang::FrontendOptions const&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Frontend/InitPreprocessor.cpp:1117:5
    #5 0x76a550e in clang::CompilerInstance::createPreprocessor(clang::TranslationUnitKind) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:392:3
    #6 0x779314b in clang::FrontendAction::BeginSourceFile(clang::CompilerInstance&, clang::FrontendInputFile const&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:742:8
    #7 0x76b49f8 in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:943:13
    #8 0x79d07f6 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:271:25
    #9 0xb19a3a in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/tools/driver/cc1_main.cpp:225:15
    #10 0xb12983 in ExecuteCC1Tool /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/tools/driver/driver.cpp:309:12
    #11 0xb12983 in main /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/tools/driver/driver.cpp:381
    #12 0x7f69ddc1c2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #13 0xa91139 in _start (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/clang-9+0xa91139)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/lib/Basic/Targets/AArch64.cpp:197:7 in clang::targets::AArch64TargetInfo::getTargetDefines(clang::LangOptions const&, clang::MacroBuilder&) const
Exiting
FileCheck error: '-' is empty.
FileCheck command line:  /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm/tools/clang/test/AST/float16.cpp --strict-whitespace

FYI @hctim

fixed with r359366