kuhar added a comment.

In D119061#3312571 <https://reviews.llvm.org/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 (like 
> always_inline_recursively) before any patch like this one.

SGTM.

>>> When I worked on my implementation, I remember that I also ran into the 
>>> issue of conflicting attributes. I defaulted to whatever the inliner 
>>> behavior was at the time, but a few folks pointed out to me that this can 
>>> be very confusing. I think this needs thorough documentation / testing.
>
> Yes, as we mentioned it already, for example always_inline on decl, and 
> noinline on callsite. We should diagnose these cases and always prefer call 
> site attribute. (and as I said, some fixes for inliner are needed).

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 remember.


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

Reply via email to