[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