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

steplong updated this revision to Diff 436551. steplong added a comment. - Remove -Og, and all non-zero optimization levels - Fix up docs - Modified tests to reflect -Og and non-zero opt level change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126

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

aaron.ballman added a comment. In D126984#3578950 , @steplong wrote: >>> 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 wi

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

steplong added a comment. >> 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? Hmm, I think I'm ok with ignor

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

xbolva00 added a comment. >> Would that work for you (and you as well @aeubanks)? yes :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://reviews.llvm.org/D126984 ___ cfe-commits mailin

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

aaron.ballman added a comment. In 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

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

steplong updated this revision to Diff 436443. steplong added a comment. - Add llvm::Attribute::MinSize when OptimizeAttr::Oz - Add test for checking minsize Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://reviews.llvm.org/D126984

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

xbolva00 added inline comments. Comment at: clang/test/CodeGen/attr-optimize.c:16 + +__attribute__((optimize("Oz"))) void f4(void) {} +// O2: @f4{{.*}}[[ATTR_OPTSIZE]] For -Os, clang adds optsize. For -Oz, clang adds optsize and minsize. So tests should check it

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

steplong added inline comments. Comment at: clang/test/CodeGen/attr-optimize.c:4 + +__attribute__((optimize("O0"))) void f1(void) {} +// O2: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]] xbolva00 wrote: > No support for > ``` > __attribute__ ((__optimize__ (0))) > ``` > >

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

xbolva00 added inline comments. Comment at: clang/test/CodeGen/attr-optimize.c:4 + +__attribute__((optimize("O0"))) void f1(void) {} +// O2: @f1{{.*}}[[ATTR_OPTNONE:#[0-9]+]] No support for ``` __attribute__ ((__optimize__ (0))) ``` ? GCC supports it Reposito

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

xbolva00 added a comment. But where I think this feature could be very useful in following case from gcc test suite where there is some FP computation.. Imagine you compile you program with -ffast-math and then you have function: __attribute__ ((optimize ("no-associative-math"))) double fn3

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

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 genera

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

aeubanks added a comment. 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

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

aaron.ballman added a comment. In D126984#3574445 , @xbolva00 wrote: > "The optimize attribute should be used for debugging purposes only. It is not > suitable in production code." > > Until they (users) start and any change in pipeline may surprise them

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

xbolva00 added a comment. "The optimize attribute should be used for debugging purposes only. It is not suitable in production code." Until they (users) start and any change in pipeline may surprise them. Personally I am bigger fan of more targeted attributes like we have noinline / noipa prop

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

aaron.ballman added a comment. In D126984#3574288 , @xbolva00 wrote: > [[gnu::optimize("O0")]] is okay but [[gnu::optimize("O3 > ")]] is not gonna work without > major changes. Not sure why we should deliver h

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

xbolva00 added a comment. In D126984#3574071 , @xbolva00 wrote: > I agree with @aeubanks , this feature requires major changes to pass manager > and I see no value to land this currently. > > I see that somebody may prefer “opt for size”, but this is exp

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

xbolva00 added a comment. [[gnu::optimize("O0")]] is okay but [[gnu::optimize("O3 ")]] is not gonna work without major changes. Not sure why we should deliver half-broken new attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

aaron.ballman added a comment. In D126984#3574077 , @aeubanks wrote: > In D126984#3574046 , @aaron.ballman > wrote: > >> In D126984#3571573 , @aeubanks >> wrote: >> >>>

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

aeubanks added a comment. In D126984#3574091 , @steplong wrote: > In D126984#3574077 , @aeubanks > wrote: > >> In D126984#3574046 , >> @aaron.ballman wrote: >> >>> In D1

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

steplong added a comment. In D126984#3574077 , @aeubanks wrote: > In D126984#3574046 , @aaron.ballman > wrote: > >> In D126984#3571573 , @aeubanks >> wrote: >> >>> IIRC

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

aeubanks added a comment. In D126984#3574046 , @aaron.ballman wrote: > In D126984#3571573 , @aeubanks > wrote: > >> IIRC in the past there was a strong preference to not have the pass manager >> support this so

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

xbolva00 added a comment. I agree with @aeubanks , this feature requires major changes to pass manager and I see no value to land this currently. I see that somebody may prefer “opt for size”, but this is exposed via “minsize” attribute so I see no strong need for optimize(“-Os”) Repository:

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

aeubanks added a comment. In D126984#3574033 , @steplong wrote: > In D126984#3571573 , @aeubanks > wrote: > >> IIRC in the past there was a strong preference to not have the pass manager >> support this sort of

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

aaron.ballman added a comment. In D126984#3571573 , @aeubanks wrote: > IIRC in the past there was a strong preference to not have the pass manager > support this sort of thing > if you want to support this, there should be an RFC for how the optimization

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

steplong added a comment. In D126984#3571573 , @aeubanks wrote: > IIRC in the past there was a strong preference to not have the pass manager > support this sort of thing > if you want to support this, there should be an RFC for how the optimization > p

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

aeubanks requested changes to this revision. aeubanks added a comment. This revision now requires changes to proceed. IIRC in the past there was a strong preference to not have the pass manager support this sort of thing if you want to support this, there should be an RFC for how the optimization

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

steplong updated this revision to Diff 435680. steplong added a comment. - Added release notes on attribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://reviews.llvm.org/D126984 Files: clang/docs/ReleaseNotes.rst clang/inc

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please add the release note when landing. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://revi

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

steplong added a comment. In D126984#3567822 , @aaron.ballman wrote: > Two final (I hope) nits! One is in the code, the other is that this should > have a release note for the new attribute. Assuming precommit CI pipeline > passes, then I think this is

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

steplong updated this revision to Diff 435313. steplong added a comment. - Add Arg when creating Attr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://reviews.llvm.org/D126984 Files: clang/include/clang/Basic/Attr.td clang/inc

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

aaron.ballman added a comment. Two final (I hope) nits! One is in the code, the other is that this should have a release note for the new attribute. Assuming precommit CI pipeline passes, then I think this is all set. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4860 + if (It

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

steplong updated this revision to Diff 435264. steplong added a comment. Herald added a subscriber: jdoerfert. - Fix up docs and comments - Fix failing pragma-attribute-supported-attributes-list.test - Remove debug print - Change to StringMap Repository: rG LLVM Github Monorepo CHANGES SINCE

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

aaron.ballman added a comment. I think this is getting close -- mostly just nits at this point. Comment at: clang/include/clang/Basic/AttrDocs.td:3463 +The ``optimize`` attribute, when attached to a function, indicates that the +function be compiled with a different optimizatio

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

steplong added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4841-4850 + StringRef Level; + // Check if argument is prefixed with "-O" or "O" + if (Arg.str().rfind("-O", 0) == 0) +Level = Arg.substr(2); + else if (Arg.str().rfind("O", 0) == 0) +Level =

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

steplong updated this revision to Diff 434639. steplong added a comment. - Added docs for attribute - Changed attribute to use Enum - Optsize includes -Og, -Oz, and -Ofast - Change to warning instead of error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2265 + let Spellings = [GCC<"optimize">]; + let Args = [StringArgument<"Level">]; + let Subjects = SubjectList<[Function]>; Something along these lines adds a "fake" argument to

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

steplong added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2169 +HasOptnone = TargetDecl->hasAttr() || + (TargetDecl->hasAttr() && + TargetDecl->getAttr()->getLevel() == "0"); I don't think this is the most ergon

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

steplong created this revision. Herald added a reviewer: aaron.ballman. Herald added a project: All. steplong requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. At the moment, it only supports a string argument that refers to the optimization