[PATCH] align_value attribute in Clang
Aaron Ballman
aaron.ballman at gmail.com
Thu Oct 2 14:04:07 PDT 2014
On Thu, Oct 2, 2014 at 4:10 PM, hfinkel at anl.gov <hfinkel at anl.gov> wrote:
> Alright, after much discussion, I think this is ready.
>
> To quickly recap, there had been some discussion regarding whether or not align_value needed to be part of the type system (so that it would be propagated by template type deduction, for example), after after receiving feedback from Richard, Alexey, et al., we're settled on no. This is not part of the type system, and will only "propagate" through templates, auto, etc. by optimizer deduction after inlining. This seems consistent with Intel's implementation.
>
> Aaron, this new revision should account for all additional points from your last review.
Aside from some very minor formatting nits, LGTM!
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -354,6 +354,25 @@
> let Documentation = [Undocumented];
> }
>
> +def AlignValue : Attr {
> + let Spellings = [
> + // Unfortunately, this is semantically an assertion, not a
> + // directive (something else must ensure the alignment), so
> + // aligned_value is a probably a better name. We might want
> + // to add an aligned_value spelling in the future (and a
> + // corresponding C++ attribute), but this can be done later
> + // once we decide if we also want them to have
> + // slightly-different semantics than Intel's align_value.
> + GNU<"align_value">
> + // Intel's compiler on Windows also supports:
> + // , Declspec<"align_value">
> + ];
This formatting is unlike anything else in the file. Would be good to
tighten it up a bit by shifting to be indented under the let.
> + let Args = [ExprArgument<"Alignment">];
> + let Subjects = SubjectList<[Var, TypedefName], WarnDiag,
> + "ExpectedVariableOrTypedef">;
> + let Documentation = [AlignValueDocs];
> +}
> +
> def AlignMac68k : InheritableAttr {
> // This attribute has no spellings as it is only ever created implicitly.
> let Spellings = [];
> Index: include/clang/Basic/AttrDocs.td
> ===================================================================
> --- include/clang/Basic/AttrDocs.td
> +++ include/clang/Basic/AttrDocs.td
> @@ -1011,6 +1011,26 @@
> }];
> }
>
> +def AlignValueDocs : Documentation {
> + let Category = DocCatType;
> + let Content = [{
> +The align_value attribute can be added to the typedef of a pointer type or the
> +declaration of a variable of pointer or reference type. It specifies that the
> +pointer will point to, or the reference will bind to, only objects with at
> +least the provided alignment. This alignment value must be some positive power
> +of 2.
> +
> + .. code-block:: c
> +
> + typedef double * aligned_double_ptr __attribute__((align_value(64)));
> + void foo(double & x __attribute__((align_value(128)),
> + aligned_double_ptr y) { ... }
> +
> +If the pointer value does not have the specified alignment at runtime, the
> +behavior of the program is undefined.
> + }];
> +}
> +
> def MSInheritanceDocs : Documentation {
> let Category = DocCatType;
> let Heading = "__single_inhertiance, __multiple_inheritance, __virtual_inheritance";
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -1930,8 +1930,12 @@
> "Neon vector size must be 64 or 128 bits">;
> def err_attribute_unsupported : Error<
> "%0 attribute is not supported for this target">;
> +// The err_*_attribute_argument_not_int are seperate because they're used by
> +// VerifyIntegerConstantExpression.
> def err_aligned_attribute_argument_not_int : Error<
> "'aligned' attribute requires integer constant">;
> +def err_align_value_attribute_argument_not_int : Error<
> + "'align_value' attribute requires integer constant">;
> def err_alignas_attribute_wrong_decl_type : Error<
> "%0 attribute cannot be applied to a %select{function parameter|"
> "variable with 'register' storage class|'catch' variable|bit-field}1">;
> @@ -1970,6 +1974,9 @@
> def warn_attribute_return_pointers_refs_only : Warning<
> "%0 attribute only applies to return values that are pointers or references">,
> InGroup<IgnoredAttributes>;
> +def warn_attribute_pointer_or_reference_only : Warning<
> + "%0 attribute only applies to a pointer or reference (%1 is invalid)">,
> + InGroup<IgnoredAttributes>;
> def err_attribute_no_member_pointers : Error<
> "%0 attribute cannot be used with pointers to members">;
> def err_attribute_invalid_implicit_this_argument : Error<
> @@ -2210,7 +2217,7 @@
> "functions, methods and blocks|functions, methods, and classes|"
> "functions, methods, and parameters|classes|variables|methods|"
> "variables, functions and labels|fields and global variables|structs|"
> - "variables, functions and tag types|thread-local variables|"
> + "variables and typedefs|thread-local variables|"
> "variables and fields|variables, data members and tag types|"
> "types and namespaces|Objective-C interfaces|methods and properties|"
> "struct or union|struct, union or class|types|"
> Index: include/clang/Sema/AttributeList.h
> ===================================================================
> --- include/clang/Sema/AttributeList.h
> +++ include/clang/Sema/AttributeList.h
> @@ -827,7 +827,7 @@
> ExpectedVariableFunctionOrLabel,
> ExpectedFieldOrGlobalVar,
> ExpectedStruct,
> - ExpectedVariableFunctionOrTag,
> + ExpectedVariableOrTypedef,
> ExpectedTLSVar,
> ExpectedVariableOrField,
> ExpectedVariableFieldOrTag,
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -7363,6 +7363,11 @@
> void AddAssumeAlignedAttr(SourceRange AttrRange, Decl *D, Expr *E, Expr *OE,
> unsigned SpellingListIndex);
>
> + /// AddAlignValueAttr - Adds an align_value attribute to a particular
> + /// declaration.
> + void AddAlignValueAttr(SourceRange AttrRange, Decl *D, Expr *E,
> + unsigned SpellingListIndex);
> +
> // OpenMP directives and clauses.
> private:
> void *VarDataSharingAttributesStack;
> Index: lib/CodeGen/CGCall.cpp
> ===================================================================
> --- lib/CodeGen/CGCall.cpp
> +++ lib/CodeGen/CGCall.cpp
> @@ -1743,6 +1743,25 @@
> AI->getArgNo() + 1,
> llvm::Attribute::NonNull));
> }
> +
> + const auto *AVAttr = PVD->getAttr<AlignValueAttr>();
> + if (!AVAttr)
> + if (const auto *TOTy = dyn_cast<TypedefType>(OTy))
> + AVAttr = TOTy->getDecl()->getAttr<AlignValueAttr>();
> + if (AVAttr) {
> + llvm::Value *AlignmentValue =
> + EmitScalarExpr(AVAttr->getAlignment());
> + llvm::ConstantInt *AlignmentCI =
> + cast<llvm::ConstantInt>(AlignmentValue);
> + unsigned Alignment =
> + std::min((unsigned) AlignmentCI->getZExtValue(),
> + +llvm::Value::MaximumAlignment);
> +
> + llvm::AttrBuilder Attrs;
> + Attrs.addAlignmentAttr(Alignment);
> + AI->addAttr(llvm::AttributeSet::get(getLLVMContext(),
> + AI->getArgNo() + 1, Attrs));
Formatting.
> + }
> }
>
> if (Arg->getType().isRestrictQualified())
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -2757,6 +2757,58 @@
> Attr.getAttributeSpellingListIndex()));
> }
>
> +static void handleAlignValueAttr(Sema &S, Decl *D,
> + const AttributeList &Attr) {
> + S.AddAlignValueAttr(Attr.getRange(), D, Attr.getArgAsExpr(0),
> + Attr.getAttributeSpellingListIndex());
> +}
> +
> +void Sema::AddAlignValueAttr(SourceRange AttrRange, Decl *D, Expr *E,
> + unsigned SpellingListIndex) {
> + AlignValueAttr TmpAttr(AttrRange, Context, E, SpellingListIndex);
> + SourceLocation AttrLoc = AttrRange.getBegin();
> +
> + QualType T;
> + if (TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(D))
> + T = TD->getUnderlyingType();
> + else if (ValueDecl *VD = dyn_cast<ValueDecl>(D))
> + T = VD->getType();
> + else
> + llvm_unreachable("Unknown decl type for align_value");
> +
> + if (!T->isDependentType() && !T->isAnyPointerType() &&
> + !T->isReferenceType() && !T->isMemberPointerType()) {
> + Diag(AttrLoc, diag::warn_attribute_pointer_or_reference_only)
> + << &TmpAttr /*TmpAttr.getName()*/ << T << D->getSourceRange();
> + return;
> + }
> +
> + if (!E->isValueDependent()) {
> + llvm::APSInt Alignment(32);
> + ExprResult ICE
> + = VerifyIntegerConstantExpression(E, &Alignment,
> + diag::err_align_value_attribute_argument_not_int,
> + /*AllowFold*/ false);
> + if (ICE.isInvalid())
> + return;
> +
> + if (!Alignment.isPowerOf2()) {
> + Diag(AttrLoc, diag::err_alignment_not_power_of_two)
> + << E->getSourceRange();
> + return;
> + }
> +
> + D->addAttr(::new (Context)
> + AlignValueAttr(AttrRange, Context, ICE.get(),
> + SpellingListIndex));
Formatting.
> + return;
> + }
> +
> + // Save dependent expressions in the AST to be instantiated.
> + D->addAttr(::new (Context) AlignValueAttr(TmpAttr));
> + return;
> +}
> +
> static void handleAlignedAttr(Sema &S, Decl *D, const AttributeList &Attr) {
> // check the attribute arguments.
> if (Attr.getNumArgs() > 1) {
> @@ -4181,6 +4233,9 @@
> case AttributeList::AT_Aligned:
> handleAlignedAttr(S, D, Attr);
> break;
> + case AttributeList::AT_AlignValue:
> + handleAlignValueAttr(S, D, Attr);
> + break;
> case AttributeList::AT_AlwaysInline:
> handleAlwaysInlineAttr(S, D, Attr);
> break;
> Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
> ===================================================================
> --- lib/Sema/SemaTemplateInstantiateDecl.cpp
> +++ lib/Sema/SemaTemplateInstantiateDecl.cpp
> @@ -152,6 +152,17 @@
> Aligned->getSpellingListIndex());
> }
>
> +static void instantiateDependentAlignValueAttr(
> + Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
> + const AlignValueAttr *Aligned, Decl *New) {
> + // The alignment expression is a constant expression.
> + EnterExpressionEvaluationContext Unevaluated(S, Sema::ConstantEvaluated);
> + ExprResult Result = S.SubstExpr(Aligned->getAlignment(), TemplateArgs);
> + if (!Result.isInvalid())
> + S.AddAlignValueAttr(Aligned->getLocation(), New, Result.getAs<Expr>(),
> + Aligned->getSpellingListIndex());
> +}
> +
> static void instantiateDependentEnableIfAttr(
> Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
> const EnableIfAttr *A, const Decl *Tmpl, Decl *New) {
> @@ -205,6 +216,12 @@
> continue;
> }
>
> + const AlignValueAttr *AlignValue = dyn_cast<AlignValueAttr>(TmplAttr);
> + if (AlignValue) {
> + instantiateDependentAlignValueAttr(*this, TemplateArgs, AlignValue, New);
> + continue;
> + }
> +
> const EnableIfAttr *EnableIf = dyn_cast<EnableIfAttr>(TmplAttr);
> if (EnableIf && EnableIf->getCond()->isValueDependent()) {
> instantiateDependentEnableIfAttr(*this, TemplateArgs, EnableIf, Tmpl,
> Index: test/CodeGen/align_value.cpp
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/align_value.cpp
> @@ -0,0 +1,8 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
> +
> +typedef double * __attribute__((align_value(64))) aligned_double;
> +
> +void foo(aligned_double x, double * y __attribute__((align_value(32))),
> + double & z __attribute__((align_value(128)))) { };
> +// CHECK: define void @_Z3fooPdS_Rd(double* align 64 %x, double* align 32 %y, double* dereferenceable(8) align 128 %z)
> +
> Index: test/Sema/align_value.c
> ===================================================================
> --- /dev/null
> +++ test/Sema/align_value.c
> @@ -0,0 +1,32 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +typedef double * __attribute__((align_value(64))) aligned_double;
> +
> +void foo(aligned_double x, double * y __attribute__((align_value(32)))) { };
> +
> +// expected-error at +1 {{requested alignment is not a power of 2}}
> +typedef double * __attribute__((align_value(63))) aligned_double1;
> +
> +// expected-error at +1 {{requested alignment is not a power of 2}}
> +typedef double * __attribute__((align_value(-2))) aligned_double2;
> +
> +// expected-error at +1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value(63, 4))) aligned_double3;
> +
> +// expected-error at +1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value())) aligned_double3a;
> +
> +// expected-error at +1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value)) aligned_double3b;
> +
> +// expected-error at +1 {{'align_value' attribute requires integer constant}}
> +typedef double * __attribute__((align_value(4.5))) aligned_double4;
> +
> +// expected-warning at +1 {{'align_value' attribute only applies to a pointer or reference ('int' is invalid)}}
> +typedef int __attribute__((align_value(32))) aligned_int;
> +
> +typedef double * __attribute__((align_value(32*2))) aligned_double5;
> +
> +// expected-warning at +1 {{'align_value' attribute only applies to variables and typedefs}}
> +void foo() __attribute__((align_value(32)));
> +
> Index: test/SemaCXX/align_value.cpp
> ===================================================================
> --- /dev/null
> +++ test/SemaCXX/align_value.cpp
> @@ -0,0 +1,26 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +typedef double * __attribute__((align_value(64))) aligned_double;
> +
> +void foo(aligned_double x, double * y __attribute__((align_value(32))),
> + double & z __attribute__((align_value(128)))) { };
> +
> +template <typename T, int Q>
> +struct x {
> + typedef T* aligned_int __attribute__((align_value(32+8*Q)));
> + aligned_int V;
> +
> + void foo(aligned_int a, T &b __attribute__((align_value(sizeof(T)*4))));
> +};
> +
> +x<float, 4> y;
> +
> +template <typename T, int Q>
> +struct nope {
> + // expected-error at +1 {{requested alignment is not a power of 2}}
> + void foo(T &b __attribute__((align_value(sizeof(T)+1))));
> +};
> +
> +// expected-note at +1 {{in instantiation of template class 'nope<long double, 4>' requested here}}
> +nope<long double, 4> y2;
> +
>
~Aaron
More information about the cfe-commits
mailing list