[PATCH] D119061: [Clang] noinline call site attribute

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG223b82402235: [Clang] noinline call site attribute (authored by xbolva00). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHA

[PATCH] D119061: [Clang] noinline call site attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 411589. xbolva00 added a comment. Fixed failing Parser test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/incl

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 411495. xbolva00 added a comment. Small doc update. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. I think I will update release notes later, after always_inline variant of this patch. So looks like we are almost done here? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 411489. xbolva00 added a comment. Addressed review comments. Feel free to suggest better wording :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/inclu

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added inline comments. Comment at: clang/test/Sema/attr-noinline.cpp:23 + + [[gnu::noinline]] bar(); // expected-warning {{'noinline' attribute is ignored in statements as it only applies to functions}} +} aaron.ballman wrote: > xbolva00 wrote: > > aar

[PATCH] D119061: [Clang] noinline call site attribute

aaron.ballman added inline comments. Comment at: clang/test/Sema/attr-noinline.cpp:23 + + [[gnu::noinline]] bar(); // expected-warning {{'noinline' attribute is ignored in statements as it only applies to functions}} +} xbolva00 wrote: > aaron.ballman wrote: >

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added inline comments. Comment at: clang/test/Sema/attr-noinline.cpp:23 + + [[gnu::noinline]] bar(); // expected-warning {{'noinline' attribute is ignored in statements as it only applies to functions}} +} aaron.ballman wrote: > It might be nice to cla

[PATCH] D119061: [Clang] noinline call site attribute

aaron.ballman added a comment. Also, it looks like there are some clang-format nits that can be addressed. Comment at: clang/include/clang/Basic/AttrDocs.td:525 + +``noinline`` attribute can also be used as a Clang-specific statement attribute. +If a statement is marked ``noin

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1764 +def NoInline : DeclOrStmtAttr { + let Spellings = [Clang<"noinline">, Declspec<"noinline">]; + let Documentation = [NoInlineDocs]; aaron.ballman wrote: > xbolva00 wrote: > > Wha

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 411383. xbolva00 added a comment. Herald added a subscriber: jdoerfert. Fixed tests. Addressed review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td

[PATCH] D119061: [Clang] noinline call site attribute

aaron.ballman added a comment. Precommit CI seems to be failing with relevant failures: Failed Tests (2): Clang :: Parser/stmt-attributes.c Clang :: Sema/attr-noinline.c Comment at: clang/include/clang/Basic/Attr.td:1764 +def NoInline : DeclOrStmtAttr { + let Spelli

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. In D119061#3340884 , @kuhar wrote: > LGTM but please get at least one additional approval before submitting Thank you. Yeah, I would like to get lgtm from @aaron.ballman here :) CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D119061: [Clang] noinline call site attribute

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. LGTM put please get at least one additional approval before submitting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 __

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. ping :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1764 +def NoInline : DeclOrStmtAttr { + let Spellings = [Clang<"noinline">, Declspec<"noinline">]; + let Documentation = [NoInlineDocs]; What I need to mention here. With Clang<"noinl

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added inline comments. Comment at: clang/test/CodeGen/attr-noinline.cpp:11 + +void foo(int i) { + [[clang::noinline]] bar(); kuhar wrote: > Could you also add a test function that is already marked as noinline at the > declaration? Yes, added CHANGES

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 408588. xbolva00 added a comment. More tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 408580. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Code

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 408578. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Code

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:522 + let Content = [{ +This function attribute suppresses the inlining of a function at the call sites +of the function. Feel free to suggest better wording :)

[PATCH] D119061: [Clang] noinline call site attribute

kuhar added inline comments. Comment at: clang/test/CodeGen/attr-noinline.cpp:11 + +void foo(int i) { + [[clang::noinline]] bar(); Could you also add a test function that is already marked as noinline at the declaration? CHANGES SINCE LAST ACTION https://re

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 updated this revision to Diff 408573. xbolva00 added a comment. Added check to warn for conflicting attributes. More docs, tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 Files: clang/include/clang/Basic/Attr.td clang/inclu

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. Fixes for inliners landed, working on this now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. Interestly, clang/llvm happily ignores noinline on static functions, gcc does not. https://godbolt.org/z/jr3Wx87rb CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 ___ cfe-

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. >> It would be good to check with a language expert if this can break some >> code. I'm thinking of situations when someone relies on a function being >> emitted (or not) and ending up with linking issues. I'm not an expert here, >> but this is a past concern I vaguely

[PATCH] D119061: [Clang] noinline call site attribute

kuhar added a comment. In D119061#3312571 , @xbolva00 wrote: >>> Do you plan to also add inline and flatten? > > You mean always_inline? Yes, after noinline. The flatten call site attribute > - theoretically why not, but it needs to be reworked in LLVM (

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 added a comment. >> Do you plan to also add inline and flatten? You mean always_inline? Yes, after noinline. The flatten call site attribute - theoretically why not, but it needs to be reworked in LLVM (like always_inline_recursively) before any patch like this one. >> When I worked o

[PATCH] D119061: [Clang] noinline call site attribute

kuhar added a comment. Thanks for working on this, @xbolva00! I don't know this part of the codebase, so won't comment on the patch itself. Just a few questions/suggestions: 1. Do you plan to also add `inline` and `flatten`? 2. When I worked on my implementation, I remember that I also ran into

[PATCH] D119061: [Clang] noinline call site attribute

xbolva00 planned changes to this revision. xbolva00 added a comment. Plan: 1. https://reviews.llvm.org/D119451 2. fix inliner in LLVM 3. continue with this patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061

[PATCH] D119061: [Clang] noinline call site attribute

aeubanks added inline comments. Comment at: clang/test/Sema/attr-noinline.cpp:14 +} + +[[clang::noinline]] static int i = bar(); // expected-error {{'noinline' attribute only applies to functions and statements}} xbolva00 wrote: > aaron.ballman wrote: > > xbolva