aaron.ballman added a comment. In D126984#3574077 <https://reviews.llvm.org/D126984#3574077>, @aeubanks wrote:
> In D126984#3574046 <https://reviews.llvm.org/D126984#3574046>, @aaron.ballman > wrote: > >> In D126984#3571573 <https://reviews.llvm.org/D126984#3571573>, @aeubanks >> wrote: >> >>> IIRC in the past there was a strong preference to not have the pass manager >>> support this sort of thing >>> if you want to support this, there should be an RFC for how the >>> optimization part of this will work as it may require invasive changes to >>> the LLVM pass manager >>> >>> (if this is purely a clang frontend thing then ignore me) >> >> Hmm, does the pass manager have to support anything here? The only Clang >> codegen changes are for emitting IR attributes that we already emitted based >> on command line flags/other attributes, so I had the impression this would >> not be invasive for the backend at all. > > if we're allowing individual functions to specify that they want the `-O1` > pipeline when everything else in the module should be compiled with `-O2`, > that's a huge change in the pass manager. but perhaps I'm misunderstanding > the point of this patch I guess I'm not seeing what burden is being added here (you may still be correct though!) The codegen changes basically boil down to: if (!HasOptnone) { if (CodeGenOpts.OptimizeSize || HasOptsize) // This was updated for || HasOptsize FuncAttrs.addAttribute(llvm::Attribute::OptimizeForSize); if (CodeGenOpts.OptimizeSize == 2) FuncAttrs.addAttribute(llvm::Attribute::MinSize); } and bool HasOptimizeAttrO0 = false; // NEW if (const auto *OA = D->getAttr<OptimizeAttr>()) HasOptimizeAttrO0 = OA->getOptLevel() == OptimizeAttr::O0; // Add optnone, but do so only if the function isn't always_inline. if ((ShouldAddOptNone || D->hasAttr<OptimizeNoneAttr>() || HasOptimizeAttrO0) && // NEW !F->hasFnAttribute(llvm::Attribute::AlwaysInline)) { B.addAttribute(llvm::Attribute::OptimizeNone); For my own education, are you saying that these changes are specifying the entire -O<N> pipeline? The patch looks for `O0` in the attribute, but the other O<N> values are noops. We do some mapping though: OptimizeAttr::OptLevelKind Kind = OA->getOptLevel(); HasOptnone = HasOptnone || (Kind == OptimizeAttr::O0); HasOptsize = Kind == OptimizeAttr::Os || Kind == OptimizeAttr::Og || Kind == OptimizeAttr::Ofast || Kind == OptimizeAttr::Oz; but I'm failing to understand why this would be a concern for the backend given that we already support setting the LLVM IR flags based on other attributes. e.g., why is `[[clang::optnone]]` okay but `[[gnu::optimize("O0")]]` a problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126984/new/ https://reviews.llvm.org/D126984 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
