mehdi_amini added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912
// OptimizeNone wins over OptimizeForSize and MinSize.
F->removeFnAttr(llvm::Attribute::OptimizeForSize);
F->removeFnAttr(llvm::Attribute::MinSize);
----------------
probinson wrote:
> mehdi_amini wrote:
> > chandlerc wrote:
> > > Is this still at all correct? Why? it seems pretty confusing especially
> > > in conjunction with the code below.
> > >
> > >
> > > I think this may force you to either:
> > > a) stop early-marking of -Os and -Oz flags with these attributes (early:
> > > prior to calling this routine) and handling all of the -O flag
> > > synthesized attributes here, or
> > > b) set optnone for -O0 wher ewe set optsize for -Os and friends, and then
> > > remove it where necessary here.
> > >
> > > I don't have any strong opinion about a vs. b.
> > I believe it is still correct: during Os/Oz we reach this point and figure
> > that there is `__attribute__((optnone))` in the *source* (not `-O0`), we
> > remove the attributes, nothing changes. Did I miss something?
> >
> Hmmm the Os/Oz attributes are added in CGCall.cpp, and are guarded with a
> check on the presence of the Optnone source attribute, so if the Optnone
> source attribute is present we should never see these. And Os/Oz set
> OptimizationLevel to 2, which is not zero, so we won't come through here for
> ShouldAddOptNone reasons either.
> Therefore these 'remove' calls should be no-ops and could be removed. (For
> paranoia you could turn them into asserts, and do some experimenting to see
> whether I'm confused about how this all fits together.)
The verifier is already complaining if we get this wrong, and indeed it
complains if I'm removing these.
See clang/test/CodeGen/attr-func-def.c:
```
int foo1(int);
int foo2(int a) {
return foo1(a + 2);
}
__attribute__((optnone))
int foo1(int a) {
return a + 1;
}
```
Here we have the attributed optnone on the definition but not the declaration,
and the check you're mentioning in CGCalls is only applying to the declaration.
https://reviews.llvm.org/D28404
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
