awarzynski added a comment. Mats, thanks for working on this! Just a few minor suggestions from me.
================ Comment at: clang/lib/Driver/ToolChains/Flang.cpp:55 +static bool shouldLoopVersion(const ArgList &Args) { + if (Arg *A = Args.getLastArg(options::OPT_Ofast, options::OPT_O, ---------------- Could you add a short Docstring, pls? In particular, is the logic implemented by this method consistent with GFortran? Are there any external docs that could be referred to here? ================ Comment at: clang/lib/Driver/ToolChains/Flang.cpp:56 +static bool shouldLoopVersion(const ArgList &Args) { + if (Arg *A = Args.getLastArg(options::OPT_Ofast, options::OPT_O, + options::OPT_O4, options::OPT_floop_versioning, ---------------- Please rewrite this to use [[ https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code | early exit ]], e.g.: ``` Arg *LoopVersionOption = Args.getLastArg(options::OPT_Ofast, options::OPT_O, options::OPT_O4, options::OPT_floop_versioning, options::OPT_fno_loop_versioning) if !(LoopVersionOption) return false; // The remaining logic HERE ``` ================ Comment at: clang/lib/Driver/ToolChains/Flang.cpp:109-110 addDebugInfoKind(CmdArgs, DebugInfoKind); + if (shouldLoopVersion(Args)) + CmdArgs.push_back("-fversion-loops-for-stride"); } ---------------- Would you classify this as a code-gen option? Alongside `-fstack-arrays` and `-flang-experimental-hlfir`? It sound like we could introduce another hook here, `adddCodeGenOptions`? ================ Comment at: flang/test/Driver/version-loops.f90:2 +! Test that flang-new forwards the -f{no-,}version-loops-for-stride +! options corredly to flang-new -fc1 for different variants of optimisation +! and explicit flags. ---------------- ================ Comment at: flang/test/Driver/version-loops.f90:5 + +! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 \ +! RUN: -O3 \ ---------------- [nit] If you are using `-###`, then you can just skip `-fsyntax-only` (it doesn't really matter what "action" is requested from the frontend driver). ================ Comment at: flang/test/Driver/version-loops.f90:33-34 + +! CHECK: "-fversion-loops-for-stride" +! CHECK: "-O3" + ---------------- Similar suggestion for other `CHECK` lines - this will make the test a bit more explicit about the expected output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141307/new/ https://reviews.llvm.org/D141307 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits