* [Fortran-dev][Patch] Some (ubound-lbound+1) -> extent cleanup
@ 2012-07-15 11:24 Tobias Burnus
2012-07-15 12:08 ` Mikael Morin
0 siblings, 1 reply; 2+ messages in thread
From: Tobias Burnus @ 2012-07-15 11:24 UTC (permalink / raw)
To: gcc patches, gfortran
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
This patch cleans up the source code and generated code ("dump") by
changing (ubound-lbound+1) calculations to directly taking the "extent".
Except for a faster -O0 performance and saving some cycles during code
generation, the code should be the same.
The only real code change I did was to gfc_grow_array. I didn't
understand the previous code, I think the current code makes more sense.
I believe the old code did:
desc->extent = desc.extent + extra;
realloc (&desc->data, (desc.extent + 1)*element_size);
while I think the latter should be "+ extra" and not "+ 1". From the
callee, I couldn't deduce whether extra is always unity in practice, in
any case it doesn't look like.
For fcncall_realloc_result, I didn't check whether the new code
effectively matches the old one, I just followed the comment which
states: "Check that the shapes are the same between lhs and expression.".
There are some more cases, but there the code wasn't as obvious. For
instance "extent = ubound - lbound"; there I was unsure whether that's a
bug (missing "+1") or valid.
Build and regtested with no new failures.
OK for the branch?
Tobias
[-- Attachment #2: extent-cleanup.diff --]
[-- Type: text/x-patch, Size: 10663 bytes --]
2012-07-15 Tobias Burnus <burnus@net-b.de>
* trans-intrinsic.c (gfc_conv_intrinsic_size,
gfc_conv_intrinsic_sizeof): Replace (ubound-lbound+1) calculation
by "extent".
* trans-expr.c (fcncall_realloc_result): Ditto.
* trans-io.c (gfc_convert_array_to_string): Ditto.
* trans-openmp.c (gfc_omp_clause_default_ctor,
gfc_omp_clause_copy_ctor, gfc_omp_clause_assign_op,
gfc_trans_omp_array_reduction): Ditto.
* trans-array.c (array_parameter_size): Ditto.
(gfc_grow_array): Ditto - and fix size calculation for realloc.
2012-07-15 Tobias Burnus <burnus@net-b.de>
* gfortran.dg/array_section_2.f90: Update scan-tree-dump pattern.
Index: gcc/fortran/trans-intrinsic.c
===================================================================
--- gcc/fortran/trans-intrinsic.c (Revision 189492)
+++ gcc/fortran/trans-intrinsic.c (Arbeitskopie)
@@ -5145,17 +5145,8 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * e
if (se->expr == NULL_TREE)
{
- tree ubound, lbound;
-
- arg1 = build_fold_indirect_ref_loc (input_location,
- arg1);
- ubound = gfc_conv_descriptor_ubound_get (arg1, argse.expr);
- lbound = gfc_conv_descriptor_lbound_get (arg1, argse.expr);
- se->expr = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, ubound, lbound);
- se->expr = fold_build2_loc (input_location, PLUS_EXPR,
- gfc_array_index_type,
- se->expr, gfc_index_one_node);
+ arg1 = build_fold_indirect_ref_loc (input_location, arg1);
+ se->expr = gfc_conv_descriptor_extent_get (arg1, argse.expr);
se->expr = fold_build2_loc (input_location, MAX_EXPR,
gfc_array_index_type, se->expr,
gfc_index_zero_node);
@@ -5194,8 +5185,6 @@ gfc_conv_intrinsic_sizeof (gfc_se *se, gfc_expr *e
tree source_bytes;
tree type;
tree tmp;
- tree lower;
- tree upper;
int n;
arg = expr->value.function.actual->expr;
@@ -5240,12 +5229,7 @@ gfc_conv_intrinsic_sizeof (gfc_se *se, gfc_expr *e
{
tree idx;
idx = gfc_rank_cst[n];
- lower = gfc_conv_descriptor_lbound_get (argse.expr, idx);
- upper = gfc_conv_descriptor_ubound_get (argse.expr, idx);
- tmp = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, upper, lower);
- tmp = fold_build2_loc (input_location, PLUS_EXPR,
- gfc_array_index_type, tmp, gfc_index_one_node);
+ tmp = gfc_conv_descriptor_extent_get (argse.expr, idx);
tmp = fold_build2_loc (input_location, MULT_EXPR,
gfc_array_index_type, tmp, source_bytes);
gfc_add_modify (&argse.pre, source_bytes, tmp);
Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c (Revision 189481)
+++ gcc/fortran/trans-expr.c (Arbeitskopie)
@@ -6476,19 +6481,10 @@ fcncall_realloc_result (gfc_se *se, int rank)
for (n = 0 ; n < rank; n++)
{
tree tmp1;
- tmp = gfc_conv_descriptor_lbound_get (desc, gfc_rank_cst[n]);
- tmp1 = gfc_conv_descriptor_lbound_get (res_desc, gfc_rank_cst[n]);
- tmp = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, tmp, tmp1);
- tmp1 = gfc_conv_descriptor_ubound_get (desc, gfc_rank_cst[n]);
- tmp = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, tmp, tmp1);
- tmp1 = gfc_conv_descriptor_ubound_get (res_desc, gfc_rank_cst[n]);
- tmp = fold_build2_loc (input_location, PLUS_EXPR,
- gfc_array_index_type, tmp, tmp1);
+ tmp = gfc_conv_descriptor_extent_get (desc, gfc_rank_cst[n]);
+ tmp1 = gfc_conv_descriptor_extent_get (res_desc, gfc_rank_cst[n]);
tmp = fold_build2_loc (input_location, NE_EXPR,
- boolean_type_node, tmp,
- gfc_index_zero_node);
+ boolean_type_node, tmp, tmp1);
tmp = gfc_evaluate_now (tmp, &se->post);
zero_cond = fold_build2_loc (input_location, TRUTH_OR_EXPR,
boolean_type_node, tmp,
Index: gcc/fortran/trans-io.c
===================================================================
--- gcc/fortran/trans-io.c (Revision 189480)
+++ gcc/fortran/trans-io.c (Arbeitskopie)
@@ -639,16 +639,10 @@ gfc_convert_array_to_string (gfc_se * se, gfc_expr
else
{
gcc_assert (GFC_DESCRIPTOR_TYPE_P (type));
- size = gfc_conv_array_stride (array, rank);
- tmp = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type,
- gfc_conv_array_ubound (array, rank),
- gfc_conv_array_lbound (array, rank));
- tmp = fold_build2_loc (input_location, PLUS_EXPR,
- gfc_array_index_type, tmp,
- gfc_index_one_node);
size = fold_build2_loc (input_location, MULT_EXPR,
- gfc_array_index_type, tmp, size);
+ gfc_array_index_type,
+ gfc_conv_array_stride (array, rank),
+ gfc_conv_array_extent (array, rank));
}
gcc_assert (size);
Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c (Revision 189481)
+++ gcc/fortran/trans-array.c (Arbeitskopie)
@@ -1287,27 +1287,25 @@ gfc_grow_array (stmtblock_t * pblock, tree desc, t
tree arg0, arg1;
tree tmp;
tree size;
- tree ubound;
+ tree extent;
if (integer_zerop (extra))
return;
- ubound = gfc_conv_descriptor_ubound_get (desc, gfc_rank_cst[0]);
+ extent = gfc_conv_descriptor_extent_get (desc, gfc_rank_cst[0]);
- /* Add EXTRA to the upper bound. */
- tmp = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- ubound, extra);
- gfc_conv_descriptor_ubound_set (pblock, desc, gfc_rank_cst[0], tmp);
+ /* Add EXTRA to the extent. */
+ extent = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
+ extent, extra);
+ gfc_conv_descriptor_extent_set (pblock, desc, gfc_rank_cst[0], extent);
/* Get the value of the current data pointer. */
arg0 = gfc_conv_descriptor_data_get (desc);
/* Calculate the new array size. */
size = TYPE_SIZE_UNIT (gfc_get_element_type (TREE_TYPE (desc)));
- tmp = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- ubound, gfc_index_one_node);
arg1 = fold_build2_loc (input_location, MULT_EXPR, size_type_node,
- fold_convert (size_type_node, tmp),
+ fold_convert (size_type_node, extent),
fold_convert (size_type_node, size));
/* Call the realloc() function. */
@@ -6927,13 +6925,7 @@ array_parameter_size (tree desc, gfc_expr *expr, t
gfc_build_addr_expr (NULL, desc));
else
{
- tree ubound = gfc_conv_descriptor_ubound_get (desc, gfc_index_zero_node);
- tree lbound = gfc_conv_descriptor_lbound_get (desc, gfc_index_zero_node);
-
- *size = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, ubound, lbound);
- *size = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- *size, gfc_index_one_node);
+ *size = gfc_conv_descriptor_extent_get (desc, gfc_index_zero_node);
*size = fold_build2_loc (input_location, MAX_EXPR, gfc_array_index_type,
*size, gfc_index_zero_node);
}
Index: gcc/fortran/trans-openmp.c
===================================================================
--- gcc/fortran/trans-openmp.c (Revision 189480)
+++ gcc/fortran/trans-openmp.c (Arbeitskopie)
@@ -173,11 +173,7 @@ gfc_omp_clause_default_ctor (tree clause, tree dec
gfc_add_modify (&cond_block, decl, outer);
rank = gfc_rank_cst[GFC_TYPE_ARRAY_RANK (type) - 1];
- size = gfc_conv_descriptor_ubound_get (decl, rank);
- size = fold_build2_loc (input_location, MINUS_EXPR, gfc_array_index_type,
- size, gfc_conv_descriptor_lbound_get (decl, rank));
- size = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- size, gfc_index_one_node);
+ size = gfc_conv_descriptor_extent_get (decl, rank);
if (GFC_TYPE_ARRAY_RANK (type) > 1)
size = fold_build2_loc (input_location, MULT_EXPR, gfc_array_index_type,
size, gfc_conv_descriptor_stride_get (decl, rank));
@@ -230,11 +226,7 @@ gfc_omp_clause_copy_ctor (tree clause, tree dest,
gfc_add_modify (&cond_block, dest, src);
rank = gfc_rank_cst[GFC_TYPE_ARRAY_RANK (type) - 1];
- size = gfc_conv_descriptor_ubound_get (dest, rank);
- size = fold_build2_loc (input_location, MINUS_EXPR, gfc_array_index_type,
- size, gfc_conv_descriptor_lbound_get (dest, rank));
- size = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- size, gfc_index_one_node);
+ size = gfc_conv_descriptor_extent_get (dest, rank);
if (GFC_TYPE_ARRAY_RANK (type) > 1)
size = fold_build2_loc (input_location, MULT_EXPR, gfc_array_index_type,
size, gfc_conv_descriptor_stride_get (dest, rank));
@@ -287,11 +279,7 @@ gfc_omp_clause_assign_op (tree clause ATTRIBUTE_UN
gfc_start_block (&block);
rank = gfc_rank_cst[GFC_TYPE_ARRAY_RANK (type) - 1];
- size = gfc_conv_descriptor_ubound_get (dest, rank);
- size = fold_build2_loc (input_location, MINUS_EXPR, gfc_array_index_type,
- size, gfc_conv_descriptor_lbound_get (dest, rank));
- size = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- size, gfc_index_one_node);
+ size = gfc_conv_descriptor_extent_get (dest, rank);
if (GFC_TYPE_ARRAY_RANK (type) > 1)
size = fold_build2_loc (input_location, MULT_EXPR, gfc_array_index_type,
size, gfc_conv_descriptor_stride_get (dest, rank));
@@ -664,12 +652,7 @@ gfc_trans_omp_array_reduction (tree c, gfc_symbol
gfc_add_modify (&block, decl, outer_sym.backend_decl);
rank = gfc_rank_cst[GFC_TYPE_ARRAY_RANK (type) - 1];
- size = gfc_conv_descriptor_ubound_get (decl, rank);
- size = fold_build2_loc (input_location, MINUS_EXPR,
- gfc_array_index_type, size,
- gfc_conv_descriptor_lbound_get (decl, rank));
- size = fold_build2_loc (input_location, PLUS_EXPR, gfc_array_index_type,
- size, gfc_index_one_node);
+ size = gfc_conv_descriptor_extent_get (decl, rank);
if (GFC_TYPE_ARRAY_RANK (type) > 1)
size = fold_build2_loc (input_location, MULT_EXPR,
gfc_array_index_type, size,
Index: gcc/testsuite/gfortran.dg/array_section_2.f90
===================================================================
--- gcc/testsuite/gfortran.dg/array_section_2.f90 (Revision 189480)
+++ gcc/testsuite/gfortran.dg/array_section_2.f90 (Arbeitskopie)
@@ -12,5 +12,5 @@ program test
allocate(a(n), temp(n))
temp(1:size(a)) = a
end program
-! { dg-final { scan-tree-dump-times "MAX_EXPR\[^\n\t\]+extent\[^\n\t\]+, 0" 1 "original" } }
+! { dg-final { scan-tree-dump-times "MAX_EXPR\[^\n\t\]+extent, 0" 1 "original" } }
! { dg-final { cleanup-tree-dump "original" } }
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Fortran-dev][Patch] Some (ubound-lbound+1) -> extent cleanup
2012-07-15 11:24 [Fortran-dev][Patch] Some (ubound-lbound+1) -> extent cleanup Tobias Burnus
@ 2012-07-15 12:08 ` Mikael Morin
0 siblings, 0 replies; 2+ messages in thread
From: Mikael Morin @ 2012-07-15 12:08 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc patches, gfortran
On 15/07/2012 13:24, Tobias Burnus wrote:
> This patch cleans up the source code and generated code ("dump") by
> changing (ubound-lbound+1) calculations to directly taking the "extent".
> Except for a faster -O0 performance and saving some cycles during code
> generation, the code should be the same.
>
A small, yet welcome simplification :-).
>
> The only real code change I did was to gfc_grow_array. I didn't
> understand the previous code, I think the current code makes more sense.
> I believe the old code did:
>
> desc->extent = desc.extent + extra;
> realloc (&desc->data, (desc.extent + 1)*element_size);
If you are talking about the old code, I think it does:
desc.ubound[0] += extra;
realloc (desc.data, (desc.ubound[0]+1)*element_size);
>
> while I think the latter should be "+ extra" and not "+ 1". From the
> callee, I couldn't deduce whether extra is always unity in practice, in
> any case it doesn't look like.
>
I think the old code is correct, under the assumption that desc.rank is
1, and desc.lbound[0] is 0, which is the case in the contexts where the
functions is called (array constructors).
>
> For fcncall_realloc_result, I didn't check whether the new code
> effectively matches the old one, I just followed the comment which
> states: "Check that the shapes are the same between lhs and expression.".
>
I think the old code is:
res_desc.ubound[n] - res_desc.lbound[n]
- (desc.ubound[n] - desc.lbound[n]) != 0
while the new one is:
res_desc.extent[n] != res.extent[n]
>
> There are some more cases, but there the code wasn't as obvious. For
> instance "extent = ubound - lbound"; there I was unsure whether that's a
> bug (missing "+1") or valid.
>
>
> Build and regtested with no new failures.
> OK for the branch?
>
Yes, thanks.
Mikael
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-07-15 12:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-15 11:24 [Fortran-dev][Patch] Some (ubound-lbound+1) -> extent cleanup Tobias Burnus
2012-07-15 12:08 ` Mikael Morin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).