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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
