Page MenuHomePhabricator

[flang][driver] Add `-fno-analyzed-objects-for-unparse`
ClosedPublic

Authored by awarzynski on Jun 3 2021, 5:58 AM.

Details

Summary

This patch adds a new option for the new Flang driver:
-fno-analyzed-objects-for-unparse. The semantics are similar to
-funparse-typed-exprs-to-f18-fc from f18. For consistency, the
latter is replaced with -fno-analyzed-objects-for-unparse.

The new option controls the behaviour of the unparser (i.e. the action
corresponding to -fdebug-unparse). The default behaviour is to use the
analyzed objects when unparsing. The new flag can be used to turn this
off, so that the original parse-tree objects are used. The analyzed
objects are generated during the semantic checks [1].

This patch also updates the semantics of
-fno-analyzed-objects-for-unparse/-funparse-typed-exprs-to-f18-fc
in f18, so that this flag is always taken into account when Unparse
is used (this way the semantics in f18 and flang-new are identical).

The added test file is based on example from Peter Steinfeld.

[1] https://github.com/llvm/llvm-project/blob/main/flang/docs/Semantics.md

Diff Detail

Event Timeline

awarzynski created this revision.Jun 3 2021, 5:58 AM
awarzynski requested review of this revision.Jun 3 2021, 5:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2021, 5:58 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Rebased on top of D103613

awarzynski updated this revision to Diff 349868.Jun 4 2021, 7:27 AM

Rebase on top of main, revert one small change uploaded accidentally

awarzynski updated this revision to Diff 349881.Jun 4 2021, 8:26 AM

Fix build and test failure

Rebase on top of main

Refine the semantics after some discussion offline

awarzynski retitled this revision from [flang][driver] Add `-fno-unparse-typed-exprs` to [flang][driver] Add `-funparse-typed-exprs-as-fortran`.Jun 18 2021, 9:43 AM
awarzynski edited the summary of this revision. (Show Details)
klausler added a comment.EditedJun 18 2021, 9:55 AM

The default behaviour is to always decorate unparsed typed expression with e.g. their KIND. The new flag can be used to turn this off, so that the generated output uses valid Fortran syntax and can be fed to another Fortran compiler.

The output of Expr<T>::AsFortran() should be valid Fortran, and it's a bug if it's not.

EDIT: To be more clear: the output of Expr<T>::AsFortran() should be syntactically valid Fortran, but it may contain references to symbols that might cause errors when recompiled; for example, references to generic interfaces will have been resolved to specific subprograms, and it's possible that those identifiers might be PRIVATE or not in scope. But the syntax of the dump of the analyzed expression should be valid Fortran.

The default behaviour is to always decorate unparsed typed expression with e.g. their KIND. The new flag can be used to turn this off, so that the generated output uses valid Fortran syntax and can be fed to another Fortran compiler.

The output of Expr<T>::AsFortran() should be valid Fortran, and it's a bug if it's not.

This is the output that I get from the unparser (input file: flang/test/Driver/unparse-typed-exprs.f95):

PROGRAM test_allocated
 INTEGER :: i = 13_4
 REAL(KIND=2_4), ALLOCATABLE :: x(:)
 IF (.NOT.allocated(x)) ALLOCATE(x(i))
END PROGRAM test_allocated

This is not valid, is it? Or am I missing something?

The default behaviour is to always decorate unparsed typed expression with e.g. their KIND. The new flag can be used to turn this off, so that the generated output uses valid Fortran syntax and can be fed to another Fortran compiler.

The output of Expr<T>::AsFortran() should be valid Fortran, and it's a bug if it's not.

This is the output that I get from the unparser (input file: flang/test/Driver/unparse-typed-exprs.f95):

PROGRAM test_allocated
 INTEGER :: i = 13_4
 REAL(KIND=2_4), ALLOCATABLE :: x(:)
 IF (.NOT.allocated(x)) ALLOCATE(x(i))
END PROGRAM test_allocated

This is not valid, is it? Or am I missing something?

Kind suffixes are described in subclause 7.4.3 of Fortran 2018, e.g. R708 on p. 58.

Kind suffixes are described in subclause 7.4.3 of Fortran 2018, e.g. R708 on p. 58.

Many thanks for this reference.

So when calling Unparse, one specifes AnalyzedObjectsAsFortran (i.e. the optional last argument) to get more semantic context in the dump, right?

awarzynski edited the summary of this revision. (Show Details)Jun 21 2021, 3:45 AM

Kind suffixes are described in subclause 7.4.3 of Fortran 2018, e.g. R708 on p. 58.

Many thanks for this reference.

So when calling Unparse, one specifes AnalyzedObjectsAsFortran (i.e. the optional last argument) to get more semantic context in the dump, right?

It's for better code coverage when testing. When the internal representations of expressions & variables are dumped by unparsing rather than the original parse trees, it allows semantic analysis (esp. folding) to be exercised. If an unparsed program compiles and runs fine when unparsed into gfortran without this flag, but fails when the flag is present, there's a bug. This feature allows much of the frontend to undergo some testing and debugging in the absence of a code generator.

It's also useful for self-testing via a "parse/unparse/reparse/unparse" mode, in which we recompile unparsed Fortran and check that it unparses to the same output.

awarzynski added a subscriber: PeteSteinfeld.

Rename the new flag as -fno-analyzed-objects-for-unparse

Based on the discussion, I've renamed the flag to better reflect the intention. Does it make more sense now?

Note, the default behaviour of the drivers does not change with this patch (i.e. {f18|flang-new} -fdebug-unparse will generate identical results _before_ and _after_).

@klousler & @PeteSteinfeld, many thanks for all the feedback!

awarzynski retitled this revision from [flang][driver] Add `-funparse-typed-exprs-as-fortran` to [flang][driver] Add `-fno-analyzed-objects-for-unparse`.Jun 24 2021, 10:25 AM
awarzynski edited the summary of this revision. (Show Details)
awarzynski added a reviewer: PeteSteinfeld.
PeteSteinfeld accepted this revision.Jun 24 2021, 1:11 PM

All builds, tests, and looks good.

This revision is now accepted and ready to land.Jun 24 2021, 1:11 PM

This patch may have broken the shared library buildbots.

This patch may have broken the shared library buildbots.

Sorry about that. A fix is on its way.

I pushed a fix without a review: https://reviews.llvm.org/rGc3ebb53eabb7f851687f66ada88aa16f768d76ce. Please let me know if you prefer such changes to go through a regular review in the future!