[flang-commits] [flang] 543cd89 - [flang] Fix problems with constant arrays with lower bounds that are not 1
Peter Steinfeld via flang-commits
flang-commits at lists.llvm.org
Fri Jan 29 08:15:27 PST 2021
Author: Peter Steinfeld
Date: 2021-01-29T08:05:10-08:00
New Revision: 543cd89d3fb5a108d4050635c00093695b2b6c6d
URL: https://github.com/llvm/llvm-project/commit/543cd89d3fb5a108d4050635c00093695b2b6c6d
DIFF: https://github.com/llvm/llvm-project/commit/543cd89d3fb5a108d4050635c00093695b2b6c6d.diff
LOG: [flang] Fix problems with constant arrays with lower bounds that are not 1
There were two problems with constant arrays whose lower bound is not 1.
First, when folding the arrays, we were creating the folded array to have lower
bounds of 1 but, we were not re-adjusting their lower bounds to the
declared values. Second, we were not calculating the extents correctly.
Both of these problems led to bogus error messages.
I fixed the first problem by adjusting the lower bounds in
NonPointerInitializationExpr() in Evaluate/check-expression.cpp. I wrote the
class ArrayConstantBoundChanger, which is similar to the existing class
ScalarConstantExpander. In the process of implementing and testing it, I found
a bug that I fixed in ScalarConstantExpander which caused it to infinitely
recurse on parenthesized expressions. I also removed the unrelated class
ScalarExpansionVisitor, which was not used.
I fixed the second problem by changing the formula that calculates upper bounds
in in the function ComputeUpperBound() in Evaluate/shape.cpp.
I added tests that trigger the bogus error messages mentioned above along with
a constant folding tests that uses array operands with shapes that conform but
have different bounds.
In the process of adding tests, I discovered that tests in
Evaluate/folding09.f90 and folding16.f90 were written incorrectly, and I
fixed them. This also revealed a bug in contant folding of the
intrinsic "lbounds" which I plan to fix in a later change.
Differential Revision: https://reviews.llvm.org/D95449
Added:
Modified:
flang/include/flang/Evaluate/tools.h
flang/lib/Evaluate/check-expression.cpp
flang/lib/Evaluate/shape.cpp
flang/test/Evaluate/folding09.f90
flang/test/Evaluate/folding16.f90
flang/test/Evaluate/test_folding.sh
flang/test/Semantics/array-constr-values.f90
Removed:
################################################################################
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 351dc8715cdd..3210ab50cdfe 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -895,8 +895,8 @@ class ScalarConstantExpander {
}
return expanded;
}
- template <typename T> Constant<T> Expand(Parentheses<T> &&x) {
- return Expand(std::move(x)); // Constant<> can be parenthesized
+ template <typename T> Expr<T> Expand(Parentheses<T> &&x) {
+ return Expand(std::move(x.left())); // Constant<> can be parenthesized
}
template <typename T> Expr<T> Expand(Expr<T> &&x) {
return std::visit([&](auto &&x) { return Expr<T>{Expand(std::move(x))}; },
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 4bedbe8d1d8f..2e061f0fe3fe 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -305,26 +305,30 @@ bool IsInitialProcedureTarget(const Expr<SomeType> &expr) {
}
}
-class ScalarExpansionVisitor : public AnyTraverse<ScalarExpansionVisitor,
- std::optional<Expr<SomeType>>> {
+class ArrayConstantBoundChanger {
public:
- using Result = std::optional<Expr<SomeType>>;
- using Base = AnyTraverse<ScalarExpansionVisitor, Result>;
- ScalarExpansionVisitor(
- ConstantSubscripts &&shape, std::optional<ConstantSubscripts> &&lb)
- : Base{*this}, shape_{std::move(shape)}, lbounds_{std::move(lb)} {}
- using Base::operator();
- template <typename T> Result operator()(const Constant<T> &x) {
- auto expanded{x.Reshape(std::move(shape_))};
- if (lbounds_) {
- expanded.set_lbounds(std::move(*lbounds_));
- }
- return AsGenericExpr(std::move(expanded));
+ ArrayConstantBoundChanger(ConstantSubscripts &&lbounds)
+ : lbounds_{std::move(lbounds)} {}
+
+ template <typename A> A ChangeLbounds(A &&x) const {
+ return std::move(x); // default case
+ }
+ template <typename T> Constant<T> ChangeLbounds(Constant<T> &&x) {
+ x.set_lbounds(std::move(lbounds_));
+ return std::move(x);
+ }
+ template <typename T> Expr<T> ChangeLbounds(Parentheses<T> &&x) {
+ return ChangeLbounds(
+ std::move(x.left())); // Constant<> can be parenthesized
+ }
+ template <typename T> Expr<T> ChangeLbounds(Expr<T> &&x) {
+ return std::visit(
+ [&](auto &&x) { return Expr<T>{ChangeLbounds(std::move(x))}; },
+ std::move(x.u)); // recurse until we hit a constant
}
private:
- ConstantSubscripts shape_;
- std::optional<ConstantSubscripts> lbounds_;
+ ConstantSubscripts &&lbounds_;
};
// Converts, folds, and then checks type, rank, and shape of an
@@ -351,7 +355,11 @@ std::optional<Expr<SomeType>> NonPointerInitializationExpr(const Symbol &symbol,
symbol.name(), symRank, folded.Rank());
}
} else if (auto extents{AsConstantExtents(context, symTS->shape())}) {
- if (folded.Rank() == 0 && symRank > 0) {
+ if (folded.Rank() == 0 && symRank == 0) {
+ // symbol and constant are both scalars
+ return {std::move(folded)};
+ } else if (folded.Rank() == 0 && symRank > 0) {
+ // expand the scalar constant to an array
return ScalarConstantExpander{std::move(*extents),
AsConstantExtents(
context, GetLowerBounds(context, NamedEntity{symbol}))}
@@ -360,7 +368,11 @@ std::optional<Expr<SomeType>> NonPointerInitializationExpr(const Symbol &symbol,
if (CheckConformance(context.messages(), symTS->shape(),
*resultShape, "initialized object",
"initialization expression", false, false)) {
- return {std::move(folded)};
+ // make a constant array with adjusted lower bounds
+ return ArrayConstantBoundChanger{
+ std::move(*AsConstantExtents(
+ context, GetLowerBounds(context, NamedEntity{symbol})))}
+ .ChangeLbounds(std::move(folded));
}
}
} else if (IsNamedConstant(symbol)) {
diff --git a/flang/lib/Evaluate/shape.cpp b/flang/lib/Evaluate/shape.cpp
index eb94139635ef..6dc2edd2355b 100644
--- a/flang/lib/Evaluate/shape.cpp
+++ b/flang/lib/Evaluate/shape.cpp
@@ -373,7 +373,7 @@ MaybeExtentExpr GetExtent(FoldingContext &context, const Subscript &subscript,
MaybeExtentExpr ComputeUpperBound(
ExtentExpr &&lower, MaybeExtentExpr &&extent) {
if (extent) {
- return std::move(*extent) - std::move(lower) + ExtentExpr{1};
+ return std::move(*extent) + std::move(lower) - ExtentExpr{1};
} else {
return std::nullopt;
}
diff --git a/flang/test/Evaluate/folding09.f90 b/flang/test/Evaluate/folding09.f90
index 6efd3c0b6669..ed60f08d001e 100644
--- a/flang/test/Evaluate/folding09.f90
+++ b/flang/test/Evaluate/folding09.f90
@@ -13,16 +13,16 @@ subroutine test(arr1, arr2, arr3, mat)
real, intent(in) :: arr1(:), arr2(10), mat(10, 10)
real, intent(in), contiguous :: arr3(:)
real :: scalar
- logical, parameter :: isc01 = is_contiguous(0)
- logical, parameter :: isc02 = is_contiguous(scalar)
- logical, parameter :: isc03 = is_contiguous(scalar + scalar)
- logical, parameter :: isc04 = is_contiguous([0, 1, 2])
- logical, parameter :: isc05 = is_contiguous(arr1 + 1.0)
- logical, parameter :: isc06 = is_contiguous(arr2)
- logical, parameter :: isc07 = is_contiguous(mat)
- logical, parameter :: isc08 = is_contiguous(mat(1:10,1))
- logical, parameter :: isc09 = is_contiguous(arr2(1:10:1))
- logical, parameter :: isc10 = is_contiguous(arr3)
- logical, parameter :: isc11 = is_contiguous(f())
+ logical, parameter :: test_isc01 = is_contiguous(0)
+ logical, parameter :: test_isc02 = is_contiguous(scalar)
+ logical, parameter :: test_isc03 = is_contiguous(scalar + scalar)
+ logical, parameter :: test_isc04 = is_contiguous([0, 1, 2])
+ logical, parameter :: test_isc05 = is_contiguous(arr1 + 1.0)
+ logical, parameter :: test_isc06 = is_contiguous(arr2)
+ logical, parameter :: test_isc07 = is_contiguous(mat)
+ logical, parameter :: test_isc08 = is_contiguous(mat(1:10,1))
+ logical, parameter :: test_isc09 = is_contiguous(arr2(1:10:1))
+ logical, parameter :: test_isc10 = is_contiguous(arr3)
+ logical, parameter :: test_isc11 = is_contiguous(f())
end subroutine
end module
diff --git a/flang/test/Evaluate/folding16.f90 b/flang/test/Evaluate/folding16.f90
index 0918381040bf..a21f5a762235 100644
--- a/flang/test/Evaluate/folding16.f90
+++ b/flang/test/Evaluate/folding16.f90
@@ -1,8 +1,17 @@
! RUN: %S/test_folding.sh %s %t %f18
! Ensure that lower bounds are accounted for in intrinsic folding;
! this is a regression test for a bug in which they were not
-real, parameter :: a(-1:-1) = 1.
-real, parameter :: b(-1:-1) = log(a)
-logical, parameter :: test = lbound(a,1)==-1 .and. lbound(b,1)==-1 .and. &
- lbound(log(a),1)==1 .and. all(b==0)
+module m
+ real, parameter :: a(-1:-1) = 1.
+ real, parameter :: b(-1:-1) = log(a)
+ integer, parameter :: c(-1:1) = [33, 22, 11]
+ integer, parameter :: d(1:3) = [33, 22, 11]
+ integer, parameter :: e(-2:0) = ([33, 22, 11])
+ ! The following test is commented out because constant folding for "lbound"
+ ! is currently broken
+ !logical, parameter :: test_1 = lbound(a,1)==-1 .and. lbound(b,1)==-1 .and. &
+ ! lbound(log(a),1)==1 .and. all(b==0)
+ logical, parameter :: test_2 = all(c .eq. d)
+ logical, parameter :: test_3 = all(c .eq. e)
+ logical, parameter :: test_4 = all(d .eq. e)
end
diff --git a/flang/test/Evaluate/test_folding.sh b/flang/test/Evaluate/test_folding.sh
index 20f7d1672246..81ecbea55274 100755
--- a/flang/test/Evaluate/test_folding.sh
+++ b/flang/test/Evaluate/test_folding.sh
@@ -5,8 +5,8 @@
# To check folding of an expression EXPR, the fortran program passed to this script
# must contain the following:
# logical, parameter :: test_x = <compare EXPR to expected value>
-# This script will test that all parameter with a name starting with "test_" have
-# been folded to .true.
+# This script will test that all parameter with a name starting with "test_"
+# have been folded to .true.
# For instance, acos folding can be tested with:
#
# real(4), parameter :: res_acos = acos(0.5_4)
diff --git a/flang/test/Semantics/array-constr-values.f90 b/flang/test/Semantics/array-constr-values.f90
index b0b21fbf7f07..ddab8a607b62 100644
--- a/flang/test/Semantics/array-constr-values.f90
+++ b/flang/test/Semantics/array-constr-values.f90
@@ -54,12 +54,28 @@ end subroutine arrayconstructorvalues
subroutine checkC7115()
real, dimension(10), parameter :: good1 = [(99.9, i = 1, 10)]
real, dimension(100), parameter :: good2 = [((88.8, i = 1, 10), j = 1, 10)]
+ real, dimension(-1:0), parameter :: good3 = [77.7, 66.6]
!ERROR: Implied DO index is active in surrounding implied DO loop and may not have the same name
real, dimension(100), parameter :: bad = [((88.8, i = 1, 10), i = 1, 10)]
!ERROR: Value of named constant 'bad2' ([INTEGER(4)::(int(j,kind=4),INTEGER(8)::j=1_8,1_8,0_8)]) cannot be computed as a constant value
!ERROR: The stride of an implied DO loop must not be zero
integer, parameter :: bad2(*) = [(j, j=1,1,0)]
+ integer, parameter, dimension(-1:0) :: negLower = (/343,512/)
+ integer, parameter, dimension(-1:0) :: negLower1 = ((/343,512/))
+
+ real :: local
+
+ local = good3(0)
+ !ERROR: Subscript value (2) is out of range on dimension 1 in reference to a constant array value
+ local = good3(2)
+ call inner(negLower(:)) ! OK
+ call inner(negLower1(:)) ! OK
+
+ contains
+ subroutine inner(arg)
+ integer :: arg(:)
+ end subroutine inner
end subroutine checkC7115
subroutine checkOkDuplicates
real :: realArray(21) = &
More information about the flang-commits
mailing list