[clang] [clang] Allow builtin addc/subc to be constant evaluated (PR #81656)
Bryce Wilson via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 15 06:58:46 PST 2024
https://github.com/Bryce-MW updated https://github.com/llvm/llvm-project/pull/81656
>From 9bea6282aae73372a80aa3d0532ae0532b4ca948 Mon Sep 17 00:00:00 2001
From: Bryce Wilson <bryce.wilson at oldmissioncapital.com>
Date: Fri, 9 Feb 2024 16:56:57 -0600
Subject: [PATCH 1/3] [clang] Allow builtin addc/subc to be constant evaluated
---
clang/docs/LanguageExtensions.rst | 10 ++++++
clang/include/clang/Basic/Builtins.td | 4 +--
clang/lib/AST/ExprConstant.cpp | 50 +++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index e91156837290f7..e8557a22662c07 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -5241,6 +5241,11 @@ Intrinsics Support within Constant Expressions
The following builtin intrinsics can be used in constant expressions:
+* ``__builtin_addcb``
+* ``__builtin_addcs``
+* ``__builtin_addc``
+* ``__builtin_addcl``
+* ``__builtin_addcll``
* ``__builtin_bitreverse8``
* ``__builtin_bitreverse16``
* ``__builtin_bitreverse32``
@@ -5287,6 +5292,11 @@ The following builtin intrinsics can be used in constant expressions:
* ``__builtin_rotateright16``
* ``__builtin_rotateright32``
* ``__builtin_rotateright64``
+* ``__builtin_subcb``
+* ``__builtin_subcs``
+* ``__builtin_subc``
+* ``__builtin_subcl``
+* ``__builtin_subcll``
The following x86-specific intrinsics can be used in constant expressions:
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 31a2bdeb2d3e5e..59dc0e20393b5f 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -4061,14 +4061,14 @@ class MPATemplate : Template<
def Addc : Builtin, MPATemplate {
let Spellings = ["__builtin_addc"];
- let Attributes = [NoThrow];
+ let Attributes = [NoThrow, Constexpr];
// FIXME: Why are these argumentes marked const?
let Prototype = "T(T const, T const, T const, T*)";
}
def Subc : Builtin, MPATemplate {
let Spellings = ["__builtin_subc"];
- let Attributes = [NoThrow];
+ let Attributes = [NoThrow, Constexpr];
// FIXME: Why are these argumentes marked const?
let Prototype = "T(T const, T const, T const, T*)";
}
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 089bc2094567f7..42e134e70c6e8e 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12691,6 +12691,56 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
return BuiltinOp == Builtin::BI__atomic_always_lock_free ?
Success(0, E) : Error(E);
}
+ case Builtin::BI__builtin_addcb:
+ case Builtin::BI__builtin_addcs:
+ case Builtin::BI__builtin_addc:
+ case Builtin::BI__builtin_addcl:
+ case Builtin::BI__builtin_addcll:
+ case Builtin::BI__builtin_subcb:
+ case Builtin::BI__builtin_subcs:
+ case Builtin::BI__builtin_subc:
+ case Builtin::BI__builtin_subcl:
+ case Builtin::BI__builtin_subcll: {
+ LValue CarryOutLValue;
+ APSInt LHS, RHS, CarryIn, Result;
+ QualType ResultType = E->getArg(0)->getType();
+ if (!EvaluateInteger(E->getArg(0), LHS, Info) ||
+ !EvaluateInteger(E->getArg(1), RHS, Info) ||
+ !EvaluateInteger(E->getArg(2), CarryIn, Info) ||
+ !EvaluatePointer(E->getArg(3), CarryOutLValue, Info))
+ return false;
+
+ bool FirstOverflowed = false;
+ bool SecondOverflowed = false;
+ switch (BuiltinOp) {
+ default:
+ llvm_unreachable("Invalid value for BuiltinOp");
+ case Builtin::BI__builtin_addcb:
+ case Builtin::BI__builtin_addcs:
+ case Builtin::BI__builtin_addc:
+ case Builtin::BI__builtin_addcl:
+ case Builtin::BI__builtin_addcll:
+ Result =
+ LHS.uadd_ov(RHS, FirstOverflowed).uadd_ov(CarryIn, SecondOverflowed);
+ break;
+ case Builtin::BI__builtin_subcb:
+ case Builtin::BI__builtin_subcs:
+ case Builtin::BI__builtin_subc:
+ case Builtin::BI__builtin_subcl:
+ case Builtin::BI__builtin_subcll:
+ Result =
+ LHS.usub_ov(RHS, FirstOverflowed).usub_ov(CarryIn, SecondOverflowed);
+ break;
+ }
+
+ // It is possible for both overflows to happen but CGBuiltin uses an OR so
+ // this is consistent
+ APSInt API{(uint32_t)(FirstOverflowed | SecondOverflowed)};
+ APValue APV{API};
+ if (!handleAssignment(Info, E, CarryOutLValue, ResultType, APV))
+ return false;
+ return Success(Result, E);
+ }
case Builtin::BI__builtin_add_overflow:
case Builtin::BI__builtin_sub_overflow:
case Builtin::BI__builtin_mul_overflow:
>From 52ad49ce3eef5a98b779c13277af6f430a9b2a4a Mon Sep 17 00:00:00 2001
From: Bryce Wilson <bryce.wilson at oldmissioncapital.com>
Date: Wed, 14 Feb 2024 16:52:09 -0600
Subject: [PATCH 2/3] Add tests and fix discovered bugs
---
clang/lib/AST/ExprConstant.cpp | 9 +++--
clang/test/SemaCXX/builtins-overflow.cpp | 49 ++++++++++++++++++++++++
2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index c8f09782c42de7..bbe05398de172b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12707,13 +12707,16 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
case Builtin::BI__builtin_subcl:
case Builtin::BI__builtin_subcll: {
LValue CarryOutLValue;
- APSInt LHS, RHS, CarryIn, Result;
+ APSInt LHS, RHS, CarryIn, CarryOut, Result;
QualType ResultType = E->getArg(0)->getType();
if (!EvaluateInteger(E->getArg(0), LHS, Info) ||
!EvaluateInteger(E->getArg(1), RHS, Info) ||
!EvaluateInteger(E->getArg(2), CarryIn, Info) ||
!EvaluatePointer(E->getArg(3), CarryOutLValue, Info))
return false;
+ // Copy the number of bits and sign
+ Result = LHS;
+ CarryOut = LHS;
bool FirstOverflowed = false;
bool SecondOverflowed = false;
@@ -12740,8 +12743,8 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
// It is possible for both overflows to happen but CGBuiltin uses an OR so
// this is consistent
- APSInt API{(uint32_t)(FirstOverflowed | SecondOverflowed)};
- APValue APV{API};
+ CarryOut = (uint64_t)(FirstOverflowed | SecondOverflowed);
+ APValue APV{CarryOut};
if (!handleAssignment(Info, E, CarryOutLValue, ResultType, APV))
return false;
return Success(Result, E);
diff --git a/clang/test/SemaCXX/builtins-overflow.cpp b/clang/test/SemaCXX/builtins-overflow.cpp
index c84b7da00b543c..1b1e46ae751329 100644
--- a/clang/test/SemaCXX/builtins-overflow.cpp
+++ b/clang/test/SemaCXX/builtins-overflow.cpp
@@ -94,3 +94,52 @@ static_assert(smul(17,22) == Result<int>{false, 374});
static_assert(smul(INT_MAX / 22, 23) == Result<int>{true, -2049870757});
static_assert(smul(INT_MIN / 22, -23) == Result<int>{true, -2049870757});
+template<typename T>
+struct CarryResult {
+ T CarryOut;
+ T Value;
+ constexpr bool operator==(const CarryResult<T> &Other) {
+ return CarryOut == Other.CarryOut && Value == Other.Value;
+ }
+};
+
+constexpr CarryResult<unsigned char> addcb(unsigned char lhs, unsigned char rhs, unsigned char carry) {
+ unsigned char carry_out{};
+ unsigned char sum{};
+ sum = __builtin_addcb(lhs, rhs, carry, &carry_out);
+ return {carry_out, sum};
+}
+
+static_assert(addcb(120, 10, 0) == CarryResult<unsigned char>{0, 130});
+static_assert(addcb(250, 10, 0) == CarryResult<unsigned char>{1, 4});
+static_assert(addcb(255, 255, 0) == CarryResult<unsigned char>{1, 254});
+static_assert(addcb(255, 255, 1) == CarryResult<unsigned char>{1, 255});
+static_assert(addcb(255, 0, 1) == CarryResult<unsigned char>{1, 0});
+static_assert(addcb(255, 1, 0) == CarryResult<unsigned char>{1, 0});
+static_assert(addcb(255, 1, 1) == CarryResult<unsigned char>{1, 1});
+// This is currently supported with the carry still producing a value of 1.
+// If support for carry outside of 0-1 is removed, change this test to check
+// that it is not supported.
+static_assert(addcb(255, 255, 2) == CarryResult<unsigned char>{1, 0});
+
+constexpr CarryResult<unsigned char> subcb(unsigned char lhs, unsigned char rhs, unsigned char carry) {
+ unsigned char carry_out{};
+ unsigned char sum{};
+ sum = __builtin_subcb(lhs, rhs, carry, &carry_out);
+ return {carry_out, sum};
+}
+
+static_assert(subcb(20, 10, 0) == CarryResult<unsigned char>{0, 10});
+static_assert(subcb(10, 10, 0) == CarryResult<unsigned char>{0, 0});
+static_assert(subcb(10, 15, 0) == CarryResult<unsigned char>{1, 251});
+// The carry is subtracted from the result
+static_assert(subcb(10, 15, 1) == CarryResult<unsigned char>{1, 250});
+static_assert(subcb(0, 0, 1) == CarryResult<unsigned char>{1, 255});
+static_assert(subcb(0, 1, 0) == CarryResult<unsigned char>{1, 255});
+static_assert(subcb(0, 1, 1) == CarryResult<unsigned char>{1, 254});
+static_assert(subcb(0, 255, 0) == CarryResult<unsigned char>{1, 1});
+static_assert(subcb(0, 255, 1) == CarryResult<unsigned char>{1, 0});
+// This is currently supported with the carry still producing a value of 1.
+// If support for carry outside of 0-1 is removed, change this test to check
+// that it is not supported.
+static_assert(subcb(0, 255, 2) == CarryResult<unsigned char>{1, 255});
>From 2e28c1cbdc5df9852ee37fecc54e618bf476bf86 Mon Sep 17 00:00:00 2001
From: Bryce Wilson <bryce.wilson at oldmissioncapital.com>
Date: Thu, 15 Feb 2024 08:58:33 -0600
Subject: [PATCH 3/3] Fix nits and add release note
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/AST/ExprConstant.cpp | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e12a802e2e9ede..2f01e2f4ef97ef 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -120,6 +120,9 @@ Non-comprehensive list of changes in this release
- Added ``__builtin_readsteadycounter`` for reading fixed frequency hardware
counters.
+- ``__builtin_addc``, ``__builtin_subc``, and the other sizes of those
+ builtins are now constexpr and may be used in constant expressions.
+
New Compiler Flags
------------------
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index bbe05398de172b..010010120cf6dd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12714,7 +12714,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
!EvaluateInteger(E->getArg(2), CarryIn, Info) ||
!EvaluatePointer(E->getArg(3), CarryOutLValue, Info))
return false;
- // Copy the number of bits and sign
+ // Copy the number of bits and sign.
Result = LHS;
CarryOut = LHS;
@@ -12742,7 +12742,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
}
// It is possible for both overflows to happen but CGBuiltin uses an OR so
- // this is consistent
+ // this is consistent.
CarryOut = (uint64_t)(FirstOverflowed | SecondOverflowed);
APValue APV{CarryOut};
if (!handleAssignment(Info, E, CarryOutLValue, ResultType, APV))
More information about the cfe-commits
mailing list