mehdi_amini marked 6 inline comments as done.
mehdi_amini added inline comments.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:760-762
Fn->removeFnAttr(llvm::Attribute::NoInline);
+ Fn->removeFnAttr(llvm::Attribute::OptimizeNone);
Fn->addFnAttr(llvm::Attribute::AlwaysInline);
----------------
chandlerc wrote:
> At point where we are in numerous places doing 3 coupled calls, we should add
> some routine to do this... Maybe we should have when I added the noinline bit.
>
> I don't have a good idea of where best to do this -- as part of or as an
> alternative to `SetInternalFunctionAttributes`? Something else?
>
> I'm imagining something like `SetAlwaysInlinedRuntimeFunctionAttributes` or
> something. Need a clang IRGen person to help push the organization in the
> right direction.
Yes some refactoring of all this custom handling would be welcome. I'll take
any pointer to how to do it in clang (I'm not familiar enough with clang).
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:892
+ // -O0 adds the optnone attribute, except if specific attributes prevents it.
+ bool ShouldAddOptNone =
----------------
chandlerc wrote:
> attributes prevents -> attributes prevent
>
> ACtually, what do you mean by attributes here? Or should this comment instead
> go below, where we start to branch on the actual 'hasAttr' calls?
>
> After reading below, I understand better. Maybe:
>
> // Track whether we need to add the optnone LLVM attribute,
> // starting with the default for this optimization level.
Actually I instead moved it all together.
================
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);
----------------
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?
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:962
+ ShouldAddOptNone &= !D->hasAttr<MinSizeAttr>();
+ ShouldAddOptNone &= !D->hasAttr<ColdAttr>();
+ ShouldAddOptNone &= !F->hasFnAttribute(llvm::Attribute::AlwaysInline);
----------------
chandlerc wrote:
> why is optnone incompatible with *cold*....
The source attribute "Cold" adds `llvm::Attribute::OptimizeForSize` even at O0
right now, I changed this and now we emit `optnone` at O0 in this case.
https://reviews.llvm.org/D28404
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
