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

Arthur Eubanks via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 10 09:44:32 PST 2022


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:
> > xbolva00 wrote:
> > > aaron.ballman wrote:
> > > > Should there be any diagnostic in a situation like this?
> > > > ```
> > > > [[gnu::always_inline]] int f() { return 0; }
> > > > 
> > > > void func() {
> > > >   [[clang::noinline]] int i = f(); // Should there be some way to know of the conflict?
> > > > }
> > > > ```
> > > Yeah, this is tricky. LLVM will currently prefer always_inline. 
> > > 
> > > I was wondering about a new warning as well, I think it is a good idea to warn tor this scenario, I was just not sure where is a good place for such check. Maybe collect all callexprs in CallExprFinder and detech this colision?
> > Yeah, if always_inline wins, that'd be a pretty big surprise. Is that intentional, or expected to change? (I would have expected the call site attribute to "win" because it knows more about its inlining needs than the author of the declaration.)
> > 
> > As for the diagnostic, the call expression finder is where I'd expect the logic to start from. That'll find you all the call expressions involved, and for each one of those you can look to see if you can find a decl for the call, and if there is one, check to see if it has an `AlwaysInlineAttr` on it, then diagnose from there.
> > 
> > 
> Yes, I think that noinline should win as well :) but it is kinda edge case scenario, so I am not too worried for now. 
> 
> But yeah, fix for inliner probably makes sense.
> 
> WDYT @aeubanks ? Would be such small change acceptable? I could send a patch.
favoring call-site attributes over callee attributes in the inliner seems reasonable


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119061/new/

https://reviews.llvm.org/D119061



More information about the cfe-commits mailing list