This is an archive of the discontinued LLVM Phabricator instance.

[flang][driver] Make `flang-new` always generate run-time type info
ClosedPublic

Authored by awarzynski on Feb 17 2022, 6:17 AM.

Details

Summary

Currently, the driver generates the tables with "run-time type
information for derived types" only when specific actions are run.
However, the corresponding data might be required by the subsequent
compilation stages (e.g. lowering, code-gen) and should be generated
unconditionally. Note that this is only possible once the semantic
checks have been run.

Note that when generating these tables, extra semantic errors might be
generated. The driver will always report these and in most cases such
semantic errors will cause the driver to exit immediately. The only
exception are actions inheriting from PrescanAndSemaDebugAction.
Currently, there's only one such action: DebugDumpAllAction
(corresponds to -fdebug-dump-all command-line flag). I've updated the
comments for this action to clarify this.

This change will mostly affect lowering, which currently is only
available for most basic examples (e.g. empty programs). I wasn't able
to find a working case that would demonstrate the new behaviour. I
hope that this change is straightforward enough and am submitting it
without a test.

Diff Detail

Event Timeline

awarzynski created this revision.Feb 17 2022, 6:17 AM
Herald added a project: Restricted Project. · View Herald Transcript
awarzynski requested review of this revision.Feb 17 2022, 6:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2022, 6:17 AM

For a bit more context, after https://reviews.llvm.org/D119555 landed, SNAP no longer builds with flang-new (see the instructions here). This patch fixes that, but we can only be verified with fir-dev. I wasn't sure how to go about fixing this, but IIUC, it's preferred to create initial patches here and then cherry-pick them onto fir-dev. Also, this change is relatively straightforward (I hope!). Let me know what you think!

jeanPerier added inline comments.
flang/include/flang/Frontend/FrontendActions.h
138

semantinc -> semantic

flang/lib/Frontend/FrontendAction.cpp
191

@klausler , do can you confirm the runtime type info generation will not generate bogus semantics errors ?

194

I wonder if you should not always complain here if !ci.getRtTyTables().schemata. Currently, it seems DebugDumpSymbolsAction and DebugDumpSymbolsAction would complain. I think at least the regular
compiler workflows should complain if it was not able to generate those. Better than to wait for more cryptic link time errors.

awarzynski added inline comments.Feb 17 2022, 7:09 AM
flang/lib/Frontend/FrontendAction.cpp
194

Good point - this would also mean better code re-use. Let me update!

Move the schemata check to aGenerateRtTypeTables`

Thanks for taking a look @jeanPerier!

klausler added inline comments.Feb 17 2022, 9:00 AM
flang/lib/Frontend/FrontendAction.cpp
191

Runtime derived type description generation can emit a fatal error in the case that a specification expression for a component bound or type parameter does not resolve to a constant or LEN type parameter.

jeanPerier added inline comments.Feb 18 2022, 3:05 AM
flang/lib/Frontend/FrontendAction.cpp
203

Mmh. I gave some bad advice... It looks like the builds are failing because this error is reported when compiling the builtin modules before __fortran_type_info.f90 was compiled to create the module (this includes the bootstrapping case of __fortran_type_info.f90). I am not sure how solide it would be to determine if this is a builtin module compilation and that this message should not be emitted.

awarzynski added inline comments.Feb 22 2022, 7:29 AM
flang/lib/Frontend/FrontendAction.cpp
203

I don't have a good solution. We could either:

  1. disable this diagnostic when __fortran_builtins is being compiled (that's the only dependency from __fortran_type_info.f90 that needs to be processed before __fortran_type_info.f90), or
  2. implement a dedicated compiler flag to disable this particular diagnostic and use it in CMake for when __fortran_builtins is being compiled.

I'm not sure how to achieve 1. in a reliable way (i.e. how to identify that it's indeed the builtin module is being processed?). As for 2., I really dislike the idea of adding a dedicated compiler flag to work around a very specific corner case. Any other suggestions?

I think that moving this bit back to DebugDumpSymbolsAction::ExecuteAction and DebugDumpAllAction::ExecuteAction would be a good compromise for now. We know that these run-time tables should be generated, but we cannot reliably check whether schemata was indeed set. Not yet.

Move back the if (!ci.getRtTyTables().schemata) checks. As discussed inline, this condition is false when compiling __fortran_builtins.f90 and there's no mechanism (yet) to apply this selectively.

PeteSteinfeld accepted this revision.Feb 22 2022, 9:28 AM

All builds and tests correctly and looks good.

This revision is now accepted and ready to land.Feb 22 2022, 9:28 AM