[clang][Interp] Implement __builtin_bitreverse #71687
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesSince the return value of this function is slightly more involved than the void/bool/int/size_t return values we've seen so far, also refactor this. Full diff: https://github.com/llvm/llvm-project/pull/71687.diff 2 Files Affected:
diff --git a/clang/lib/AST/Interp/InterpBuiltin.cpp b/clang/lib/AST/Interp/InterpBuiltin.cpp
index f26d298f5b60045..b5a9af7e334624e 100644
--- a/clang/lib/AST/Interp/InterpBuiltin.cpp
+++ b/clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -59,13 +59,54 @@ static void pushInt(InterpState &S, int32_t Val) {
llvm_unreachable("Int isn't 16 or 32 bit?");
}
-static bool retInt(InterpState &S, CodePtr OpPC, APValue &Result) {
- PrimType IntType = getIntPrimType(S);
- if (IntType == PT_Sint32)
- return Ret<PT_Sint32>(S, OpPC, Result);
- else if (IntType == PT_Sint16)
- return Ret<PT_Sint16>(S, OpPC, Result);
- llvm_unreachable("Int isn't 16 or 32 bit?");
+static void pushAPSInt(InterpState &S, const APSInt &Val) {
+ bool Signed = Val.isSigned();
+
+ if (Signed) {
+ switch (Val.getBitWidth()) {
+ case 64:
+ S.Stk.push<Integral<64, true>>(
+ Integral<64, true>::from(Val.getSExtValue()));
+ break;
+ case 32:
+ S.Stk.push<Integral<32, true>>(
+ Integral<32, true>::from(Val.getSExtValue()));
+ break;
+ case 16:
+ S.Stk.push<Integral<16, true>>(
+ Integral<16, true>::from(Val.getSExtValue()));
+ break;
+ case 8:
+ S.Stk.push<Integral<8, true>>(
+ Integral<8, true>::from(Val.getSExtValue()));
+ break;
+ default:
+ llvm_unreachable("Invalid integer bitwidth");
+ }
+ return;
+ }
+
+ // Unsigned.
+ switch (Val.getBitWidth()) {
+ case 64:
+ S.Stk.push<Integral<64, false>>(
+ Integral<64, false>::from(Val.getZExtValue()));
+ break;
+ case 32:
+ S.Stk.push<Integral<32, false>>(
+ Integral<32, false>::from(Val.getZExtValue()));
+ break;
+ case 16:
+ S.Stk.push<Integral<16, false>>(
+ Integral<16, false>::from(Val.getZExtValue()));
+ break;
+ case 8:
+ S.Stk.push<Integral<8, false>>(
+ Integral<8, false>::from(Val.getZExtValue()));
+ break;
+ default:
+ llvm_unreachable("Invalid integer bitwidth");
+ }
}
static void pushSizeT(InterpState &S, uint64_t Val) {
@@ -87,20 +128,29 @@ static void pushSizeT(InterpState &S, uint64_t Val) {
}
}
-static bool retSizeT(InterpState &S, CodePtr OpPC, APValue &Result) {
- const TargetInfo &TI = S.getCtx().getTargetInfo();
- unsigned SizeTWidth = TI.getTypeWidth(TI.getSizeType());
-
- switch (SizeTWidth) {
- case 64:
- return Ret<PT_Uint64>(S, OpPC, Result);
- case 32:
- return Ret<PT_Uint32>(S, OpPC, Result);
- case 16:
- return Ret<PT_Uint16>(S, OpPC, Result);
+static bool retPrimValue(InterpState &S, CodePtr OpPC, APValue &Result,
+ std::optional<PrimType> &T) {
+ if (!T)
+ return RetVoid(S, OpPC, Result);
+
+#define RET_CASE(X) \
+ case X: \
+ return Ret<X>(S, OpPC, Result);
+ switch (*T) {
+ RET_CASE(PT_Float);
+ RET_CASE(PT_Bool);
+ RET_CASE(PT_Sint8);
+ RET_CASE(PT_Uint8);
+ RET_CASE(PT_Sint16);
+ RET_CASE(PT_Uint16);
+ RET_CASE(PT_Sint32);
+ RET_CASE(PT_Uint32);
+ RET_CASE(PT_Sint64);
+ RET_CASE(PT_Uint64);
+ default:
+ llvm_unreachable("Unsupported return type for builtin function");
}
-
- llvm_unreachable("size_t isn't 64 or 32 bit?");
+#undef RET_CASE
}
static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
@@ -439,40 +489,55 @@ static bool interp__builtin_popcount(InterpState &S, CodePtr OpPC,
return true;
}
+static bool interp__builtin_bitreverse(InterpState &S, CodePtr OpPC,
+ const InterpFrame *Frame,
+ const Function *Func,
+ const CallExpr *Call) {
+ PrimType ArgT = *S.getContext().classify(Call->getArg(0)->getType());
+ APSInt Val = peekToAPSInt(S.Stk, ArgT);
+ pushAPSInt(S, APSInt(Val.reverseBits(), /*IsUnsigend=*/true));
+ return true;
+}
+
bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
const CallExpr *Call) {
InterpFrame *Frame = S.Current;
APValue Dummy;
+ QualType ReturnType = Call->getCallReturnType(S.getCtx());
+ std::optional<PrimType> ReturnT = S.getContext().classify(ReturnType);
+ // If classify failed, we assume void.
+ assert(ReturnT || ReturnType->isVoidType());
+
switch (F->getBuiltinID()) {
case Builtin::BI__builtin_is_constant_evaluated:
S.Stk.push<Boolean>(Boolean::from(S.inConstantContext()));
- return Ret<PT_Bool>(S, OpPC, Dummy);
+ break;
case Builtin::BI__builtin_assume:
- return RetVoid(S, OpPC, Dummy);
+ break;
case Builtin::BI__builtin_strcmp:
- if (interp__builtin_strcmp(S, OpPC, Frame))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_strcmp(S, OpPC, Frame))
+ return false;
break;
case Builtin::BI__builtin_strlen:
- if (interp__builtin_strlen(S, OpPC, Frame))
- return retSizeT(S, OpPC, Dummy);
+ if (!interp__builtin_strlen(S, OpPC, Frame))
+ return false;
break;
case Builtin::BI__builtin_nan:
case Builtin::BI__builtin_nanf:
case Builtin::BI__builtin_nanl:
case Builtin::BI__builtin_nanf16:
case Builtin::BI__builtin_nanf128:
- if (interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/false))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/false))
+ return false;
break;
case Builtin::BI__builtin_nans:
case Builtin::BI__builtin_nansf:
case Builtin::BI__builtin_nansl:
case Builtin::BI__builtin_nansf16:
case Builtin::BI__builtin_nansf128:
- if (interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/true))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_nan(S, OpPC, Frame, F, /*Signaling=*/true))
+ return false;
break;
case Builtin::BI__builtin_huge_val:
@@ -485,15 +550,15 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
case Builtin::BI__builtin_infl:
case Builtin::BI__builtin_inff16:
case Builtin::BI__builtin_inff128:
- if (interp__builtin_inf(S, OpPC, Frame, F))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_inf(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_copysign:
case Builtin::BI__builtin_copysignf:
case Builtin::BI__builtin_copysignl:
case Builtin::BI__builtin_copysignf128:
- if (interp__builtin_copysign(S, OpPC, Frame, F))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_copysign(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_fmin:
@@ -501,8 +566,8 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
case Builtin::BI__builtin_fminl:
case Builtin::BI__builtin_fminf16:
case Builtin::BI__builtin_fminf128:
- if (interp__builtin_fmin(S, OpPC, Frame, F))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_fmin(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_fmax:
@@ -510,60 +575,60 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
case Builtin::BI__builtin_fmaxl:
case Builtin::BI__builtin_fmaxf16:
case Builtin::BI__builtin_fmaxf128:
- if (interp__builtin_fmax(S, OpPC, Frame, F))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_fmax(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_isnan:
- if (interp__builtin_isnan(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_isnan(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_issignaling:
- if (interp__builtin_issignaling(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_issignaling(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_isinf:
- if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
+ return false;
break;
case Builtin::BI__builtin_isinf_sign:
- if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
+ return false;
break;
case Builtin::BI__builtin_isfinite:
- if (interp__builtin_isfinite(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_isfinite(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_isnormal:
- if (interp__builtin_isnormal(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_isnormal(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_issubnormal:
- if (interp__builtin_issubnormal(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_issubnormal(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_iszero:
- if (interp__builtin_iszero(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_iszero(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_isfpclass:
- if (interp__builtin_isfpclass(S, OpPC, Frame, F, Call))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_isfpclass(S, OpPC, Frame, F, Call))
+ return false;
break;
case Builtin::BI__builtin_fpclassify:
- if (interp__builtin_fpclassify(S, OpPC, Frame, F))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_fpclassify(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_fabs:
case Builtin::BI__builtin_fabsf:
case Builtin::BI__builtin_fabsl:
case Builtin::BI__builtin_fabsf128:
- if (interp__builtin_fabs(S, OpPC, Frame, F))
- return Ret<PT_Float>(S, OpPC, Dummy);
+ if (!interp__builtin_fabs(S, OpPC, Frame, F))
+ return false;
break;
case Builtin::BI__builtin_popcount:
@@ -572,15 +637,23 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
case Builtin::BI__popcnt16: // Microsoft variants of popcount
case Builtin::BI__popcnt:
case Builtin::BI__popcnt64:
- if (interp__builtin_popcount(S, OpPC, Frame, F, Call))
- return retInt(S, OpPC, Dummy);
+ if (!interp__builtin_popcount(S, OpPC, Frame, F, Call))
+ return false;
+ break;
+
+ case Builtin::BI__builtin_bitreverse8:
+ case Builtin::BI__builtin_bitreverse16:
+ case Builtin::BI__builtin_bitreverse32:
+ case Builtin::BI__builtin_bitreverse64:
+ if (!interp__builtin_bitreverse(S, OpPC, Frame, F, Call))
+ return false;
break;
default:
return false;
}
- return false;
+ return retPrimValue(S, OpPC, Dummy, ReturnT);
}
bool InterpretOffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E,
diff --git a/clang/test/AST/Interp/builtin-functions.cpp b/clang/test/AST/Interp/builtin-functions.cpp
index a78a0fbdf11b1d7..1422674cd20bbdd 100644
--- a/clang/test/AST/Interp/builtin-functions.cpp
+++ b/clang/test/AST/Interp/builtin-functions.cpp
@@ -295,3 +295,10 @@ namespace popcount {
char popcount9[__builtin_popcountll(0xF0F0LL) == 8 ? 1 : -1];
char popcount10[__builtin_popcountll(~0LL) == BITSIZE(long long) ? 1 : -1];
}
+
+namespace bitreverse {
+ char bitreverse1[__builtin_bitreverse8(0x01) == 0x80 ? 1 : -1];
+ char bitreverse2[__builtin_bitreverse16(0x3C48) == 0x123C ? 1 : -1];
+ char bitreverse3[__builtin_bitreverse32(0x12345678) == 0x1E6A2C48 ? 1 : -1];
+ char bitreverse4[__builtin_bitreverse64(0x0123456789ABCDEFULL) == 0xF7B3D591E6A2C480 ? 1 : -1];
+}
|
|
Ping |
AaronBallman
approved these changes
Nov 15, 2023
There was a problem hiding this comment.
The changes seem reasonable to me but I think we should start putting some effort in to support _BitInt because I have a sneaking suspicion that will be hard to support with all the power-of-two literals being used, and the more integer handling code we add, the harder it will be to support in the future. WDYT?
| Integral<8, true>::from(Val.getSExtValue())); | ||
| break; | ||
| default: | ||
| llvm_unreachable("Invalid integer bitwidth"); |
There was a problem hiding this comment.
_BitInt?
Not that you need to support it in this patch, but I do wonder if this design is painting us into a corner.
There was a problem hiding this comment.
I thought BitInt wasn't possible here since the builtins have l, ll, etc. variants... but I guess that's not true for the ones defined as variadic functions?
There was a problem hiding this comment.
Yeah, that's the situation I'm worried about (not really specific to bitreverse as that only has hardcoded bit widths). However, now that I look at builtins.def more closely, most of the ones taking variadic arguments are doing so because they have custom type-checking that often disallows non-power-of-two-types. So I think we're fine for now, probably.
Since the return value of this function is slightly more involved than the void/bool/int/size_t return values we've seen so far, also refactor this.
tbaederr
force-pushed
the
builtin-bitreverse
branch
from
140555c
to
f10454f
Compare
AaronBallman
approved these changes
Nov 16, 2023
| Integral<8, true>::from(Val.getSExtValue())); | ||
| break; | ||
| default: | ||
| llvm_unreachable("Invalid integer bitwidth"); |
There was a problem hiding this comment.
Yeah, that's the situation I'm worried about (not really specific to bitreverse as that only has hardcoded bit widths). However, now that I look at builtins.def more closely, most of the ones taking variadic arguments are doing so because they have custom type-checking that often disallows non-power-of-two-types. So I think we're fine for now, probably.
sr-tream
pushed a commit
to sr-tream/llvm-project
that referenced
this pull request
Nov 20, 2023
Since the return value of this function is slightly more involved than the void/bool/int/size_t return values we've seen so far, also refactor this.
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
Since the return value of this function is slightly more involved than the void/bool/int/size_t return values we've seen so far, also refactor this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since the return value of this function is slightly more involved than the void/bool/int/size_t return values we've seen so far, also refactor this.