[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

Mon Jun 13 10:47:54 PDT 2022

aaron.ballman added a comment.

In D126984#3574550 <https://reviews.llvm.org/D126984#3574550>, @aeubanks wrote:

> I can't speak for @xbolva00 but the only part I'm against is the user-visible feature of
>
>> - Added preliminary support for GCC's attribute `optimize`, which allows functions to be compiled with different optimization options than what was specified on the command line.
>
> which implies that we're on the way to support per-function optimization levels (which we aren't)
> the internal clang representation changes are all fine

Ah, would you be okay if we retained the user-facing feature but more clearly documented (in release notes and documentation) the differences from GCC and that we do not currently intend to close that gap?

> and even for the MSVC pragma `#pragma optimize("t", on)`, what are we supporting if the user compiles their code with `-O0`? because right now we won't optimize anything with `-O0`

@steplong -- what are your thoughts on this?

In D126984#3574592 <https://reviews.llvm.org/D126984#3574592>, @xbolva00 wrote:

>> Or are you opposed to the notion of having one semantic attribute to control all of this and you prefer to see multiple individual semantic attributes and all that comes along with them in terms of combinations?
>
> But to use different pipeline for different functions (here I mean -O1, O2 <https://reviews.llvm.org/owners/package/2/>, O3 <https://reviews.llvm.org/owners/package/3/>) is a major change to LLVM pass manager and I think this use case does not justify it.

Thanks for clarifying! I'd be fine changing the internal enumeration for the attribute to represent a better subset of what we intend to implement support for (rather than making it look like we intend to support O1 <https://reviews.llvm.org/owners/package/1/>-O4). Would that work for you (and you as well @aeubanks)?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984