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.wil...@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.wil...@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.wil...@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)) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits