Re: [PATCH] Add -funknown-commons to work around PR/69368 (and others) in SPEC2006

[Alan Lawrence <alan dot lawrence at arm dot com>]

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