[PATCH] D105811: [flang][driver] Delete `f18` (i.e. the old Flang driver)

Andrzej Warzynski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 23 07:46:24 PDT 2021


awarzynski marked 3 inline comments as done.
awarzynski added inline comments.


================
Comment at: flang/docs/Overview.md:36
 
-**Command:** `f18 -E src.f90` dumps the cooked character stream
+**Command:** `flang-new -fc1 -E src.f90` dumps the cooked character stream
 
----------------
Meinersbur wrote:
> Shouldn't the docs refer to `flang` instead of `flang-new`? AFAIU `flang` will eventually be replaced by a proper executable that does not call gfortran anymore and `flang-new` go away.
I think that this document should only refer to the frontend driver rather than the compiler driver. In our case, that's either `f18` (compiler driver + frontend driver) or `flang-new -fc1` (new frontend driver).  In fact, some options referred here are only available in `flang-new -fc1` (i..e. these options are frontend-specific).

Currently, `flang` is a yet another tool - it's neither a compiler nor a frontend driver, so I don't want to use it here. IMO, we should first rename it as e.g. `flang-to-gfortran` (or something similar) to clarify what it actually does. Then, we can claim `flang` for the actual driver and update all the docs accordingly.


================
Comment at: flang/test/lit.cfg.py:43
 # config.
-if config.include_flang_new_driver_test:
-  config.available_features.add('new-flang-driver')
-else:
-  config.available_features.add('old-flang-driver')
+config.available_features.add('new-flang-driver')
 
----------------
kiranchandramohan wrote:
> Is this required?
Some tests use it, e.g.: https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/missing-arg.f90#L4

Technically speaking, this patch makes it irrelevant. However, I wanted to keep this change relatively small and easy to review. If I do remove `new-flang-driver`, I will have to update further 20 files. I'm fine with that, but I can also remove it in a subsequent NFC patch. What do you recommend?


================
Comment at: flang/tools/f18/flang:239
 
   local -r ext_fc="${F18_FC:-gfortran}"
   local -r wd=$(cd "$(dirname "$0")/.." && pwd)
----------------
Meinersbur wrote:
> Is `F18_FC` still defined?
Only here: https://github.com/llvm/llvm-project/blob/main/flang/docs/ReleaseNotes.md#using-flang. I think that we do want to support compilers other than `gfortran` (so we need to keep this variable).


================
Comment at: flang/tools/f18/flang:317
   # flag that the driver will see (otherwise it assumes compiler/toolchain
   # driver mode).`f18` will just ignore this flag when uparsing, so it's fine
   # to add it here unconditionally.
----------------
Meinersbur wrote:
> 
This last sentence only applies to `f18`. I am deleting it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105811/new/

https://reviews.llvm.org/D105811



More information about the llvm-commits mailing list