This is an archive of the discontinued LLVM Phabricator instance.

[flang] Prevent rewrite of shape inquiries to non constant explicit expr
AbandonedPublic

Authored by jeanPerier on Jul 6 2021, 10:12 AM.

Details

Summary

Shape analysis was building expressions using non constant explicit bounds.
This caused issues because shape analysis is actively used in front-end
expression rewrites, regardless of the context. When rewriting expressions
inside execution parts it is not guaranteed that non constant specification
expressions will evaluate to the same value they would inside the specification
parts.

Example:

fortran
subroutine foo(n)
  real :: x(n)
  n = n + 100
  print *, size(x, 1)
end subroutine

Was unparsed with f18 -funparse to the following program that is not
semantically equivalent:

fortran
SUBROUTINE foo (n)
 REAL x(n)
  n=n+100_4
 PRINT *, int(int(n,kind=8)-1_8+1_8,kind=4)
END SUBROUTINE

Currently, this impacted at least all rewrites of LBOUND, UBOUND, SIZE, SHAPE,
and SIZEOF.

This patch prevents usage of non constant explicit bounds in shape analysis
results, except when requested by the shape analysis user (in contexts where it
is known to be safe to use these). Otherwise, evaluate::DescriptorInquiry are
generated. The only place where keeping non constant explicit bounds in shape
analysis appeared to be needed so far was runtime type info generation (for
derived types).

I considered fixing it at the folding level (by discarding non constant results from
shape analysis), but it seemed less solid (more places to secure, hard to enforce all
usages of shape analysis do not end-up used somewhere else now or in the future). Also,
Being able to rewrite size(x+y) where y is an array to size(y) is safe and still an
improvement, and naively discarding non constant shape analysis results in folding was
preventing that from happening.

Diff Detail

Event Timeline

jeanPerier created this revision.Jul 6 2021, 10:12 AM
jeanPerier requested review of this revision.Jul 6 2021, 10:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2021, 10:12 AM
PeteSteinfeld accepted this revision.Jul 6 2021, 2:10 PM

All builds, tests, and looks good.

flang/include/flang/Evaluate/shape.h
69

Typo, should be "contexts" rather than "context".

This revision is now accepted and ready to land.Jul 6 2021, 2:10 PM

Fix typo in comments.

jeanPerier marked an inline comment as done.Jul 7 2021, 12:46 AM

Are there any clients of these APIs that should have the new flag arguments set to "true"?

Are there any clients of these APIs that should have the new flag arguments set to "true"?

Yes, the derived type info generation needs to be able to access length type parameters symbols in the returned expressions. runtime-type-info.cpp changes are setting the useNonConstantExplicitExpr to true.

klausler accepted this revision.Jul 7 2021, 1:18 PM

It may be the case that shape analysis could always create expressions containing descriptor inquiry operations for non-constant bounds (&c.), and the decision whether to rewrite those inquiries (or not) be moved into folding and controlled by the FoldingContext instead. Shape analysis, per se, shouldn't be the part of the code that cares about specification vs. executable parts.

jeanPerier abandoned this revision.Nov 2 2021, 8:09 AM

A better approach is being worked on by @klausler.