[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