RFA: Fix path based disambiguation in RTL oracle

Hi,

I haven't yet checked in my patch from last week about not creating 
alias-set conflicts for stack slot sharing mostly due to the caveat I 
mentioned.  Namely that the RTL disambiguator still uses path-based means 
(which belong to type-based aliasing) even in the write_dependence tests.  
That's really wrong, but I couldn't (and I think can't) create a testcase 
using stack slot sharing that would fail with the current unchanged 
compiler.

Luckily, we don't need stack slot sharing to demonstrate the bug.  The 
testcase below will fail on x86_64 with -O2 -fschedule-insns.  I've added 
it to the torture so it has a chance to fail on other targets too (e.g. on 
i686 it doesn't because it doesn't want to do the "wrong" swap of two 
insns).

I've retained the path based disambiguation in the true_dependence 
variants, where it is okay according to our mem model.

There are some cleanup opportunities in checking what the O(N^2) algorithm 
of nonoverlapping_component_refs_p catches that rtx_refs_may_alias_p 
doesn't.  We know already some cases where MEM_OFFSET_KNOWN_P is set too 
conservatively for the latter to work on some cases.  In any case that 
would be a follow-up.

So, patch fixes testcase and is in regstrap on x86_64-linux.  Okay if that 
passes?


Ciao,
Michael.
------------------------
	* alias.c (nonoverlapping_component_refs_p): Take two rtx arguments.
	(nonoverlapping_memrefs_p): Don't call it here ...
	(true_dependence_1): ... but here.

testsuite/
	* gcc.dg/torture/alias-1.c: New test.

On Mon, Jun 11, 2012 at 4:19 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> I haven't yet checked in my patch from last week about not creating
> alias-set conflicts for stack slot sharing mostly due to the caveat I
> mentioned.  Namely that the RTL disambiguator still uses path-based means
> (which belong to type-based aliasing) even in the write_dependence tests.
> That's really wrong, but I couldn't (and I think can't) create a testcase
> using stack slot sharing that would fail with the current unchanged
> compiler.
>
> Luckily, we don't need stack slot sharing to demonstrate the bug.  The
> testcase below will fail on x86_64 with -O2 -fschedule-insns.  I've added
> it to the torture so it has a chance to fail on other targets too (e.g. on
> i686 it doesn't because it doesn't want to do the "wrong" swap of two
> insns).
>
> I've retained the path based disambiguation in the true_dependence
> variants, where it is okay according to our mem model.
>
> There are some cleanup opportunities in checking what the O(N^2) algorithm
> of nonoverlapping_component_refs_p catches that rtx_refs_may_alias_p
> doesn't.  We know already some cases where MEM_OFFSET_KNOWN_P is set too
> conservatively for the latter to work on some cases.  In any case that
> would be a follow-up.
>
> So, patch fixes testcase and is in regstrap on x86_64-linux.  Okay if that
> passes?

Ok.

Thanks,
Richard.

>
> Ciao,
> Michael.
> ------------------------
>        * alias.c (nonoverlapping_component_refs_p): Take two rtx arguments.
>        (nonoverlapping_memrefs_p): Don't call it here ...
>        (true_dependence_1): ... but here.
>
> testsuite/
>        * gcc.dg/torture/alias-1.c: New test.
>
> Index: alias.c
> ===================================================================
> --- alias.c     (revision 188384)
> +++ alias.c     (working copy)
> @@ -156,7 +156,7 @@ static rtx find_base_value (rtx);
>  static int mems_in_disjoint_alias_sets_p (const_rtx, const_rtx);
>  static int insert_subset_children (splay_tree_node, void*);
>  static alias_set_entry get_alias_set_entry (alias_set_type);
> -static bool nonoverlapping_component_refs_p (const_tree, const_tree);
> +static bool nonoverlapping_component_refs_p (const_rtx, const_rtx);
>  static tree decl_for_component_ref (tree);
>  static int write_dependence_p (const_rtx, const_rtx, int);
>
> @@ -2184,11 +2184,15 @@ read_dependence (const_rtx mem, const_rt
>    overlap for any pair of objects.  */
>
>  static bool
> -nonoverlapping_component_refs_p (const_tree x, const_tree y)
> +nonoverlapping_component_refs_p (const_rtx rtlx, const_rtx rtly)
>  {
> +  const_tree x = MEM_EXPR (rtlx), y = MEM_EXPR (rtly);
>   const_tree fieldx, fieldy, typex, typey, orig_y;
>
> -  if (!flag_strict_aliasing)
> +  if (!flag_strict_aliasing
> +      || !x || !y
> +      || TREE_CODE (x) != COMPONENT_REF
> +      || TREE_CODE (y) != COMPONENT_REF)
>     return false;
>
>   do
> @@ -2307,13 +2311,6 @@ nonoverlapping_memrefs_p (const_rtx x, c
>          && ! MEM_OFFSET_KNOWN_P (y)))
>     return 0;
>
> -  /* If both are field references, we may be able to determine something.  */
> -  if (TREE_CODE (exprx) == COMPONENT_REF
> -      && TREE_CODE (expry) == COMPONENT_REF
> -      && nonoverlapping_component_refs_p (exprx, expry))
> -    return 1;
> -
> -
>   /* If the field reference test failed, look at the DECLs involved.  */
>   moffsetx_known_p = MEM_OFFSET_KNOWN_P (x);
>   if (moffsetx_known_p)
> @@ -2519,6 +2516,9 @@ true_dependence_1 (const_rtx mem, enum m
>   if (nonoverlapping_memrefs_p (mem, x, false))
>     return 0;
>
> +  if (nonoverlapping_component_refs_p (mem, x))
> +    return 0;
> +
>   return rtx_refs_may_alias_p (x, mem, true);
>  }
>
> Index: testsuite/gcc.dg/torture/alias-1.c
> ===================================================================
> --- testsuite/gcc.dg/torture/alias-1.c  (revision 0)
> +++ testsuite/gcc.dg/torture/alias-1.c  (revision 0)
> @@ -0,0 +1,38 @@
> +/* { dg-do run } */
> +/* { dg-options "-fschedule-insns" } */
> +
> +extern void abort (void) __attribute__((noreturn));
> +
> +struct B { int a; int b;};
> +struct wrapper {
> +union setconflict
> +{
> +  struct S { char one1; struct B b1; } s;
> +  struct T { struct B b2; char two2; } t;
> +} a;
> +};
> +
> +int
> +main ()
> +{
> +  int sum = 0;
> +  int i;
> +  struct wrapper w;
> +  struct B *p;
> +
> +  p = &w.a.s.b1;
> +  asm ("": "=r" (p):"0" (p));
> +  p->a = 0;
> +  asm ("": "=r" (p):"0" (p));
> +  sum += p->a;
> +
> +  p = &w.a.t.b2;
> +  asm ("": "=r" (p):"0" (p));
> +  p->b = 1;
> +  asm ("": "=r" (p):"0" (p));
> +  sum += p->b;
> +
> +  if (sum != 1)
> +    abort();
> +  return 0;
> +}