awarzynski added a comment. Thanks for implementing this, @tblah!
Two high level questions/requests: - are you confident that we will need LangOptions.def? - can you upload a patch with full context? (some details can be found here: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface) -Andrzej ================ Comment at: clang/lib/Driver/ToolChains/Flang.cpp:85-86 + ArgStringList &CmdArgs) { + // TODO: share RenderFloatingPointOptions from ./Clang.cpp and use that + // instead of duplicating code here + StringRef FPContract; ---------------- What's RenderFloatingPointOptions? ================ Comment at: flang/lib/Frontend/CompilerInvocation.cpp:664 +/// +/// \param [out] res Stores the processed arguments +/// \param [in] args The arguments to parse ---------------- `res` is a very confusing name (that I used myself in various places). Basically, it's the `CompilerInvocation` instance ... result. Perhaps use `invoc`? ================ Comment at: flang/lib/Frontend/CompilerInvocation.cpp:665 +/// \param [out] res Stores the processed arguments +/// \param [in] args The arguments to parse +/// \param [out] diags DiagnosticsEngine to report erros with ---------------- [nit] "The compiler invocation args to parse" ================ Comment at: flang/test/Driver/driver-help.f90:108 ! HELP-FC1-NEXT: Use <value> as character line width in fixed mode +! HELP-FC1-NEXT: -ffp-contract=<value> Form fused FP ops (e.g. FMAs): fast (fuses across statements disregarding pragmas) | on (only fuses in the same statement unless dictated by pragmas) | off (never fuses) | fast-honor-pragmas (fuses across statements unless diectated by pragmas). Default is 'fast' for CUDA, 'fast-honor-pragmas' for HIP, and 'on' otherwise. ! HELP-FC1-NEXT: -ffree-form Process source files in free form ---------------- Why not expose this flag in `flang-new`? (as well as `flang-new -fc1`?) ================ Comment at: flang/test/Driver/flang_fp_opts.f90:1-2 +! Test for passing of floating point options between the compiler and frontend +! drivers. + ---------------- Sounds like a test for [[ https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90 | frontend-forwarding.f90 ]] ================ Comment at: flang/test/Driver/flang_fp_opts.f90:6-7 + +! CHECK1-LABEL: "-fc1" +! CHECK1: -ffp-contract=fast + ---------------- Could you use more descriptive prefixes? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136080/new/ https://reviews.llvm.org/D136080 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
