On Thu, Oct 2, 2014 at 4:10 PM, [email protected] <[email protected]> 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@+1 {{requested alignment is not a power of 2}}
> +typedef double * __attribute__((align_value(63))) aligned_double1;
> +
> +// expected-error@+1 {{requested alignment is not a power of 2}}
> +typedef double * __attribute__((align_value(-2))) aligned_double2;
> +
> +// expected-error@+1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value(63, 4))) aligned_double3;
> +
> +// expected-error@+1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value())) aligned_double3a;
> +
> +// expected-error@+1 {{attribute takes one argument}}
> +typedef double * __attribute__((align_value)) aligned_double3b;
> +
> +// expected-error@+1 {{'align_value' attribute requires integer constant}}
> +typedef double * __attribute__((align_value(4.5))) aligned_double4;
> +
> +// expected-warning@+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@+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@+1 {{requested alignment is not a power of 2}}
> + void foo(T &b __attribute__((align_value(sizeof(T)+1))));
> +};
> +
> +// expected-note@+1 {{in instantiation of template class 'nope<long double,
> 4>' requested here}}
> +nope<long double, 4> y2;
> +
>
~Aaron
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
