This is an archive of the discontinued LLVM Phabricator instance.

[Clang] always_inline statement attribute
ClosedPublic

Authored by xbolva00 on Mar 1 2022, 5:56 AM.

Details

Summary

Motivation:

int test(int x, int y) {
    int r = 0;
    [[clang::always_inline]] r += foo(x, y); // force compiler to inline this function here
    return r;
}

In 2018, @kuhar proposed "Introduce per-callsite inline intrinsics" in https://reviews.llvm.org/D51200 to solve this motivation case (and many others).

This patch solves this problem with call site attribute. "noinline" statement attribute already landed in D119061. Also, some LLVM Inliner fixes landed so call site attribute is stronger than function attribute.

Diff Detail

Event Timeline

xbolva00 created this revision.Mar 1 2022, 5:56 AM
xbolva00 requested review of this revision.Mar 1 2022, 5:56 AM
xbolva00 retitled this revision from [Clang] noinline call site attribute to [Clang] always_inline call site attribute.
xbolva00 edited the summary of this revision. (Show Details)
xbolva00 updated this revision to Diff 412073.Mar 1 2022, 6:02 AM

Version for review.

ping

(I will fix clang format nits later)

Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2022, 4:53 AM
xbolva00 retitled this revision from [Clang] always_inline call site attribute to [Clang] always_inline statement attribute.Mar 9 2022, 3:19 AM

Ping

Cool. Given D119061 is landed, I think this one should be good.

aaron.ballman added inline comments.Mar 9 2022, 3:57 AM
clang/include/clang/Basic/Attr.td
714

I'm not certain we should add ErrorDiag here. That's likely to break existing code that's ignoring the attribute.

clang/include/clang/Basic/AttrDocs.td
6246

We should document more clearly (perhaps just with an example) what "contains calls" means. e.g.,

[[clang::always_inline]] return foo(bar(), baz(), bing() + 12);

Do we attempt to inline everything? Just the simple calls with no expressions? Just the call to foo()?

It's probably also worth reminding people that a declaration statement, which is a statement, is not a statement that can have an attribute associated with it (the attribute applies to the declaration, not the statement in that case). So this won't work, and it's not a bug:

[[clang::always_inline]] int i = bar();

(If this case matters to you, you could have the attribute also apply to VarDecl or some subset of that.)

clang/lib/CodeGen/CGCall.cpp
5252–5253

You should fix the clang-format issue here.

clang/lib/CodeGen/CGStmt.cpp
695

clang-format here as well.

clang/lib/Sema/SemaStmtAttr.cpp
247

Should we create this after the early return below so that we don't do the work of finding the call exprs just to ignore the results because the spelling was wrong?

xbolva00 updated this revision to Diff 414168.Mar 9 2022, 11:25 AM

Addressed review feedback.

xbolva00 marked 3 inline comments as done.Mar 9 2022, 11:27 AM
xbolva00 added inline comments.
clang/include/clang/Basic/Attr.td
714

Yeah. WarnDiag.

clang/include/clang/Basic/AttrDocs.td
6246

Right, doc updated.

(If this case matters to you, you could have the attribute also apply to VarDecl or some subset of that.)

Not so much currently.

aaron.ballman accepted this revision.Mar 14 2022, 12:52 PM

You should also add a release note for the new functionality, but other than that and a nit, this LGTM.

clang/include/clang/Basic/AttrDocs.td
6258

Declaration statement -> A declaration statement

This revision is now accepted and ready to land.Mar 14 2022, 12:52 PM
xbolva00 updated this revision to Diff 415206.Mar 14 2022, 1:42 PM

fixed nit

This revision was landed with ongoing or failed builds.Mar 14 2022, 1:45 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2022, 1:45 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript