This is an archive of the discontinued LLVM Phabricator instance.

[flang][driver] Add support for `-cpp/-nocpp`
ClosedPublic

Authored by awarzynski on Mar 24 2021, 12:21 PM.

Details

Summary

This patch adds support for the -cpp and -nocpp flags. The
implemented semantics match f18 (i.e. the "throwaway" driver), but are
different to gfortran. In Flang the preprocessor is always run and
-cpp/-nocpp are only used to control whether standard macro
predefinitions are added or not. In practice this is sufficient to model
gfortran`s -cpp/-nocpp.

In the absence of -cpp/-nocpp, the driver will use the extension of
the input file to decide whether to include the standard macro
predefinitions. gfortran's documentation [1] was used to decide which
file extension to use for this. The extension for:

  • predefined-macros-compiler-version.f90

had to be updated accordingly.

The logic mentioned above was added in FrontendAction::BeginSourceFile.
That's relatively late in the driver set-up, but this roughly where the
name of the input file becomes available. The logic for deciding between
fixed and free form works in an a similar way and was also moved to
FrontendAction::BeginSourceFile for consistency (and to reduce
code-duplication).

The -cpp/-nocpp flags are respected also when the input is read from
stdin. This is different to:

  • gfortran (behaves as if -cpp was used)
  • f18 (behaves as if -nocpp was used)

[1] https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html

Diff Detail

Unit TestsFailed

Event Timeline

awarzynski created this revision.Mar 24 2021, 12:21 PM
awarzynski requested review of this revision.Mar 24 2021, 12:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2021, 12:21 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
awarzynski added inline comments.Mar 24 2021, 12:24 PM
flang/include/flang/Frontend/CompilerInvocation.h
145–148

DELETEME

minor comments.

flang/lib/Frontend/FrontendAction.cpp
73

Nit: prefernce -> preference
below also.

flang/lib/Frontend/FrontendOptions.cpp
23

Unrelated comment: f77 is probably not free form.

30

Consider adding fpp and making it match the behaviour of gfortran as given below.

"The preprocessor is automatically invoked if the file extension is .fpp, .FPP, .F, .FOR, .FTN, .F90, .F95, .F03 or .F08."
https://gcc.gnu.org/onlinedocs/gfortran/Preprocessing-Options.html

Address PR comments + updated comments + rebase

I refined some comments and added -fpp to the list of file extensions. Thank you @kiranchandramohan for pointing this out!

tskeith added inline comments.Mar 29 2021, 7:38 AM
clang/include/clang/Driver/Options.td
4305

This option affects command line macro definitions too. So maybe something like:
Enable predefined and command line preprocessor macros

Make sure that -cpp\-nocpp controls command line macro definitions too

As @tskeith pointed out, -cpp\-nocpp should also affect command line macro
definitions (on top of standard macro predefinitions). With this change, the
file extension becomes significant in all tests that depend on macros. I've
renamed some of them accordingly. Few other tests are updated to
demonstrate that the new flags work as expected.

awarzynski added inline comments.Mar 29 2021, 12:00 PM
clang/include/clang/Driver/Options.td
4305

I've actually missed this case altogether - thank you for pointing it out. Updated accordingly.

flang/lib/Frontend/FrontendOptions.cpp
23

Thanks for noticing and pointing out! https://reviews.llvm.org/D99494

LGTM.

flang/include/flang/Frontend/CompilerInvocation.h
142

Nit: Misplaced? Don't see any params here.

147

Nit: Should this have a param in?

flang/include/flang/Frontend/FrontendOptions.h
147

Nit: implment -> implement

flang/lib/Frontend/FrontendActions.cpp
68

Nit: Looks similar to function above.

This revision is now accepted and ready to land.Mar 30 2021, 1:55 PM
awarzynski updated this revision to Diff 334667.Apr 1 2021, 6:09 AM
awarzynski marked 3 inline comments as done.

Update comments

flang/include/flang/Frontend/CompilerInvocation.h
142

Yes, needs updating.

147

Should be deleted, sorry!

flang/lib/Frontend/FrontendActions.cpp
68

Yes, they are identical up to L85 (in which Parsing starts):

// Parse. In case of failure, report and return.
ci.parsing().Parse(llvm::outs());

I agree that this is not great and should be refactored at some point. But it will be much easier once we have more actions and a better understanding of what is needed. I think that https://reviews.llvm.org/D99645 is an important step in this direction. It adds a new abstract class for frontend actions. Once it's merged, we will have 3 classes of actions:

  • prescan
  • prescan + parse
  • prescan + parse + sema

So there will be a very clear split into 3 groups.

awarzynski edited the summary of this revision. (Show Details)Apr 1 2021, 6:11 AM
This revision was landed with ongoing or failed builds.Apr 7 2021, 6:02 AM
This revision was automatically updated to reflect the committed changes.

As there have been no new comments and this patch has already been accepted, I decided to merge it without waiting for more reviews. Please leave post-commit comments if I missed anything and I will address them. Thank you all for reviewing!

flang/test/Driver/driver-help.f90