Skip to content

[clang][ExprConst] allow single element access of vector object to be constant expression #72607

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

Closed
wants to merge 1 commit into from
Closed

[clang][ExprConst] allow single element access of vector object to be constant expression #72607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ sections with improvements to Clang's support for those languages.

C++ Language Changes
--------------------
- Allow single element access of vector object to be constant expression.
Supports the `V.xyzw` syntax and other tidbits as seen in OpenCL.
Selecting multiple elements is left as a future work.

C++20 Feature Support
^^^^^^^^^^^^^^^^^^^^^
Expand Down
103 changes: 100 additions & 3 deletions clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ namespace {
ArraySize = 2;
MostDerivedLength = I + 1;
IsArray = true;
} else if (const auto *VT = Type->getAs<VectorType>()) {
Type = VT->getElementType();
ArraySize = VT->getNumElements();
MostDerivedLength = I + 1;
IsArray = true;
} else if (const FieldDecl *FD = getAsField(Path[I])) {
Type = FD->getType();
ArraySize = 0;
Expand Down Expand Up @@ -437,6 +442,16 @@ namespace {
MostDerivedArraySize = 2;
MostDerivedPathLength = Entries.size();
}
void addVectorUnchecked(QualType EltTy, uint64_t Size, uint64_t Idx) {
Entries.push_back(PathEntry::ArrayIndex(Idx));

// This is technically a most-derived object, though in practice this
// is unlikely to matter.
MostDerivedType = EltTy;
MostDerivedIsArrayElement = true;
MostDerivedArraySize = Size;
Comment on lines +451 to +452
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this: it seems to indicate that an expression like &V[0] ought to be valid, but it's often not possible to take a pointer to a vector element (i.e. when they're living in a vector register).

That said, I see it's what the Complex type is doing, and I whose elements I think are also not individually addressable, so 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

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

That said, I see it's what the Complex type is doing, and I whose elements I think are also not individually addressable, so 🤷

C23 6.2.5p17: Each complex type has the same representation and alignment requirements as an array type containing exactly two elements of the corresponding real type; the first element is equal to the real part, and the second element to the imaginary part, of the complex number.

CC @jcranmer-intel @arsenm for some other opinions on whether you should be able to address a vector type in a constant expression.

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 accessing individual fields should be OK. Taking the address of a vector element feels wrong, but I haven't thought too deeply about it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Until we have evidence that &V[0] makes sense for vector types, I think we should disallow it. It's easier for us to introduce support once we have a use case than to have to maintain support should someone start relying on this without us intending it as a stable extension.

MostDerivedPathLength = Entries.size();
}
void diagnoseUnsizedArrayPointerArithmetic(EvalInfo &Info, const Expr *E);
void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E,
const APSInt &N);
Expand Down Expand Up @@ -1732,6 +1747,11 @@ namespace {
if (checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real))
Designator.addComplexUnchecked(EltTy, Imag);
}
void addVectorElement(EvalInfo &Info, const Expr *E, QualType EltTy,
uint64_t Size, uint64_t Idx) {
if (checkSubobject(Info, E, CSK_VectorElement))
Designator.addVectorUnchecked(EltTy, Size, Idx);
}
void clearIsNullPointer() {
IsNullPtr = false;
}
Expand Down Expand Up @@ -3278,6 +3298,19 @@ static bool HandleLValueComplexElement(EvalInfo &Info, const Expr *E,
return true;
}

static bool HandleLValueVectorElement(EvalInfo &Info, const Expr *E,
LValue &LVal, QualType EltTy,
uint64_t Size, uint64_t Idx) {
if (Idx) {
CharUnits SizeOfElement;
if (!HandleSizeof(Info, E->getExprLoc(), EltTy, SizeOfElement))
return false;
LVal.Offset += SizeOfElement * Idx;
}
LVal.addVectorElement(Info, E, EltTy, Size, Idx);
return true;
}

/// Try to evaluate the initializer for a variable declaration.
///
/// \param Info Information about the ongoing evaluation.
Expand Down Expand Up @@ -3823,6 +3856,21 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
return handler.found(Index ? O->getComplexFloatImag()
: O->getComplexFloatReal(), ObjType);
}
} else if (const auto *VT = ObjType->getAs<VectorType>()) {
uint64_t Index = Sub.Entries[I].getAsArrayIndex();
if (Index >= VT->getNumElements()) {
if (Info.getLangOpts().CPlusPlus11)
Info.FFDiag(E, diag::note_constexpr_access_past_end)
<< handler.AccessKind;
else
Info.FFDiag(E);
return handler.failed();
}

ObjType = VT->getElementType();

assert(I == N - 1 && "extracting subobject of scalar?");
return handler.found(O->getVectorElt(Index), ObjType);
} else if (const FieldDecl *Field = getAsField(Sub.Entries[I])) {
if (Field->isMutable() &&
!Obj.mayAccessMutableMembers(Info, handler.AccessKind)) {
Expand Down Expand Up @@ -8432,6 +8480,7 @@ class LValueExprEvaluator
bool VisitCXXTypeidExpr(const CXXTypeidExpr *E);
bool VisitCXXUuidofExpr(const CXXUuidofExpr *E);
bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
bool VisitExtVectorElementExpr(const ExtVectorElementExpr *E);
bool VisitUnaryDeref(const UnaryOperator *E);
bool VisitUnaryReal(const UnaryOperator *E);
bool VisitUnaryImag(const UnaryOperator *E);
Expand Down Expand Up @@ -8755,15 +8804,63 @@ bool LValueExprEvaluator::VisitMemberExpr(const MemberExpr *E) {
return LValueExprEvaluatorBaseTy::VisitMemberExpr(E);
}

bool LValueExprEvaluator::VisitExtVectorElementExpr(
const ExtVectorElementExpr *E) {
bool Success = true;

APValue Val;
if (!Evaluate(Val, Info, E->getBase())) {
if (!Info.noteFailure())
return false;
Success = false;
}

SmallVector<uint32_t, 4> Indices;
E->getEncodedElementAccess(Indices);
// FIXME: support accessing more than one element
if (Indices.size() > 1)
return false;

if (Success) {
Result.setFrom(Info.Ctx, Val);
const auto *VT = E->getBase()->getType()->castAs<VectorType>();
HandleLValueVectorElement(Info, E, Result, VT->getElementType(),
VT->getNumElements(), Indices[0]);
}

return Success;
}

bool LValueExprEvaluator::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
// FIXME: Deal with vectors as array subscript bases.
if (E->getBase()->getType()->isVectorType() ||
E->getBase()->getType()->isSveVLSBuiltinType())
if (E->getBase()->getType()->isSveVLSBuiltinType())
return Error(E);

APSInt Index;
bool Success = true;

if (const auto *VT = E->getBase()->getType()->getAs<VectorType>()) {
APValue Val;
if (!Evaluate(Val, Info, E->getBase())) {
if (!Info.noteFailure())
return false;
Success = false;
}

if (!EvaluateInteger(E->getIdx(), Index, Info)) {
if (!Info.noteFailure())
return false;
Success = false;
}

if (Success) {
Result.setFrom(Info.Ctx, Val);
HandleLValueVectorElement(Info, E, Result, VT->getElementType(),
VT->getNumElements(), Index.getExtValue());
}

return Success;
}

// C++17's rules require us to evaluate the LHS first, regardless of which
// side is the base.
for (const Expr *SubExpr : {E->getLHS(), E->getRHS()}) {
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/Interp/State.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ enum CheckSubobjectKind {
CSK_ArrayToPointer,
CSK_ArrayIndex,
CSK_Real,
CSK_Imag
CSK_Imag,
CSK_VectorElement
};

namespace interp {
Expand Down
43 changes: 21 additions & 22 deletions clang/test/CodeGenCXX/temporaries.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,27 @@ namespace RefTempSubobject {
constexpr const SelfReferential &sr = SelfReferential();
}

namespace Vector {
typedef __attribute__((vector_size(16))) int vi4a;
typedef __attribute__((ext_vector_type(4))) int vi4b;
struct S {
vi4a v;
vi4b w;
};

int &&r = S().v[1];
// CHECK: @_ZGRN6Vector1rE_ = internal global i32 0, align 4
// CHECK: @_ZN6Vector1rE = constant ptr @_ZGRN6Vector1rE_, align 8

int &&s = S().w[1];
// CHECK: @_ZGRN6Vector1sE_ = internal global i32 0, align 4
// CHECK: @_ZN6Vector1sE = constant ptr @_ZGRN6Vector1sE_, align 8

int &&t = S().w.y;
// CHECK: @_ZGRN6Vector1tE_ = internal global i32 0, align 4
// CHECK: @_ZN6Vector1tE = constant ptr @_ZGRN6Vector1tE_, align 8
}

struct A {
A();
~A();
Expand Down Expand Up @@ -666,28 +687,6 @@ namespace Bitfield {
int &&r = S().a;
}

namespace Vector {
typedef __attribute__((vector_size(16))) int vi4a;
typedef __attribute__((ext_vector_type(4))) int vi4b;
struct S {
vi4a v;
vi4b w;
};
// CHECK: alloca
// CHECK: extractelement
// CHECK: store i32 {{.*}}, ptr @_ZGRN6Vector1rE_
// CHECK: store ptr @_ZGRN6Vector1rE_, ptr @_ZN6Vector1rE,
int &&r = S().v[1];

// CHECK: alloca
// CHECK: extractelement
// CHECK: store i32 {{.*}}, ptr @_ZGRN6Vector1sE_
// CHECK: store ptr @_ZGRN6Vector1sE_, ptr @_ZN6Vector1sE,
int &&s = S().w[1];
// FIXME PR16204: The following code leads to an assertion in Sema.
//int &&s = S().w.y;
}

namespace ImplicitTemporaryCleanup {
struct A { A(int); ~A(); };
void g();
Expand Down
29 changes: 29 additions & 0 deletions clang/test/SemaCXX/constexpr-vectors-access-elements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++17 -fsyntax-only -verify

namespace Vector {

using TwoIntsVecSize __attribute__((vector_size(8))) = int;

constexpr TwoIntsVecSize a = {1,2};
static_assert(a[1] == 2);
static_assert(a[2]); // expected-error {{not an integral constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}

}

namespace ExtVector {

using FourIntsExtVec __attribute__((ext_vector_type(4))) = int;

constexpr FourIntsExtVec b = {1,2,3,4};
static_assert(b[0] == 1 && b[1] == 2 && b[2] == 3 && b[3] == 4);
static_assert(b.s0 == 1 && b.s1 == 2 && b.s2 == 3 && b.s3 == 4);
static_assert(b.x == 1 && b.y == 2 && b.z == 3 && b.w == 4);
static_assert(b.r == 1 && b.g == 2 && b.b == 3 && b.a == 4);
static_assert(b[5]); // expected-error {{not an integral constant expression}} expected-note {{read of dereferenced one-past-the-end pointer}}

// FIXME: support selecting multiple elements
static_assert(b.lo.lo == 1); // expected-error {{not an integral constant expression}}
// static_assert(b.lo.lo==1 && b.lo.hi==2 && b.hi.lo == 3 && b.hi.hi == 4);
// static_assert(b.odd[0]==1 && b.odd[1]==2 && b.even[0] == 3 && b.even[1] == 4);

}