Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/Builtins.td
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,12 @@ def BSwap : Builtin, Template<["unsigned short", "uint32_t", "uint64_t"],
let Prototype = "T(T)";
}

def BSwapg : Builtin {
let Spellings = ["__builtin_bswapg"];
let Attributes = [NoThrow, Const, Constexpr, CustomTypeChecking];
let Prototype = "int(...)";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let Prototype = "int(...)";
let Prototype = "T(T)";

Should this match the other generic ones?

Copy link
Author

Choose a reason for hiding this comment

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

No, this will cause a build error:
llvm-project/clang/include/clang/Basic/Builtins.td:761:7: error: Not a template
let Prototype = "T(T)";
^

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC T(T) assumes an explicit set of type mappings. In principle I think this could be labeled as T(T) with the customtypechecking flag. That would be more correct than int(..), and the Sema work is already doing the custom checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether we should have a CustomTypeCheckingBuiltin that doesn't ask for a prototype. I don't think that would work without any modification to Clang though. Some builtins might assume the return type is correct and won't set it again during custom type checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice if we could automate type checking for truly template/generic builtins. I feel like builtins using CustomTypeChecking should have an accurate prototype, but I agree that no prototype is better than a fake one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, some more infrastructure for this would be nice, especially since we seem to go in the direction of "all the builtins are generic".

Copy link
Author

Choose a reason for hiding this comment

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

IIRC T(T) assumes an explicit set of type mappings. In principle I think this could be labeled as T(T) with the customtypechecking flag. That would be more correct than int(..), and the Sema work is already doing the custom checking.

In fact, at first I attempted to use T(T) with an explicit set of type mappings, but the automatically generated builtin functions are bswapg16, bswapg32, etc., rather than a single generic bswapg. I'm not sure if I can use T(T) without making more modifications to clang. It would be nice if that worked.

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, missed this reply - yeah it exists to automate the bindings for multiple types rather than just specifying a generic function :-/

}

def Bitreverse : BitInt8_16_32_64BuiltinsTemplate, Builtin {
let Spellings = ["__builtin_bitreverse"];
let Attributes = [NoThrow, Const, Constexpr];
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/AST/ByteCode/InterpBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1004,8 +1004,10 @@ static bool interp__builtin_bswap(InterpState &S, CodePtr OpPC,
const CallExpr *Call) {
const APSInt &Val = popToAPSInt(S, Call->getArg(0));
assert(Val.getActiveBits() <= 64);

pushInteger(S, Val.byteSwap(), Call->getType());
if (Val.getBitWidth() == 8)
pushInteger(S, Val, Call->getType());
else
pushInteger(S, Val.byteSwap(), Call->getType());
return true;
}

Expand Down Expand Up @@ -3288,7 +3290,7 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call,
case Builtin::BI__builtin_elementwise_ctzg:
return interp__builtin_elementwise_countzeroes(S, OpPC, Frame, Call,
BuiltinID);

case Builtin::BI__builtin_bswapg:
case Builtin::BI__builtin_bswap16:
case Builtin::BI__builtin_bswap32:
case Builtin::BI__builtin_bswap64:
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13982,6 +13982,15 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,

return Success(Val.reverseBits(), E);
}
case Builtin::BI__builtin_bswapg: {
APSInt Val;
if (!EvaluateInteger(E->getArg(0), Val, Info))
return false;
if (Val.getBitWidth() == 8)
return Success(Val, E);

return Success(Val.byteSwap(), E);
}

case Builtin::BI__builtin_bswap16:
case Builtin::BI__builtin_bswap32:
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CodeGen/CGBuiltin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3622,6 +3622,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
Builder.CreateArithmeticFence(ArgValue, ConvertType(ArgType)));
return RValue::get(ArgValue);
}
case Builtin::BI__builtin_bswapg:
case Builtin::BI__builtin_bswap16:
case Builtin::BI__builtin_bswap32:
case Builtin::BI__builtin_bswap64:
Expand Down
30 changes: 30 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2200,6 +2200,32 @@ static bool BuiltinCpu(Sema &S, const TargetInfo &TI, CallExpr *TheCall,
return false;
}

/// Checks that __builtin_bswapg was called with a single argument, which is an
/// unsigned integer, and overrides the return value type to the integer type.
static bool BuiltinBswapg(Sema &S, CallExpr *TheCall) {
if (S.checkArgCount(TheCall, 1))
return true;
ExprResult ArgRes = S.DefaultLvalueConversion(TheCall->getArg(0));
if (ArgRes.isInvalid())
return true;

Expr *Arg = ArgRes.get();
TheCall->setArg(0, Arg);
if (Arg->isTypeDependent())
return false;

QualType ArgTy = Arg->getType();

if (!ArgTy->isIntegerType()) {
S.Diag(Arg->getBeginLoc(), diag::err_builtin_invalid_arg_type)
<< 1 << /* scalar */ 1 << /* unsigned integer ty */ 1 << /* no fp */ 0
<< ArgTy;
return true;
}
TheCall->setType(ArgTy);
return false;
}

/// Checks that __builtin_popcountg was called with a single argument, which is
/// an unsigned integer.
static bool BuiltinPopcountg(Sema &S, CallExpr *TheCall) {
Expand Down Expand Up @@ -3448,6 +3474,10 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
}
break;
}
case Builtin::BI__builtin_bswapg:
if (BuiltinBswapg(*this, TheCall))
return ExprError();
break;
case Builtin::BI__builtin_popcountg:
if (BuiltinPopcountg(*this, TheCall))
return ExprError();
Expand Down
4 changes: 4 additions & 0 deletions clang/test/AST/ByteCode/builtin-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,10 @@ namespace bswap {
int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
int h4 = __builtin_bswap32(0x1234) == 0x34120000 ? 1 : f();
int h5 = __builtin_bswap64(0x1234) == 0x3412000000000000 ? 1 : f();
int h6 = __builtin_bswapg(0x12) == 0x12 ? 1 : f();
int h7 = __builtin_bswapg(0x1234) == 0x3412 ? 1 : f();
int h8 = __builtin_bswapg(0x00001234) == 0x34120000 ? 1 : f();
int h9 = __builtin_bswapg(0x0000000000001234) == 0x3412000000000000 ? 1 : f();
}

#define CFSTR __builtin___CFStringMakeConstantString
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGen/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ int main(void) {
P(object_size, (s0, 3));

// Whatever

P(bswapg, (N));
P(bswap16, (N));
P(bswap32, (N));
P(bswap64, (N));
Expand Down
4 changes: 4 additions & 0 deletions clang/test/Sema/constant-builtins-2.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,10 @@ int h0 = __builtin_types_compatible_p(int, float);
int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
int h4 = __builtin_bswap32(0x1234) == 0x34120000 ? 1 : f();
int h5 = __builtin_bswap64(0x1234) == 0x3412000000000000 ? 1 : f();
int h6 = __builtin_bswapg((char)(0x12)) == (char)(0x12) ? 1 : f();
int h7 = __builtin_bswapg((short)(0x1234)) == (short)(0x3412) ? 1 : f();
int h8 = __builtin_bswapg(0x00001234) == 0x34120000 ? 1 : f();
int h9 = __builtin_bswapg(0x0000000000001234ULL) == 0x3412000000000000 ? 1 : f();
extern long int bi0;
extern __typeof__(__builtin_expect(0, 0)) bi0;

Expand Down
5 changes: 4 additions & 1 deletion clang/test/Sema/constant-builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ int h0 = __builtin_types_compatible_p(int,float);
int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
int h4 = __builtin_bswap32(0x1234) == 0x34120000 ? 1 : f();
int h5 = __builtin_bswap64(0x1234) == 0x3412000000000000 ? 1 : f();

int h6 = __builtin_bswapg((char)0x12) == (char)0x12 ? 1 : f();
int h7 = __builtin_bswapg((short)(0x1234)) == (short)(0x3412) ? 1 : f();
int h8 = __builtin_bswapg(0x00001234) == 0x34120000 ? 1 : f();
int h9 = __builtin_bswapg(0x0000000000001234ULL) == 0x3412000000000000 ? 1 : f();
short somefunc(void);

short t = __builtin_constant_p(5353) ? 42 : somefunc();
Expand Down
Loading