Skip to content

Commit ca47447

Browse files
klauslerklausler
committed
[flang] Don't reference non-invariant symbols in shape expressions
When an array's shape involves references to symbols that are not invariant in a scope -- the classic example being a dummy array with an explicit shape involving other dummy arguments -- the compiler was creating shape expressions that referenced those symbols. This might be valid if those symbols are somehow captured and copied at each entry point to a subprogram, and the copies referenced in the shapes instead, but that's not the case. This patch introduces a new expression predicate IsScopeInvariantExpr(), which defines a class of expressions that contains constant expressions (in the sense that the standard uses that term) as well as references to items that may be safely accessed in a context-free way throughout their scopes. This includes dummy arguments that are INTENT(IN) and not VALUE, descriptor inquiries into descriptors that cannot change, and bare LEN type parameters within the definitions of derived types. The new predicate is then used in shape analysis to winnow out results that would have otherwise been contextual. Differential Revision: https://reviews.llvm.org/D113309
1 parent fae4409 commit ca47447

File tree

5 files changed

+85
-36
lines changed

5 files changed

+85
-36
lines changed

‎flang/include/flang/Evaluate/check-expression.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ extern template bool IsConstantExpr(const Expr<SomeInteger> &);
3434
extern template bool IsConstantExpr(const Expr<SubscriptInteger> &);
3535
extern template bool IsConstantExpr(const StructureConstructor &);
3636

37+
// Predicate: true when an expression is a constant expression (in the
38+
// strict sense of the Fortran standard) or a dummy argument with
39+
// INTENT(IN) and no VALUE. This is useful for representing explicit
40+
// shapes of other dummy arguments.
41+
template <typename A> bool IsScopeInvariantExpr(const A &);
42+
extern template bool IsScopeInvariantExpr(const Expr<SomeType> &);
43+
extern template bool IsScopeInvariantExpr(const Expr<SomeInteger> &);
44+
extern template bool IsScopeInvariantExpr(const Expr<SubscriptInteger> &);
45+
3746
// Predicate: true when an expression actually is a typed Constant<T>,
3847
// perhaps with parentheses and wrapping around it. False for all typeless
3948
// expressions, including BOZ literals.

‎flang/lib/Evaluate/check-expression.cpp

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,18 @@
1818

1919
namespace Fortran::evaluate {
2020

21-
// Constant expression predicate IsConstantExpr().
21+
// Constant expression predicates IsConstantExpr() & IsScopeInvariantExpr().
2222
// This code determines whether an expression is a "constant expression"
2323
// in the sense of section 10.1.12. This is not the same thing as being
2424
// able to fold it (yet) into a known constant value; specifically,
2525
// the expression may reference derived type kind parameters whose values
2626
// are not yet known.
27-
class IsConstantExprHelper : public AllTraverse<IsConstantExprHelper, true> {
27+
//
28+
// The variant form (IsScopeInvariantExpr()) also accepts symbols that are
29+
// INTENT(IN) dummy arguments without the VALUE attribute.
30+
template <bool INVARIANT>
31+
class IsConstantExprHelper
32+
: public AllTraverse<IsConstantExprHelper<INVARIANT>, true> {
2833
public:
2934
using Base = AllTraverse<IsConstantExprHelper, true>;
3035
IsConstantExprHelper() : Base{*this} {}
@@ -36,12 +41,15 @@ class IsConstantExprHelper : public AllTraverse<IsConstantExprHelper, true> {
3641
}
3742

3843
bool operator()(const TypeParamInquiry &inq) const {
39-
return semantics::IsKindTypeParameter(inq.parameter());
44+
return INVARIANT || semantics::IsKindTypeParameter(inq.parameter());
4045
}
4146
bool operator()(const semantics::Symbol &symbol) const {
4247
const auto &ultimate{GetAssociationRoot(symbol)};
4348
return IsNamedConstant(ultimate) || IsImpliedDoIndex(ultimate) ||
44-
IsInitialProcedureTarget(ultimate);
49+
IsInitialProcedureTarget(ultimate) ||
50+
ultimate.has<semantics::TypeParamDetails>() ||
51+
(INVARIANT && IsIntentIn(symbol) &&
52+
!symbol.attrs().test(semantics::Attr::VALUE));
4553
}
4654
bool operator()(const CoarrayRef &) const { return false; }
4755
bool operator()(const semantics::ParamValue &param) const {
@@ -72,15 +80,21 @@ class IsConstantExprHelper : public AllTraverse<IsConstantExprHelper, true> {
7280
}
7381

7482
bool operator()(const Constant<SomeDerived> &) const { return true; }
75-
bool operator()(const DescriptorInquiry &) const { return false; }
83+
bool operator()(const DescriptorInquiry &x) const {
84+
const Symbol &sym{x.base().GetLastSymbol()};
85+
return INVARIANT && !IsAllocatable(sym) &&
86+
(!IsDummy(sym) ||
87+
(IsIntentIn(sym) && !sym.attrs().test(semantics::Attr::VALUE)));
88+
}
7689

7790
private:
7891
bool IsConstantStructureConstructorComponent(
7992
const Symbol &, const Expr<SomeType> &) const;
8093
bool IsConstantExprShape(const Shape &) const;
8194
};
8295

83-
bool IsConstantExprHelper::IsConstantStructureConstructorComponent(
96+
template <bool INVARIANT>
97+
bool IsConstantExprHelper<INVARIANT>::IsConstantStructureConstructorComponent(
8498
const Symbol &component, const Expr<SomeType> &expr) const {
8599
if (IsAllocatable(component)) {
86100
return IsNullPointer(expr);
@@ -92,7 +106,9 @@ bool IsConstantExprHelper::IsConstantStructureConstructorComponent(
92106
}
93107
}
94108

95-
bool IsConstantExprHelper::operator()(const ProcedureRef &call) const {
109+
template <bool INVARIANT>
110+
bool IsConstantExprHelper<INVARIANT>::operator()(
111+
const ProcedureRef &call) const {
96112
// LBOUND, UBOUND, and SIZE with DIM= arguments will have been rewritten
97113
// into DescriptorInquiry operations.
98114
if (const auto *intrinsic{std::get_if<SpecificIntrinsic>(&call.proc().u)}) {
@@ -122,7 +138,9 @@ bool IsConstantExprHelper::operator()(const ProcedureRef &call) const {
122138
return false;
123139
}
124140

125-
bool IsConstantExprHelper::IsConstantExprShape(const Shape &shape) const {
141+
template <bool INVARIANT>
142+
bool IsConstantExprHelper<INVARIANT>::IsConstantExprShape(
143+
const Shape &shape) const {
126144
for (const auto &extent : shape) {
127145
if (!(*this)(extent)) {
128146
return false;
@@ -132,13 +150,21 @@ bool IsConstantExprHelper::IsConstantExprShape(const Shape &shape) const {
132150
}
133151

134152
template <typename A> bool IsConstantExpr(const A &x) {
135-
return IsConstantExprHelper{}(x);
153+
return IsConstantExprHelper<false>{}(x);
136154
}
137155
template bool IsConstantExpr(const Expr<SomeType> &);
138156
template bool IsConstantExpr(const Expr<SomeInteger> &);
139157
template bool IsConstantExpr(const Expr<SubscriptInteger> &);
140158
template bool IsConstantExpr(const StructureConstructor &);
141159

160+
// IsScopeInvariantExpr()
161+
template <typename A> bool IsScopeInvariantExpr(const A &x) {
162+
return IsConstantExprHelper<true>{}(x);
163+
}
164+
template bool IsScopeInvariantExpr(const Expr<SomeType> &);
165+
template bool IsScopeInvariantExpr(const Expr<SomeInteger> &);
166+
template bool IsScopeInvariantExpr(const Expr<SubscriptInteger> &);
167+
142168
// IsActuallyConstant()
143169
struct IsActuallyConstantHelper {
144170
template <typename A> bool operator()(const A &) { return false; }

‎flang/lib/Evaluate/shape.cpp

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "flang/Common/idioms.h"
1111
#include "flang/Common/template.h"
1212
#include "flang/Evaluate/characteristics.h"
13+
#include "flang/Evaluate/check-expression.h"
1314
#include "flang/Evaluate/fold.h"
1415
#include "flang/Evaluate/intrinsics.h"
1516
#include "flang/Evaluate/tools.h"
@@ -249,7 +250,8 @@ auto GetLowerBoundHelper::operator()(const Symbol &symbol0) -> Result {
249250
int j{0};
250251
for (const auto &shapeSpec : details->shape()) {
251252
if (j++ == dimension_) {
252-
if (const auto &bound{shapeSpec.lbound().GetExplicit()}) {
253+
const auto &bound{shapeSpec.lbound().GetExplicit()};
254+
if (bound && IsScopeInvariantExpr(*bound)) {
253255
return *bound;
254256
} else if (IsDescriptor(symbol)) {
255257
return ExtentExpr{DescriptorInquiry{NamedEntity{symbol0},
@@ -282,7 +284,8 @@ auto GetLowerBoundHelper::operator()(const Component &component) -> Result {
282284
int j{0};
283285
for (const auto &shapeSpec : details->shape()) {
284286
if (j++ == dimension_) {
285-
if (const auto &bound{shapeSpec.lbound().GetExplicit()}) {
287+
const auto &bound{shapeSpec.lbound().GetExplicit()};
288+
if (bound && IsScopeInvariantExpr(*bound)) {
286289
return *bound;
287290
} else if (IsDescriptor(symbol)) {
288291
return ExtentExpr{
@@ -340,9 +343,21 @@ static MaybeExtentExpr GetNonNegativeExtent(
340343
} else {
341344
return ExtentExpr{*uval - *lval + 1};
342345
}
346+
} else if (lbound && ubound && IsScopeInvariantExpr(*lbound) &&
347+
IsScopeInvariantExpr(*ubound)) {
348+
// Apply effective IDIM (MAX calculation with 0) so thet the
349+
// result is never negative
350+
if (lval.value_or(0) == 1) {
351+
return ExtentExpr{Extremum<SubscriptInteger>{
352+
Ordering::Greater, ExtentExpr{0}, common::Clone(*ubound)}};
353+
} else {
354+
return ExtentExpr{
355+
Extremum<SubscriptInteger>{Ordering::Greater, ExtentExpr{0},
356+
common::Clone(*ubound) - common::Clone(*lbound) + ExtentExpr{1}}};
357+
}
358+
} else {
359+
return std::nullopt;
343360
}
344-
return common::Clone(ubound.value()) - common::Clone(lbound.value()) +
345-
ExtentExpr{1};
346361
}
347362

348363
MaybeExtentExpr GetExtent(const NamedEntity &base, int dimension) {
@@ -372,21 +387,15 @@ MaybeExtentExpr GetExtent(const NamedEntity &base, int dimension) {
372387
int j{0};
373388
for (const auto &shapeSpec : details->shape()) {
374389
if (j++ == dimension) {
375-
if (const auto &ubound{shapeSpec.ubound().GetExplicit()}) {
376-
if (shapeSpec.ubound().GetExplicit()) {
377-
// 8.5.8.2, paragraph 3. If the upper bound is less than the
378-
// lower bound, the extent is zero.
379-
if (shapeSpec.lbound().GetExplicit()) {
380-
return GetNonNegativeExtent(shapeSpec);
381-
} else {
382-
return ubound.value();
383-
}
384-
}
390+
if (auto extent{GetNonNegativeExtent(shapeSpec)}) {
391+
return extent;
385392
} else if (details->IsAssumedSize() && j == symbol.Rank()) {
386393
return std::nullopt;
387394
} else if (semantics::IsDescriptor(symbol)) {
388395
return ExtentExpr{DescriptorInquiry{NamedEntity{base},
389396
DescriptorInquiry::Field::Extent, dimension}};
397+
} else {
398+
break;
390399
}
391400
}
392401
}
@@ -437,7 +446,11 @@ MaybeExtentExpr GetExtent(FoldingContext &context, const Subscript &subscript,
437446
MaybeExtentExpr ComputeUpperBound(
438447
ExtentExpr &&lower, MaybeExtentExpr &&extent) {
439448
if (extent) {
440-
return std::move(*extent) + std::move(lower) - ExtentExpr{1};
449+
if (ToInt64(lower).value_or(0) == 1) {
450+
return std::move(*extent);
451+
} else {
452+
return std::move(*extent) + std::move(lower) - ExtentExpr{1};
453+
}
441454
} else {
442455
return std::nullopt;
443456
}
@@ -454,7 +467,8 @@ MaybeExtentExpr GetUpperBound(const NamedEntity &base, int dimension) {
454467
int j{0};
455468
for (const auto &shapeSpec : details->shape()) {
456469
if (j++ == dimension) {
457-
if (const auto &bound{shapeSpec.ubound().GetExplicit()}) {
470+
const auto &bound{shapeSpec.ubound().GetExplicit()};
471+
if (bound && IsScopeInvariantExpr(*bound)) {
458472
return *bound;
459473
} else if (details->IsAssumedSize() && dimension + 1 == symbol.Rank()) {
460474
break;
@@ -487,10 +501,10 @@ Shape GetUpperBounds(const NamedEntity &base) {
487501
Shape result;
488502
int dim{0};
489503
for (const auto &shapeSpec : details->shape()) {
490-
if (const auto &bound{shapeSpec.ubound().GetExplicit()}) {
504+
const auto &bound{shapeSpec.ubound().GetExplicit()};
505+
if (bound && IsScopeInvariantExpr(*bound)) {
491506
result.push_back(*bound);
492-
} else if (details->IsAssumedSize()) {
493-
CHECK(dim + 1 == base.Rank());
507+
} else if (details->IsAssumedSize() && dim + 1 == base.Rank()) {
494508
result.emplace_back(std::nullopt); // UBOUND folding replaces with -1
495509
} else {
496510
result.emplace_back(

‎flang/test/Semantics/modfile33.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ subroutine s1(n, x, y, z, a, b)
572572
! real(4) :: x
573573
! real(4) :: y(1_8:4_8, 1_8:n)
574574
! real(4) :: z(1_8:2_8, 1_8:2_8, 1_8:2_8)
575-
! real(4) :: a(1_8:int(int(4_8*(n-1_8+1_8),kind=4),kind=8))
575+
! real(4) :: a(1_8:int(int(4_8*size(y,dim=2),kind=4),kind=8))
576576
! real(4) :: b(1_8:add(y, z))
577577
! end
578578
!end

‎flang/test/Semantics/offsets01.f90

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ subroutine s4
3838
! Descriptors with length parameters
3939
subroutine s5(n)
4040
integer :: n
41-
type :: t1(l)
42-
integer, len :: l
43-
real :: a(l)
41+
type :: t1(n)
42+
integer, len :: n
43+
real :: a(n)
4444
end type
45-
type :: t2(l1, l2)
46-
integer, len :: l1
47-
integer, len :: l2
48-
real :: b(l1, l2)
45+
type :: t2(n1, n2)
46+
integer, len :: n1
47+
integer, len :: n2
48+
real :: b(n1, n2)
4949
end type
5050
type(t1(n)) :: x1 !CHECK: x1 size=40 offset=
5151
type(t2(n,n)) :: x2 !CHECK: x2 size=48 offset=

0 commit comments

Comments
 (0)