Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AArch64: add __builtin_arm_trap #85054

Closed
wants to merge 1 commit into from

Conversation

TNorthover
Copy link
Contributor

It's useful to provide an indicator code with the trap, which the generic __builtin_trap can't do. asm("brk #N") is an option, but following that with a __builtin_unreachable() leads to two traps when the compiler doesn't know the block can't return. So compiler support like this is useful.

It's useful to provide an indicator code with the trap, which the generic
__builtin_trap can't do. asm("brk #N") is an option, but following that with a
__builtin_unreachable() leads to two traps when the compiler doesn't know the
block can't return. So compiler support like this is useful.
@TNorthover TNorthover requested a review from aemerson March 13, 2024 10:08
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Mar 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 13, 2024

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Tim Northover (TNorthover)

Changes

It's useful to provide an indicator code with the trap, which the generic __builtin_trap can't do. asm("brk #N") is an option, but following that with a __builtin_unreachable() leads to two traps when the compiler doesn't know the block can't return. So compiler support like this is useful.


Full diff: https://github.com/llvm/llvm-project/pull/85054.diff

4 Files Affected:

  • (modified) clang/include/clang/Basic/BuiltinsAArch64.def (+3)
  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+6)
  • (modified) clang/test/CodeGen/builtins-arm64.c (+6)
  • (modified) clang/test/Sema/builtins-arm64.c (+9)
diff --git a/clang/include/clang/Basic/BuiltinsAArch64.def b/clang/include/clang/Basic/BuiltinsAArch64.def
index b5cbe90c8fd6a3..cf8711c6eaee37 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -50,6 +50,9 @@ BUILTIN(__builtin_arm_wfi, "v", "")
 BUILTIN(__builtin_arm_sev, "v", "")
 BUILTIN(__builtin_arm_sevl, "v", "")
 
+// Like __builtin_trap but provide an 16-bit immediate reason code (which goes into `brk #N`).
+BUILTIN(__builtin_arm_trap, "vUIs", "nr")
+
 // CRC32
 TARGET_BUILTIN(__builtin_arm_crc32b, "UiUiUc", "nc", "crc")
 TARGET_BUILTIN(__builtin_arm_crc32cb, "UiUiUc", "nc", "crc")
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 93ab465079777b..528a13fb275124 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -10686,6 +10686,12 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
     return Builder.CreateCall(F, llvm::ConstantInt::get(Int32Ty, HintID));
   }
 
+  if (BuiltinID == clang::AArch64::BI__builtin_arm_trap) {
+    Function *F = CGM.getIntrinsic(Intrinsic::aarch64_break);
+    llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+    return Builder.CreateCall(F, Builder.CreateZExt(Arg, CGM.Int32Ty));
+  }
+
   if (BuiltinID == clang::AArch64::BI__builtin_arm_get_sme_state) {
     // Create call to __arm_sme_state and store the results to the two pointers.
     CallInst *CI = EmitRuntimeCall(CGM.CreateRuntimeFunction(
diff --git a/clang/test/CodeGen/builtins-arm64.c b/clang/test/CodeGen/builtins-arm64.c
index 05ea1c719edff3..8bd68d9ceb48ec 100644
--- a/clang/test/CodeGen/builtins-arm64.c
+++ b/clang/test/CodeGen/builtins-arm64.c
@@ -156,4 +156,10 @@ int rndrrs(uint64_t *__addr) {
   return __builtin_arm_rndrrs(__addr);
 }
 
+// CHECK-LABEL: @trap(
+// CHECK: call void @llvm.aarch64.break(i32 42)
+void trap() {
+  __builtin_arm_trap(42);
+}
+
 // CHECK: ![[M0]] = !{!"1:2:3:4:5"}
diff --git a/clang/test/Sema/builtins-arm64.c b/clang/test/Sema/builtins-arm64.c
index e711121f7260ff..f094162b3aadc9 100644
--- a/clang/test/Sema/builtins-arm64.c
+++ b/clang/test/Sema/builtins-arm64.c
@@ -29,3 +29,12 @@ void test_prefetch(void) {
   __builtin_arm_prefetch(0, 0, 0, 2, 0); // expected-error-re {{argument value {{.*}} is outside the valid range}}
   __builtin_arm_prefetch(0, 0, 0, 0, 2); // expected-error-re {{argument value {{.*}} is outside the valid range}}
 }
+
+void test_trap(short s, unsigned short us) {
+  __builtin_arm_trap(42);
+  __builtin_arm_trap(65535);
+  __builtin_arm_trap(-1);
+  __builtin_arm_trap(65536); // expected-warning {{implicit conversion from 'int' to 'unsigned short' changes value from 65536 to 0}}
+  __builtin_arm_trap(s); // expected-error {{argument to '__builtin_arm_trap' must be a constant integer}}
+  __builtin_arm_trap(us); // expected-error {{argument to '__builtin_arm_trap' must be a constant integer}}
+}
\ No newline at end of file

Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@DanielKristofKiss DanielKristofKiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a line or two in the clang/docs/LanguageExtensions.rst would be useful.

@TNorthover
Copy link
Contributor Author

Thanks. Good idea on the docs, I've added some wording and pushed the change (4299c72)

@TNorthover TNorthover closed this Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants