r202131 - Attr: Remove ForceInline
David Majnemer
david.majnemer at gmail.com
Tue Feb 25 10:09:51 PST 2014
As implemented before r202131, all the code that checked for AlwaysInline
also checked for ForceInline and treated it identically.
This commit, r202131, does nothing to change the semantics of how clang
feels about ForceInline. It is just as correct or wrong as it was before.
Quite frankly, many of the limitations that __forceinline is documented to
have are ridiculous, it seems like their implementation is incredibly
dependent on the frontend being able to do some analysis.
For example, they mention that they won't inline if the function has a
variable argument list. That just seems like a bug, I don't see why we
would need to mimic that behavior.
MSVC has /Ob0, which we bind to -fno-inline. I don't think we have this on
by default, they do. The two flags don't have quite the same semantics. I
think if there is something to fix, it should be done by fixing /Ob0.
--
David Majnemer
On Tue Feb 25 2014 at 5:24:05 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:
> I don't believe the semantics for __forceinline and always_inline
> really match up. As best I can tell, always_inline will *always*
> inline the function, regardless of optimization level, which is not
> something that __forceinline does. As a simple example:
>
> __forceinline int i() { return 9; }
>
> int main(void) {
> int j = i();
> return 0;
> }
>
> In debug mode in MSVC 2013, this produces (in part):
>
> ; 3 : int main(void) {
>
> push ebp
> mov ebp, esp
> sub esp, 204 ; 000000ccH
> push ebx
> push esi
> push edi
> lea edi, DWORD PTR [ebp-204]
> mov ecx, 51 ; 00000033H
> mov eax, -858993460 ; ccccccccH
> rep stosd
>
> ; 4 : int j = i();
>
> call ?i@@YAHXZ ; i
> mov DWORD PTR _j$[ebp], eax
>
> ; 5 : return 0;
>
> xor eax, eax
>
> ; 6 : }
>
> When I compile in GCC (using always_inline) at -O0, I get:
>
> main:
> push rbp
> mov rbp, rsp
> mov eax, 9
> mov DWORD PTR [rbp-4], eax
> mov eax, 0
> pop rbp
> ret
>
> Clang may not support __forceinline semantics properly yet, but I am
> not keen on this behavior change. It also deviates from the
> documentation for both (gcc claims "this attribute inlines the
> function even if no optimization level was specified." and msvc claims
> "You cannot force the compiler to inline a particular function, even
> with the __forceinline keyword.").
>
> ~Aaron
>
> On Tue, Feb 25, 2014 at 4:53 AM, David Majnemer
> <david.majnemer at gmail.com> wrote:
> > Author: majnemer
> > Date: Tue Feb 25 03:53:29 2014
> > New Revision: 202131
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=202131&view=rev
> > Log:
> > Attr: Remove ForceInline
> >
> > The __forceinline keyword's semantics are now recast as AlwaysInline and
> > the kw___forceinline token has its language mode set for KEYMS.
> >
> > This preserves the semantics of the previous implementation but with
> > less duplication of code.
> >
> > Modified:
> > cfe/trunk/include/clang/Basic/Attr.td
> > cfe/trunk/include/clang/Basic/TokenKinds.def
> > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> > cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> > cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/Attr.td?rev=202131&r1=202130&r2=202131&view=diff
> > ============================================================
> ==================
> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> > +++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 25 03:53:29 2014
> > @@ -357,7 +357,7 @@ def AlignMac68k : InheritableAttr {
> > }
> >
> > def AlwaysInline : InheritableAttr {
> > - let Spellings = [GCC<"always_inline">];
> > + let Spellings = [GCC<"always_inline">, Keyword<"__forceinline">];
> > let Subjects = SubjectList<[Function]>;
> > let Documentation = [Undocumented];
> > }
> > @@ -1676,12 +1676,6 @@ def DLLImport : InheritableAttr, TargetS
> > let Documentation = [Undocumented];
> > }
> >
> > -def ForceInline : InheritableAttr {
> > - let Spellings = [Keyword<"__forceinline">];
> > - let LangOpts = [MicrosoftExt];
> > - let Documentation = [Undocumented];
> > -}
> > -
> > def SelectAny : InheritableAttr {
> > let Spellings = [Declspec<"selectany">];
> > let LangOpts = [MicrosoftExt];
> >
> > Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/TokenKinds.def?rev=202131&r1=202130&r2=202131&view=diff
> > ============================================================
> ==================
> > --- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
> > +++ cfe/trunk/include/clang/Basic/TokenKinds.def Tue Feb 25 03:53:29
> 2014
> > @@ -455,7 +455,7 @@ KEYWORD(__cdecl , KE
> > KEYWORD(__stdcall , KEYALL)
> > KEYWORD(__fastcall , KEYALL)
> > KEYWORD(__thiscall , KEYALL)
> > -KEYWORD(__forceinline , KEYALL)
> > +KEYWORD(__forceinline , KEYMS)
> > KEYWORD(__unaligned , KEYMS)
> >
> > // OpenCL address space qualifiers
> >
> > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CodeGenFunction.cpp?rev=202131&r1=202130&r2=202131&view=diff
> > ============================================================
> ==================
> > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Tue Feb 25 03:53:29 2014
> > @@ -527,8 +527,7 @@ void CodeGenFunction::StartFunction(Glob
> > Fn->addFnAttr(llvm::Attribute::InlineHint);
> > break;
> > }
> > - } else if (!FD->hasAttr<AlwaysInlineAttr>() &&
> > - !FD->hasAttr<ForceInlineAttr>())
> > + } else if (!FD->hasAttr<AlwaysInlineAttr>())
> > Fn->addFnAttr(llvm::Attribute::NoInline);
> > }
> >
> >
> > Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CodeGenModule.cpp?rev=202131&r1=202130&r2=202131&view=diff
> > ============================================================
> ==================
> > --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Feb 25 03:53:29 2014
> > @@ -635,8 +635,7 @@ void CodeGenModule::SetLLVMFunctionAttri
> > B.addAttribute(llvm::Attribute::NoDuplicate);
> > } else if (D->hasAttr<NoInlineAttr>()) {
> > B.addAttribute(llvm::Attribute::NoInline);
> > - } else if ((D->hasAttr<AlwaysInlineAttr>() ||
> > - D->hasAttr<ForceInlineAttr>()) &&
> > + } else if (D->hasAttr<AlwaysInlineAttr>() &&
> > !F->getAttributes().hasAttribute(llvm::
> AttributeSet::FunctionIndex,
> >
> llvm::Attribute::NoInline)) {
> > // (noinline wins over always_inline, and we can't specify both in
> IR)
> > @@ -1245,8 +1244,7 @@ CodeGenModule::shouldEmitFunction(Global
> > if (getFunctionLinkage(GD) != llvm::Function::
> AvailableExternallyLinkage)
> > return true;
> > const FunctionDecl *F = cast<FunctionDecl>(GD.getDecl());
> > - if (CodeGenOpts.OptimizationLevel == 0 &&
> > - !F->hasAttr<AlwaysInlineAttr>() && !F->hasAttr<ForceInlineAttr>()
> )
> > + if (CodeGenOpts.OptimizationLevel == 0 &&
> !F->hasAttr<AlwaysInlineAttr>())
> > return false;
> > // PR9614. Avoid cases where the source code is lying to us. An
> available
> > // externally function should have an equivalent function somewhere
> else,
> >
> > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDeclAttr.cpp?rev=202131&r1=202130&r2=202131&view=diff
> > ============================================================
> ==================
> > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Feb 25 03:53:29 2014
> > @@ -4269,8 +4269,6 @@ static void ProcessDeclAttribute(Sema &S
> > break;
> > case AttributeList::AT_MSInheritance:
> > handleMSInheritanceAttr(S, D, Attr); break;
> > - case AttributeList::AT_ForceInline:
> > - handleSimpleAttribute<ForceInlineAttr>(S, D, Attr); break;
> > case AttributeList::AT_SelectAny:
> > handleSimpleAttribute<SelectAnyAttr>(S, D, Attr); break;
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140225/4ae80d5b/attachment.html>
More information about the cfe-commits
mailing list