This is an archive of the discontinued LLVM Phabricator instance.

[flang] Handle common block with different sizes in same file
ClosedPublic

Authored by jeanPerier on Apr 28 2022, 8:47 AM.

Details

Summary

Semantics is not preventing a named common block to appear with
different size in a same file (named common block should always have
the same storage size (see Fortran 2018 8.10.2.5), but it is a common
extension to accept different sizes).

Lowering was not coping with this well, since it just use the first
common block appearance, starting with BLOCK DATAs to define common
blocks (this also was an issue with the blank common block, which can
legally appear with different size in different scoping units).

Semantics is also not preventing named common from being initialized
outside of a BLOCK DATA, and lowering was dealing badly with this,
since it only gave an initial value to common blocks Globals if the
first common block appearance, starting with BLOCK DATAs had an initial
value.

Semantics is also allowing blank common to be initialized, while
lowering was assuming this would never happen, and was never creating
an initial value for it.

Lastly, semantics was not complaining if a COMMON block was initialized
in several scoping unit in a same file, while lowering can only generate
one of these initial value.

To fix this, add a structure to keep track of COMMON block properties
(biggest size, and initial value if any) at the Program level. Once the
size of a common block appearance is know, the common block appearance
is checked against this information. It allows semantics to emit an error
in case of multiple initialization in different scopes of a same common
block, and to warn in case named common blocks appears with different
sizes. Lastly, this allows lowering to use the Program level info about
common blocks to emit the right GlobalOp for a Common Block, regardless
of the COMMON Block appearances order: It emits a GlobalOp with the
biggest size, whose lowest bytes are initialized with the initial value
if any is given in a scope where the common block appears.

Lowering is updated to go emit the common blocks before anything else so
that the related GlobalOps are available when lowering the scopes where
common block appear. It is also updated to not assume that blank common
are never initialized.

Diff Detail

Event Timeline

jeanPerier created this revision.Apr 28 2022, 8:47 AM
Herald added a project: Restricted Project. · View Herald Transcript
jeanPerier requested review of this revision.Apr 28 2022, 8:47 AM
klausler accepted this revision.Apr 28 2022, 9:15 AM

Looks great. Could you add comments to docs/Extensions.md to describe that (1) initializations of common outside BLOCK DATA and (2) initializations of blank COMMON are both non-standard, but are near-universal extensions that we have to support? Thanks.

flang/include/flang/Lower/PFTBuilder.h
33

It seems a little risky to define this typedef in two distinct header files; what if one of them changes?

flang/include/flang/Semantics/semantics.h
208

"depend on" or "depend upon"

flang/test/Lower/common-block-2.f90
20

"appearance"

This revision is now accepted and ready to land.Apr 28 2022, 9:15 AM
jeanPerier marked 3 inline comments as done.Apr 29 2022, 1:10 AM

Could you add comments to docs/Extensions.md to describe that (1) initializations of common outside BLOCK DATA and (2) initializations of blank COMMON are both non-standard, but are near-universal extensions that we have to support?

Thanks for the reminder. (2) was already describe there, I added (1) right next to it.

flang/include/flang/Lower/PFTBuilder.h
33

In this specific case, both definitions are visible in PFTBuilder.cpp file, so if there is a conflict, the C++ compiler (at least clang) would raise "error: type alias redefinition with different types".

But I agree it is more maintenance to maintain two definition, I included semantics.h. It is not too template heavy, so there is little point avoiding to include it here.

flang/include/flang/Semantics/semantics.h
208

Thanks

jeanPerier marked 2 inline comments as done.
  • Add note in docs/Extensions.md.
  • Inlcude semantics.h in PFTBuilder.h instead of redefining the typdef
  • Fix typos