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

Andrzej Warzynski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 18 10:21:18 PST 2021


awarzynski added inline comments.


================
Comment at: flang/test/Flang-Driver/fdefault.f90:1
+! Ensure argument -fdefault* works as expected.
+! TODO: Add checks when actual codegen is possible for this family
----------------
I think that this test is a great improvement compared to what we had before. Thank you @tskeith for the suggestion and @arnamoy10 for implementing it!

A have a few additional ideas how to strengthen this:
1) It would be great to see `FileCheck` instead of `grep` (with `FileCheck` you can check more lines easily)

2)  One `RUN` line per flag. When we mix flags (e.g. `-fdefault-real-8 -flarge-sizes`), it's not clear whether we are testing one particular flag (e.g. `-flarge-sizes`) or how various flags interact with each other (e.g. `-fdefault-real-8` and `-flarge-sizes`).

What about:
* 1 `RUN` line without any flags + `FileCheck` checks for both REAL and DOUBLE
* 1 `RUN` line with `-fdefault-real-8` + FileCheck` checks for both REAL and DOUBLE
* 1 `RUN` line with `-fdefault-double-8 -fdefault-real-8` + `FileCheck` checks  for both REAL and DOUBLE
* 1 `RUN` line with `-fdefault-double-8` + `FileCheck` check to demonstrates that this is an error 

3) Separate file for `-flarge-sizes`. Otherwise there's a lot being tested here. No need to mix with other flags.


================
Comment at: flang/test/Flang-Driver/fdefault.f90:36
+end
\ No newline at end of file

----------------
FIXME


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

https://reviews.llvm.org/D96344



More information about the cfe-commits mailing list