Bug 34677 - PREs insert_fake_stores is mostly useless
Summary: PREs insert_fake_stores is mostly useless
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P3 enhancement
Target Milestone: 4.4.0
Assignee: Richard Biener
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2008-01-04 16:18 UTC by Richard Biener
Modified: 2008-03-10 17:15 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-03-10 14:39:27


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2008-01-04 16:18:57 UTC
Because it only handles stores through direct INDIRECT_REFs.  So while we handle
testcases like loadpre4.c:

int main(int *a, int argc)
{
  int b;
  int c;
  int i;
  int d, e;

  for (i = 0; i < argc; i++)
    {
      e = *a;
      *a = 9;
    }
  return d + e;
}

the much simpler

int a;

int foo(int argc)
{
  int b;
  int c;
  int i;
  int d, e;

  for (i = 0; i < argc; i++)
    {
      e = a;
      a = 9;
    }
  return d + e;
}

or the only slightly more complex

struct {
  int a;
  int large[100];
} x;

int foo(int argc)
{
  int b;
  int c;
  int i;
  int d, e;

  for (i = 0; i < argc; i++)
    {
      e = x.a;
      x.a = 9;
    }
  return d + e;
}

is not handled.  If you disable insertion of fake stores completely, only
two loadpre tests fail in tree-ssa.exp.  Did the new SCCVN obsolete some
cases where we needed the fake stores before but do not so now?
Comment 1 Andrew Pinski 2008-01-04 16:57:08 UTC
Isn't there already a bug about load PRE with global variables?
Comment 2 Richard Biener 2008-01-04 17:00:21 UTC
The plain DECL does not work because

  1) we don't do the storetmp
  2) can_PRE_operation and can_value_number_operation return false (so we
     do not enter a into EXP_GEN
  3) create_value_expr_from cannot handle it

most of the rest of PRE looks like it may handle DECLs, but if you fix the
above, vn_lookup_with_vuses still not finds VH.10, because it is not ANTIC_IN.

Comment 3 Richard Biener 2008-01-04 17:03:05 UTC
I think the other PR is only about VN global constants.  But also an incoming
pointer to a structure is not handled:

struct X { int i; };
int foo(struct X *a, int argc)
{
  int b;
  int c;
  int i;
  int d, e;

  for (i = 0; i < argc; i++)
    {
      e = a->i;
      a->i = 9;
    }
  return d + e;
}
Comment 4 Daniel Berlin 2008-01-04 17:31:02 UTC
Subject: Re:  PREs insert_fake_stores is mostly useless

FRE will handle DECL's, and VN will value number them.
I've made small attempts at make PRE work with globals, but i'm lazy
and haven't done it for real yet :)
It is trickier than it would initially seem.

On 4 Jan 2008 17:00:21 -0000, rguenth at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #2 from rguenth at gcc dot gnu dot org  2008-01-04 17:00 -------
> The plain DECL does not work because
>
>   1) we don't do the storetmp
>   2) can_PRE_operation and can_value_number_operation return false (so we
>      do not enter a into EXP_GEN
>   3) create_value_expr_from cannot handle it
>
> most of the rest of PRE looks like it may handle DECLs, but if you fix the
> above, vn_lookup_with_vuses still not finds VH.10, because it is not ANTIC_IN.
>
>
>
> --
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34677
>
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
>
Comment 5 Richard Biener 2008-03-10 14:39:26 UTC
I have a patch for the non-PRE-of-globals part.
Comment 6 Richard Biener 2008-03-10 17:15:31 UTC
Subject: Bug 34677

Author: rguenth
Date: Mon Mar 10 17:14:45 2008
New Revision: 133081

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133081
Log:
2008-03-10  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/34677
	* tree-ssa-pre.c (modify_expr_node_pool): Remove.
	(poolify_tree): Likewise.
	(modify_expr_template): Likewise.
	(poolify_modify_stmt): Likewise.
	(insert_fake_stores): Handle all component-ref style stores
	in addition to INDIRECT_REF.  Also handle complex types.
	Do not poolify the inserted load.
	(realify_fake_stores): Do not rebuild the tree but only
	make it a SSA_NAME copy.
	(init_pre): Remove initialzation of modify_expr_template.
	Do not allocate modify_expr_node_pool.
	(fini_pre): Do not free modify_expr_node_pool.

	* gcc.dg/tree-ssa/loadpre23.c: New testcase.
	* gcc.dg/tree-ssa/loadpre24.c: Likewise.
	* gcc.dg/tree-ssa/loadpre25.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/loadpre23.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/loadpre24.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/loadpre25.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-pre.c

Comment 7 Richard Biener 2008-03-10 17:15:35 UTC
Fixed.