chandlerc 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);
----------------
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.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:892
+ // -O0 adds the optnone attribute, except if specific attributes prevents it.
+ bool ShouldAddOptNone =
----------------
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.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:899-900
- // OptimizeNone implies noinline; we should not be inlining such functions.
+ // OptimizeNone implies noinline; we should not be inlining such
+ // functions.
B.addAttribute(llvm::Attribute::NoInline);
----------------
Unrelated (and unnecessary) formatting change?
================
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);
----------------
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.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:962
+ ShouldAddOptNone &= !D->hasAttr<MinSizeAttr>();
+ ShouldAddOptNone &= !D->hasAttr<ColdAttr>();
+ ShouldAddOptNone &= !F->hasFnAttribute(llvm::Attribute::AlwaysInline);
----------------
why is optnone incompatible with *cold*....
https://reviews.llvm.org/D28404
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
