[flang] Change the default F18_FC to gfortran

[AlexisPerry]

Summary

Changed default F18_FC from pgf90 to gfortran. Removed unnecessary references to pgf90 in favor of more generic naming.

[AlexisPerry]

Extended the flang driver options to include gfortran equivalents to pgf90 specific options.

[clementval]

I don't see the previous change made to set gfortran as default. Are they gone with your diff update?

[AlexisPerry]

Changed the default external compiler used by the flang temporary driver.

• Changed default F18_FC from pgf90 to gfortran.
• Removed unneccesary references to pgf90 in favor of more generic naming.
• Extended the flang driver options to include gfortran equivalents to pgf90 specific options.

[AlexisPerry]

In D83488#2142950, @clementval wrote:

I don't see the previous change made to set gfortran as default. Are they gone with your diff update?

My mistake, sorry about that. They should be back now.

[clementval]

It's weird, the changes are in but both files are identical ... so it looks like there is nothing to review

[AlexisPerry]

I'll admit, this is my first time trying to commit to LLVM directly and using Phabricator (rather than on a fork that uses pull requests), and I'm definitely still learning the workflow. I had some git issues when first creating the patch that meant my first commit was lost locally, so when I went to update the patch it only had the second commit which caused the original changes to disappear. I then created a new branch and re-did the changes and updated again, using only a single commit, so that the diff would now have everything in it.

If this has resulted in an unreviewable state for this diff, then I will close it and submit a new one which will hopefully go more smoothly now that I've learned a few things. Sorry for the confusion.

[clementval]

In D83488#2142974, @AlexisPerry wrote:

I'll admit, this is my first time trying to commit to LLVM directly and using Phabricator (rather than on a fork that uses pull requests), and I'm definitely still learning the workflow. I had some git issues when first creating the patch that meant my first commit was lost locally, so when I went to update the patch it only had the second commit which caused the original changes to disappear. I then created a new branch and re-did the changes and updated again, using only a single commit, so that the diff would now have everything in it.

If this has resulted in an unreviewable state for this diff, then I will close it and submit a new one which will hopefully go more smoothly now that I've learned a few things. Sorry for the confusion.

The diff looks fine now! If you don't use it already, I would recommend to use arcanist to deal with the diff and phabricator ... it makes things easier

[clementval]

LGTM. You might want to wait to see if anybody has something to say on this.

[richard.barton.arm]

Thanks for picking up the work on this one @AlexisPerry!

I have replied to the RFC (late - my apologies) with a slightly different proposal. I'd like to make a final decision at the developers call on Monday. I don't hold my position too strongly though and would be happy to go in this direction. I also think we can go ahead with this patch because it moves us to a better place than we are now. Even if we chose my proposal to have no default most of this patch would still be kept because having gfortran support is a useful feature anyway.

On the patch itself, I have two requests:

1. This patch seems to be doing a couple of different things now that don't seem to be strictly linked:
   • Switching the default F18_FC to gfortran
   • Add support for a number of new gfortran options

I think these would be better submitted as two patches. Do you agree?

2. The patch should add a regression test for these fallback and option translation behaviours in test/Driver. It is a shame there are none already.

[clementval]

+1 to split this in two patches

[AlexisPerry]

Splitting this into two patches as requested by reviewers.

Change the default external compiler used by the flang temporary driver

- Changed default F18_FC from pgf90 to gfortran.
- Removed unneccesary references to pgf90 in favor of more generic naming.

[AlexisPerry]

The second part of the split can be found here: https://reviews.llvm.org/D83687

[richard.barton.arm]

Request one change to OpenMP handling, but the rest looks fine.

flang/tools/f18/f18.cpp

643

This needs to be amended to support gfortran. We'd need a similar switch on isPGF90 to pass -fopenmp here. I think it might work better to be moved into the next if/else block so it can be set to gfortran/pgfortran there.

Might also be better to add this as part of D83687 rather than this patch.

[AlexisPerry]

@richard.barton.arm I have made the requested adjustments in the other patch. Thanks for the feedback!

flang/tools/f18/f18.cpp

643

I agree this would be better as part of D83687, so I will add the fix over there. Good catch!

[tskeith]

When I build with this change I'm seeing a couple of modfile tests failing. The problem is that when isPGF90 is false, parser::useHexadecimalEscapeSequences is set to be true which affects how escape sequences are written in character literals in modfiles.

The choice of the compiler shouldn't be affecting modfile format, so I've created https://reviews.llvm.org/D83703 to fix that.

[AlexisPerry]

Updates following a rebase on master (due to changes on D83687)

[AlexisPerry]

@richard.barton.arm Is this patch acceptable now? Thanks!

[richard.barton.arm]

LGTM - thanks for doing this one for splitting the patch!

flang/tools/f18/f18.cpp

while (!args.empty()) {
     } else if (arg == "-") {
       fortranSources.push_back("-");
     } else if (arg == "--") {
       while (!args.empty()) {
         fortranSources.emplace_back(std::move(args.front()));
         args.pop_front();
       }
       break;
-    } else if (arg == "-Mfixed") {
+    } else if (arg == "-Mfixed" || arg == "-ffixed-form") {
       driver.forcedForm = true;
       options.isFixedForm = true;
-    } else if (arg == "-Mfree") {
+    } else if (arg == "-Mfree" || arg == "-ffree-form") {
       driver.forcedForm = true;
       options.isFixedForm = false;
-    } else if (arg == "-Mextend") {
+    } else if (arg == "-Mextend" || arg == "-ffixed-line-length-132") {
       options.fixedFormColumns = 132;
-    } else if (arg == "-Munlimited") {
+    } else if (arg == "-Munlimited" || arg == "-ffree-line-length-none" ||
+        arg == "-ffree-line-length-0" || arg == "-ffixed-line-length-none" ||
+        arg == "-ffixed-line-length-0") {
       // For reparsing f18's -E output of fixed-form cooked character stream
       options.fixedFormColumns = 1000000;
     } else if (arg == "-Mbackslash") {
       options.features.Enable(
           Fortran::common::LanguageFeature::BackslashEscapes, false);
     } else if (arg == "-Mnobackslash") {
       options.features.Enable(
           Fortran::common::LanguageFeature::BackslashEscapes, true);
-    } else if (arg == "-Mstandard") {
+    } else if (arg == "-Mstandard" || arg == "-std=f95" ||
+        arg == "-std=f2003" || arg == "-std=f2008" || arg == "-std=legacy") {
       driver.warnOnNonstandardUsage = true;
     } else if (arg == "-fopenmp") {
       options.features.Enable(Fortran::common::LanguageFeature::OpenMP);
       options.predefinitions.emplace_back("_OPENMP", "201511");
     } else if (arg == "-Werror") {
       driver.warningsAreErrors = true;
     } else if (arg == "-ed") {
       options.features.Enable(Fortran::common::LanguageFeature::OldDebugLines);
if (arg.empty()) {
         options.predefinitions.emplace_back(arg.substr(2), "1");
       } else {
         options.predefinitions.emplace_back(
             arg.substr(2, eq - 2), arg.substr(eq + 1));
       }
     } else if (arg.substr(0, 2) == "-U") {
       options.predefinitions.emplace_back(
           arg.substr(2), std::optional<std::string>{});
+    } else if (arg == "-fdefault-double-8") {
+      defaultKinds.set_defaultRealKind(4);
     } else if (arg == "-r8" || arg == "-fdefault-real-8") {
       defaultKinds.set_defaultRealKind(8);
     } else if (arg == "-i8" || arg == "-fdefault-integer-8") {
       defaultKinds.set_defaultIntegerKind(8);
       defaultKinds.set_subscriptIntegerKind(8);
       defaultKinds.set_sizeIntegerKind(8);
     } else if (arg == "-Mlargearray") {
     } else if (arg == "-Mnolargearray") {
if (arg.empty()) {
         args.pop_front();
       }
       driver.getDefinitionArgs = {arguments[0], arguments[1], arguments[2]};
     } else if (arg == "-fget-symbols-sources") {
       driver.getSymbolsSources = true;
     } else if (arg == "-help" || arg == "--help" || arg == "-?") {
       llvm::errs()
           << "f18 options:\n"
-          << "  -Mfixed | -Mfree     force the source form\n"
-          << "  -Mextend             132-column fixed form\n"
+          << "  -Mfixed | -Mfree | -ffixed-form | -ffree-form   force the "
+             "source form\n"
+          << "  -Mextend | -ffixed-line-length-132   132-column fixed form\n"
           << "  -f[no-]backslash     enable[disable] \\escapes in literals\n"
           << "  -M[no]backslash      disable[enable] \\escapes in literals\n"
           << "  -Mstandard           enable conformance warnings\n"
+          << "  -std=<standard>      enable conformance warnings\n"
           << "  -fenable=<feature>   enable a language feature\n"
           << "  -fdisable=<feature>  disable a language feature\n"
-          << "  -r8 | -fdefault-real-8 | -i8 | -fdefault-integer-8  "
-             "change default kinds of intrinsic types\n"
+          << "  -r8 | -fdefault-real-8 | -i8 | -fdefault-integer-8 | "
+             "-fdefault-double-8   change default kinds of intrinsic types\n"
           << "  -Werror              treat warnings as errors\n"
           << "  -ed                  enable fixed form D lines\n"
           << "  -E                   prescan & preprocess only\n"
           << "  -module dir          module output directory (default .)\n"
           << "  -flatin              interpret source as Latin-1 (ISO 8859-1) "
              "rather than UTF-8\n"
           << "  -fparse-only         parse only, no output except messages\n"
           << "  -funparse            parse & reformat only, no code "
if (arg.empty()) {
       }
     }
   }
 
   if (driver.warnOnNonstandardUsage) {
     options.features.WarnOnAllNonstandard();
   }
   if (options.features.IsEnabled(Fortran::common::LanguageFeature::OpenMP)) {
     driver.F18_FCArgs.push_back("-mp");
   }
   if (isPGF90) {
     if (!options.features.IsEnabled(
             Fortran::common::LanguageFeature::BackslashEscapes)) {
       driver.F18_FCArgs.push_back(
           "-Mbackslash"); // yes, this *disables* them in pgf90
     }
     Fortran::parser::useHexadecimalEscapeSequences = false;