[PATCH] D28404: IRGen: Add optnone attribute on function during O0

Paul Robinson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 6 17:18:36 PST 2017


probinson 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);
----------------
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.)


https://reviews.llvm.org/D28404





More information about the cfe-commits mailing list