This is an archive of the discontinued LLVM Phabricator instance.

[flang] Don't perform macro replacement unless *.F, *.F90, &c.
ClosedPublic

Authored by klausler on Feb 10 2021, 4:19 PM.

Details

Summary

Avoid spurious and confusing macro replacements from things like
-DPIC on Fortran source files whose suffixes indicate that preprocessing
is not expected.

Add gfortran-like "-cpp" and "-nocpp" flags to f18 to force predefinition
of macros independent of the source file suffix.

Diff Detail

Event Timeline

klausler created this revision.Feb 10 2021, 4:19 PM
klausler requested review of this revision.Feb 10 2021, 4:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2021, 4:19 PM
PeteSteinfeld accepted this revision.Feb 10 2021, 7:44 PM

f18.cpp needs to have clang-format run on it. Otherwise, all builds, tests, and looks good.

This revision is now accepted and ready to land.Feb 10 2021, 7:44 PM

Hello @klausler, thank you for working on this!

@PeteSteinfeld and @klausler, could you please clarify the following before this is merged. I want to make sure that the two Flang compiler drivers don't diverge too much.

Q1:
According Parsing.md:

The preprocessor always runs.

Doesn't this patch change this? Or indicate that such change is coming? If that's the case, could you please update Parsing.md?

Q2:
This patch adds -cpp/-nocpp flags. Are these meant to be used to enable/disable preprocessing? (like in e.g. gfortran). If I understand correctly, currently even with -nocpp the preprocessor is still run.

Q3:
Could the following condition be extracted into a separate method (e.g. bool shouldRunPP):

if (suffix == "F" || suffix == "F90" || suffix == "F95" ||
    suffix == "CUF" || suffix == "F18") {
  options.predefinitions = predefinitions;

? It would be very helpful if this could be shared between the drivers. Alternatively, could you suggest _where_ such method could be added?

The new driver assumes that the preprocessor always runs and adds standard predefinitions unconditionally here. From what I can see, this patch shouldn't cause any of the driver tests to fail, but it does mean that the currently implemented behavior becomes incorrect. I'm happy to update the new driver in a separate patch, but could you clarify Q1, Q2 and Q3 first? Your input is much appreciated!

The behavior of a preprocessor is always present, even with "-nocpp"; preprocessing is an integrated part of prescanning, not a separate pass or process. The preprocessor instances used to be unconditionally initialized with some macro definitions from -D options and built-in names like FILE; those initializations are now conditional, so that unexpected macro replacement will not occur. The motivation for this change is from an application that defined -DPIC on the command line and had a variable named PIC in a *.f90 file.

The list of file suffixes that will observe predefined macros in f18 is probably incomplete; I don't know what to do with *.ff files, for example. I would not view it as being authoritative if I were you.

Thank you for clarifying, this helps a lot!

As there is no intention for f18 -nocpp (or flang-new -nocpp) to match the behavior of gfortran -nocpp, could we use different spellings? Also, I doubt that we would be able to re-use -cpp/-nocpp in flang-new for something other than enabling/disabling the preprocessor (these options are already present in Options.td).

I'm guessing that want something with semantics similar to -ffree-form/-ffixed-form. Naming is hard, but let me try suggesting something:

  • -fadd-pp-predefs/-fno-pp-predefs (add preprocessor predefinitions)
  • -fno-macro-predefs/-fmacro-predefs (add macro predefinitions)
  • -fno-std-macro-predefs/-fstd-macro-predefs (add standard macro predefinitions)

Would any of these make sense to you?

Thank you for clarifying, this helps a lot!

As there is no intention for f18 -nocpp (or flang-new -nocpp) to match the behavior of gfortran -nocpp, could we use different spellings? Also, I doubt that we would be able to re-use -cpp/-nocpp in flang-new for something other than enabling/disabling the preprocessor (these options are already present in Options.td).

I'm guessing that want something with semantics similar to -ffree-form/-ffixed-form. Naming is hard, but let me try suggesting something:

  • -fadd-pp-predefs/-fno-pp-predefs (add preprocessor predefinitions)
  • -fno-macro-predefs/-fmacro-predefs (add macro predefinitions)
  • -fno-std-macro-predefs/-fstd-macro-predefs (add standard macro predefinitions)

Would any of these make sense to you?

I would not enjoy explaining to a user why they have to modify their decades-old makefiles to build their existing applications with your new driver; I advise that you avoid gratuitous incompatibility.

This revision was landed with ongoing or failed builds.Feb 11 2021, 11:25 AM
This revision was automatically updated to reflect the committed changes.