[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Tim Keith via Phabricator via cfe-commits]

tskeith added inline comments.


================
Comment at: flang/test/Flang-Driver/fdefault.f90:4
+
+! REQUIRES: new-flang-driver
+
----------------
awarzynski wrote:
> tskeith wrote:
> > Can't this work with the f18 driver too? That's the best way to ensure they are consistent.
> I think that this is a good idea, but there are two things that need to be addressed first:
> 
> **OPTION NAMES**
> 
>  `f18` uses `-module` rather than `-module-dir`. I suggest adding an alias, `-module-dir`, to `f18` (e.g. similarly to how `-fparse-only` and `-fsyntax-only` are handled)
> 
> **IMPLEMENTATION**
> If I understand correctly, the current implementation of `-fdefault-real-8` in `f18` is incomplete, see [[ https://github.com/llvm/llvm-project/blob/b6db47d7e044730dc3c9b35dae6697eee0885dbf/flang/tools/f18/f18.cpp#L568-L569 | here ]]:
> ```
>     } else if (arg == "-r8" || arg == "-fdefault-real-8") {
>       defaultKinds.set_defaultRealKind(8);
> ```
> I think that there should be `defaultKinds.set_defaultRealKind(16);` as well.
> 
> @tskeith could you confirm?
> I think that this is a good idea, but there are two things that need to be addressed first:
> 
> **OPTION NAMES**
> 
>  `f18` uses `-module` rather than `-module-dir`. I suggest adding an alias, `-module-dir`, to `f18` (e.g. similarly to how `-fparse-only` and `-fsyntax-only` are handled)

Yes, in general when an option is given a different name in the new driver that option should be added to f18. Not only for testing consistent behavior but also because f18 gets more usage so any problems are more likely to turn up.

> **IMPLEMENTATION**
> If I understand correctly, the current implementation of `-fdefault-real-8` in `f18` is incomplete, see [[ https://github.com/llvm/llvm-project/blob/b6db47d7e044730dc3c9b35dae6697eee0885dbf/flang/tools/f18/f18.cpp#L568-L569 | here ]]:
> ```
>     } else if (arg == "-r8" || arg == "-fdefault-real-8") {
>       defaultKinds.set_defaultRealKind(8);
> ```
> I think that there should be `defaultKinds.set_defaultRealKind(16);` as well.

Do you mean `set_doublePrecisionKind(16)`? Yes, if that's how `-fdefault-real-8` is supposed to behave, it should work that way in f18 too.






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

https://reviews.llvm.org/D96344