[flang-commits] [PATCH] D126291: [flang][Driver] Update link job on windows

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added a comment.

In D126291#3577133 <https://reviews.llvm.org/D126291#3577133>, @rovka wrote:

> I had the same idea about switching the tests to using target triples instead of having separate files for it, but initially I had some issues getting that to work properly.

Could you give it another go? I agree with @mstorsjo that ... it should just work when using `-###` :)

In D126291#3577160 <https://reviews.llvm.org/D126291#3577160>, @rovka wrote:

> In D126291#3572796 <https://reviews.llvm.org/D126291#3572796>, @mmuetzel wrote:
>
>> In D126291#3572777 <https://reviews.llvm.org/D126291#3572777>, @rovka wrote:
>>
>>> Moved the check for `-flang-experimental-exec` into addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski does this look like a good idea?
>>
>> If moving that check to inside that function is ok, should the same check be added to `addFortranRuntimeLibs`, too?
>> Edit: And also retain that condition for any flags that are added in the respective part of the toolchain files that don't use any of these two functions?
>
> I don't think we need to add it to the other function since we'll get an error anyway if the linker can't find the libraries. In any case, the right way to handle this is probably to error out in the driver before trying to compose a link job when `-flang-experimental-exec` is not specified, rather than rely on a specific linker error. But that should probably be a different patch. @awarzynski wdyt?

IMHO, `addFortranRuntimeLibraryPath` should be adding Fortran runtime library path unconditionally (as the name suggests). Similarly, `addFortranRuntimeLibs` should be adding Fortran library libs unconditionally. But unfortunately, we need to deal with `-flang-experimental-exec` (I really wish we didn't). What you are suggesting here makes a lot of sense to me, but it is also worth keeping in mind that this flag is here only temporarily ;-) So, I'd mostly focus on making sure that removing it is easy (and that everything else is rock solid). We do need `Args.hasArg(options::OPT_flang_experimental_exec)` there somewhere - I'll be happy with whatever you decide!


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

https://reviews.llvm.org/D126291