[PATCH] Disable generation of SFTs and adjust testsuite accordingly

[PATCH] Disable generation of SFTs and adjust testsuite accordingly

[Richard Guenther]

This patch disables generating SFTs by default and adjusts a few testcases
for this.  This is to let us see now where this makes a difference
(and thus the alias oracles need to be improved or their use spreaded).
Previous testing of this patch on SPEC2000 showed increased performance
without SFTs, possibly related to partitioning oddities especially with
SFTs.  Disabling SFTs should also address some of the compile-time
bugreports that are still open.

Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for mainline?

Thanks,
Richard.

2008-04-15  Richard Guenther  <rguenther@suse.de>

	* params.def (PARAM_MAX_FIELDS_FOR_FIELD_SENSITIVE): Set default
	to zero, thus disable creation of SFTs.

	* gcc.dg/tree-ssa/salias-1.c: Remove.
	* gcc.dg/tree-ssa/pr26421.c: Adjust pattern.
	* gcc.dg/tree-ssa/alias-15.c: Likewise.
	* gcc.dg/tree-ssa/ssa-lim-3.c: Run at -O2.

Index: params.def
===================================================================
*** params.def	(revision 134313)
--- params.def	(working copy)
*************** DEFPARAM (PARAM_MAX_JUMP_THREAD_DUPLICAT
*** 652,658 ****
  DEFPARAM (PARAM_MAX_FIELDS_FOR_FIELD_SENSITIVE,
            "max-fields-for-field-sensitive",
  	  "Maximum number of fields in a structure before pointer analysis treats the structure as a single variable",
! 	  100, 0, 0)
  
  DEFPARAM(PARAM_MAX_SCHED_READY_INSNS,
  	 "max-sched-ready-insns",
--- 652,658 ----
  DEFPARAM (PARAM_MAX_FIELDS_FOR_FIELD_SENSITIVE,
            "max-fields-for-field-sensitive",
  	  "Maximum number of fields in a structure before pointer analysis treats the structure as a single variable",
! 	  0, 0, 0)
  
  DEFPARAM(PARAM_MAX_SCHED_READY_INSNS,
  	 "max-sched-ready-insns",
Index: testsuite/gcc.dg/tree-ssa/salias-1.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/salias-1.c	(revision 134313)
--- testsuite/gcc.dg/tree-ssa/salias-1.c	(working copy)
***************
*** 1,19 ****
- /* { dg-do compile } */
- /* { dg-options "-O2 -fdump-tree-salias" } */
- 
- struct {
-   struct {
-     struct {
- 	int i, j;
-     } c;
-   } b;
- } a;
- 
- int foo(void)
- {
-   a.b.c.i = 0;
-   return a.b.c.j;
- }
- 
- /* { dg-final { scan-tree-dump-times "structure field tag SFT" 2 "salias" } } */
- /* { dg-final { cleanup-tree-dump "salias" } } */
--- 0 ----
Index: testsuite/gcc.dg/tree-ssa/pr26421.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/pr26421.c	(revision 134313)
--- testsuite/gcc.dg/tree-ssa/pr26421.c	(working copy)
*************** int foo(void)
*** 16,20 ****
    return a.i;
  }
  
! /* { dg-final { scan-tree-dump-times "VDEF" 4 "salias" } } */
  /* { dg-final { cleanup-tree-dump "salias" } } */
--- 16,22 ----
    return a.i;
  }
  
! /* Verify the call clobbers all of a.  */
! 
! /* { dg-final { scan-tree-dump-times "VDEF <a_" 2 "salias" } } */
  /* { dg-final { cleanup-tree-dump "salias" } } */
Index: testsuite/gcc.dg/tree-ssa/alias-15.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/alias-15.c	(revision 134313)
--- testsuite/gcc.dg/tree-ssa/alias-15.c	(working copy)
*************** static inline struct X *wrap(struct X *p
*** 11,20 ****
  int test2(void)
  {
    struct X *p = wrap(&m.b);
!   /* Both memory references need to alias the same SFT.  */
    return p->b[3] - m.b.b[3];
  }
  
! /* { dg-final { scan-tree-dump "SFT.5 created for var m offset 128" "salias" } } */
! /* { dg-final { scan-tree-dump-times "VUSE <SFT.5_" 2 "salias" } } */
  /* { dg-final { cleanup-tree-dump "salias" } } */
--- 11,19 ----
  int test2(void)
  {
    struct X *p = wrap(&m.b);
!   /* Both memory references need to alias the same tags.  */
    return p->b[3] - m.b.b[3];
  }
  
! /* { dg-final { scan-tree-dump-times "VUSE <m_.\\\(D\\\)>" 2 "salias" } } */
  /* { dg-final { cleanup-tree-dump "salias" } } */
Index: testsuite/gcc.dg/tree-ssa/ssa-lim-3.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/ssa-lim-3.c	(revision 134313)
--- testsuite/gcc.dg/tree-ssa/ssa-lim-3.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-O -fdump-tree-lim-details" } */
  
  struct { int x; int y; } global;
  void foo(int n)
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-O2 -fdump-tree-lim-details" } */
  
  struct { int x; int y; } global;
  void foo(int n)

Re: [PATCH] Disable generation of SFTs and adjust testsuite accordingly

[Diego Novillo]

On 4/15/08 11:22 AM, Richard Guenther wrote:

> 	* params.def (PARAM_MAX_FIELDS_FOR_FIELD_SENSITIVE): Set default
> 	to zero, thus disable creation of SFTs.
> 
> 	* gcc.dg/tree-ssa/salias-1.c: Remove.
> 	* gcc.dg/tree-ssa/pr26421.c: Adjust pattern.
> 	* gcc.dg/tree-ssa/alias-15.c: Likewise.
> 	* gcc.dg/tree-ssa/ssa-lim-3.c: Run at -O2.

OK.  Thank you very much for doing this.

Perhaps we should plan to remove SFTs altogether in stage 2.  In time, 
they're going to rot.


Diego.

Re: [PATCH] Disable generation of SFTs and adjust testsuite accordingly

[Richard Guenther]

On Tue, 15 Apr 2008, Diego Novillo wrote:

> On 4/15/08 11:22 AM, Richard Guenther wrote:
> 
> >  * params.def (PARAM_MAX_FIELDS_FOR_FIELD_SENSITIVE): Set default
> >  to zero, thus disable creation of SFTs.
> > 
> >  * gcc.dg/tree-ssa/salias-1.c: Remove.
> >  * gcc.dg/tree-ssa/pr26421.c: Adjust pattern.
> >  * gcc.dg/tree-ssa/alias-15.c: Likewise.
> >  * gcc.dg/tree-ssa/ssa-lim-3.c: Run at -O2.
> 
> OK.  Thank you very much for doing this.
> 
> Perhaps we should plan to remove SFTs altogether in stage 2.  In time, they're
> going to rot.

Yes, I plan to do this as well.  For the moment let's keep them to
be able to easily compare results with the same code-base otherwise.

Richard.

Re: [PATCH] Disable generation of SFTs and adjust testsuite accordingly

[David Edelsohn]

Richi,

	Removing SFTs is a great goal and removing the infrastructure
early during this GCC development cycle is a good choice.  Did you test
the performance impact of diabling SFTs on architectures other than x86,
either yourself or asking others for help?

	The change likely is a general win, but I think it would have been
a good idea to test the effect of disabling an entire facet of GCC's alias
analysis infrastructure on multiple architectures before applying the
patch.  Waiting for people to notice something amiss on auto-testers and
tracking it back to the patch is not very friendly.  Again, I am not
suggesting that you measure the performance yourself on all architectures
-- I am sure that other GCC developers would have been happy to test the
patch if asked.

	I am glad that you are taking the lead to move GCC to a better
alias analysis infrastructure.  I think you would have a stronger
justification if you worked with other people to test the patch instead of
waiting for problem reports.

Thanks, David

Re: [PATCH] Disable generation of SFTs and adjust testsuite accordingly

[Richard Guenther]

On Tue, 15 Apr 2008, David Edelsohn wrote:

> Richi,
> 
> 	Removing SFTs is a great goal and removing the infrastructure
> early during this GCC development cycle is a good choice.  Did you test
> the performance impact of diabling SFTs on architectures other than x86,
> either yourself or asking others for help?
> 
> 	The change likely is a general win, but I think it would have been
> a good idea to test the effect of disabling an entire facet of GCC's alias
> analysis infrastructure on multiple architectures before applying the
> patch.  Waiting for people to notice something amiss on auto-testers and
> tracking it back to the patch is not very friendly.  Again, I am not
> suggesting that you measure the performance yourself on all architectures
> -- I am sure that other GCC developers would have been happy to test the
> patch if asked.
> 
> 	I am glad that you are taking the lead to move GCC to a better
> alias analysis infrastructure.  I think you would have a stronger
> justification if you worked with other people to test the patch instead of
> waiting for problem reports.

Disabling SFTs didn't come as a surprise.  SPEC and testsuite results
for x86_64 were posted early in March

http://gcc.gnu.org/ml/gcc/2008-03/msg00249.html

which is where you could have followed up with either a request for
more testing or proactively could have followed up with results for
the architecture you care about.

I will happily look into fallout of disabling SFTs, but as SPEC
results alone are not a good testcase for testing all fallout there
wasn't much point in delaying this "big" change in infrastructure
behavior.  The plan was to expose this change as early as possible
to have as much time as possible to extend the alias-oracle
infrastructure or shortcomings of it.

You are of course free to contribute with either reporting bugs
or fixing them if they happen.

Thanks,
Richard.