[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:590 + std::optional OptLevelOrNone = + CodeGenOpt::getLevel(CGOpts.OptimizationLevel); + assert(OptLevelOrNone && "Invalid optimization level!"); aaronpuchert wrote: >

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

aaronpuchert added inline comments. Comment at: flang/lib/Frontend/FrontendActions.cpp:590 + std::optional OptLevelOrNone = + CodeGenOpt::getLevel(CGOpts.OptimizationLevel); + assert(OptLevelOrNone && "Invalid optimization level!"); Does this need

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added a comment. I also ended up changing the underlying type to `int` to resolve https://lab.llvm.org/buildbot/#/builders/61/builds/38754 , and I removed the `getID` function (with the implicit conversions it would never get used anyway). Repository: rG LLVM Github Monorepo

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:67 + inline std::optional getLevel(IDType ID) { +if (ID < 0 || ID > 3) + return std::nullopt; mstorsjo wrote: > scott.linder wrote: > > barannikov88 wrote: > > > As I

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

mstorsjo added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:67 + inline std::optional getLevel(IDType ID) { +if (ID < 0 || ID > 3) + return std::nullopt; scott.linder wrote: > barannikov88 wrote: > > As I can see, clients do not

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG25c0ea2a5370: [NFC] Consolidate llvm::CodeGenOpt::Level handling (authored by scott.linder). Changed prior to commit:

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

barannikov88 added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 scott.linder wrote: > barannikov88 wrote: > > arsenm wrote: > > > scott.linder

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 barannikov88 wrote: > arsenm wrote: > > scott.linder wrote: > > > This is ABI

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder updated this revision to Diff 490321. scott.linder marked 2 inline comments as done. scott.linder added a comment. - Fix return type of `getID` - Fix mistakenly updated option help text - Add back in missing `assert`s Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

barannikov88 added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:67 + inline std::optional getLevel(IDType ID) { +if (ID < 0 || ID > 3) + return std::nullopt; As I can see, clients do not check for nullopt. Either add checks or

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

barannikov88 added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 arsenm wrote: > scott.linder wrote: > > This is ABI breaking, so maybe we

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

arsenm added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 scott.linder wrote: > This is ABI breaking, so maybe we don't want/need to define

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added inline comments. Comment at: llvm/include/llvm/Support/CodeGen.h:57 + /// Code generation optimization level. + enum Level : IDType { +None = 0, ///< -O0 This is ABI breaking, so maybe we don't want/need to define the underlying

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

arsenm accepted this revision. arsenm added a comment. This revision is now accepted and ready to land. The error handling inconsistency is kind of annoying too but this is an improvement. I thought traditionally GCC accepted whatever big number you put after -O Repository: rG LLVM Github

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. I wasn't sure whether to include `getID` and unit-test it, or just drop it until it is used. If anyone has a preference let me know and I'll update the review. Repository: rG LLVM Github

[PATCH] D141968: [NFC] Consolidate llvm::CodeGenOpt::Level handling

scott.linder created this revision. Herald added subscribers: ormris, steven_wu, hiraditya. Herald added projects: Flang, All. scott.linder requested review of this revision. Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, jdoerfert. Herald added projects: clang, OpenMP, LLVM.