[PATCH] Implement the flatten attribute.

[Peter Collingbourne]

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