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

Fri Jun 10 13:49:52 PDT 2022

xbolva00 added a comment.

>> Are you opposed to exposing #pragma optimize? (https://docs.microsoft.com/en-us/cpp/preprocessor/optimize?view=msvc-170) If yes, I think Stephen should run an RFC on Discourse to see if there's general agreement.

No, I like it, seems more useful and general than "pragma clang optimize on/off"

>> Are you opposed to the direction of condensing the optimization semantic attributes (the things in the AST) down into one? If yes, I'd like to understand why better.

No :)

>> Are you still opposed to exposing a neutered form of the GCC optimize attribute as a parsed attribute (the thing users write in their source)? If yes, that's fine by me, but then I'd still like to see most of this patch land, just without a way for the user to spell the attribute themselves. We can adjust the semantic attribute's enumeration to cover only the cases we want to support.

Not entirely opposed, GCC optimize attribute could partially work fine, O0 maps to optnone, Os to optsize, Oz to minsize. I am more worried about next steps, see below.

>> 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?

Not strongly opposed, just some concerns how this could work together with LLVM. Example: attribute((optimize("-fno-licm"))) -> 'optimize="no-licm" '? This could work. Possibly also allow this form: attribute((optimize(no-licm,  optsize))) ?

What about current attributes? Should/can we drop them and use  for example 'optimize="no-ipa,no-clone" '? Not strongly opposed, but probably a lot of work.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984