[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

richard.barton.arm added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:185 {"flang", "--driver-mode=flang"}, + {"flang-new", "--driver-mode=flang"}, {"clang-dxc", "--driver-mode=dxc"}, clementval wrote: > This is counter

[PATCH] D122008: [flang][driver] Add support for generating executables

richard.barton.arm added a comment. Option 1 for me, let's not delay. With upstreaming of fir-dev making great progress, we can start to think about having a working flang in LLVM 15. For that we would need to have upstreaming finished, the driver finished and CMake support finished before end

[PATCH] D97119: [flang][driver] Add options for -std=f2018

richard.barton.arm added a comment. I agree with @tskeith that `-Mstandard` is not exactly orthogonal to `-std`, the former being about warning for non-standard extensions/deviations and the latter being about use of standard fortran, but from a different standard version. I would expect

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

richard.barton.arm added a comment. I think the right long-term solution is going to be SUGGESTION 2, or a variant of that. We have implemented this feature downstream in Arm Compiler for Linux, i.e. a separate, normally longer and more detailed, Doc string for the CommandLineReference.rst

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

richard.barton.arm added a comment. Could we tweak the wording to clarify that it is Clang-specific? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94169/new/ https://reviews.llvm.org/D94169 ___

[PATCH] D94169: [clang][driver] Restore the original help text for `-I`

richard.barton.arm accepted this revision. richard.barton.arm added a comment. This revision is now accepted and ready to land. The rationale for this revert looks good to me. I don't see the rationale behind the original change as it was committed without review. If @andrei99 gets back to us

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

richard.barton.arm accepted this revision. richard.barton.arm added a comment. I'm happy to accept this revision based on @SouraVX 's code review and the fact that Andrzej has sent multiple RFCs covering the clang-side changes, including the Options flags (latest here

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

richard.barton.arm accepted this revision. richard.barton.arm added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86089/new/ https://reviews.llvm.org/D86089 ___ cfe-commits mailing list

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

richard.barton.arm accepted this revision. richard.barton.arm added a comment. This revision is now accepted and ready to land. This LGTM. I think all the previous comments from other reviewers and me have been dealt with so I am happy to accept this revision based on the reviews so far. I

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

richard.barton.arm added a comment. Another random thought that just came to me: what does the new driver do when you invoke it with no input files or options? I could imagine a few sensible outcomes (error: no input (clang and gcc/gfortran behaviour), print version and exit, print help and

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

richard.barton.arm requested changes to this revision. richard.barton.arm added a comment. This revision now requires changes to proceed. Requesting changes mostly because of the exit status issue on the Driver tests. A few general questions as well: 1. Why not implement `-###` as part of this

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

richard.barton.arm added inline comments. Comment at: flang/test/lit.cfg.py:40 +# exclude the tests for flang_new driver while there are two drivers +if config.include_flang_new_driver_test == "OFF": + config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt',

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

richard.barton.arm requested changes to this revision. richard.barton.arm added a comment. A few specific comments to address here as well as the pre-commit linting ones. Comment at: clang/lib/Driver/Driver.cpp:1584 void Driver::PrintVersion(const Compilation , raw_ostream )

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm added a comment. So, coming back to this after a few weeks break. I agree with the point @awarzynski makes on this option being powerful and I think your alternative suggestion of using a different hard-coded name could work well at this stage of the project. I think the

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm added inline comments. Comment at: clang/test/Driver/flang/clang-driver-2-frontend01.f90:10 +! RUN: cp %clang %t1 +! RUN: %clang --driver-mode=flang -fortran-fe %basename_t.tmp1 -### %s 2>&1 | FileCheck --check-prefixes=ALL %s +

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm added inline comments. Comment at: clang/include/clang/Driver/Options.td:268 +def fortran_fe : Separate<["-"], "fortran-fe">, InternalDriverOpt, + HelpText<"Name for native Fortran compiler">; richard.barton.arm wrote: > This is not right.

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm added a comment. Still not sure why all the tests are needed here. The first one looks fine, i.e. we test that --fortran-fe= calls to that copy. The second one appears to test the default behaviour with no option, but why does it do it via a different name for clang, why

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm added a comment. I'd still like to see the nits addressed and comments on the tests addressed before approving. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73951/new/ https://reviews.llvm.org/D73951

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm added inline comments. Comment at: clang/include/clang/Driver/Options.td:264 MetaVarName<"">; +def fcc_fortran_name : Separate<["-"], "ffc-fortran-name">, InternalDriverOpt, + HelpText<"Name for native Fortran compiler">; CarolineConcatto

[PATCH] D73951: [Clang] [Driver]Add logic to search for flang frontend

richard.barton.arm requested changes to this revision. richard.barton.arm added a comment. A few comments running through that need addressing. In addition - have you checked the behaviour of this option with `-Bprefix`? Looking at the code for Driver.GetProgramPath, it seems like that would

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

richard.barton.arm added a comment. I think the behaviour for missing flang is fine for now, and I think we can improve on it later on. We ought to codify (if it is not done already) where flang looks for tools to exec, because I think PATH is probably not the only place it could look to

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

richard.barton.arm added a comment. Thanks for the updates. I think this is now looking good and matches the RFC already posted. One thought that occurs to me when reviewing this. I think we will assume that F18 /flang when it lands in the LLVM project will be an

[PATCH] D63607: [clang][driver] Add basic --driver-mode=fortran support for flang

richard.barton.arm added a comment. Herald added a subscriber: usaxena95. Hi Peter The overall approach seems good to me and matches how the driver is integrated in the original flang project so not too many surprises. I left a few comments mostly about the scope of the original patch. I

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

richard.barton.arm added a comment. Hi @nickdesaulniers - thanks for the clarification. I was suffering from some PEBCAK of my own when I thought the commits were not on master. Thanks for these patches - a great help. Repository: rC Clang https://reviews.llvm.org/D53210

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

richard.barton.arm added a comment. Hi @nickdesaulniers I have run into this too recently so would love to see this patch land. Did you get anywhere with those lld test failures? Ta Rich Repository: rC Clang https://reviews.llvm.org/D53210 ___

[PATCH] D29773: Add support for armv7ve flag in clang (PR31358).

richard.barton.arm accepted this revision. richard.barton.arm added a comment. This revision is now accepted and ready to land. This all LGTM. If I can assume I am allow to approve, then I approve. Comment at: test/Preprocessor/arm-acle-6.4.c:136 +// CHECK-V7VE: