* [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
@ 2016-02-19 17:42 Alan Lawrence
2016-02-19 17:53 ` Jakub Jelinek
0 siblings, 1 reply; 12+ messages in thread
From: Alan Lawrence @ 2016-02-19 17:42 UTC (permalink / raw)
To: gcc-patches
This relates to FORTRAN code where different modules give different sizes to the
same array in a COMMON block (contrary to the fortran language specification).
SPEC have refused to patch the source code
(https://www.spec.org/cpu2006/Docs/faq.html#Run.05).
Hence, this patch provides a Fortran-specific option -funknown-commons that
marks such arrays as having unknown size - that is, NULL_TREE for both
TYPE_SIZE and max value of TYPE_DOMAIN. DECL_SIZE is preserved for e.g. output
in varasm.c.
On AArch64, it fixes the 416.gamess issue, and allows compiling 416.gamess
without the -fno-aggressive-loop-optimizations previously required (numerous
other PRs relating to 416.gamess).
I had to fix up a couple of places to deal with null TYPE_SIZE but in most cases
a test for such was already present. (omp-low.c had too many so I gave up: in
practice I think it's OK to just not use the new flag at the same time as
-fopenmp).
Bootstrapped + check-gcc + check-g++ on AArch64 and x86.
Also check-fortran with a variant enabling the option in all cases (to allow
compilation of C): here, only gfortran.dg/gomp/appendix-a/a.24.1.f90 fails,
this uses -fopenmp mentioned above.
Testcase fails both the FIND and one of the assignments if -funknown-commons is removed.
OK for trunk? I'd be happy to move this under -std=legacy if the Fortran folks
prefer. (-funcommon-commons has also been humourously suggested!)
gcc/ChangeLog:
* expr.c (store_field): Handle null TYPE_SIZE.
* tree-vect-data-refs.c (vect_analyze_data_refs): Bail out if TYPE_SIZE
is null.
* tree-dfa.c (get_ref_base_and_extent): Keep maxsize==-1 when reading
the whole of a DECL.
gcc/fortran/ChangeLog:
* lang.opt: Add -funknown-commons.
* trans-common.ci (build_field): If flag_unknown_commons is on, set
upper bound and size of arrays in common block to NULL_TREE.
(create_common): Explicitly set DECL_SIZE and DECL_SIZE_UNIT.
gcc/testsuite/ChangeLog:
* gfortran.dg/unknown_commons.f: New.
---
gcc/expr.c | 1 +
gcc/fortran/lang.opt | 4 ++++
gcc/fortran/trans-common.c | 35 +++++++++++++++++++++++++++++
gcc/testsuite/gfortran.dg/unknown_commons.f | 20 +++++++++++++++++
gcc/tree-dfa.c | 6 ++++-
gcc/tree-vect-data-refs.c | 8 +++++++
6 files changed, 73 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/unknown_commons.f
diff --git a/gcc/expr.c b/gcc/expr.c
index f4bac36..609bf59 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -6638,6 +6638,7 @@ store_field (rtx target, HOST_WIDE_INT bitsize, HOST_WIDE_INT bitpos,
RHS isn't the same size as the bitfield, we must use bitfield
operations. */
|| (bitsize >= 0
+ && TYPE_SIZE (TREE_TYPE (exp))
&& TREE_CODE (TYPE_SIZE (TREE_TYPE (exp))) == INTEGER_CST
&& compare_tree_int (TYPE_SIZE (TREE_TYPE (exp)), bitsize) != 0
/* Except for initialization of full bytes from a CONSTRUCTOR, which
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 45428d8..b1b4641 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -686,6 +686,10 @@ funderscoring
Fortran Var(flag_underscoring) Init(1)
Append underscores to externally visible names.
+funknown-commons
+Fortran Var(flag_unknown_commons)
+Treat arrays in common blocks as having unknown size.
+
fwhole-file
Fortran Ignore
Does nothing. Preserved for backward compatibility.
diff --git a/gcc/fortran/trans-common.c b/gcc/fortran/trans-common.c
index 21d1928..bf99c56 100644
--- a/gcc/fortran/trans-common.c
+++ b/gcc/fortran/trans-common.c
@@ -284,8 +284,41 @@ build_field (segment_info *h, tree union_type, record_layout_info rli)
unsigned HOST_WIDE_INT desired_align, known_align;
name = get_identifier (h->sym->name);
+
+ gcc_assert (TYPE_P (h->field));
+ tree size = TYPE_SIZE (h->field);
+ tree size_unit = TYPE_SIZE_UNIT (h->field);
+ if (GFC_ARRAY_TYPE_P (h->field)
+ && !h->sym->value
+ && flag_unknown_commons)
+ {
+ gcc_assert (!GFC_DESCRIPTOR_TYPE_P (h->field));
+ gcc_assert (TREE_CODE (h->field) == ARRAY_TYPE);
+
+ /* Could be merged at link time with a larger array whose size we don't
+ know. */
+ static tree range_type = NULL_TREE;
+ if (!range_type)
+ range_type = build_range_type (gfc_array_index_type,
+ gfc_index_zero_node, NULL_TREE);
+ tree type = copy_node (h->field);
+ TYPE_DOMAIN (type) = range_type;
+ TYPE_SIZE (type) = NULL_TREE;
+ TYPE_SIZE_UNIT (type) = NULL_TREE;
+ SET_TYPE_MODE (type, BLKmode);
+ gcc_assert (TYPE_CANONICAL (h->field) == h->field);
+ gcc_assert (TYPE_MAIN_VARIANT (h->field) == h->field);
+ TYPE_CANONICAL (type) = type;
+ TYPE_MAIN_VARIANT (type) = type;
+ layout_type (type);
+ h->field = type;
+ }
+
field = build_decl (h->sym->declared_at.lb->location,
FIELD_DECL, name, h->field);
+ DECL_SIZE (field) = size;
+ DECL_SIZE_UNIT (field) = size_unit;
+
known_align = (offset & -offset) * BITS_PER_UNIT;
if (known_align == 0 || known_align > BIGGEST_ALIGNMENT)
known_align = BIGGEST_ALIGNMENT;
@@ -703,6 +736,8 @@ create_common (gfc_common_head *com, segment_info *head, bool saw_equiv)
VAR_DECL, DECL_NAME (s->field),
TREE_TYPE (s->field));
TREE_STATIC (var_decl) = TREE_STATIC (decl);
+ DECL_SIZE (var_decl) = DECL_SIZE (s->field);
+ DECL_SIZE_UNIT (var_decl) = DECL_SIZE_UNIT (s->field);
/* Mark the variable as used in order to avoid warnings about
unused variables. */
TREE_USED (var_decl) = 1;
diff --git a/gcc/testsuite/gfortran.dg/unknown_commons.f b/gcc/testsuite/gfortran.dg/unknown_commons.f
new file mode 100644
index 0000000..97e3ce3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unknown_commons.f
@@ -0,0 +1,20 @@
+! { dg-do compile }
+! { dg-options "-O3 -funknown-commons -fdump-tree-dom2-details" }
+
+! Test for PR69368: a single-element array in a common block, which will be
+! overridden with a larger size at link time (contrary to language spec).
+! Dominator opts considers accesses to differently-computed elements of X as
+! equivalent, unless -funknown-commons is passed in.
+ SUBROUTINE FOO
+ IMPLICIT DOUBLE PRECISION (X)
+ INTEGER J
+ COMMON /MYCOMMON / X(1)
+ DO 10 J=1,1024
+ X(J+1)=X(J+7)
+ 10 CONTINUE
+ RETURN
+ END
+! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
+! We should retain both a read and write of mycommon.x.
+! { dg-final { scan-tree-dump-times " _\[0-9\]+ = mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
+! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] = _\[0-9\]+;" 1 "dom2" } }
diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
index 0e98056..340c04c 100644
--- a/gcc/tree-dfa.c
+++ b/gcc/tree-dfa.c
@@ -617,7 +617,11 @@ get_ref_base_and_extent (tree exp, HOST_WIDE_INT *poffset,
if (maxsize == -1
&& DECL_SIZE (exp)
&& TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
- maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
+ {
+ maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
+ if (maxsize == bitsize)
+ maxsize = -1;
+ }
}
else if (CONSTANT_CLASS_P (exp))
{
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 4c0e135..f8e8dce 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -3412,6 +3412,14 @@ again:
return false;
}
+ if (TYPE_SIZE (TREE_TYPE (DR_REF (dr))) == NULL_TREE)
+ {
+ if (dump_enabled_p ())
+ dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+ "not vectorized: data-ref of unknown size\n");
+ return false;
+ }
+
stmt = DR_STMT (dr);
stmt_info = vinfo_for_stmt (stmt);
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-19 17:42 [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006 Alan Lawrence
@ 2016-02-19 17:53 ` Jakub Jelinek
2016-02-22 11:46 ` Alan Lawrence
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2016-02-19 17:53 UTC (permalink / raw)
To: Alan Lawrence; +Cc: gcc-patches
On Fri, Feb 19, 2016 at 05:42:34PM +0000, Alan Lawrence wrote:
> This relates to FORTRAN code where different modules give different sizes to the
> same array in a COMMON block (contrary to the fortran language specification).
> SPEC have refused to patch the source code
> (https://www.spec.org/cpu2006/Docs/faq.html#Run.05).
>
> Hence, this patch provides a Fortran-specific option -funknown-commons that
> marks such arrays as having unknown size - that is, NULL_TREE for both
> TYPE_SIZE and max value of TYPE_DOMAIN. DECL_SIZE is preserved for e.g. output
> in varasm.c.
>
> On AArch64, it fixes the 416.gamess issue, and allows compiling 416.gamess
> without the -fno-aggressive-loop-optimizations previously required (numerous
> other PRs relating to 416.gamess).
>
> I had to fix up a couple of places to deal with null TYPE_SIZE but in most cases
I think it is wrong to touch TYPE_SIZE/TYPE_SIZE_UNIT, IMHO it is much better just
to ignore DECL_SIZE/DECL_SIZE_UNIT in the selected few places
(get_ref_base_and_extent, the tree-ssa-loop-niters.c analysis) if the switch
is on, for selected decls (aggregates with flexible array members and other
similar trailing arrays, arrays themselves; all only if DECL_COMMON).
> a test for such was already present. (omp-low.c had too many so I gave up: in
> practice I think it's OK to just not use the new flag at the same time as
> -fopenmp).
That will just be an endless source of bugreports when people will report ICEs
with -funknown-commons -fopenmp (or -fopenacc, or -fcilkplus, or
-ftree-parallelize-loops, ...). For OpenMP the TYPE_SIZE* is essential, if
you privatize variables, you need to know how big variable to allocate etc.
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-19 17:53 ` Jakub Jelinek
@ 2016-02-22 11:46 ` Alan Lawrence
2016-02-22 12:03 ` Jakub Jelinek
0 siblings, 1 reply; 12+ messages in thread
From: Alan Lawrence @ 2016-02-22 11:46 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 19/02/16 17:52, Jakub Jelinek wrote:
> On Fri, Feb 19, 2016 at 05:42:34PM +0000, Alan Lawrence wrote:
>> This relates to FORTRAN code where different modules give different sizes to the
>> same array in a COMMON block (contrary to the fortran language specification).
>> SPEC have refused to patch the source code
>> (https://www.spec.org/cpu2006/Docs/faq.html#Run.05).
>>
>> Hence, this patch provides a Fortran-specific option -funknown-commons that
>> marks such arrays as having unknown size - that is, NULL_TREE for both
>> TYPE_SIZE and max value of TYPE_DOMAIN. DECL_SIZE is preserved for e.g. output
>> in varasm.c.
>>
>> On AArch64, it fixes the 416.gamess issue, and allows compiling 416.gamess
>> without the -fno-aggressive-loop-optimizations previously required (numerous
>> other PRs relating to 416.gamess).
>>
>> I had to fix up a couple of places to deal with null TYPE_SIZE but in most cases
>
> I think it is wrong to touch TYPE_SIZE/TYPE_SIZE_UNIT, IMHO it is much better just
> to ignore DECL_SIZE/DECL_SIZE_UNIT in the selected few places
> (get_ref_base_and_extent, the tree-ssa-loop-niters.c analysis) if the switch
> is on, for selected decls (aggregates with flexible array members and other
> similar trailing arrays, arrays themselves; all only if DECL_COMMON).
So do you see...
(a) A global command-line option, which we check alongside DECL_COMMON, in (all
or some of the) places which currently deal with flexible array members? (I
guess the flag would have no effect for compiling C code...or
(b) do we require it to 'enable' the existing flexible array member support?)
(c) A new flag on each DECL, which we check in the places dealing with flexible
array members, which we set on those decls in the fortran frontend if a
fortran-frontend-only command-line option is passed?
(d) A new flag on each DECL, which replaces the check for flexible array
members, plus a global command-line option, which controls both setting the flag
(on DECL_COMMONs) in the fortran frontend, and also setting it on flexible array
members in the C frontend? This might be 'cleanest' but is also probably the
most change and I doubt I'm going to have time to do this for GCC 6...
(e) Some other scheme that I've not understood yet?
Thanks,
Alan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-22 11:46 ` Alan Lawrence
@ 2016-02-22 12:03 ` Jakub Jelinek
2016-02-23 11:42 ` Alan Lawrence
2016-02-25 18:00 ` Alan Lawrence
0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-02-22 12:03 UTC (permalink / raw)
To: Alan Lawrence; +Cc: gcc-patches
On Mon, Feb 22, 2016 at 11:46:19AM +0000, Alan Lawrence wrote:
> On 19/02/16 17:52, Jakub Jelinek wrote:
> >On Fri, Feb 19, 2016 at 05:42:34PM +0000, Alan Lawrence wrote:
> >>This relates to FORTRAN code where different modules give different sizes to the
> >>same array in a COMMON block (contrary to the fortran language specification).
> >>SPEC have refused to patch the source code
> >>(https://www.spec.org/cpu2006/Docs/faq.html#Run.05).
> >>
> >>Hence, this patch provides a Fortran-specific option -funknown-commons that
> >>marks such arrays as having unknown size - that is, NULL_TREE for both
> >>TYPE_SIZE and max value of TYPE_DOMAIN. DECL_SIZE is preserved for e.g. output
> >>in varasm.c.
> >>
> >>On AArch64, it fixes the 416.gamess issue, and allows compiling 416.gamess
> >>without the -fno-aggressive-loop-optimizations previously required (numerous
> >>other PRs relating to 416.gamess).
> >>
> >>I had to fix up a couple of places to deal with null TYPE_SIZE but in most cases
> >
> >I think it is wrong to touch TYPE_SIZE/TYPE_SIZE_UNIT, IMHO it is much better just
> >to ignore DECL_SIZE/DECL_SIZE_UNIT in the selected few places
> >(get_ref_base_and_extent, the tree-ssa-loop-niters.c analysis) if the switch
> >is on, for selected decls (aggregates with flexible array members and other
> >similar trailing arrays, arrays themselves; all only if DECL_COMMON).
>
> So do you see...
>
> (a) A global command-line option, which we check alongside DECL_COMMON, in
> (all or some of the) places which currently deal with flexible array
> members? (I guess the flag would have no effect for compiling C code...or
> (b) do we require it to 'enable' the existing flexible array member support?)
>
> (c) A new flag on each DECL, which we check in the places dealing with
> flexible array members, which we set on those decls in the fortran frontend
> if a fortran-frontend-only command-line option is passed?
>
> (d) A new flag on each DECL, which replaces the check for flexible array
> members, plus a global command-line option, which controls both setting the
> flag (on DECL_COMMONs) in the fortran frontend, and also setting it on
> flexible array members in the C frontend? This might be 'cleanest' but is
> also probably the most change and I doubt I'm going to have time to do this
> for GCC 6...
>
> (e) Some other scheme that I've not understood yet?
(f) A global command-line option, which we check alongside DECL_COMMON and
further tests (basically, we want only DECL_COMMON decls that either have
ARRAY_TYPE, or some other aggregate type with flexible array member or some
other trailing array in the struct), and use the resulting predicate in
places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
predicate returns true, we assume the DECL_SIZE is
"variable"/"unlimited"/whatever.
The two spots known to me that would need to use this new predicate would
be:
tree.c (array_at_struct_end_p):
/* If the reference is based on a declared entity, the size of the array
is constrained by its given domain. */
if (DECL_P (ref))
return false;
This would need to do if (DECL_P (ref) && !the_new_predicate (ref)) return false;
tree-dfa.c (get_ref_base_and_extent):
if (DECL_P (exp))
{
...
}
You want to do inside of that block something like
if (the_new_predicate (exp))
{
/* We need to deal with variable arrays ending structures. */
if (seen_variable_array_ref
&& maxsize != -1
&& (TYPE_SIZE (TREE_TYPE (exp)) == NULL_TREE
|| TREE_CODE (TYPE_SIZE (TREE_TYPE (exp))) != INTEGER_CST
|| (bit_offset + maxsize
== wi::to_offset (TYPE_SIZE (TREE_TYPE (exp))))))
maxsize = -1;
else if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE)
maxsize = -1;
}
/* If maxsize is unknown adjust it according to the size of the
base decl. */
else if ((maxsize == -1
&& DECL_SIZE (exp)
&& TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
Thus, basically for the DECL_COMMON with trailing array readd the
r233153 hunk (though it can be readded in slightly different spot),
somehow deal with the case of ARRAY_TYPE itself, and only apply DEC_SIZE if
the predicate is false.
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-22 12:03 ` Jakub Jelinek
@ 2016-02-23 11:42 ` Alan Lawrence
2016-02-25 18:00 ` Alan Lawrence
1 sibling, 0 replies; 12+ messages in thread
From: Alan Lawrence @ 2016-02-23 11:42 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 22/02/16 12:03, Jakub Jelinek wrote:
>
> (f) A global command-line option, which we check alongside DECL_COMMON and
> further tests (basically, we want only DECL_COMMON decls that either have
> ARRAY_TYPE, or some other aggregate type with flexible array member or some
> other trailing array in the struct), and use the resulting predicate in
> places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
> predicate returns true, we assume the DECL_SIZE is
> "variable"/"unlimited"/whatever.
> The two spots known to me that would need to use this new predicate would
> be:
> tree.c (array_at_struct_end_p):
> tree-dfa.c (get_ref_base_and_extent):
Well, with just those two, this fixes 416.gamess, and passes all tests in
gfortran, with only a few scan-dump/warning tests failing in gcc...
--Alan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-22 12:03 ` Jakub Jelinek
2016-02-23 11:42 ` Alan Lawrence
@ 2016-02-25 18:00 ` Alan Lawrence
2016-03-03 10:20 ` Alan Lawrence
2016-03-04 13:27 ` Richard Biener
1 sibling, 2 replies; 12+ messages in thread
From: Alan Lawrence @ 2016-02-25 18:00 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub, tkoenig
On 22/02/16 12:03, Jakub Jelinek wrote:
> (f) A global command-line option, which we check alongside DECL_COMMON and
> further tests (basically, we want only DECL_COMMON decls that either have
> ARRAY_TYPE, or some other aggregate type with flexible array member or some
> other trailing array in the struct), and use the resulting predicate in
> places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
> predicate returns true, we assume the DECL_SIZE is
> "variable"/"unlimited"/whatever.
> The two spots known to me that would need to use this new predicate would
> be:
> tree.c (array_at_struct_end_p):
[...]
> tree-dfa.c (get_ref_base_and_extent):
[...]
Close enough to what I meant by (a)/(b), but never mind that, and I hadn't
looked at where the change for aggressive loop opts needed to be. However...
Well you are essentially writing the patch for me at this point (so, thanks!),
but here's a patch that puts that under a global -funknown-commons flag.
Testcase as before.
Bootstrapped (with flag overridden to true) on x86_64 and aarch64, check-gcc,
check-fortran, and tested 416.gamess on AArch64, where this patch enables
running *without* the -fno-aggressive-loop-opts previously required.
In the gcc testsuite, these fail with the option turned on:
gcc.dg/pr53265.c (test for warnings, line 137)
gcc.dg/pr53265.c (test for warnings, line 138)
gcc.dg/pr64277.c scan-tree-dump cunroll ... (*2)
gcc.dg/tree-ssa/cunroll-{1,2,3,4,9,10,11}.c scan-tree-dump cunroll/cunrolli/ivcanon (various)
gcc.dg/tree-ssa/loop-38.c scan-tree-dump cunrolli "Loop 1 iterates at most 11 times"
...which all appear harmless.
Thomas Koenig suggests (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69368#c80)
that this be combined with some flag fiddling and warnings in the Fortran
front-end; this patch doesn't do that, as I'm not very familiar with the
frontends, but that can follow in a separate patch. (Thomas?)
OK for trunk?
Cheers, Alan
gcc/ChangeLog:
DATE Alan Lawrence <alan.lawrence@arm.com>
Jakub Jelinek <jakub@redhat.com>
* common.opt (funknown-commons, flag_unknown_commons): New.
* tree.c (array_at_struct_end_p): Do not limit to size of decl for
DECL_COMMONS if flag_unknown_commons is set.
* tree-dfa.c (get_ref_base_and_extent): Likewise.
gcc/testsuite/ChangeLog:
* gfortran.dg/unknown_commons.f: New.
---
gcc/common.opt | 4 ++++
gcc/testsuite/gfortran.dg/unknown_commons.f | 20 ++++++++++++++++++++
gcc/tree-dfa.c | 15 ++++++++++++++-
gcc/tree.c | 6 ++++--
4 files changed, 42 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/unknown_commons.f
diff --git a/gcc/common.opt b/gcc/common.opt
index 520fa9c..b5b1282 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2455,6 +2455,10 @@ funit-at-a-time
Common Report Var(flag_unit_at_a_time) Init(1)
Compile whole compilation unit at a time.
+funknown-commons
+Common Var(flag_unknown_commons)
+Assume common declarations may be overridden with unknown larger sizes
+
funroll-loops
Common Report Var(flag_unroll_loops) Optimization
Perform loop unrolling when iteration count is known.
diff --git a/gcc/testsuite/gfortran.dg/unknown_commons.f b/gcc/testsuite/gfortran.dg/unknown_commons.f
new file mode 100644
index 0000000..97e3ce3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unknown_commons.f
@@ -0,0 +1,20 @@
+! { dg-do compile }
+! { dg-options "-O3 -funknown-commons -fdump-tree-dom2-details" }
+
+! Test for PR69368: a single-element array in a common block, which will be
+! overridden with a larger size at link time (contrary to language spec).
+! Dominator opts considers accesses to differently-computed elements of X as
+! equivalent, unless -funknown-commons is passed in.
+ SUBROUTINE FOO
+ IMPLICIT DOUBLE PRECISION (X)
+ INTEGER J
+ COMMON /MYCOMMON / X(1)
+ DO 10 J=1,1024
+ X(J+1)=X(J+7)
+ 10 CONTINUE
+ RETURN
+ END
+! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
+! We should retain both a read and write of mycommon.x.
+! { dg-final { scan-tree-dump-times " _\[0-9\]+ = mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
+! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] = _\[0-9\]+;" 1 "dom2" } }
diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
index 0e98056..017e98e 100644
--- a/gcc/tree-dfa.c
+++ b/gcc/tree-dfa.c
@@ -612,9 +612,22 @@ get_ref_base_and_extent (tree exp, HOST_WIDE_INT *poffset,
if (DECL_P (exp))
{
+ if (flag_unknown_commons
+ && TREE_CODE (exp) == VAR_DECL && DECL_COMMON (exp))
+ {
+ tree sz_tree = TYPE_SIZE (TREE_TYPE (exp));
+ /* If size is unknown, or we have read to the end, assume there
+ may be more to the structure than we are told. */
+ if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
+ || (seen_variable_array_ref
+ && (sz_tree == NULL_TREE
+ || TREE_CODE (sz_tree) != INTEGER_CST
+ || (bit_offset + maxsize == wi::to_offset (sz_tree)))))
+ maxsize = -1;
+ }
/* If maxsize is unknown adjust it according to the size of the
base decl. */
- if (maxsize == -1
+ else if (maxsize == -1
&& DECL_SIZE (exp)
&& TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
diff --git a/gcc/tree.c b/gcc/tree.c
index 07cb9d9..eb6b642 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -12957,8 +12957,10 @@ array_at_struct_end_p (tree ref)
}
/* If the reference is based on a declared entity, the size of the array
- is constrained by its given domain. */
- if (DECL_P (ref))
+ is constrained by its given domain. (Do not trust commons PR/69368). */
+ if (DECL_P (ref)
+ && !(flag_unknown_commons
+ && TREE_CODE (ref) == VAR_DECL && DECL_COMMON (ref)))
return false;
return true;
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-25 18:00 ` Alan Lawrence
@ 2016-03-03 10:20 ` Alan Lawrence
2016-03-04 13:27 ` Richard Biener
1 sibling, 0 replies; 12+ messages in thread
From: Alan Lawrence @ 2016-03-03 10:20 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub, tkoenig, Wilco Dijkstra
On 25/02/16 18:00, Alan Lawrence wrote:
> On 22/02/16 12:03, Jakub Jelinek wrote:
>> (f) A global command-line option, which we check alongside DECL_COMMON and
>> further tests (basically, we want only DECL_COMMON decls that either have
>> ARRAY_TYPE, or some other aggregate type with flexible array member or some
>> other trailing array in the struct), and use the resulting predicate in
>> places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
>> predicate returns true, we assume the DECL_SIZE is
>> "variable"/"unlimited"/whatever.
>> The two spots known to me that would need to use this new predicate would
>> be:
>> tree.c (array_at_struct_end_p):
> [...]
>> tree-dfa.c (get_ref_base_and_extent):
> [...]
>
> Close enough to what I meant by (a)/(b), but never mind that, and I hadn't
> looked at where the change for aggressive loop opts needed to be. However...
> Well you are essentially writing the patch for me at this point (so, thanks!),
> but here's a patch that puts that under a global -funknown-commons flag.
> Testcase as before.
>
> Bootstrapped (with flag overridden to true) on x86_64 and aarch64, check-gcc,
> check-fortran, and tested 416.gamess on AArch64, where this patch enables
> running *without* the -fno-aggressive-loop-opts previously required.
>
> In the gcc testsuite, these fail with the option turned on:
> gcc.dg/pr53265.c (test for warnings, line 137)
> gcc.dg/pr53265.c (test for warnings, line 138)
> gcc.dg/pr64277.c scan-tree-dump cunroll ... (*2)
> gcc.dg/tree-ssa/cunroll-{1,2,3,4,9,10,11}.c scan-tree-dump cunroll/cunrolli/ivcanon (various)
> gcc.dg/tree-ssa/loop-38.c scan-tree-dump cunrolli "Loop 1 iterates at most 11 times"
> ...which all appear harmless.
>
> Thomas Koenig suggests (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69368#c80)
> that this be combined with some flag fiddling and warnings in the Fortran
> front-end; this patch doesn't do that, as I'm not very familiar with the
> frontends, but that can follow in a separate patch. (Thomas?)
>
> OK for trunk?
>
> Cheers, Alan
>
> gcc/ChangeLog:
>
> DATE Alan Lawrence <alan.lawrence@arm.com>
> Jakub Jelinek <jakub@redhat.com>
>
> * common.opt (funknown-commons, flag_unknown_commons): New.
> * tree.c (array_at_struct_end_p): Do not limit to size of decl for
> DECL_COMMONS if flag_unknown_commons is set.
> * tree-dfa.c (get_ref_base_and_extent): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/unknown_commons.f: New.
Ping.
(Besides my original patch
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01356.html which we decided was
too fragile, I believe this is the only patch proposed that actually fixes the
miscompare in 416.gamess.)
Thanks, Alan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-02-25 18:00 ` Alan Lawrence
2016-03-03 10:20 ` Alan Lawrence
@ 2016-03-04 13:27 ` Richard Biener
2016-03-04 13:33 ` Jakub Jelinek
2016-03-07 11:03 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 (was: Re: [PATCH] Add -funknown-commons ...) Alan Lawrence
1 sibling, 2 replies; 12+ messages in thread
From: Richard Biener @ 2016-03-04 13:27 UTC (permalink / raw)
To: Alan Lawrence; +Cc: GCC Patches, Jakub Jelinek, Thomas Koenig
On Thu, Feb 25, 2016 at 7:00 PM, Alan Lawrence <alan.lawrence@arm.com> wrote:
> On 22/02/16 12:03, Jakub Jelinek wrote:
>> (f) A global command-line option, which we check alongside DECL_COMMON and
>> further tests (basically, we want only DECL_COMMON decls that either have
>> ARRAY_TYPE, or some other aggregate type with flexible array member or some
>> other trailing array in the struct), and use the resulting predicate in
>> places where we optimize based on DECL_SIZE{,_UNIT} of the decl - if that
>> predicate returns true, we assume the DECL_SIZE is
>> "variable"/"unlimited"/whatever.
>> The two spots known to me that would need to use this new predicate would
>> be:
>> tree.c (array_at_struct_end_p):
> [...]
>> tree-dfa.c (get_ref_base_and_extent):
> [...]
>
> Close enough to what I meant by (a)/(b), but never mind that, and I hadn't
> looked at where the change for aggressive loop opts needed to be. However...
> Well you are essentially writing the patch for me at this point (so, thanks!),
> but here's a patch that puts that under a global -funknown-commons flag.
> Testcase as before.
>
> Bootstrapped (with flag overridden to true) on x86_64 and aarch64, check-gcc,
> check-fortran, and tested 416.gamess on AArch64, where this patch enables
> running *without* the -fno-aggressive-loop-opts previously required.
>
> In the gcc testsuite, these fail with the option turned on:
> gcc.dg/pr53265.c (test for warnings, line 137)
> gcc.dg/pr53265.c (test for warnings, line 138)
> gcc.dg/pr64277.c scan-tree-dump cunroll ... (*2)
> gcc.dg/tree-ssa/cunroll-{1,2,3,4,9,10,11}.c scan-tree-dump cunroll/cunrolli/ivcanon (various)
> gcc.dg/tree-ssa/loop-38.c scan-tree-dump cunrolli "Loop 1 iterates at most 11 times"
> ...which all appear harmless.
>
> Thomas Koenig suggests (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69368#c80)
> that this be combined with some flag fiddling and warnings in the Fortran
> front-end; this patch doesn't do that, as I'm not very familiar with the
> frontends, but that can follow in a separate patch. (Thomas?)
>
> OK for trunk?
Comments below.
> Cheers, Alan
>
> gcc/ChangeLog:
>
> DATE Alan Lawrence <alan.lawrence@arm.com>
> Jakub Jelinek <jakub@redhat.com>
>
> * common.opt (funknown-commons, flag_unknown_commons): New.
> * tree.c (array_at_struct_end_p): Do not limit to size of decl for
> DECL_COMMONS if flag_unknown_commons is set.
> * tree-dfa.c (get_ref_base_and_extent): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/unknown_commons.f: New.
> ---
> gcc/common.opt | 4 ++++
> gcc/testsuite/gfortran.dg/unknown_commons.f | 20 ++++++++++++++++++++
> gcc/tree-dfa.c | 15 ++++++++++++++-
> gcc/tree.c | 6 ++++--
> 4 files changed, 42 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gfortran.dg/unknown_commons.f
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 520fa9c..b5b1282 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2455,6 +2455,10 @@ funit-at-a-time
> Common Report Var(flag_unit_at_a_time) Init(1)
> Compile whole compilation unit at a time.
>
> +funknown-commons
> +Common Var(flag_unknown_commons)
> +Assume common declarations may be overridden with unknown larger sizes
I think to make it work with LTO you need to mark it 'Optimization'.
Also it's about
arrays so maybe
'Assume common declarations may be overridden with ones with a larger
trailing array'
also if we document it here we should eventually document it in invoke.texi.
Not sure if "unknown commons" is a good term, maybe "unconstrained
commons" instead?
Otherwise looks ok to me.
Richard.
> +
> funroll-loops
> Common Report Var(flag_unroll_loops) Optimization
> Perform loop unrolling when iteration count is known.
> diff --git a/gcc/testsuite/gfortran.dg/unknown_commons.f b/gcc/testsuite/gfortran.dg/unknown_commons.f
> new file mode 100644
> index 0000000..97e3ce3
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/unknown_commons.f
> @@ -0,0 +1,20 @@
> +! { dg-do compile }
> +! { dg-options "-O3 -funknown-commons -fdump-tree-dom2-details" }
> +
> +! Test for PR69368: a single-element array in a common block, which will be
> +! overridden with a larger size at link time (contrary to language spec).
> +! Dominator opts considers accesses to differently-computed elements of X as
> +! equivalent, unless -funknown-commons is passed in.
> + SUBROUTINE FOO
> + IMPLICIT DOUBLE PRECISION (X)
> + INTEGER J
> + COMMON /MYCOMMON / X(1)
> + DO 10 J=1,1024
> + X(J+1)=X(J+7)
> + 10 CONTINUE
> + RETURN
> + END
> +! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
> +! We should retain both a read and write of mycommon.x.
> +! { dg-final { scan-tree-dump-times " _\[0-9\]+ = mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
> +! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] = _\[0-9\]+;" 1 "dom2" } }
> diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
> index 0e98056..017e98e 100644
> --- a/gcc/tree-dfa.c
> +++ b/gcc/tree-dfa.c
> @@ -612,9 +612,22 @@ get_ref_base_and_extent (tree exp, HOST_WIDE_INT *poffset,
>
> if (DECL_P (exp))
> {
> + if (flag_unknown_commons
> + && TREE_CODE (exp) == VAR_DECL && DECL_COMMON (exp))
> + {
> + tree sz_tree = TYPE_SIZE (TREE_TYPE (exp));
> + /* If size is unknown, or we have read to the end, assume there
> + may be more to the structure than we are told. */
> + if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
> + || (seen_variable_array_ref
> + && (sz_tree == NULL_TREE
> + || TREE_CODE (sz_tree) != INTEGER_CST
> + || (bit_offset + maxsize == wi::to_offset (sz_tree)))))
> /* If maxsize is unknown adjust it according to the size of the
> base decl. */
> - if (maxsize == -1
> + else if (maxsize == -1
> && DECL_SIZE (exp)
> && TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
> maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 07cb9d9..eb6b642 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -12957,8 +12957,10 @@ array_at_struct_end_p (tree ref)
> }
>
> /* If the reference is based on a declared entity, the size of the array
> - is constrained by its given domain. */
> - if (DECL_P (ref))
> + is constrained by its given domain. (Do not trust commons PR/69368). */
> + if (DECL_P (ref)
> + && !(flag_unknown_commons
> + && TREE_CODE (ref) == VAR_DECL && DECL_COMMON (ref)))
> return false;
>
> return true;
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006
2016-03-04 13:27 ` Richard Biener
@ 2016-03-04 13:33 ` Jakub Jelinek
2016-03-07 11:03 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 (was: Re: [PATCH] Add -funknown-commons ...) Alan Lawrence
1 sibling, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-03-04 13:33 UTC (permalink / raw)
To: Richard Biener; +Cc: Alan Lawrence, GCC Patches, Thomas Koenig
On Fri, Mar 04, 2016 at 02:27:46PM +0100, Richard Biener wrote:
> > +funknown-commons
> > +Common Var(flag_unknown_commons)
> > +Assume common declarations may be overridden with unknown larger sizes
>
> I think to make it work with LTO you need to mark it 'Optimization'.
> Also it's about
> arrays so maybe
>
> 'Assume common declarations may be overridden with ones with a larger
> trailing array'
>
> also if we document it here we should eventually document it in invoke.texi.
Also, isn't the *.opt description line supposed to end with a full stop?
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 (was: Re: [PATCH] Add -funknown-commons ...)
2016-03-04 13:27 ` Richard Biener
2016-03-04 13:33 ` Jakub Jelinek
@ 2016-03-07 11:03 ` Alan Lawrence
2016-03-09 17:54 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 Alan Lawrence
1 sibling, 1 reply; 12+ messages in thread
From: Alan Lawrence @ 2016-03-07 11:03 UTC (permalink / raw)
To: gcc-patches; +Cc: richard.guenther, jakub, tkoenig
On 04/03/16 13:27, Richard Biener wrote:
> I think to make it work with LTO you need to mark it 'Optimization'.
> Also it's about
> arrays so maybe
>
> 'Assume common declarations may be overridden with ones with a larger
> trailing array'
>
> also if we document it here we should eventually document it in invoke.texi.
>
> Not sure if "unknown commons" is a good term, maybe "unconstrained
> commons" instead?
All done; I doubt there is really a good word, unconstrained seems as good as
any. I've reused much the same wording in invoke.texi, unless you think there
is more to add.
On 04/03/16 13:33, Jakub Jelinek wrote:
> Also, isn't the *.opt description line supposed to end with a full stop?
Ah, yes, thanks.
Is this version OK for trunk?
gcc/ChangeLog:
DATE Alan Lawrence <alan.lawrence@arm.com>
Jakub Jelinek <jakub@redhat.com>
* common.opt (funconstrained-commons, flag_unconstrained_commons): New.
* tree.c (array_at_struct_end_p): Do not limit to size of decl for
DECL_COMMONS if flag_unconstrained_commons is set.
* tree-dfa.c (get_ref_base_and_extent): Likewise.
gcc/testsuite/ChangeLog:
* gfortran.dg/unconstrained_commons.f: New.
---
gcc/common.opt | 5 +++++
gcc/doc/invoke.texi | 8 +++++++-
gcc/testsuite/gfortran.dg/unconstrained_commons.f | 20 ++++++++++++++++++++
gcc/tree-dfa.c | 15 ++++++++++++++-
gcc/tree.c | 6 ++++--
5 files changed, 50 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/unconstrained_commons.f
diff --git a/gcc/common.opt b/gcc/common.opt
index 520fa9c..bbf79ef 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2451,6 +2451,11 @@ fsplit-paths
Common Report Var(flag_split_paths) Init(0) Optimization
Split paths leading to loop backedges.
+funconstrained-commons
+Common Var(flag_unconstrained_commons) Optimization
+Assume common declarations may be overridden with ones with a larger
+trailing array.
+
funit-at-a-time
Common Report Var(flag_unit_at_a_time) Init(1)
Compile whole compilation unit at a time.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 0a2a6f4..68933a1 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -407,7 +407,7 @@ Objective-C and Objective-C++ Dialects}.
-ftree-parallelize-loops=@var{n} -ftree-pre -ftree-partial-pre -ftree-pta @gol
-ftree-reassoc -ftree-sink -ftree-slsr -ftree-sra @gol
-ftree-switch-conversion -ftree-tail-merge -ftree-ter @gol
--ftree-vectorize -ftree-vrp @gol
+-ftree-vectorize -ftree-vrp -funconstrained-commons @gol
-funit-at-a-time -funroll-all-loops -funroll-loops @gol
-funsafe-loop-optimizations -funsafe-math-optimizations -funswitch-loops @gol
-fipa-ra -fvariable-expansion-in-unroller -fvect-cost-model -fvpt @gol
@@ -6659,6 +6659,12 @@ the loop optimizer itself cannot prove that these assumptions are valid.
If you use @option{-Wunsafe-loop-optimizations}, the compiler warns you
if it finds this kind of loop.
+@item -funconstrained-commons
+@opindex funconstrained-commons
+This option tells the compiler that variables declared in common blocks
+(e.g. Fortran) may later be overridden with longer trailing arrays. This
+prevents certain optimizations that depend on knowing the array bounds.
+
@item -fcrossjumping
@opindex fcrossjumping
Perform cross-jumping transformation.
diff --git a/gcc/testsuite/gfortran.dg/unconstrained_commons.f b/gcc/testsuite/gfortran.dg/unconstrained_commons.f
new file mode 100644
index 0000000..f9fc471
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unconstrained_commons.f
@@ -0,0 +1,20 @@
+! { dg-do compile }
+! { dg-options "-O3 -funconstrained-commons -fdump-tree-dom2-details" }
+
+! Test for PR69368: a single-element array in a common block, which will be
+! overridden with a larger size at link time (contrary to language spec).
+! Dominator opts considers accesses to differently-computed elements of X as
+! equivalent, unless -funconstrained-commons is passed in.
+ SUBROUTINE FOO
+ IMPLICIT DOUBLE PRECISION (X)
+ INTEGER J
+ COMMON /MYCOMMON / X(1)
+ DO 10 J=1,1024
+ X(J+1)=X(J+7)
+ 10 CONTINUE
+ RETURN
+ END
+! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
+! We should retain both a read and write of mycommon.x.
+! { dg-final { scan-tree-dump-times " _\[0-9\]+ = mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
+! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] = _\[0-9\]+;" 1 "dom2" } }
diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
index 0e98056..f133abc 100644
--- a/gcc/tree-dfa.c
+++ b/gcc/tree-dfa.c
@@ -612,9 +612,22 @@ get_ref_base_and_extent (tree exp, HOST_WIDE_INT *poffset,
if (DECL_P (exp))
{
+ if (flag_unconstrained_commons
+ && TREE_CODE (exp) == VAR_DECL && DECL_COMMON (exp))
+ {
+ tree sz_tree = TYPE_SIZE (TREE_TYPE (exp));
+ /* If size is unknown, or we have read to the end, assume there
+ may be more to the structure than we are told. */
+ if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
+ || (seen_variable_array_ref
+ && (sz_tree == NULL_TREE
+ || TREE_CODE (sz_tree) != INTEGER_CST
+ || (bit_offset + maxsize == wi::to_offset (sz_tree)))))
+ maxsize = -1;
+ }
/* If maxsize is unknown adjust it according to the size of the
base decl. */
- if (maxsize == -1
+ else if (maxsize == -1
&& DECL_SIZE (exp)
&& TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
diff --git a/gcc/tree.c b/gcc/tree.c
index 07cb9d9..9179b37 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -12957,8 +12957,10 @@ array_at_struct_end_p (tree ref)
}
/* If the reference is based on a declared entity, the size of the array
- is constrained by its given domain. */
- if (DECL_P (ref))
+ is constrained by its given domain. (Do not trust commons PR/69368). */
+ if (DECL_P (ref)
+ && !(flag_unconstrained_commons
+ && TREE_CODE (ref) == VAR_DECL && DECL_COMMON (ref)))
return false;
return true;
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006
2016-03-07 11:03 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 (was: Re: [PATCH] Add -funknown-commons ...) Alan Lawrence
@ 2016-03-09 17:54 ` Alan Lawrence
2016-03-10 10:51 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Alan Lawrence @ 2016-03-09 17:54 UTC (permalink / raw)
To: gcc-patches; +Cc: richard.guenther, jakub, tkoenig
On 07/03/16 11:02, Alan Lawrence wrote:
> On 04/03/16 13:27, Richard Biener wrote:
>> I think to make it work with LTO you need to mark it 'Optimization'.
>> Also it's about
>> arrays so maybe
>>
>> 'Assume common declarations may be overridden with ones with a larger
>> trailing array'
>>
>> also if we document it here we should eventually document it in invoke.texi.
>>
>> Not sure if "unknown commons" is a good term, maybe "unconstrained
>> commons" instead?
>
> All done; I doubt there is really a good word, unconstrained seems as good as
> any. I've reused much the same wording in invoke.texi, unless you think there
> is more to add.
>
> On 04/03/16 13:33, Jakub Jelinek wrote:
>> Also, isn't the *.opt description line supposed to end with a full stop?
>
> Ah, yes, thanks.
>
> Is this version OK for trunk?
>
> gcc/ChangeLog:
>
> DATE Alan Lawrence <alan.lawrence@arm.com>
> Jakub Jelinek <jakub@redhat.com>
>
> * common.opt (funconstrained-commons, flag_unconstrained_commons): New.
> * tree.c (array_at_struct_end_p): Do not limit to size of decl for
> DECL_COMMONS if flag_unconstrained_commons is set.
> * tree-dfa.c (get_ref_base_and_extent): Likewise.
And add to that
* doc/invoke.texi (Optimize Options): Add -funconstrained-commons.
(funconstrained-commons): Document.
Thanks,
Alan
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/unconstrained_commons.f: New.
> ---
> gcc/common.opt | 5 +++++
> gcc/doc/invoke.texi | 8 +++++++-
> gcc/testsuite/gfortran.dg/unconstrained_commons.f | 20 ++++++++++++++++++++
> gcc/tree-dfa.c | 15 ++++++++++++++-
> gcc/tree.c | 6 ++++--
> 5 files changed, 50 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gfortran.dg/unconstrained_commons.f
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 520fa9c..bbf79ef 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2451,6 +2451,11 @@ fsplit-paths
> Common Report Var(flag_split_paths) Init(0) Optimization
> Split paths leading to loop backedges.
>
> +funconstrained-commons
> +Common Var(flag_unconstrained_commons) Optimization
> +Assume common declarations may be overridden with ones with a larger
> +trailing array.
> +
> funit-at-a-time
> Common Report Var(flag_unit_at_a_time) Init(1)
> Compile whole compilation unit at a time.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 0a2a6f4..68933a1 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -407,7 +407,7 @@ Objective-C and Objective-C++ Dialects}.
> -ftree-parallelize-loops=@var{n} -ftree-pre -ftree-partial-pre -ftree-pta @gol
> -ftree-reassoc -ftree-sink -ftree-slsr -ftree-sra @gol
> -ftree-switch-conversion -ftree-tail-merge -ftree-ter @gol
> --ftree-vectorize -ftree-vrp @gol
> +-ftree-vectorize -ftree-vrp -funconstrained-commons @gol
> -funit-at-a-time -funroll-all-loops -funroll-loops @gol
> -funsafe-loop-optimizations -funsafe-math-optimizations -funswitch-loops @gol
> -fipa-ra -fvariable-expansion-in-unroller -fvect-cost-model -fvpt @gol
> @@ -6659,6 +6659,12 @@ the loop optimizer itself cannot prove that these assumptions are valid.
> If you use @option{-Wunsafe-loop-optimizations}, the compiler warns you
> if it finds this kind of loop.
>
> +@item -funconstrained-commons
> +@opindex funconstrained-commons
> +This option tells the compiler that variables declared in common blocks
> +(e.g. Fortran) may later be overridden with longer trailing arrays. This
> +prevents certain optimizations that depend on knowing the array bounds.
> +
> @item -fcrossjumping
> @opindex fcrossjumping
> Perform cross-jumping transformation.
> diff --git a/gcc/testsuite/gfortran.dg/unconstrained_commons.f b/gcc/testsuite/gfortran.dg/unconstrained_commons.f
> new file mode 100644
> index 0000000..f9fc471
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/unconstrained_commons.f
> @@ -0,0 +1,20 @@
> +! { dg-do compile }
> +! { dg-options "-O3 -funconstrained-commons -fdump-tree-dom2-details" }
> +
> +! Test for PR69368: a single-element array in a common block, which will be
> +! overridden with a larger size at link time (contrary to language spec).
> +! Dominator opts considers accesses to differently-computed elements of X as
> +! equivalent, unless -funconstrained-commons is passed in.
> + SUBROUTINE FOO
> + IMPLICIT DOUBLE PRECISION (X)
> + INTEGER J
> + COMMON /MYCOMMON / X(1)
> + DO 10 J=1,1024
> + X(J+1)=X(J+7)
> + 10 CONTINUE
> + RETURN
> + END
> +! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
> +! We should retain both a read and write of mycommon.x.
> +! { dg-final { scan-tree-dump-times " _\[0-9\]+ = mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
> +! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] = _\[0-9\]+;" 1 "dom2" } }
> diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
> index 0e98056..f133abc 100644
> --- a/gcc/tree-dfa.c
> +++ b/gcc/tree-dfa.c
> @@ -612,9 +612,22 @@ get_ref_base_and_extent (tree exp, HOST_WIDE_INT *poffset,
>
> if (DECL_P (exp))
> {
> + if (flag_unconstrained_commons
> + && TREE_CODE (exp) == VAR_DECL && DECL_COMMON (exp))
> + {
> + tree sz_tree = TYPE_SIZE (TREE_TYPE (exp));
> + /* If size is unknown, or we have read to the end, assume there
> + may be more to the structure than we are told. */
> + if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
> + || (seen_variable_array_ref
> + && (sz_tree == NULL_TREE
> + || TREE_CODE (sz_tree) != INTEGER_CST
> + || (bit_offset + maxsize == wi::to_offset (sz_tree)))))
> + maxsize = -1;
> + }
> /* If maxsize is unknown adjust it according to the size of the
> base decl. */
> - if (maxsize == -1
> + else if (maxsize == -1
> && DECL_SIZE (exp)
> && TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
> maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
> diff --git a/gcc/tree.c b/gcc/tree.c
> index 07cb9d9..9179b37 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -12957,8 +12957,10 @@ array_at_struct_end_p (tree ref)
> }
>
> /* If the reference is based on a declared entity, the size of the array
> - is constrained by its given domain. */
> - if (DECL_P (ref))
> + is constrained by its given domain. (Do not trust commons PR/69368). */
> + if (DECL_P (ref)
> + && !(flag_unconstrained_commons
> + && TREE_CODE (ref) == VAR_DECL && DECL_COMMON (ref)))
> return false;
>
> return true;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006
2016-03-09 17:54 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 Alan Lawrence
@ 2016-03-10 10:51 ` Richard Biener
0 siblings, 0 replies; 12+ messages in thread
From: Richard Biener @ 2016-03-10 10:51 UTC (permalink / raw)
To: Alan Lawrence; +Cc: GCC Patches, Jakub Jelinek, Thomas Koenig
On Wed, Mar 9, 2016 at 6:54 PM, Alan Lawrence
<alan.lawrence@foss.arm.com> wrote:
> On 07/03/16 11:02, Alan Lawrence wrote:
>>
>> On 04/03/16 13:27, Richard Biener wrote:
>>>
>>> I think to make it work with LTO you need to mark it 'Optimization'.
>>> Also it's about
>>> arrays so maybe
>>>
>>> 'Assume common declarations may be overridden with ones with a larger
>>> trailing array'
>>>
>>> also if we document it here we should eventually document it in
>>> invoke.texi.
>>>
>>> Not sure if "unknown commons" is a good term, maybe "unconstrained
>>> commons" instead?
>>
>>
>> All done; I doubt there is really a good word, unconstrained seems as good
>> as
>> any. I've reused much the same wording in invoke.texi, unless you think
>> there
>> is more to add.
>>
>> On 04/03/16 13:33, Jakub Jelinek wrote:
>>>
>>> Also, isn't the *.opt description line supposed to end with a full stop?
>>
>>
>> Ah, yes, thanks.
>>
>> Is this version OK for trunk?
>>
>> gcc/ChangeLog:
>>
>> DATE Alan Lawrence <alan.lawrence@arm.com>
>> Jakub Jelinek <jakub@redhat.com>
>>
>> * common.opt (funconstrained-commons,
>> flag_unconstrained_commons): New.
>> * tree.c (array_at_struct_end_p): Do not limit to size of decl
>> for
>> DECL_COMMONS if flag_unconstrained_commons is set.
>> * tree-dfa.c (get_ref_base_and_extent): Likewise.
>
>
> And add to that
> * doc/invoke.texi (Optimize Options): Add -funconstrained-commons.
> (funconstrained-commons): Document.
Ok.
Richard.
> Thanks,
> Alan
>
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gfortran.dg/unconstrained_commons.f: New.
>> ---
>> gcc/common.opt | 5 +++++
>> gcc/doc/invoke.texi | 8 +++++++-
>> gcc/testsuite/gfortran.dg/unconstrained_commons.f | 20
>> ++++++++++++++++++++
>> gcc/tree-dfa.c | 15 ++++++++++++++-
>> gcc/tree.c | 6 ++++--
>> 5 files changed, 50 insertions(+), 4 deletions(-)
>> create mode 100644 gcc/testsuite/gfortran.dg/unconstrained_commons.f
>>
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index 520fa9c..bbf79ef 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -2451,6 +2451,11 @@ fsplit-paths
>> Common Report Var(flag_split_paths) Init(0) Optimization
>> Split paths leading to loop backedges.
>>
>> +funconstrained-commons
>> +Common Var(flag_unconstrained_commons) Optimization
>> +Assume common declarations may be overridden with ones with a larger
>> +trailing array.
>> +
>> funit-at-a-time
>> Common Report Var(flag_unit_at_a_time) Init(1)
>> Compile whole compilation unit at a time.
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 0a2a6f4..68933a1 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -407,7 +407,7 @@ Objective-C and Objective-C++ Dialects}.
>> -ftree-parallelize-loops=@var{n} -ftree-pre -ftree-partial-pre
>> -ftree-pta @gol
>> -ftree-reassoc -ftree-sink -ftree-slsr -ftree-sra @gol
>> -ftree-switch-conversion -ftree-tail-merge -ftree-ter @gol
>> --ftree-vectorize -ftree-vrp @gol
>> +-ftree-vectorize -ftree-vrp -funconstrained-commons @gol
>> -funit-at-a-time -funroll-all-loops -funroll-loops @gol
>> -funsafe-loop-optimizations -funsafe-math-optimizations -funswitch-loops
>> @gol
>> -fipa-ra -fvariable-expansion-in-unroller -fvect-cost-model -fvpt @gol
>> @@ -6659,6 +6659,12 @@ the loop optimizer itself cannot prove that these
>> assumptions are valid.
>> If you use @option{-Wunsafe-loop-optimizations}, the compiler warns you
>> if it finds this kind of loop.
>>
>> +@item -funconstrained-commons
>> +@opindex funconstrained-commons
>> +This option tells the compiler that variables declared in common blocks
>> +(e.g. Fortran) may later be overridden with longer trailing arrays. This
>> +prevents certain optimizations that depend on knowing the array bounds.
>> +
>> @item -fcrossjumping
>> @opindex fcrossjumping
>> Perform cross-jumping transformation.
>> diff --git a/gcc/testsuite/gfortran.dg/unconstrained_commons.f
>> b/gcc/testsuite/gfortran.dg/unconstrained_commons.f
>> new file mode 100644
>> index 0000000..f9fc471
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/unconstrained_commons.f
>> @@ -0,0 +1,20 @@
>> +! { dg-do compile }
>> +! { dg-options "-O3 -funconstrained-commons -fdump-tree-dom2-details" }
>> +
>> +! Test for PR69368: a single-element array in a common block, which will
>> be
>> +! overridden with a larger size at link time (contrary to language spec).
>> +! Dominator opts considers accesses to differently-computed elements of X
>> as
>> +! equivalent, unless -funconstrained-commons is passed in.
>> + SUBROUTINE FOO
>> + IMPLICIT DOUBLE PRECISION (X)
>> + INTEGER J
>> + COMMON /MYCOMMON / X(1)
>> + DO 10 J=1,1024
>> + X(J+1)=X(J+7)
>> + 10 CONTINUE
>> + RETURN
>> + END
>> +! { dg-final { scan-tree-dump-not "FIND" "dom2" } }
>> +! We should retain both a read and write of mycommon.x.
>> +! { dg-final { scan-tree-dump-times " _\[0-9\]+ =
>> mycommon\\.x\\\[_\[0-9\]+\\\];" 1 "dom2" } }
>> +! { dg-final { scan-tree-dump-times " mycommon\\.x\\\[_\[0-9\]+\\\] =
>> _\[0-9\]+;" 1 "dom2" } }
>> diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c
>> index 0e98056..f133abc 100644
>> --- a/gcc/tree-dfa.c
>> +++ b/gcc/tree-dfa.c
>> @@ -612,9 +612,22 @@ get_ref_base_and_extent (tree exp, HOST_WIDE_INT
>> *poffset,
>>
>> if (DECL_P (exp))
>> {
>> + if (flag_unconstrained_commons
>> + && TREE_CODE (exp) == VAR_DECL && DECL_COMMON (exp))
>> + {
>> + tree sz_tree = TYPE_SIZE (TREE_TYPE (exp));
>> + /* If size is unknown, or we have read to the end, assume there
>> + may be more to the structure than we are told. */
>> + if (TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE
>> + || (seen_variable_array_ref
>> + && (sz_tree == NULL_TREE
>> + || TREE_CODE (sz_tree) != INTEGER_CST
>> + || (bit_offset + maxsize == wi::to_offset
>> (sz_tree)))))
>> + maxsize = -1;
>> + }
>> /* If maxsize is unknown adjust it according to the size of the
>> base decl. */
>> - if (maxsize == -1
>> + else if (maxsize == -1
>> && DECL_SIZE (exp)
>> && TREE_CODE (DECL_SIZE (exp)) == INTEGER_CST)
>> maxsize = wi::to_offset (DECL_SIZE (exp)) - bit_offset;
>> diff --git a/gcc/tree.c b/gcc/tree.c
>> index 07cb9d9..9179b37 100644
>> --- a/gcc/tree.c
>> +++ b/gcc/tree.c
>> @@ -12957,8 +12957,10 @@ array_at_struct_end_p (tree ref)
>> }
>>
>> /* If the reference is based on a declared entity, the size of the
>> array
>> - is constrained by its given domain. */
>> - if (DECL_P (ref))
>> + is constrained by its given domain. (Do not trust commons
>> PR/69368). */
>> + if (DECL_P (ref)
>> + && !(flag_unconstrained_commons
>> + && TREE_CODE (ref) == VAR_DECL && DECL_COMMON (ref)))
>> return false;
>>
>> return true;
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-03-10 10:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-19 17:42 [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006 Alan Lawrence
2016-02-19 17:53 ` Jakub Jelinek
2016-02-22 11:46 ` Alan Lawrence
2016-02-22 12:03 ` Jakub Jelinek
2016-02-23 11:42 ` Alan Lawrence
2016-02-25 18:00 ` Alan Lawrence
2016-03-03 10:20 ` Alan Lawrence
2016-03-04 13:27 ` Richard Biener
2016-03-04 13:33 ` Jakub Jelinek
2016-03-07 11:03 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 (was: Re: [PATCH] Add -funknown-commons ...) Alan Lawrence
2016-03-09 17:54 ` [PATCH] Add -funconstrained-commons to work around PR/69368 (and others) in SPEC2006 Alan Lawrence
2016-03-10 10:51 ` Richard Biener
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).