Re: [PATCH] Require type compatible bases in DDR initialization (PR tree-optimization/70127)

[Richard Biener <rguenther at suse dot de>]

On March 8, 2016 7:04:37 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>Honza has removed types_compatible_p call from operand_equal_p of
>*MEM_REF.
>This breaks tree-data-ref.c stuff, as the access fns are built with the
>assumption that they refer to the same stuff (array indices or field
>bit
>offsets) between all DRs whose bases are operand_equal_p, but now that
>it ensures just that they have the same size, one could have a type of
>single element array of records, the other the record itself, or one
>could
>be a record with a single field, another that field's type, and we
>suddenly
>can have DR_ACCESS_FN (, 0) in one DR being array index and bitfield
>offset
>in another one etc.
>
>To me the safest fix looks to be just to revert to what 5.x used to do
>here
>for initialize_data_dependence_relation, i.e. require compatible types,
>perhaps later we could carefully lift up that restriction to either
>not creating access fns for single element arrays (but watch for
>flexible
>array-like arrays) or fields over whole structure side, or compare
>types
>more losely, or compare also sizes of refs themselves, ...

I believe the safest fix is to re-instantiate the compatibility check by refactoring operand_equal_p to perform it on the full ref (but not recursions where it would be redundant and maybe too conservative).
I've noticed this as well when doing the last operand_equal_p surgery, esp. The incomplete and bogus half-way type checking done at its top.

Richard.

>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
>2016-03-08  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/70127
>	* tree-data-ref.c (initialize_data_dependence_relation): Return
>	chrec_dont_know if types of DR_BASE_OBJECT aren't compatible.
>
>	* gcc.c-torture/execute/pr70127.c: New test.
>
>--- gcc/tree-data-ref.c.jj	2016-01-27 12:38:52.000000000 +0100
>+++ gcc/tree-data-ref.c	2016-03-08 12:01:11.865034378 +0100
>@@ -1539,7 +1539,13 @@ initialize_data_dependence_relation (str
> 
>   /* If the references do not access the same object, we do not know
>      whether they alias or not.  */
>-  if (!operand_equal_p (DR_BASE_OBJECT (a), DR_BASE_OBJECT (b), 0))
>+  if (!operand_equal_p (DR_BASE_OBJECT (a), DR_BASE_OBJECT (b), 0)
>+      /* operand_equal_p checked just type sizes; with single element
>+	 array types and/or fields that have the same size as containing
>+	 struct/union those could match, even when DR_ACCESS_FN count
>+	 different things between the two DRs.  See PR70127.  */
>+      || !types_compatible_p (TREE_TYPE (DR_BASE_OBJECT (a)),
>+			      TREE_TYPE (DR_BASE_OBJECT (b))))
>     {
>       DDR_ARE_DEPENDENT (res) = chrec_dont_know;
>       return res;
>--- gcc/testsuite/gcc.c-torture/execute/pr70127.c.jj	2016-03-08
>12:11:11.890835632 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/pr70127.c	2016-03-08
>12:10:58.000000000 +0100
>@@ -0,0 +1,23 @@
>+/* PR tree-optimization/70127 */
>+
>+struct S { int f; signed int g : 2; } a[1], c = {5, 1}, d;
>+short b;
>+
>+__attribute__((noinline, noclone)) void
>+foo (int x)
>+{
>+  if (x != 1)
>+    __builtin_abort ();
>+}
>+
>+int
>+main ()
>+{
>+  while (b++ <= 0)
>+    {
>+      struct S e = {1, 1};
>+      d = e = a[0] = c;
>+    }
>+  foo (a[0].g);
>+  return 0;
>+}
>
>	Jakub