[PATCH] align_value attribute in Clang
Aaron Ballman
aaron.ballman at gmail.com
Thu Jul 24 09:21:10 PDT 2014
On Tue, Jul 22, 2014 at 10:09 PM, hfinkel at anl.gov <hfinkel at anl.gov> wrote:
> Hi rsmith, aaron.ballman,
>
> This patch introduces support for the align_value attribute. This attribute is supported by Intel's compiler (versions 14.0+), and several of my HPC users have requested support in Clang. It specifies an alignment assumption on the values to which a pointer points, and is used by numerical libraries to encourage efficient generation of vector code.
>
> Of course, we already have an aligned attribute that can specify enhanced alignment for a type, so why is this additional attribute important? The problem is that if you want to specify that an input array of T is, say, 64-byte aligned, you could try this:
>
> typedef double aligned_double __attribute__((aligned(64)));
> void foo(aligned_double *P) {
> double x = P[0]; // This is fine.
> double y = P[1]; // What alignment did those doubles have again?
> }
>
> the access here to P[1] causes problems. P was specified as a pointer to type aligned_double, and any object of type aligned_double must be 64-byte aligned. But if P[0] is 64-byte aligned, then P[1] cannot be, and this access causes undefined behavior. Getting round this problem requires a lot of awkward casting and hand-unrolling of loops, all of which is bad.
>
> With the align_value attribute, we can accomplish what we'd like in a well defined way:
> typedef double *aligned_double_ptr __attribute__((align_value(64)));
> void foo(aligned_double_ptr P) {
> double x = P[0]; // This is fine.
> double y = P[1]; // This is fine too.
> }
>
> This patch is not predicated on any uncommitted LLVM features -- CodeGen just adds the align attribute on function arguments, which has the desired effect as of r213670 -- (although, as follow-up work, can be enhanced to apply to more than just function parameters when combined with the in-development llvm.assume functionality).
>
> Some documentation on the align_value attribute from Intel is on this page:
> https://software.intel.com/en-us/articles/data-alignment-to-assist-vectorization
>
> Thanks again!
>
> P.S. I would have chosen to call this aligned_value, not align_value, for better naming consistency with the aligned attribute, but I think it would be more useful to users to adopt Intel's name.
>
> http://reviews.llvm.org/D4635
>
> Files:
> include/clang/Basic/Attr.td
> include/clang/Basic/AttrDocs.td
> include/clang/Basic/DiagnosticSemaKinds.td
> include/clang/Sema/Sema.h
> lib/CodeGen/CGCall.cpp
> lib/Sema/SemaDeclAttr.cpp
> lib/Sema/SemaTemplateInstantiateDecl.cpp
> test/CodeGen/align_value.cpp
> test/Sema/align_value.cpp
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -354,6 +354,12 @@
> let Documentation = [Undocumented];
> }
>
> +def AlignValue : InheritableAttr {
> + let Spellings = [GNU<"align_value">, Declspec<"align_value">];
I cannot find any documentation on this being a valid Declspec
attribute on MSDN (and VS 2013 concurs). Is this something in one of
the newest CTPs?
Do we want a clang-scoped C++11 attribute for this?
> + let Args = [ExprArgument<"Alignment">];
> + let Documentation = [AlignValueDocs];
What subjects does this attribute appertain to?
> +}
> +
> 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
> @@ -987,6 +987,24 @@
> }];
> }
>
> +def AlignValueDocs : Documentation {
> + let Category = DocCatType;
> + let Heading = "align_value";
Shouldn't be a need to specify the heading here since the spellings
are all identical.
> + 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) { ... }
> + }];
> +}
> +
> 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
> @@ -1899,6 +1899,8 @@
> "%0 attribute is not supported for this target">;
> 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">;
This should be combined with the preceding diagnostic and just use a %select.
> 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">;
> @@ -1934,6 +1936,9 @@
> def warn_attribute_return_pointers_only : Warning<
> "%0 attribute only applies to return values that are pointers">,
> 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<
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -7283,6 +7283,11 @@
> void AddAlignedAttr(SourceRange AttrRange, Decl *D, TypeSourceInfo *T,
> unsigned SpellingListIndex, bool IsPackExpansion);
>
> + /// 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
> @@ -1527,6 +1527,25 @@
> AI->getArgNo() + 1,
> llvm::Attribute::NonNull));
> }
> +
> + auto *AVAttr = PVD->getAttr<AlignValueAttr>();
const auto *?
> + 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));
> + }
> }
>
> if (Arg->getType().isRestrictQualified())
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -2679,6 +2679,62 @@
> Attr.getAttributeSpellingListIndex()));
> }
>
> +static void handleAlignValueAttr(Sema &S, Decl *D,
> + const AttributeList &Attr) {
> + S.AddAlignValueAttr(Attr.getRange(), D, Attr.getArgAsExpr(0),
> + Attr.getAttributeSpellingListIndex());
> + return;
Spurious return.
> +}
> +
> +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 {
> + Diag(D->getLocation(), diag::err_attr_wrong_decl)
> + << &TmpAttr /*TmpAttr.getName()*/ << AttrRange;
> + return;
This is usually diagnosed with warn_attribute_wrong_decl_type or
err_attribute_wrong_decl_type.
> + }
> +
> + 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->isTypeDependent() && !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 && !llvm::isPowerOf2_64(Alignment.getZExtValue())) {
Perhaps: Alignment.isPowerOf2() instead?
> + Diag(AttrLoc, diag::err_attribute_aligned_not_power_of_two)
> + << E->getSourceRange();
> + return;
> + }
> +
> + D->addAttr(::new (Context)
> + AlignValueAttr(AttrRange, Context, ICE.get(),
> + SpellingListIndex));
> + 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) {
> @@ -4102,6 +4158,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
> @@ -129,6 +129,17 @@
> }
> }
>
> +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) {
> @@ -183,6 +194,12 @@
> continue;
> }
>
> + const AlignValueAttr *AlignValue = dyn_cast<AlignValueAttr>(TmplAttr);
> + if (AlignValue && AlignValue->getAlignment()->isValueDependent()) {
> + instantiateDependentAlignValueAttr(*this, TemplateArgs, AlignValue, New);
> + continue;
> + }
> +
> assert(!TmplAttr->isPackExpansion());
> if (TmplAttr->isLateParsed() && LateAttrs) {
> // Late parsed attributes must be instantiated and attached after the
> 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.cpp
> ===================================================================
> --- /dev/null
> +++ test/Sema/align_value.cpp
Since this test is in Sema, it should be align_value.c instead.
> @@ -0,0 +1,49 @@
> +// 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)))) { };
> +
> +// expected-error at +1 {{requested alignment is not a power of 2}}
I'm totally unfamiliar with expected-error at +1; what does that mean/do?
> +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_double3;
> +
> +// expected-error at +1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value)) aligned_double3;
> +
> +// 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;
> +
> +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;
> +
So this test could either move into SemaCXX, or could be split into
two tests. If we add a C++11 attribute spelling, I think splitting
into two tests would probably make the most sense. If we don't, then
perhaps just move the entire test into SemaCXX.
~Aaron
More information about the cfe-commits
mailing list