awarzynski added a comment. Thanks for taking a look, @peixin! Just to clarify - I'm not really looking into `-Os`, `-Ofast` or `-Oz` at the moment. But I'm always happy to review driver patches :)
================ Comment at: clang/include/clang/Driver/Options.td:732 +def O_flag : Flag<["-"], "O">, Flags<[CC1Option,FC1Option]>, Alias<O>, AliasArgs<["1"]>; def Ofast : Joined<["-"], "Ofast">, Group<O_Group>, Flags<[CC1Option]>; def P : Flag<["-"], "P">, Flags<[CC1Option,FlangOption,FC1Option]>, Group<Preprocessor_Group>, ---------------- peixin wrote: > Will enabling Ofast require more changes in the flang frontend? If yes, it is > OK to support it in another patch later. I would much prefer to have it implemented it in a dedicated patch. For every option, one has to consider the semantics in the compiler driver (`flang-new`) vs frontend driver (`flang-new -fc1`) and then, in case of code-gen flags, the difference between middle-end and back-end pass pipelines. So quite a few things :) In general, I'm in favor of doing this incrementally, in small patches. This makes reviewing and testing easier and more self-contained. Is that OK? ================ Comment at: flang/include/flang/Frontend/CodeGenOptions.def:30 + +VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified. + ---------------- peixin wrote: > I saw `clang/include/clang/Basic/CodeGenOptions.def` has the following: > ``` > VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified. > VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is > specified. > ``` > > Do `-Os` and `-Oz` need extra processing in flang drivers? If yes, it is OK > to support it in another patch later. > Do -Os and -Oz need extra processing in flang drivers? These are code-size optimisation flags, so the logic will be a bit different. Also, I'm very much in favor of small, incremental and self-contained patches. Splitting this into regular and size optimizations seemed like a natural split. ================ Comment at: flang/lib/Frontend/CodeGenOptions.cpp:8 +//===----------------------------------------------------------------------===// + +#include "flang/Frontend/CodeGenOptions.h" ---------------- peixin wrote: > Miss the following? > ``` > // > // Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/ > // > //===----------------------------------------------------------------------===// > ``` > Good catch, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128043/new/ https://reviews.llvm.org/D128043 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits