[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`
Andrzej Warzynski via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 30 03:46:48 PDT 2022
awarzynski 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"},
----------------
richard.barton.arm wrote:
> clementval wrote:
> > This is counter intuitive. We rename flang-new but we add flang-new here. It should be configurable.
> This is a list of all the valid prefixes of binaries that the driver supports. With this change, an additional one will be supported so I don't think it's an issue to have both flang and flang-new here.
>
> The thing that confuses me is how flang-new works today without this change, given that "flang" is not a suffix of "flang-new"? @awarzynski , do you know why that is? Perhaps the line here is not needed at all? Or is this a bug fix for flang-new that is actually unrelated to this change?
> This is counter intuitive.
I can add a comment to clarify this.
> It should be configurable.
It is, in Flang's [[ https://github.com/llvm/llvm-project/blob/main/flang/tools/flang-driver/driver.cpp | driver.cpp ]]. Originally, the suffix was hard-coded as:
```
clang::driver::ParsedClangName targetandMode("flang", "--driver-mode=flang");
```
(i.e. the `clangDriver` library used `flang` internally despite the actual name being `flang-new`). This is now being replaced with (see in "driver.cpp"):
```
clang::driver::ParsedClangName targetandMode =
clang::driver::ToolChain::getTargetAndModeFromProgramName(argv[0]);
```
But the change in "driver.cpp" means that we can no longer make assumptions about the suffix and hence the update here.
Like I mentioned earlier, we should not make this file build-time configurable. One possible option would be to try to update Clang's [[ https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Config/config.h.cmake | config.h.cmake ]], but that's would lead to more Flang-specific changes in Clang's core set-up. Also, I'm not convinced it would work here.
> @awarzynski , do you know why that is?
Yeah, check Flang's "driver.cpp". We should've captured this earlier. The original set-up from ToolChain.cpp predates `flang-new`. And then in "driver.cpp" we just matched what was here. There was a lot of confusion around naming back then and this has slipped in.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125788/new/
https://reviews.llvm.org/D125788
More information about the cfe-commits
mailing list