[PATCH 1/4] fortran: Pre-evaluate string pointers. [PR102043]
This avoids a regression on deferred_character_23.f90 later in the
patch series when array references are rewritten to use pointer
arithmetic.
The problem is a SAVE_EXPR tree as TYPE_SIZE_UNIT of one array element
type, which is used by the pointer arithmetic expressions. As these
expressions appear in both branches of an if-then-else block, the tree
is lowered to a variable in one of the branches but it’s used in both
branches, which is invalid middle-end code.
This change pre-evaluates the array references or pointer arithmetics
to variables before the if-then-else block, so that the SAVE_EXPR are
expanded to variables in the parent scope of the if-then-else block,
and expressions referencing the variables remain valid in both
branches.
PR fortran/102043
gcc/fortran/ChangeLog:
* trans-expr.cc: Pre-evaluate src and dest to variables
before using them.
gcc/testsuite/ChangeLog:
* gfortran.dg/dependency_49.f90: Update variable occurence
count.
---
gcc/fortran/trans-expr.cc | 7 +++
gcc/testsuite/gfortran.dg/dependency_49.f90 | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 06713f24f95..3962b6848ce 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -8093,6 +8093,13 @@ gfc_trans_string_copy (stmtblock_t * block, tree
dlength, tree dest,
cond2 = fold_build2_loc (input_location, LT_EXPR, logical_type_node, slen,
dlen);
+ /* Pre-evaluate pointers unless one of the IF arms will be optimized away.
*/
+ if (!CONSTANT_CLASS_P (cond2))
+{
+ dest = gfc_evaluate_now (dest, block);
+ src = gfc_evaluate_now (src, block);
+}
+
/* Copy and pad with spaces. */
tmp3 = build_call_expr_loc (input_location,
builtin_decl_explicit (BUILT_IN_MEMMOVE),
diff --git a/gcc/testsuite/gfortran.dg/dependency_49.f90
b/gcc/testsuite/gfortran.dg/dependency_49.f90
index 73d517e8f76..9638f65c069 100644
--- a/gcc/testsuite/gfortran.dg/dependency_49.f90
+++ b/gcc/testsuite/gfortran.dg/dependency_49.f90
@@ -11,4 +11,5 @@ program main
a%x = a%x(2:3)
print *,a%x
end program main
-! { dg-final { scan-tree-dump-times "__var_1" 4 "original" } }
+! The temporary var appears three times: declaration, copy-in and copy-out
+! { dg-final { scan-tree-dump-times "__var_1" 3 "original" } }
--
2.35.1
[PATCH 3/4] fortran: Generate an array temporary reference [PR102043]
This avoids regressing on char_cast_1.f90 and char_cast_2.f90 later in
the patch series when the code generation for array references is
changed to use pointer arithmetic.
The regressing testcases match part of an array reference in the
generated tree dump and it’s not clear how the pattern should be
rewritten to match the equivalent with pointer arithmetic.
This change uses a method specific to array temporaries to generate
array-references, so that these array references are flagged as safe
for array indexing and will not be updated to use pointer arithmetic.
PR fortran/102043
gcc/fortran/ChangeLog:
* trans-array.cc (gfc_conv_expr_descriptor): Use
gfc_conv_tmp_array_ref.
---
gcc/fortran/trans-array.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index b3f8871ff22..11e47c0c1ca 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -7723,7 +7723,7 @@ gfc_conv_expr_descriptor (gfc_se *se, gfc_expr *expr)
lse.ss = loop.temp_ss;
rse.ss = ss;
- gfc_conv_scalarized_array_ref (&lse, NULL);
+ gfc_conv_tmp_array_ref (&lse);
if (expr->ts.type == BT_CHARACTER)
{
gfc_conv_expr (&rse, expr);
--
2.35.1
[PATCH 0/4] Use pointer arithmetic for array references [PR102043]
Hello, this is a fix for PR102043, which is a wrong code bug caused by the middle-end concluding from array indexing that the array index is non-negative. This is a wrong assumption for "reversed arrays", that is arrays whose descriptor makes accesses to the array from last element to first element. More generally the wrong cases are arrays with a descriptor having a negative stride for at least one dimension. I have been trying to fix this by stopping the front-end from generating bogus code, by either stripping array-ness from descriptor data pointers, or by changing the initialization of data pointers to point to the first element in memory order instead of the first element in access order (which is the last in memory order for reversed arrays). Both ways are very impacting changes to the frontend and I haven’t been able to eliminate all the regressions in time using either way. However, Richi showed with a patch attached to the PR that array references are crucial for the problem to appear, and everything works if array indexing is replaced with pointer arithmetic. This is much simpler and doesn’t imply invasive changes to the frontend. I have built on top of his patch to keep the array indexing in cases where the change to pointer arithmetic is not necessary, either because the array is not a fortran array with a descriptor, or because it’s known to be contiguous. This has the benefit of reducing the churn in the dumped code patterns used in the testsuite. It also avoids ICE regression such as interface_12.f90 or result_in_spec.f90, but I can’t exclude that those could be a real problem made latent. Patches 1 to 3 are preliminary changes to avoid regressions. The main change is number 4, the last in the series. Regression tested on x86_64-pc-linux-gnu. OK for master? Mikael Morin (4): fortran: Pre-evaluate string pointers. [PR102043] fortran: Update index extraction code. [PR102043] fortran: Generate an array temporary reference [PR102043] fortran: Use pointer arithmetic to index arrays [PR102043] gcc/fortran/trans-array.cc| 60 +- gcc/fortran/trans-expr.cc | 9 +- gcc/fortran/trans-io.cc | 48 - gcc/fortran/trans.cc | 42 +++- gcc/fortran/trans.h | 4 +- .../gfortran.dg/array_reference_3.f90 | 195 ++ gcc/testsuite/gfortran.dg/c_loc_test_22.f90 | 4 +- gcc/testsuite/gfortran.dg/dependency_49.f90 | 3 +- gcc/testsuite/gfortran.dg/finalize_10.f90 | 2 +- .../gfortran.dg/negative_stride_1.f90 | 25 +++ .../gfortran.dg/vector_subscript_8.f90| 16 ++ .../gfortran.dg/vector_subscript_9.f90| 21 ++ 12 files changed, 401 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/array_reference_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/negative_stride_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/vector_subscript_8.f90 create mode 100644 gcc/testsuite/gfortran.dg/vector_subscript_9.f90 -- 2.35.1
[PATCH 2/4] fortran: Update index extraction code. [PR102043]
This avoids a regression on hollerith4.f90 and hollerith6.f90 later in
the patch series when code generation for array references is changed
to use pointer arithmetic.
The problem comes from the extraction of the array index from an
ARRAY_REF tree, which doesn’t work if the tree is not an ARRAY_REF
any more.
This updates the code generated for remaining size evaluation to work
with a source tree that uses either array indexing or pointer
arithmetic.
PR fortran/102043
gcc/fortran/ChangeLog:
* trans-io.cc: Add handling for the case where the array
is referenced using pointer arithmetic.
---
gcc/fortran/trans-io.cc | 48 +++--
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/gcc/fortran/trans-io.cc b/gcc/fortran/trans-io.cc
index 732221f848b..9f86815388c 100644
--- a/gcc/fortran/trans-io.cc
+++ b/gcc/fortran/trans-io.cc
@@ -737,7 +737,6 @@ set_parameter_ref (stmtblock_t *block, stmtblock_t
*postblock,
static void
gfc_convert_array_to_string (gfc_se * se, gfc_expr * e)
{
- tree size;
if (e->rank == 0)
{
@@ -755,12 +754,13 @@ gfc_convert_array_to_string (gfc_se * se, gfc_expr * e)
array = sym->backend_decl;
type = TREE_TYPE (array);
+ tree elts_count;
if (GFC_ARRAY_TYPE_P (type))
- size = GFC_TYPE_ARRAY_SIZE (type);
+ elts_count = GFC_TYPE_ARRAY_SIZE (type);
else
{
gcc_assert (GFC_DESCRIPTOR_TYPE_P (type));
- size = gfc_conv_array_stride (array, rank);
+ tree stride = gfc_conv_array_stride (array, rank);
tmp = fold_build2_loc (input_location, MINUS_EXPR,
gfc_array_index_type,
gfc_conv_array_ubound (array, rank),
@@ -768,23 +768,49 @@ gfc_convert_array_to_string (gfc_se * se, gfc_expr * e)
tmp = fold_build2_loc (input_location, PLUS_EXPR,
gfc_array_index_type, tmp,
gfc_index_one_node);
+ elts_count = fold_build2_loc (input_location, MULT_EXPR,
+ gfc_array_index_type, tmp, stride);
+ }
+ gcc_assert (elts_count);
+
+ tree elt_size = TYPE_SIZE_UNIT (gfc_get_element_type (type));
+ elt_size = fold_convert (gfc_array_index_type, elt_size);
+
+ tree size;
+ if (TREE_CODE (se->expr) == ARRAY_REF)
+ {
+ tree index = TREE_OPERAND (se->expr, 1);
+ index = fold_convert (gfc_array_index_type, index);
+
+ elts_count = fold_build2_loc (input_location, MINUS_EXPR,
+ gfc_array_index_type,
+ elts_count, index);
+
size = fold_build2_loc (input_location, MULT_EXPR,
- gfc_array_index_type, tmp, size);
+ gfc_array_index_type, elts_count, elt_size);
+ }
+ else
+ {
+ gcc_assert (TREE_CODE (se->expr) == INDIRECT_REF);
+ tree ptr = TREE_OPERAND (se->expr, 0);
+
+ gcc_assert (TREE_CODE (ptr) == POINTER_PLUS_EXPR);
+ tree offset = fold_convert_loc (input_location, gfc_array_index_type,
+ TREE_OPERAND (ptr, 1));
+
+ size = fold_build2_loc (input_location, MULT_EXPR,
+ gfc_array_index_type, elts_count, elt_size);
+ size = fold_build2_loc (input_location, MINUS_EXPR,
+ gfc_array_index_type, size, offset);
}
gcc_assert (size);
- size = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, size,
- TREE_OPERAND (se->expr, 1));
se->expr = gfc_build_addr_expr (NULL_TREE, se->expr);
- tmp = TYPE_SIZE_UNIT (gfc_get_element_type (type));
- size = fold_build2_loc (input_location, MULT_EXPR,
- gfc_array_index_type, size,
- fold_convert (gfc_array_index_type, tmp));
se->string_length = fold_convert (gfc_charlen_type_node, size);
return;
}
+ tree size;
gfc_conv_array_parameter (se, e, true, NULL, NULL, &size);
se->string_length = fold_convert (gfc_charlen_type_node, size);
}
--
2.35.1
[PATCH 4/4] fortran: Use pointer arithmetic to index arrays [PR102043]
The code generated for array references used to be ARRAY_REF trees as
could be expected. However, the middle-end may conclude from those
trees that the indexes used are non-negative (more precisely not below
the lower bound), which is a wrong assumption in the case of "reversed-
order" arrays.
The problematic arrays are those with a descriptor and having a negative
stride for at least one dimension. The descriptor data points to the
first element in array order (which is not the first in memory order in
that case), and the negative stride(s) makes walking the array backwards
(towards lower memory addresses), and we can access elements with
negative index wrt data pointer.
With this change, pointer arithmetic is generated by default for array
references, unless we are in a case where negative indexes can’t happen
(array descriptor’s dim element, substrings, explicit shape,
allocatable, or assumed shape contiguous). A new flag is added to
choose between array indexing and pointer arithmetic, and it’s set
if the context can tell array indexing is safe (descriptor dim
element, substring, temporary array), or a new method is called
to decide on whether the flag should be set for one given array
expression.
PR fortran/102043
gcc/fortran/ChangeLog:
* trans.h (gfc_build_array_ref): Add non_negative_offset
argument.
* trans.cc (gfc_build_array_ref): Ditto. Use pointer arithmetic
if non_negative_offset is false.
* trans-expr.cc (gfc_conv_substring): Set flag in the call to
gfc_build_array_ref.
* trans-array.cc (gfc_get_cfi_dim_item,
gfc_conv_descriptor_dimension): Same.
(build_array_ref): Decide on whether to set the flag and update
the call.
(gfc_conv_scalarized_array_ref): Same. New argument tmp_array.
(gfc_conv_tmp_array_ref): Update call to
gfc_conv_scalarized_ref.
(non_negative_strides_array_p): New function.
gcc/testsuite/ChangeLog:
* gfortran.dg/array_reference_3.f90: New.
* gfortran.dg/negative_stride_1.f90: New.
* gfortran.dg/vector_subscript_8.f90: New.
* gfortran.dg/vector_subscript_9.f90: New.
* gfortran.dg/c_loc_test_22.f90: Update dump patterns.
* gfortran.dg/finalize_10.f90: Same.
Co-Authored-By: Richard Biener
---
gcc/fortran/trans-array.cc| 58 +-
gcc/fortran/trans-expr.cc | 2 +-
gcc/fortran/trans.cc | 42 +++-
gcc/fortran/trans.h | 4 +-
.../gfortran.dg/array_reference_3.f90 | 195 ++
gcc/testsuite/gfortran.dg/c_loc_test_22.f90 | 4 +-
gcc/testsuite/gfortran.dg/finalize_10.f90 | 2 +-
.../gfortran.dg/negative_stride_1.f90 | 25 +++
.../gfortran.dg/vector_subscript_8.f90| 16 ++
.../gfortran.dg/vector_subscript_9.f90| 21 ++
10 files changed, 354 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/array_reference_3.f90
create mode 100644 gcc/testsuite/gfortran.dg/negative_stride_1.f90
create mode 100644 gcc/testsuite/gfortran.dg/vector_subscript_8.f90
create mode 100644 gcc/testsuite/gfortran.dg/vector_subscript_9.f90
diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 11e47c0c1ca..e4b6270ccf8 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -172,7 +172,7 @@ static tree
gfc_get_cfi_dim_item (tree desc, tree idx, unsigned field_idx)
{
tree tmp = gfc_get_cfi_descriptor_field (desc, CFI_FIELD_DIM);
- tmp = gfc_build_array_ref (tmp, idx, NULL);
+ tmp = gfc_build_array_ref (tmp, idx, NULL_TREE, true);
tree field = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (tmp)), field_idx);
gcc_assert (field != NULL_TREE);
return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field),
@@ -424,7 +424,7 @@ gfc_conv_descriptor_dimension (tree desc, tree dim)
tmp = gfc_get_descriptor_dimension (desc);
- return gfc_build_array_ref (tmp, dim, NULL);
+ return gfc_build_array_ref (tmp, dim, NULL_TREE, true);
}
@@ -3664,10 +3664,51 @@ build_class_array_ref (gfc_se *se, tree base, tree
index)
}
+/* Indicates that the tree EXPR is a reference to an array that can’t
+ have any negative stride. */
+
+static bool
+non_negative_strides_array_p (tree expr)
+{
+ if (expr == NULL_TREE)
+return false;
+
+ tree type = TREE_TYPE (expr);
+ if (POINTER_TYPE_P (type))
+type = TREE_TYPE (type);
+
+ if (TYPE_LANG_SPECIFIC (type))
+{
+ gfc_array_kind array_kind = GFC_TYPE_ARRAY_AKIND (type);
+
+ if (array_kind == GFC_ARRAY_ALLOCATABLE
+ || array_kind == GFC_ARRAY_ASSUMED_SHAPE_CONT)
+ return true;
+}
+
+ /* An array with descriptor can have negative strides.
+ We try to be conservative and return false by default here
+ if we don’t recognize a contiguous array instead of
+ returning false if we can identify a non-contiguous o
