Skip to content

[clang] Implement constexpr bit_cast for vectors #66894

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 30, 2023
Merged

[clang] Implement constexpr bit_cast for vectors #66894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38647e8
[clang] Implement constexpr bit_cast for vectors
DaMatrix Aug 13, 2023
e23b362
Change and add test case for diagnostic when doing a C-style bitcast …
DaMatrix Oct 24, 2023
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
5 changes: 4 additions & 1 deletion clang/include/clang/Basic/DiagnosticASTKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def note_constexpr_memcpy_unsupported : Note<
"source is not a contiguous array of at least %4 elements of type %3|"
"destination is not a contiguous array of at least %4 elements of type %3}2">;
def note_constexpr_bit_cast_unsupported_type : Note<
"constexpr bit_cast involving type %0 is not yet supported">;
"constexpr bit cast involving type %0 is not yet supported">;
def note_constexpr_bit_cast_unsupported_bitfield : Note<
"constexpr bit_cast involving bit-field is not yet supported">;
def note_constexpr_bit_cast_invalid_type : Note<
Expand All @@ -326,6 +326,9 @@ def note_constexpr_bit_cast_invalid_type : Note<
"%select{type|member}1 is not allowed in a constant expression">;
def note_constexpr_bit_cast_invalid_subtype : Note<
"invalid type %0 is a %select{member|base}1 of %2">;
def note_constexpr_bit_cast_invalid_vector : Note<
"bit_cast involving type %0 is not allowed in a constant expression; "
"element size %1 * element count %2 is not a multiple of the byte size %3">;
def note_constexpr_bit_cast_indet_dest : Note<
"indeterminate value can only initialize an object of type 'unsigned char'"
"%select{, 'char',|}1 or 'std::byte'; %0 is invalid">;
Expand Down
269 changes: 175 additions & 94 deletions clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2732,53 +2732,6 @@ static bool truncateBitfieldValue(EvalInfo &Info, const Expr *E,
return true;
}

static bool EvalAndBitcastToAPInt(EvalInfo &Info, const Expr *E,
llvm::APInt &Res) {
APValue SVal;
if (!Evaluate(SVal, Info, E))
return false;
if (SVal.isInt()) {
Res = SVal.getInt();
return true;
}
if (SVal.isFloat()) {
Res = SVal.getFloat().bitcastToAPInt();
return true;
}
if (SVal.isVector()) {
QualType VecTy = E->getType();
unsigned VecSize = Info.Ctx.getTypeSize(VecTy);
QualType EltTy = VecTy->castAs<VectorType>()->getElementType();
unsigned EltSize = Info.Ctx.getTypeSize(EltTy);
bool BigEndian = Info.Ctx.getTargetInfo().isBigEndian();
Res = llvm::APInt::getZero(VecSize);
for (unsigned i = 0; i < SVal.getVectorLength(); i++) {
APValue &Elt = SVal.getVectorElt(i);
llvm::APInt EltAsInt;
if (Elt.isInt()) {
EltAsInt = Elt.getInt();
} else if (Elt.isFloat()) {
EltAsInt = Elt.getFloat().bitcastToAPInt();
} else {
// Don't try to handle vectors of anything other than int or float
// (not sure if it's possible to hit this case).
Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
return false;
}
unsigned BaseEltSize = EltAsInt.getBitWidth();
if (BigEndian)
Res |= EltAsInt.zextOrTrunc(VecSize).rotr(i*EltSize+BaseEltSize);
else
Res |= EltAsInt.zextOrTrunc(VecSize).rotl(i*EltSize);
}
return true;
}
// Give up if the input isn't an int, float, or vector. For example, we
// reject "(v4i16)(intptr_t)&a".
Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
return false;
}

/// Perform the given integer operation, which is known to need at most BitWidth
/// bits, and check for overflow in the original type (if that type was not an
/// unsigned type).
Expand Down Expand Up @@ -7011,10 +6964,11 @@ class APValueToBufferConverter {
return visitArray(Val, Ty, Offset);
case APValue::Struct:
return visitRecord(Val, Ty, Offset);
case APValue::Vector:
return visitVector(Val, Ty, Offset);

case APValue::ComplexInt:
case APValue::ComplexFloat:
case APValue::Vector:
case APValue::FixedPoint:
// FIXME: We should support these.

Expand Down Expand Up @@ -7101,6 +7055,72 @@ class APValueToBufferConverter {
return true;
}

bool visitVector(const APValue &Val, QualType Ty, CharUnits Offset) {
const VectorType *VTy = Ty->castAs<VectorType>();
QualType EltTy = VTy->getElementType();
unsigned NElts = VTy->getNumElements();
unsigned EltSize =
VTy->isExtVectorBoolType() ? 1 : Info.Ctx.getTypeSize(EltTy);

if ((NElts * EltSize) % Info.Ctx.getCharWidth() != 0) {
// The vector's size in bits is not a multiple of the target's byte size,
// so its layout is unspecified. For now, we'll simply treat these cases
// as unsupported (this should only be possible with OpenCL bool vectors
// whose element count isn't a multiple of the byte size).
Info.FFDiag(BCE->getBeginLoc(),
diag::note_constexpr_bit_cast_invalid_vector)
<< Ty.getCanonicalType() << EltSize << NElts
<< Info.Ctx.getCharWidth();
return false;
}

if (EltTy->isRealFloatingType() && &Info.Ctx.getFloatTypeSemantics(EltTy) ==
&APFloat::x87DoubleExtended()) {
// The layout for x86_fp80 vectors seems to be handled very inconsistently
// by both clang and LLVM, so for now we won't allow bit_casts involving
// it in a constexpr context.
Info.FFDiag(BCE->getBeginLoc(),
diag::note_constexpr_bit_cast_unsupported_type)
<< EltTy;
return false;
}

if (VTy->isExtVectorBoolType()) {
// Special handling for OpenCL bool vectors:
// Since these vectors are stored as packed bits, but we can't write
// individual bits to the BitCastBuffer, we'll buffer all of the elements
// together into an appropriately sized APInt and write them all out at
// once. Because we don't accept vectors where NElts * EltSize isn't a
// multiple of the char size, there will be no padding space, so we don't
// have to worry about writing data which should have been left
// uninitialized.
bool BigEndian = Info.Ctx.getTargetInfo().isBigEndian();

llvm::APInt Res = llvm::APInt::getZero(NElts);
for (unsigned I = 0; I < NElts; ++I) {
const llvm::APSInt &EltAsInt = Val.getVectorElt(I).getInt();
assert(EltAsInt.isUnsigned() && EltAsInt.getBitWidth() == 1 &&
"bool vector element must be 1-bit unsigned integer!");

Res.insertBits(EltAsInt, BigEndian ? (NElts - I - 1) : I);
}

SmallVector<uint8_t, 8> Bytes(NElts / 8);
llvm::StoreIntToMemory(Res, &*Bytes.begin(), NElts / 8);
Buffer.writeObject(Offset, Bytes);
} else {
// Iterate over each of the elements and write them out to the buffer at
// the appropriate offset.
CharUnits EltSizeChars = Info.Ctx.getTypeSizeInChars(EltTy);
for (unsigned I = 0; I < NElts; ++I) {
if (!visit(Val.getVectorElt(I), EltTy, Offset + I * EltSizeChars))
return false;
}
}

return true;
}

bool visitInt(const APSInt &Val, QualType Ty, CharUnits Offset) {
APSInt AdjustedVal = Val;
unsigned Width = AdjustedVal.getBitWidth();
Expand All @@ -7109,7 +7129,7 @@ class APValueToBufferConverter {
AdjustedVal = AdjustedVal.extend(Width);
}

SmallVector<unsigned char, 8> Bytes(Width / 8);
SmallVector<uint8_t, 8> Bytes(Width / 8);
llvm::StoreIntToMemory(AdjustedVal, &*Bytes.begin(), Width / 8);
Buffer.writeObject(Offset, Bytes);
return true;
Expand Down Expand Up @@ -7310,6 +7330,77 @@ class BufferToAPValueConverter {
return ArrayValue;
}

std::optional<APValue> visit(const VectorType *VTy, CharUnits Offset) {
QualType EltTy = VTy->getElementType();
unsigned NElts = VTy->getNumElements();
unsigned EltSize =
VTy->isExtVectorBoolType() ? 1 : Info.Ctx.getTypeSize(EltTy);

if ((NElts * EltSize) % Info.Ctx.getCharWidth() != 0) {
// The vector's size in bits is not a multiple of the target's byte size,
// so its layout is unspecified. For now, we'll simply treat these cases
// as unsupported (this should only be possible with OpenCL bool vectors
// whose element count isn't a multiple of the byte size).
Info.FFDiag(BCE->getBeginLoc(),
diag::note_constexpr_bit_cast_invalid_vector)
<< QualType(VTy, 0) << EltSize << NElts << Info.Ctx.getCharWidth();
return std::nullopt;
}

if (EltTy->isRealFloatingType() && &Info.Ctx.getFloatTypeSemantics(EltTy) ==
&APFloat::x87DoubleExtended()) {
// The layout for x86_fp80 vectors seems to be handled very inconsistently
// by both clang and LLVM, so for now we won't allow bit_casts involving
// it in a constexpr context.
Info.FFDiag(BCE->getBeginLoc(),
diag::note_constexpr_bit_cast_unsupported_type)
<< EltTy;
return std::nullopt;
}

SmallVector<APValue, 4> Elts;
Elts.reserve(NElts);
if (VTy->isExtVectorBoolType()) {
// Special handling for OpenCL bool vectors:
// Since these vectors are stored as packed bits, but we can't read
// individual bits from the BitCastBuffer, we'll buffer all of the
// elements together into an appropriately sized APInt and write them all
// out at once. Because we don't accept vectors where NElts * EltSize
// isn't a multiple of the char size, there will be no padding space, so
// we don't have to worry about reading any padding data which didn't
// actually need to be accessed.
bool BigEndian = Info.Ctx.getTargetInfo().isBigEndian();

SmallVector<uint8_t, 8> Bytes;
Bytes.reserve(NElts / 8);
if (!Buffer.readObject(Offset, CharUnits::fromQuantity(NElts / 8), Bytes))
return std::nullopt;

APSInt SValInt(NElts, true);
llvm::LoadIntFromMemory(SValInt, &*Bytes.begin(), Bytes.size());

for (unsigned I = 0; I < NElts; ++I) {
llvm::APInt Elt =
SValInt.extractBits(1, (BigEndian ? NElts - I - 1 : I) * EltSize);
Elts.emplace_back(
APSInt(std::move(Elt), !EltTy->isSignedIntegerType()));
}
} else {
// Iterate over each of the elements and read them from the buffer at
// the appropriate offset.
CharUnits EltSizeChars = Info.Ctx.getTypeSizeInChars(EltTy);
for (unsigned I = 0; I < NElts; ++I) {
std::optional<APValue> EltValue =
visitType(EltTy, Offset + I * EltSizeChars);
if (!EltValue)
return std::nullopt;
Elts.push_back(std::move(*EltValue));
}
}

return APValue(Elts.data(), Elts.size());
}

std::optional<APValue> visit(const Type *Ty, CharUnits Offset) {
return unsupportedType(QualType(Ty, 0));
}
Expand Down Expand Up @@ -7409,25 +7500,15 @@ static bool checkBitCastConstexprEligibility(EvalInfo *Info,
return SourceOK;
}

static bool handleLValueToRValueBitCast(EvalInfo &Info, APValue &DestValue,
APValue &SourceValue,
static bool handleRValueToRValueBitCast(EvalInfo &Info, APValue &DestValue,
const APValue &SourceRValue,
const CastExpr *BCE) {
assert(CHAR_BIT == 8 && Info.Ctx.getTargetInfo().getCharWidth() == 8 &&
"no host or target supports non 8-bit chars");
assert(SourceValue.isLValue() &&
"LValueToRValueBitcast requires an lvalue operand!");

if (!checkBitCastConstexprEligibility(&Info, Info.Ctx, BCE))
return false;

LValue SourceLValue;
APValue SourceRValue;
SourceLValue.setFrom(Info.Ctx, SourceValue);
if (!handleLValueToRValueConversion(
Info, BCE, BCE->getSubExpr()->getType().withConst(), SourceLValue,
SourceRValue, /*WantObjectRepresentation=*/true))
return false;

// Read out SourceValue into a char buffer.
std::optional<BitCastBuffer> Buffer =
APValueToBufferConverter::convert(Info, SourceRValue, BCE);
Expand All @@ -7444,6 +7525,25 @@ static bool handleLValueToRValueBitCast(EvalInfo &Info, APValue &DestValue,
return true;
}

static bool handleLValueToRValueBitCast(EvalInfo &Info, APValue &DestValue,
APValue &SourceValue,
const CastExpr *BCE) {
assert(CHAR_BIT == 8 && Info.Ctx.getTargetInfo().getCharWidth() == 8 &&
"no host or target supports non 8-bit chars");
assert(SourceValue.isLValue() &&
"LValueToRValueBitcast requires an lvalue operand!");

LValue SourceLValue;
APValue SourceRValue;
SourceLValue.setFrom(Info.Ctx, SourceValue);
if (!handleLValueToRValueConversion(
Info, BCE, BCE->getSubExpr()->getType().withConst(), SourceLValue,
SourceRValue, /*WantObjectRepresentation=*/true))
return false;

return handleRValueToRValueBitCast(Info, DestValue, SourceRValue, BCE);
}

template <class Derived>
class ExprEvaluatorBase
: public ConstStmtVisitor<Derived, bool> {
Expand Down Expand Up @@ -10528,41 +10628,22 @@ bool VectorExprEvaluator::VisitCastExpr(const CastExpr *E) {
return Success(Elts, E);
}
case CK_BitCast: {
// Evaluate the operand into an APInt we can extract from.
llvm::APInt SValInt;
if (!EvalAndBitcastToAPInt(Info, SE, SValInt))
APValue SVal;
if (!Evaluate(SVal, Info, SE))
return false;

if (!SVal.isInt() && !SVal.isFloat() && !SVal.isVector()) {
// Give up if the input isn't an int, float, or vector. For example, we
// reject "(v4i16)(intptr_t)&a".
Info.FFDiag(E, diag::note_constexpr_invalid_cast)
<< 2 << Info.Ctx.getLangOpts().CPlusPlus;
return false;
// Extract the elements
QualType EltTy = VTy->getElementType();
unsigned EltSize = Info.Ctx.getTypeSize(EltTy);
bool BigEndian = Info.Ctx.getTargetInfo().isBigEndian();
SmallVector<APValue, 4> Elts;
if (EltTy->isRealFloatingType()) {
const llvm::fltSemantics &Sem = Info.Ctx.getFloatTypeSemantics(EltTy);
unsigned FloatEltSize = EltSize;
if (&Sem == &APFloat::x87DoubleExtended())
FloatEltSize = 80;
for (unsigned i = 0; i < NElts; i++) {
llvm::APInt Elt;
if (BigEndian)
Elt = SValInt.rotl(i * EltSize + FloatEltSize).trunc(FloatEltSize);
else
Elt = SValInt.rotr(i * EltSize).trunc(FloatEltSize);
Elts.push_back(APValue(APFloat(Sem, Elt)));
}
} else if (EltTy->isIntegerType()) {
for (unsigned i = 0; i < NElts; i++) {
llvm::APInt Elt;
if (BigEndian)
Elt = SValInt.rotl(i*EltSize+EltSize).zextOrTrunc(EltSize);
else
Elt = SValInt.rotr(i*EltSize).zextOrTrunc(EltSize);
Elts.push_back(APValue(APSInt(Elt, !EltTy->isSignedIntegerType())));
}
} else {
return Error(E);
}
return Success(Elts, E);

if (!handleRValueToRValueBitCast(Info, Result, SVal, E))
return false;

return true;
}
default:
return ExprEvaluatorBaseTy::VisitCastExpr(E);
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/CodeGen/CGExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2151,6 +2151,9 @@ llvm::Constant *ConstantEmitter::tryEmitPrivate(const APValue &Value,
Inits[I] = llvm::ConstantInt::get(CGM.getLLVMContext(), Elt.getInt());
else if (Elt.isFloat())
Inits[I] = llvm::ConstantFP::get(CGM.getLLVMContext(), Elt.getFloat());
else if (Elt.isIndeterminate())
Inits[I] = llvm::UndefValue::get(CGM.getTypes().ConvertType(
DestType->castAs<VectorType>()->getElementType()));
else
llvm_unreachable("unsupported vector element type");
}
Expand Down
9 changes: 5 additions & 4 deletions clang/test/CodeGen/const-init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,12 @@ void g28(void) {
typedef short v12i16 __attribute((vector_size(24)));
typedef long double v2f80 __attribute((vector_size(24)));
// CHECK: @g28.a = internal global <1 x i64> <i64 10>
// CHECK: @g28.b = internal global <12 x i16> <i16 0, i16 0, i16 0, i16 -32768, i16 16383, i16 0, i16 0, i16 0, i16 0, i16 -32768, i16 16384, i16 0>
// CHECK: @g28.c = internal global <2 x x86_fp80> <x86_fp80 0xK3FFF8000000000000000, x86_fp80 0xK40008000000000000000>, align 32
// @g28.b = internal global <12 x i16> <i16 0, i16 0, i16 0, i16 -32768, i16 16383, i16 0, i16 0, i16 0, i16 0, i16 -32768, i16 16384, i16 0>
// @g28.c = internal global <2 x x86_fp80> <x86_fp80 0xK3FFF8000000000000000, x86_fp80 0xK40008000000000000000>, align 32
static v1i64 a = (v1i64)10LL;
static v12i16 b = (v12i16)(v2f80){1,2};
static v2f80 c = (v2f80)(v12i16){0,0,0,-32768,16383,0,0,0,0,-32768,16384,0};
//FIXME: support constant bitcast between vectors of x86_fp80
//static v12i16 b = (v12i16)(v2f80){1,2};
//static v2f80 c = (v2f80)(v12i16){0,0,0,-32768,16383,0,0,0,0,-32768,16384,0};
Comment on lines +147 to +148
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add some test coverage that we properly diagnose this situation.

I think we'll produce a diagnostic mentioning bit_cast, which is probably not what we want here. Maybe the simplest thing to do to fix that would be to replace bit_cast with bit cast in all the diagnostics. That seems at least as good for the std::bit_cast / __builtin_bit_cast case, and better for the vector cast case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interestingly, uncommenting these lines doesn't cause any diagnostic messages to be generated, only an error stating error: initializer element is not a compile-time constant. I'll see if I can get the diagnostic to be output and change the messages as suggested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, on further investigation, it looks like the diagnostic isn't being emitted at all because the constant evaluation code is called with an EvalInfo which has diagnostics disabled, and is never called again.

https://github.com/llvm/llvm-project/blob/72f63b695c9ebd9c7032c4b754ff7965c28fad5c/clang/lib/AST/Expr.cpp#L3486
https://github.com/llvm/llvm-project/blob/72f63b695c9ebd9c7032c4b754ff7965c28fad5c/clang/lib/AST/ExprConstant.cpp#L15483-L15485

And once it's been evaluted with no diagnostic, an diag::err_init_element_not_constant is emitted which triggers an error and so it never gets evaluated again with diagnostics enabled.

https://github.com/llvm/llvm-project/blob/72f63b695c9ebd9c7032c4b754ff7965c28fad5c/clang/lib/Sema/SemaDecl.cpp#L12510-L12514

What should I do here? This feels like a weakness of the code in Sema; any changes I make here are going to break a bunch of tests which expect no diagnostics to be output in these cases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps you could add a C++ testcass with the variable declared constexpr.

}

// PR13643
Expand Down
Loading