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

[clang][builtin] Implement __builtin_allow_runtime_check #87568

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Apr 3, 2024

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Apr 3, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 3, 2024

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Vitaly Buka (vitalybuka)

Changes

RFC: https://discourse.llvm.org/t/rfc-add-llvm-experimental-hot-intrinsic-or-llvm-hot/77641


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

6 Files Affected:

  • (modified) clang/docs/LanguageExtensions.rst (+28)
  • (modified) clang/include/clang/Basic/Builtins.td (+6)
  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+11-11)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+11)
  • (added) clang/test/CodeGen/builtin-allow-runtime-check.cpp (+32)
  • (added) clang/test/Sema/builtin-allow-runtime-check.c (+22)
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 7b23e4d1c2f30c..41d364fcc9a390 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3464,6 +3464,34 @@ Query for this feature with ``__has_builtin(__builtin_trap)``.
 
 ``__builtin_arm_trap`` is lowered to the ``llvm.aarch64.break`` builtin, and then to ``brk #payload``.
 
+``__builtin_allow_runtime_check``
+------------------
+
+``__builtin_allow_runtime_check`` return true if the check at the current program location should be executed.
+
+**Syntax**:
+
+.. code-block:: c++
+
+    bool __builtin_allow_runtime_check(const char* kind)
+
+**Example of use**:
+
+.. code-block:: c++
+
+  if (__builtin_allow_runtime_check("mycheck") && !ExpensiveCheck()) {
+     abort();
+  }
+
+**Description**
+
+``__builtin_allow_runtime_check`` is lowered to ` ``llvm.allow.runtime.check`` <https://llvm.org/docs/LangRef.html#llvm-allow-runtime-check-intrinsic>`_ builtin.
+
+The ``__builtin_allow_runtime_check()`` builtin is typically used with control flow
+conditions such as in ``if`` to guard expensive runtime checks. The specific rules for selecting permitted checks can differ and are controlled by the compiler options.
+
+Query for this feature with ``__has_builtin(__builtin_allow_runtime_check)``.
+
 ``__builtin_nondeterministic_value``
 ------------------------------------
 
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index f421223ff087de..64f805ec2e4c14 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -1164,6 +1164,12 @@ def Unreachable : Builtin {
   let Prototype = "void()";
 }
 
+def AllowRuntimeCheck : Builtin {
+  let Spellings = ["__builtin_allow_runtime_check"];
+  let Attributes = [NoThrow, Pure, Const];
+  let Prototype = "bool(char const*)";
+}
+
 def ShuffleVector : Builtin {
   let Spellings = ["__builtin_shufflevector"];
   let Attributes = [NoThrow, Const, CustomTypeChecking];
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 483f9c26859923..f4809f09e025a2 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -3423,17 +3423,17 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     Builder.CreateCall(FnAssume, ArgValue);
     return RValue::get(nullptr);
   }
-  case Builtin::BI__builtin_assume_separate_storage: {
-    const Expr *Arg0 = E->getArg(0);
-    const Expr *Arg1 = E->getArg(1);
-
-    Value *Value0 = EmitScalarExpr(Arg0);
-    Value *Value1 = EmitScalarExpr(Arg1);
-
-    Value *Values[] = {Value0, Value1};
-    OperandBundleDefT<Value *> OBD("separate_storage", Values);
-    Builder.CreateAssumption(ConstantInt::getTrue(getLLVMContext()), {OBD});
-    return RValue::get(nullptr);
+  case Builtin::BI__builtin_allow_runtime_check: {
+    StringRef Kind =
+        cast<StringLiteral>(E->getArg(0)->IgnoreParenCasts())->getString();
+    LLVMContext &Ctx = CGM.getLLVMContext();
+    llvm::Metadata *KindStr[] = {llvm::MDString::get(Ctx, Kind)};
+    llvm::MDNode *KindNode = llvm::MDNode::get(Ctx, KindStr);
+    llvm::Value *KindMD = llvm::MetadataAsValue::get(Ctx, KindNode);
+    llvm::Value *Allow = Builder.CreateCall(
+        CGM.getIntrinsic(llvm::Intrinsic::allow_runtime_check), KindMD);
+
+    return RValue::get(Allow);
   }
   case Builtin::BI__arithmetic_fence: {
     // Create the builtin call if FastMath is selected, and the target
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3dcd18b3afc8b4..3ac456d5c54885 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3235,6 +3235,17 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
     if (SemaBuiltinCountZeroBitsGeneric(*this, TheCall))
       return ExprError();
     break;
+
+  case Builtin::BI__builtin_allow_runtime_check: {
+    Expr *Arg = TheCall->getArg(0)->IgnoreParenImpCasts();
+    // Check if the argument is a string literal.
+    if (!isa<StringLiteral>(Arg->IgnoreParenImpCasts())) {
+      Diag(TheCall->getBeginLoc(), diag::err_expr_not_string_literal)
+          << Arg->getSourceRange();
+      return ExprError();
+    }
+    break;
+  }
   }
 
   if (getLangOpts().HLSL && CheckHLSLBuiltinFunctionCall(BuiltinID, TheCall))
diff --git a/clang/test/CodeGen/builtin-allow-runtime-check.cpp b/clang/test/CodeGen/builtin-allow-runtime-check.cpp
new file mode 100644
index 00000000000000..bb71190db4ce67
--- /dev/null
+++ b/clang/test/CodeGen/builtin-allow-runtime-check.cpp
@@ -0,0 +1,32 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 4
+// RUN: %clang_cc1 -cc1 -triple x86_64-pc-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+static_assert(__has_builtin(__builtin_allow_runtime_check), "");
+
+// CHECK-LABEL: define dso_local noundef zeroext i1 @_Z4testv(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call i1 @llvm.allow.runtime.check(metadata [[META2:![0-9]+]])
+// CHECK-NEXT:    ret i1 [[TMP0]]
+//
+bool test() {
+  return __builtin_allow_runtime_check("mycheck");
+}
+
+// CHECK-LABEL: define dso_local noundef zeroext i1 @_Z10test_twicev(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[TMP0:%.*]] = call i1 @llvm.allow.runtime.check(metadata [[META2]])
+// CHECK-NEXT:    [[CONV:%.*]] = zext i1 [[TMP0]] to i32
+// CHECK-NEXT:    [[TMP1:%.*]] = call i1 @llvm.allow.runtime.check(metadata [[META2]])
+// CHECK-NEXT:    [[CONV1:%.*]] = zext i1 [[TMP1]] to i32
+// CHECK-NEXT:    [[OR:%.*]] = or i32 [[CONV]], [[CONV1]]
+// CHECK-NEXT:    [[TOBOOL:%.*]] = icmp ne i32 [[OR]], 0
+// CHECK-NEXT:    ret i1 [[TOBOOL]]
+//
+bool test_twice() {
+  return __builtin_allow_runtime_check("mycheck") | __builtin_allow_runtime_check("mycheck");
+}
+//.
+// CHECK: [[META2]] = !{!"mycheck"}
+//.
diff --git a/clang/test/Sema/builtin-allow-runtime-check.c b/clang/test/Sema/builtin-allow-runtime-check.c
new file mode 100644
index 00000000000000..f4a94fa66321e5
--- /dev/null
+++ b/clang/test/Sema/builtin-allow-runtime-check.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-linux-gnu -verify %s
+// RUN: %clang_cc1 -fsyntax-only -triple aarch64-linux-gnu -verify %s
+
+extern const char *str;
+
+int main(void) {
+  int r = 0;
+
+  r |= __builtin_allow_runtime_check(str); // expected-error {{expression is not a string literal}}
+
+  r |= __builtin_allow_runtime_check(5); // expected-error {{incompatible integer to pointer conversion}} expected-error {{expression is not a string literal}}
+
+  r |= __builtin_allow_runtime_check("a", "b");  // expected-error {{too many arguments to function call}}
+
+  r |= __builtin_allow_runtime_check("");
+
+  r |= __builtin_allow_runtime_check("check");
+
+  str = __builtin_allow_runtime_check("check2");  // expected-error {{incompatible integer to pointer conversion}}
+
+  return r;
+}

Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@vitalybuka
Copy link
Collaborator Author

@MaskRay @nikic @efriedma-quic Any comments?

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

This looks very reasonable to me, but I'm not a clang reviewer...

@vitalybuka

This comment was marked as resolved.

@efriedma-quic

This comment was marked as resolved.

goldsteinn and others added 3 commits April 10, 2024 11:20
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@@ -3464,6 +3464,34 @@ Query for this feature with ``__has_builtin(__builtin_trap)``.

``__builtin_arm_trap`` is lowered to the ``llvm.aarch64.break`` builtin, and then to ``brk #payload``.

``__builtin_allow_runtime_check``
Copy link
Collaborator

Choose a reason for hiding this comment

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

The docs aren't particularly clear to me -- why would the check at the current program location ever NOT be executed? What strings can be passed in? What compiler options impact it? Will it only accept string literals or are runtime values fine?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've started RFC, as @efriedma-quic suggested
https://discourse.llvm.org/t/rfc-introduce-new-clang-builtin-builtin-allow-runtime-check/78281

It needs to be compiler time lowered, so I think it should literals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It needs to be compiler time lowered, so I think it should literals.

In that case, I think we want constant expression not just literal, right? e.g.,

constexpr const char *name = "name of check";
if (__builtin_allow_runtime_check(name)) {
}

is still able to be lowered at compile time. The reason I ask is because I'm thinking about template metaprogramming cases where you might want to do something along the lines of:

template <typename Ty>
struct S {
  void mem_fun() {
    if (__builtin_allow_runtime_check(Ty::check_name) && stuff) {
    }
  }
};

but maybe this is not a compelling use case? I don't insist on constant expression support, more just trying to verify that we support the expected usage patterns.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure how to do that. I would expected that from __builtin_nan, but it can't do that:
https://godbolt.org/z/hWx47Gqvn

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similar __builtin_cpu_is, also works only with literals.

The closest thing I see is c++26 static_assert https://en.cppreference.com/w/cpp/language/static_assert
And it's processed in a complicated way EvaluateStaticAssertMessageAsString
https://godbolt.org/z/Gcf74Ysjs

It can try to port that code, but I'd keep as-is. We have a way to improve if use-case is important.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, let's just skip this for now then, thank you!

clang/lib/Sema/SemaChecking.cpp Show resolved Hide resolved
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@@ -3464,6 +3464,34 @@ Query for this feature with ``__has_builtin(__builtin_trap)``.

``__builtin_arm_trap`` is lowered to the ``llvm.aarch64.break`` builtin, and then to ``brk #payload``.

``__builtin_allow_runtime_check``
Copy link
Collaborator

Choose a reason for hiding this comment

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

It needs to be compiler time lowered, so I think it should literals.

In that case, I think we want constant expression not just literal, right? e.g.,

constexpr const char *name = "name of check";
if (__builtin_allow_runtime_check(name)) {
}

is still able to be lowered at compile time. The reason I ask is because I'm thinking about template metaprogramming cases where you might want to do something along the lines of:

template <typename Ty>
struct S {
  void mem_fun() {
    if (__builtin_allow_runtime_check(Ty::check_name) && stuff) {
    }
  }
};

but maybe this is not a compelling use case? I don't insist on constant expression support, more just trying to verify that we support the expected usage patterns.

clang/docs/LanguageExtensions.rst Outdated Show resolved Hide resolved
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.clangbuiltin-implement-__builtin_allow_runtime_check to main April 12, 2024 21:27
Created using spr 1.3.4
Created using spr 1.3.4
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

@vitalybuka vitalybuka merged commit 1f35e72 into main Apr 17, 2024
4 of 5 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/clangbuiltin-implement-__builtin_allow_runtime_check branch April 17, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

7 participants