Re: [PATCH] Remove bogus optimization from may_alias_p

[Richard Guenther <rguenther at suse dot de>]

On Mon, 20 Oct 2008, Richard Guenther wrote:

> On Mon, 20 Oct 2008, Diego Novillo wrote:
> 
> > 2008/10/19 Richard Guenther <rguenther@suse.de>:
> > 
> > > This removes a test where one subtest is always false and the
> > > other one is always true if unmodifiable_var_p (var) is true,
> > > which is obviously bogus.  On the trunk this causes false
> > > TBAA pruning of points-to sets.
> > 
> > This also prunes the case where a regular pointer is made to point to
> > a read-only variable.  The user gets a warning about mismatched const
> > settings.  So TBAA should be able to prune this.
> 
> Well, the basic problem of the way the test is implemented is that it
> compares unmodifiable_var_p on a SMT (which is always false) with
> unmodifiable_var_p on a decl.  So we have
> 
>   if ((unmodifiable_var_p (mem) && !unmodifiable_var_p (var))
>       || (unmodifiable_var_p (var) && !unmodifiable_var_p (mem)))
> 
> that is equivalent to
> 
>   if (false
>       || (unmodifiable_var_p (var) && true))
> 
> which makes us always prune readonly variables from points-to sets.
> Even if the pointer (which is represented as SMT here) is "readonly"
> (though, as you might know, the GCC middle-end doesn't know a thing
> like a pointer that points to readonly memory - at least the const
> qualifier cannot be reliably used for this)
> 
> > You also need a test case.
> 
> I can add one scanning the alias dump for the resulting points-to set.
> Or just wait for my fix of PR36509 which will cause bootstrap failures
> due to bogus alias warnings if the PTA set pruning is not fixed.

Here is one.  I didn't succeed in doing a testcase that gets miscompiled,
as our optimizers do not use points-to information (yet).

Consider

const static int a;

int foo(int i)
{
  const int *q;
  int b;
  if (i)
    q = &a;
  else
    q = &b;
  b = 1;
  return *q;
}

for which (before the patch) computed

  Points-to sets
  q_1 = { a b }

ok, that's still unpruned

  Flow-sensitive alias information for foo
  q_1, name memory tag: NMT.10, is dereferenced, points-to vars: { b }

oops, where did a go?  we pruned it.  The IL looks then like

<bb 4>:
  # q_1 = PHI <&a(2), &b(3)>
  # b_7 = VDEF <b_6(D)>
  b = 1;
  # VUSE <b_7>
  D.1239_5 = *q_1;
  return D.1239_5;

now we are lucky that nobody optimizes *q_1 to 1.  After the patch
we correctly do not prune a and we get

<bb 4>:
  # q_1 = PHI <&a(2), &b(3)>
  # b_7 = VDEF <b_6(D)>
  b = 1;
  # VUSE <a_8(D), b_7>
  D.1239_5 = *q_1;
  return D.1239_5;

which is correct.

I'll install the following.

Richard.

2008-10-21  Richard Guenther  <rguenther@suse.de>

	* gcc.dg/tree-ssa/alias-19.c: New testcase.

Index: gcc/testsuite/gcc.dg/tree-ssa/alias-19.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/alias-19.c	(revision 0)
--- gcc/testsuite/gcc.dg/tree-ssa/alias-19.c	(revision 0)
***************
*** 0 ****
--- 1,31 ----
+ /* { dg-do run } */
+ /* { dg-options "-O2 -fdump-tree-alias-vops" } */
+ 
+ const static int a;
+ 
+ int __attribute__((noinline))
+ foo(int i)
+ {
+   const int *q;
+   int b;
+   if (i)
+     q = &a;
+   else
+     q = &b;
+   b = 1;
+   /* We should not prune a from the points-to set of q.  */
+   return *q;
+ }
+ 
+ extern void abort (void);
+ int main()
+ {
+   if (foo(1) != 0)
+     abort ();
+   return 0;
+ }
+ 
+ /* { dg-final { scan-tree-dump "q_. = { a b }" "alias" } } */
+ /* { dg-final { scan-tree-dump "q_., name memory tag: NMT..., is dereferenced, points-to vars: { a b }" "alias" } } */
+ /* { dg-final { scan-tree-dump "# VUSE <a_.\\\(D\\\), b_.>" "alias" } } */
+ /* { dg-final { cleanup-tree-dump "alias" } } */