[PATCH] Implement the flatten attribute.
Aaron Ballman
aaron at aaronballman.com
Mon May 19 11:44:40 PDT 2014
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -668,6 +668,12 @@
> let Documentation = [Undocumented];
> }
>
> +def Flatten : InheritableAttr {
> + let Spellings = [GNU<"flatten">];
This should be spelled with a GCC spelling instead of GNU.
> + let Subjects = SubjectList<[Function]>;
What about member functions? Templates? Obj-C methods?
> + let Documentation = [Undocumented];
Please document this attribute prior to committing.
> +}
> +
> def Format : InheritableAttr {
> let Spellings = [GCC<"format">];
> let Args = [IdentifierArgument<"Type">, IntArgument<"FormatIdx">,
> Index: lib/CodeGen/CGCall.cpp
> ===================================================================
> --- lib/CodeGen/CGCall.cpp
> +++ lib/CodeGen/CGCall.cpp
> @@ -1063,6 +1063,7 @@
> }
>
> void CodeGenModule::ConstructAttributeList(const CGFunctionInfo &FI,
> + const Decl *CallerDecl,
> const Decl *TargetDecl,
> AttributeListType &PAL,
> unsigned &CallingConv,
> @@ -1075,6 +1076,11 @@
> if (FI.isNoReturn())
> FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
>
> + if (CallerDecl) {
> + if (CallerDecl->hasAttr<FlattenAttr>())
Second if can be hoisted into the first if. That being said, I prefer
the way Alp proposed to implement it as it is a bit more concise and
doesn't require a signature change where the caller is required.
> + FuncAttrs.addAttribute(llvm::Attribute::AlwaysInline);
> + }
> +
> // FIXME: handle sseregparm someday...
> if (TargetDecl) {
> if (TargetDecl->hasAttr<ReturnsTwiceAttr>())
> @@ -2913,7 +2919,7 @@
>
> unsigned CallingConv;
> CodeGen::AttributeListType AttributeList;
> - CGM.ConstructAttributeList(CallInfo, TargetDecl, AttributeList,
> + CGM.ConstructAttributeList(CallInfo, CurCodeDecl, TargetDecl, AttributeList,
> CallingConv, true);
> llvm::AttributeSet Attrs = llvm::AttributeSet::get(getLLVMContext(),
> AttributeList);
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -576,7 +576,7 @@
> llvm::Function *F) {
> unsigned CallingConv;
> AttributeListType AttributeList;
> - ConstructAttributeList(Info, D, AttributeList, CallingConv, false);
> + ConstructAttributeList(Info, 0, D, AttributeList, CallingConv, false);
> F->setAttributes(llvm::AttributeSet::get(getLLVMContext(), AttributeList));
> F->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
> }
> Index: lib/CodeGen/CodeGenModule.h
> ===================================================================
> --- lib/CodeGen/CodeGenModule.h
> +++ lib/CodeGen/CodeGenModule.h
> @@ -906,12 +906,14 @@
> /// function type.
> ///
> /// \param Info - The function type information.
> + /// \param CallerDecl - The decl of the caller, if available
> /// \param TargetDecl - The decl these attributes are being constructed
> /// for. If supplied the attributes applied to this decl may contribute to the
> /// function attributes and calling convention.
> /// \param PAL [out] - On return, the attribute list to use.
> /// \param CallingConv [out] - On return, the LLVM calling convention to use.
> void ConstructAttributeList(const CGFunctionInfo &Info,
> + const Decl *CallerDecl,
> const Decl *TargetDecl,
> AttributeListType &PAL,
> unsigned &CallingConv,
> Index: lib/CodeGen/MicrosoftCXXABI.cpp
> ===================================================================
> --- lib/CodeGen/MicrosoftCXXABI.cpp
> +++ lib/CodeGen/MicrosoftCXXABI.cpp
> @@ -1113,7 +1113,7 @@
>
> unsigned CallingConv;
> CodeGen::AttributeListType AttributeList;
> - CGM.ConstructAttributeList(FnInfo, MD, AttributeList, CallingConv, true);
> + CGM.ConstructAttributeList(FnInfo, 0, MD, AttributeList, CallingConv, true);
> llvm::AttributeSet Attrs =
> llvm::AttributeSet::get(CGF.getLLVMContext(), AttributeList);
>
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp
> +++ lib/Sema/SemaDeclAttr.cpp
> @@ -4140,6 +4140,9 @@
> case AttributeList::AT_OptimizeNone:
> handleOptimizeNoneAttr(S, D, Attr);
> break;
> + case AttributeList::AT_Flatten:
> + handleSimpleAttribute<FlattenAttr>(S, D, Attr);
> + break;
> case AttributeList::AT_Format:
> handleFormatAttr(S, D, Attr);
> break;
> Index: test/CodeGen/flatten.c
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/flatten.c
> @@ -0,0 +1,10 @@
> +// RUN: %clang -target x86_64-linux-gnu -S %s -emit-llvm -o - | FileCheck %s
> +
> +void f(void) {}
> +
> +__attribute__((flatten))
> +// CHECK: define void @g()
> +void g(void) {
> + // CHECK-NOT: call {{.*}} @f
> + f();
> +}
>
Missing test cases for semantics of the attribute (that it applies to
the subjects you expect, takes no arguments, etc).
~Aaron
On Mon, May 19, 2014 at 2:31 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> On Sat, May 17, 2014 at 11:51:38AM +0300, Alp Toker wrote:
>>> Index: lib/CodeGen/CGCall.cpp
>>> ===================================================================
>>> --- lib/CodeGen/CGCall.cpp
>>> +++ lib/CodeGen/CGCall.cpp
>>> @@ -1063,6 +1063,7 @@
>>> }
>>> void CodeGenModule::ConstructAttributeList(const CGFunctionInfo
>>> &FI,
>>> + const Decl *CallerDecl,
>>> const Decl *TargetDecl,
>>> AttributeListType &PAL,
>>> unsigned &CallingConv,
>>> @@ -1075,6 +1076,11 @@
>>> if (FI.isNoReturn())
>>> FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
>>> + if (CallerDecl) {
>>> + if (CallerDecl->hasAttr<FlattenAttr>())
>>> + FuncAttrs.addAttribute(llvm::Attribute::AlwaysInline);
>>> + }
>>> +
>>
>> If the callee is NoInline we should probably give that priority preserve
>> correctness.
>
> Maybe; see comment below.
>
>>> // FIXME: handle sseregparm someday...
>>> if (TargetDecl) {
>>> if (TargetDecl->hasAttr<ReturnsTwiceAttr>())
>>> @@ -2913,7 +2919,7 @@
>>> unsigned CallingConv;
>>> CodeGen::AttributeListType AttributeList;
>>> - CGM.ConstructAttributeList(CallInfo, TargetDecl, AttributeList,
>>> + CGM.ConstructAttributeList(CallInfo, CurCodeDecl, TargetDecl, AttributeList,
>>> CallingConv, true);
>>
>> I'd prefer to keep locality and handle the attribute in EmitCall(),
>> instead of adding CallerDecl and generally passing 0 to it.
>
> Either way works for me.
>
>> I don't usually work on CodeGen so I've attached my interpretation of
>> the feature based on your patch for review :-)
>
> One comment:
>
>> + if (CurCodeDecl && CurCodeDecl->hasAttr<FlattenAttr>() &&
>> + !CS.hasFnAttr(llvm::Attribute::NoInline))
>> + Attrs =
>> + Attrs.addAttribute(getLLVMContext(), llvm::AttributeSet::FunctionIndex,
>> + llvm::Attribute::AlwaysInline);
>> +
>> CS.setAttributes(Attrs);
>
> This is checking the NoInline attribute on CS before we apply the attribute
> set we got from ConstructAttributeList. If your test is passing anyway,
> this probably means that the noinline attribute in the called function is
> overriding the alwaysinline attribute on the call.
>
> I can also imagine that in an LTO situation the caller's TU may not be able
> to see the noinline attribute on the callee.
>
> Since the check has no effect even if fixed (and is unreliable) I'd suggest
> removing it.
>
> LGTM other than that.
>
> Thanks,
> --
> Peter
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list