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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits