[AttrDocs] document always_inline

[nickdesaulniers]

Summary

GNU documentaion for always_inline:
https://gcc.gnu.org/onlinedocs/gcc/Inline.html

GNU documentation for function attributes:
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

Microsoft documentation for __force_inline:
https://docs.microsoft.com/en-us/cpp/cpp/inline-functions-cpp

[jyknight]

clang/include/clang/Basic/AttrDocs.td

5441–5442

I believe the semantics are stronger than this. It should get inlined unless it cannot be.

[lebedev.ri]

clang/include/clang/Basic/AttrDocs.td

5436–5437

One too many lines

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

5442

It may make sense to link back to MSDN and GCC if we're honoring their semantics. We seem to do this for MSDN regularly, but a bit less common for the GCC docs.

5444

Can you add the various spellings to this heading?

[ojeda]

Note that the latest GCC docs (rather than 4.1.2) are at: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

For reference, this was triggered from: https://lore.kernel.org/lkml/[e-mail]/

[jdoerfert]

clang/include/clang/Basic/AttrDocs.td

5443

It is more than that. This would imply that with optimizations enabled there is no effect. I would mention that the inline heuristic is disabled and inlining is always attempted, w/ or w/o optimizations.

[nickdesaulniers]

• add links
• remove extra whitespace
• rewording, split onto separate lines to ease code review
• add spellings to heading

[nickdesaulniers]

clang/include/clang/Basic/AttrDocs.td

5441–5442

Seemingly not for indirect function calls: https://godbolt.org/z/BoND-X

[kristina]

Just linking relevant bug for the record: https://bugs.llvm.org/show_bug.cgi?id=43517

Also, I'm fairly certain __forceinline and always_inline, confusingly enough differ in semantics, with __forceinline only being a stronger hint on MSVC.

[joerg]

I wonder if we should actually enumerate evil here, i.e. give the situations in which inlining actually fails. As mentioned on IRC, I wonder if we shouldn't aim for the stronger semantics and at least warn by default of any situation that prevents always_inline from doing its job.

[nickdesaulniers]

In D68410#1694026, @kristina wrote:

Also, I'm fairly certain __forceinline and always_inline, confusingly enough differ in semantics, with __forceinline only being a stronger hint on MSVC.

Does clang handle __forceinline vs always_inline differently, today? If not, then sounds like we may need to split these in two.

In D68410#1696411, @joerg wrote:

I wonder if we should actually enumerate evil here, i.e. give the situations in which inlining actually fails.

Which is likely to change over time. I worry that enumerating such cases is compiler version specific, and might lead to developers depending/[ab]using that behavior?

As mentioned on IRC, I wonder if we shouldn't aim for the stronger semantics

As long as we error when we fail to inline, I think that matches GCC's behavior. There's likely differences in what we can inline or not.

and at least warn by default of any situation that prevents always_inline from doing its job.

Might be hard to recognize all such cases in the frontend? GCC does warn via -Wattributes when the attribute is applied to a non-inline function.

[dblaikie]

In D68410#1696411, @joerg wrote:

I wonder if we should actually enumerate evil here, i.e. give the situations in which inlining actually fails.

Which is likely to change over time. I worry that enumerating such cases is compiler version specific, and might lead to developers depending/[ab]using that behavior?

I'm jumping in part-way here, but FWIW I'd be concerned that the lack of explicit rules is actually going to make people depend on the whims of the optimizer/inliner - if we have a backend warning that only fires when inlining doesn't occur. So narrowing the set of cases we accept in the frontend seems like it reduces the risk of abuse by compiler users - leaving more freedom for the compiler optimizations to change without breaking existing code.

[lebedev.ri]

I think partial (but not wrong!) docs is better than no docs whatsoever, so i'd be inclined to proceed with this.
I, too, not really convinced that actual explicit rules should be spelled out.
Some nits, LG otherwise to me.

clang/include/clang/Basic/AttrDocs.td

5440

s/Inline/Inlining/

5443

This comment wasn't addressed.

[nickdesaulniers]

• rebased, s/Inline/Inlining/

[nickdesaulniers]

Sorry, I should not have waiting this long to update this patch...

clang/include/clang/Basic/AttrDocs.td

5443

I don't follow. I'm not sure if this comment has bit rot? I think it's critical to mention that always_inline does not mean always, as inline substitution MAY fail.

[jdoerfert]

LGTM

clang/include/clang/Basic/AttrDocs.td

5443

Original text:

Hint that inline substitution should be attempted when optimizations are

disabled. Does not guarantee that inline substitution actually occurs.

The new text is better.

[ojeda]

clang/include/clang/Basic/AttrDocs.td

5665

MSDN is gone now, Microsoft Docs is the new one.

[aaron.ballman]

LGTM aside from the minor wording nit from @ojeda.

[nickdesaulniers]

s/MSDN/Microsoft Docs/

[nickdesaulniers]

@lebedev.ri would you mind re-reviewing when you have a chance?

[lebedev.ri]

Seems fine to me.

clang/include/clang/Basic/Attr.td

def AlignMac68k : InheritableAttr {
   let Spellings = [];
   let SemaHandler = 0;
   let Documentation = [Undocumented];
 }
 
 def AlwaysInline : InheritableAttr {
   let Spellings = [GCC<"always_inline">, Keyword<"__forceinline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [AlwaysInlineDocs];
 }
 
 def Artificial : InheritableAttr {
   let Spellings = [GCC<"artificial">];
   let Subjects = SubjectList<[InlineFunction]>;
   let Documentation = [ArtificialDocs];
   let SimpleHandler = 1;
 }

clang/include/clang/Basic/AttrDocs.td

if (true) {
       IntOwner O(7);
       P = IntPointer(O); // P "points into" O
     } // P is dangling
     return P.get(); // error: Using a dangling Pointer.
   }
 
 }];
 }
 
 def ArmBuiltinAliasDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 This attribute is used in the implementation of the ACLE intrinsics.
 It allows the intrinsic functions to
 be declared using the names defined in ACLE, and still be recognized
 as clang builtins equivalent to the underlying name. For example,
 ``arm_mve.h`` declares the function ``vaddq_u32`` with
 ``__attribute__((__clang_arm_mve_alias(__builtin_arm_mve_vaddq_u32)))``,
 and similarly, one of the type-overloaded declarations of ``vaddq``
 will have the same attribute. This ensures that both functions are
 recognized as that clang builtin, and in the latter case, the choice
 of which builtin to identify the function as can be deferred until
 after overload resolution.
 
 This attribute can only be used to set up the aliases for certain Arm

 This attribute declares a function that can be called from non-secure state, or
 from secure state. Entering from and returning to non-secure state would switch
 to and from secure state, respectively, and prevent flow of information
 to non-secure state, except via return values. See `ARMv8-M Security Extensions:
 Requirements on Development Tools - Engineering Specification Documentation
 <https://developer.arm.com/docs/ecm0359818/latest/>`_ for more information.
   }];
 }
+
+def AlwaysInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Inlining heuristics are disabled and inlining is always attempted regardless of
+optimization level.
+
+Does not guarantee that inline substitution actually occurs.
+
+See also `the Microsoft Docs on Inline Functions`_, `the GCC Common Function
+Attribute docs`_, and `the GCC Inline docs`_.
+
+.. _the Microsoft Docs on Inline Functions: https://docs.microsoft.com/en-us/cpp/cpp/inline-functions-cpp
+.. _the GCC Common Function Attribute docs: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
+.. _the GCC Inline docs: https://gcc.gnu.org/onlinedocs/gcc/Inline.html
+
+}];
+  let Heading = "always_inline, __force_inline";
+}