awarzynski added a comment. Thanks for working on this, LG! I've left a few minor comments inline.
================ Comment at: clang/include/clang/Driver/Options.td:6449-6455 +def opt_record_file : Separate<["-"], "opt-record-file">, Flags<[FC1Option, CC1Option]>, + HelpText<"File name to use for YAML optimization record output">, + MarshallingInfoString<CodeGenOpts<"OptRecordFile">>; +def opt_record_passes : Separate<["-"], "opt-record-passes">, Flags<[FC1Option, CC1Option]>, + HelpText<"Only record remark information for passes whose names match the given regular expression">; +def opt_record_format : Separate<["-"], "opt-record-format">, Flags<[FC1Option, CC1Option]>, + HelpText<"The format used for serializing remarks (default: YAML)">; ---------------- Currently, these options have the following flags set: * `CC1Option` and `NoDriverOption`. See: * https://github.com/llvm/llvm-project/blob/5da317a79e3e53f17c6d356361807df1d16a0b66/clang/include/clang/Driver/Options.td#L6156 IIUC, you'd like this to be CC1Option` and `NoDriverOption` _and_ `FC1Option`. You can do that by moving these definitions here: * https://github.com/llvm/llvm-project/blob/5da317a79e3e53f17c6d356361807df1d16a0b66/clang/include/clang/Driver/Options.td#L6810-L6835 ================ Comment at: flang/lib/Frontend/CompilerInvocation.cpp:195-197 + args.getLastArg(clang::driver::options::OPT_opt_record_file)) { + opts.OptRecordFile = a->getValue(); + } ---------------- Drop braces: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements. Same for the other options below. Btw, I'd be OK with moving this to a dedicated hook for parsing e.g. "opt remark options". ================ Comment at: flang/lib/Frontend/FrontendActions.cpp:961-965 + clang::DiagnosticsEngine &diags = ci.getDiagnostics(); + const CodeGenOptions &codeGenOpts = ci.getInvocation().getCodeGenOpts(); + Fortran::lower::LoweringOptions &loweringOpts = + ci.getInvocation().getLoweringOpts(); + ---------------- victorkingi wrote: > Thought this refactoring might help making the function clearer. Let me know > if its unnecessary It's a nice clean-up! I'd move it to a separate patch, but the noise level is low, so I am not too concerned. ================ Comment at: flang/test/Driver/frontend-forwarding.f90:26 +! RUN: %flang -### %s 2>&1 \ +! RUN: -foptimization-record-file=%t.opt.yaml \ ---------------- Is a dedicated driver invocation needed for this test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155452/new/ https://reviews.llvm.org/D155452 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
