Re: spec2000 regression

Re: spec2000 regression

[Richard Kenner]

    !   if (DECL_P (t) && ref == DECL_RTL_IF_SET (t))
    !     /* Avoid returning a bogus alias set from a new MEM.  */
    !     set_mem_alias_set (ref, get_alias_set (type));
    !   else
    !     set_mem_alias_set (ref, get_alias_set (t));

I suppose, but I think this is a real kludge.

Instead, I'd abort in the first case and fix all the callers.

Re: spec2000 regression

[Jason Merrill]

>>>>> "Richard" == Richard Kenner <kenner@vlsi1.ultra.nyu.edu> writes:

>     !   if (DECL_P (t) && ref == DECL_RTL_IF_SET (t))
>     !     /* Avoid returning a bogus alias set from a new MEM.  */
>     !     set_mem_alias_set (ref, get_alias_set (type));
>     !   else
>     !     set_mem_alias_set (ref, get_alias_set (t));

> I suppose, but I think this is a real kludge.

> Instead, I'd abort in the first case and fix all the callers.

Makes sense.

Jason

Re: spec2000 regression

[Jason Merrill]

>>>>> "Jason" == Jason Merrill <jason_merrill@redhat.com> writes:

>> Instead, I'd abort in the first case and fix all the callers.

> Makes sense.

Applied:

2001-08-22  Jason Merrill  <jason_merrill@redhat.com>

	* explow.c (set_mem_attributes): Avoid returning a bogus alias set
	from a new MEM.

Re: spec2000 regression

[Jason Merrill]

>>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:

>> >>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:
>> 
>> > +       /* set_mem_attributes uses DECL_RTL to avoid re-generating of
>> > +          already computed alias sets.  Here we want to re-generate.  */
>> > +       if (TREE_CODE (decl) != SAVE_EXPR)
>> > + 	SET_DECL_RTL (decl, NULL);
>> >         set_mem_attributes (reg, decl, 1);
>> > +       if (TREE_CODE (decl) != SAVE_EXPR)
>> > + 	SET_DECL_RTL (decl, reg);
>> 
>> This should check DECL_P.  Otherwise, looks good.

> May I interpret this as approval with this change?

Yes.  Please apply all the changes you have; I haven't checked in any of
the patches I've sent out.

Jason

Re: spec2000 regression

[Jan Hubicka]

> >>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:
> 
> > +       /* set_mem_attributes uses DECL_RTL to avoid re-generating of
> > +          already computed alias sets.  Here we want to re-generate.  */
> > +       if (TREE_CODE (decl) != SAVE_EXPR)
> > + 	SET_DECL_RTL (decl, NULL);
> >         set_mem_attributes (reg, decl, 1);
> > +       if (TREE_CODE (decl) != SAVE_EXPR)
> > + 	SET_DECL_RTL (decl, reg);
> 
> This should check DECL_P.  Otherwise, looks good.
May I interpret this as approval with this change?
> 
> Jason

Re: spec2000 regression

[Jason Merrill]

>>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:

> +       /* set_mem_attributes uses DECL_RTL to avoid re-generating of
> +          already computed alias sets.  Here we want to re-generate.  */
> +       if (TREE_CODE (decl) != SAVE_EXPR)
> + 	SET_DECL_RTL (decl, NULL);
>         set_mem_attributes (reg, decl, 1);
> +       if (TREE_CODE (decl) != SAVE_EXPR)
> + 	SET_DECL_RTL (decl, reg);

This should check DECL_P.  Otherwise, looks good.

Jason

Re: spec2000 regression

[Jan Hubicka]

> 
> 
> --On Wednesday, August 15, 2001 03:10:40 PM +0100 Jason Merrill 
> <jason_merrill@redhat.com> wrote:
> 
> > This should avoid such problems.
> >
> > 2001-08-15  Jason Merrill  <jason_merrill@redhat.com>
> >
> > 	* explow.c (set_mem_attributes): Avoid returning a bogus alias set
> > 	from a new MEM.
> 
> Hmm.  That changes the semantics somewhat, in the abstract; i.e.,
> it takes advantage of how get_alias_set handles decls by looking at
> their types.  If we did something smarter (putting things in subsets
> based on information about the scope they were in, say), then the
> two calls might start meaning different things.
> 
> So, I don't know whether the safety you're introducing is better, or
> not.
As suggested by Richard, it probably makes sense to do this trick only
as sanity checking, so following patch fixes all callers a first step.

I've regtested and bootstrapped following patch together with
modified set_mem_attributes patch to abort in incorrect cases.

Except for simple grep it has caught problem in function.c that does
replace REG by memory in place. The fix is rather ugly, but I can't
come with something better.

The patch above should be probably updated to abort in first case if
testing succeds.

OK to install?

Honza

Thu Aug 16 14:50:50 CEST 2001  Jan Hubicka  <jh@suse.cz>

	* function.c (put_var_into_stack): Temporarily clear DECL_RTL.
	(assign_params): Avoid setting DECL_RTL to unfinished RTX.
	(expand_function_start): Likewise.
	* stmt.c (expand_decl): Likewise.
	* varasm.c (make_decl_rtx): Likewise.

Index: function.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/function.c,v
retrieving revision 1.295
diff -c -3 -p -r1.295 function.c
*** function.c	2001/08/13 15:52:18	1.295
--- function.c	2001/08/16 12:50:02
*************** put_var_into_stack (decl)
*** 1420,1426 ****
--- 1420,1433 ----
  
        /* Change the CONCAT into a combined MEM for both parts.  */
        PUT_CODE (reg, MEM);
+ 
+       /* set_mem_attributes uses DECL_RTL to avoid re-generating of
+          already computed alias sets.  Here we want to re-generate.  */
+       if (TREE_CODE (decl) != SAVE_EXPR)
+ 	SET_DECL_RTL (decl, NULL);
        set_mem_attributes (reg, decl, 1);
+       if (TREE_CODE (decl) != SAVE_EXPR)
+ 	SET_DECL_RTL (decl, reg);
  
        /* The two parts are in memory order already.
  	 Use the lower parts address as ours.  */
*************** assign_parms (fndecl)
*** 4688,4697 ****
  	     appropriately.  */
  	  if (passed_pointer)
  	    {
! 	      SET_DECL_RTL (parm,
! 			    gen_rtx_MEM (TYPE_MODE (TREE_TYPE (passed_type)), 
! 					 parmreg));
! 	      set_mem_attributes (DECL_RTL (parm), parm, 1);
  	    }
  	  else
  	    {
--- 4695,4704 ----
  	     appropriately.  */
  	  if (passed_pointer)
  	    {
! 	      rtx x = gen_rtx_MEM (TYPE_MODE (TREE_TYPE (passed_type)),
! 			     	   parmreg);
! 	      set_mem_attributes (x, parm, 1);
! 	      SET_DECL_RTL (parm, x);
  	    }
  	  else
  	    {
*************** assign_parms (fndecl)
*** 5030,5040 ****
        if (parm == function_result_decl)
  	{
  	  tree result = DECL_RESULT (fndecl);
! 
! 	  SET_DECL_RTL (result,
! 			gen_rtx_MEM (DECL_MODE (result), DECL_RTL (parm)));
  
! 	  set_mem_attributes (DECL_RTL (result), result, 1);
  	}
      }
  
--- 5037,5046 ----
        if (parm == function_result_decl)
  	{
  	  tree result = DECL_RESULT (fndecl);
! 	  rtx x = gen_rtx_MEM (DECL_MODE (result), DECL_RTL (parm));
  
! 	  set_mem_attributes (x, result, 1);
! 	  SET_DECL_RTL (result, x);
  	}
      }
  
*************** expand_function_start (subr, parms_have_
*** 6451,6461 ****
  	}
        if (value_address)
  	{
! 	  SET_DECL_RTL (DECL_RESULT (subr),
! 			gen_rtx_MEM (DECL_MODE (DECL_RESULT (subr)), 
! 				     value_address));
! 	  set_mem_attributes (DECL_RTL (DECL_RESULT (subr)),
! 			      DECL_RESULT (subr), 1);
  	}
      }
    else if (DECL_MODE (DECL_RESULT (subr)) == VOIDmode)
--- 6457,6465 ----
  	}
        if (value_address)
  	{
! 	  rtx x = gen_rtx_MEM (DECL_MODE (DECL_RESULT (subr)), value_address);
! 	  set_mem_attributes (x, DECL_RESULT (subr), 1);
! 	  SET_DECL_RTL (DECL_RESULT (subr), x);
  	}
      }
    else if (DECL_MODE (DECL_RESULT (subr)) == VOIDmode)
Index: stmt.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/stmt.c,v
retrieving revision 1.208
diff -c -3 -p -r1.208 stmt.c
*** stmt.c	2001/08/13 15:52:22	1.208
--- stmt.c	2001/08/16 12:50:07
*************** expand_decl (decl)
*** 3810,3824 ****
    else if (DECL_SIZE (decl) == 0)
      /* Variable with incomplete type.  */
      {
        if (DECL_INITIAL (decl) == 0)
  	/* Error message was already done; now avoid a crash.  */
! 	SET_DECL_RTL (decl, gen_rtx_MEM (BLKmode, const0_rtx));
        else
  	/* An initializer is going to decide the size of this array.
  	   Until we know the size, represent its address with a reg.  */
! 	SET_DECL_RTL (decl, gen_rtx_MEM (BLKmode, gen_reg_rtx (Pmode)));
  
!       set_mem_attributes (DECL_RTL (decl), decl, 1);
      }
    else if (DECL_MODE (decl) != BLKmode
  	   /* If -ffloat-store, don't put explicit float vars
--- 3810,3826 ----
    else if (DECL_SIZE (decl) == 0)
      /* Variable with incomplete type.  */
      {
+       rtx x;
        if (DECL_INITIAL (decl) == 0)
  	/* Error message was already done; now avoid a crash.  */
! 	x = gen_rtx_MEM (BLKmode, const0_rtx);
        else
  	/* An initializer is going to decide the size of this array.
  	   Until we know the size, represent its address with a reg.  */
! 	x = gen_rtx_MEM (BLKmode, gen_reg_rtx (Pmode));
  
!       set_mem_attributes (x, decl, 1);
!       SET_DECL_RTL (decl, x);
      }
    else if (DECL_MODE (decl) != BLKmode
  	   /* If -ffloat-store, don't put explicit float vars
*************** expand_decl (decl)
*** 3888,3894 ****
    else
      /* Dynamic-size object: must push space on the stack.  */
      {
!       rtx address, size;
  
        /* Record the stack pointer on entry to block, if have
  	 not already done so.  */
--- 3890,3896 ----
    else
      /* Dynamic-size object: must push space on the stack.  */
      {
!       rtx address, size, x;
  
        /* Record the stack pointer on entry to block, if have
  	 not already done so.  */
*************** expand_decl (decl)
*** 3913,3921 ****
  					      TYPE_ALIGN (TREE_TYPE (decl)));
  
        /* Reference the variable indirect through that rtx.  */
!       SET_DECL_RTL (decl, gen_rtx_MEM (DECL_MODE (decl), address));
  
-       set_mem_attributes (DECL_RTL (decl), decl, 1);
  
        /* Indicate the alignment we actually gave this variable.  */
  #ifdef STACK_BOUNDARY
--- 3915,3924 ----
  					      TYPE_ALIGN (TREE_TYPE (decl)));
  
        /* Reference the variable indirect through that rtx.  */
!       x = gen_rtx_MEM (DECL_MODE (decl), address);
!       set_mem_attributes (x, decl, 1);
!       SET_DECL_RTL (decl, x);
  
  
        /* Indicate the alignment we actually gave this variable.  */
  #ifdef STACK_BOUNDARY
Index: varasm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/varasm.c,v
retrieving revision 1.193
diff -c -3 -p -r1.193 varasm.c
*** varasm.c	2001/08/13 15:14:52	1.193
--- varasm.c	2001/08/16 12:50:09
*************** make_decl_rtl (decl, asmspec)
*** 591,596 ****
--- 591,597 ----
    const char *name = 0;
    const char *new_name = 0;
    int reg_number;
+   rtx x;
  
    /* Check that we are not being given an automatic variable.  */
    /* A weak alias has TREE_PUBLIC set but not the other bits.  */
*************** make_decl_rtl (decl, asmspec)
*** 758,768 ****
  	   && (TREE_PUBLIC (decl) || TREE_STATIC (decl)))))
      TREE_SIDE_EFFECTS (decl) = 1;
  
!   SET_DECL_RTL (decl, gen_rtx_MEM (DECL_MODE (decl),
! 				   gen_rtx_SYMBOL_REF (Pmode, name)));
!   SYMBOL_REF_WEAK (XEXP (DECL_RTL (decl), 0)) = DECL_WEAK (decl);
    if (TREE_CODE (decl) != FUNCTION_DECL)
!     set_mem_attributes (DECL_RTL (decl), decl, 1);
  
    /* Optionally set flags or add text to the name to record information
       such as that it is a function name.
--- 759,769 ----
  	   && (TREE_PUBLIC (decl) || TREE_STATIC (decl)))))
      TREE_SIDE_EFFECTS (decl) = 1;
  
!   x = gen_rtx_MEM (DECL_MODE (decl), gen_rtx_SYMBOL_REF (Pmode, name));
!   SYMBOL_REF_WEAK (XEXP (x, 0)) = DECL_WEAK (decl);
    if (TREE_CODE (decl) != FUNCTION_DECL)
!     set_mem_attributes (x, decl, 1);
!   SET_DECL_RTL (decl, x);
  
    /* Optionally set flags or add text to the name to record information
       such as that it is a function name.

Re: spec2000 regression

[Andreas Jaeger]

"Joseph S. Myers" <jsm28@cam.ac.uk> writes:

> On Wed, 15 Aug 2001, Jan Hubicka wrote:
>
>> Would be following testsuite OK?
>> 
>> long long a1, *a=&a1;
>> short b1, *b=&b1;
>> main()
>> {
>> #ifdef __OPTIMIZE__
>>   *a = 1;
>>   *b = 2;
>>   if (*a != 1)
>> 	  undefined ();
>> #endif
>>   return 0;
>> }
>
> Assuming that it does indeed pass with working compilers but fails with
> mainline before the problem is fixed, that testcase should be OK.

It works with 3.0 and fails with 3.1 CVS from yesterday:

gromit:/tmp:[127]$ /opt/gcc-3.1-devel/bin/gcc -Wall t.c -O2
t.c:4: warning: return type defaults to `int'
t.c: In function `main':
t.c:9: warning: implicit declaration of function `undefined'
/tmp/cc9lSIBE.o: In function `main':
/tmp/cc9lSIBE.o(.text+0x41): undefined reference to `undefined'
collect2: ld returned 1 exit status

gromit:/tmp:[1]$ /opt/gcc-3.0/bin/gcc -Wall t.c -O2
t.c:4: warning: return type defaults to `int'
t.c: In function `main':
t.c:9: warning: implicit declaration of function `undefined'

I didn't check any of the patches floating around.

But shouldn't the testcase get fixed so that it issues no warnings?

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: spec2000 regression

[Mark Mitchell]

--On Wednesday, August 15, 2001 03:10:40 PM +0100 Jason Merrill 
<jason_merrill@redhat.com> wrote:

> This should avoid such problems.
>
> 2001-08-15  Jason Merrill  <jason_merrill@redhat.com>
>
> 	* explow.c (set_mem_attributes): Avoid returning a bogus alias set
> 	from a new MEM.

Hmm.  That changes the semantics somewhat, in the abstract; i.e.,
it takes advantage of how get_alias_set handles decls by looking at
their types.  If we did something smarter (putting things in subsets
based on information about the scope they were in, say), then the
two calls might start meaning different things.

So, I don't know whether the safety you're introducing is better, or
not.

Actually, how does this interact with `restrict' now?  That's something
where I think we may need the DECL, not the TYPE.

I wish I remembered this code better; I'm just raising issues in case
they apply...

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: spec2000 regression

[Mark Mitchell]

--On Wednesday, August 15, 2001 02:41:27 PM +0200 Jan Hubicka <jh@suse.cz> 
wrote:
> the problem is that actually code first sets the DECL_RTL and then calls
> set_mem_attributes, that calls get_alias_set, but it thinks that the
> alias set is already computed, but it isn't.
>

That patch doesn't look right to me.  You should figure out why the
alias set is already initialized, and clear it if necessary before
you get here.

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: spec2000 regression

[Joseph S. Myers]

On Wed, 15 Aug 2001, Jan Hubicka wrote:

> Would be following testsuite OK?
> 
> long long a1, *a=&a1;
> short b1, *b=&b1;
> main()
> {
> #ifdef __OPTIMIZE__
>   *a = 1;
>   *b = 2;
>   if (*a != 1)
> 	  undefined ();
> #endif
>   return 0;
> }

Assuming that it does indeed pass with working compilers but fails with
mainline before the problem is fixed, that testcase should be OK.

-- 
Joseph S. Myers
jsm28@cam.ac.uk

Re: spec2000 regression

[Jan Hubicka]

> This should avoid such problems.
> 
> 2001-08-15  Jason Merrill  <jason_merrill@redhat.com>
> 
> 	* explow.c (set_mem_attributes): Avoid returning a bogus alias set
> 	from a new MEM.
> 
Hi,
I've verified that it solves my testcase.  Thanks

Honza

Re: spec2000 regression

[Jan Hubicka]

> On Wed, 15 Aug 2001, Jan Hubicka wrote:
> 
> > I am now testing following patch. OK to install if it succeeds?
> 
> You should add a testcase to the testsuite for this bug.  You can do this
> by calling a non-existent function such that the call will be optimised
> away if the optimisation succeeds.  (Look e.g. at
> gcc.c-torture/execute/builtin-abs-1.c - in this case you'll want to use
> 'set additional_flags "-fstrict-aliasing"' in the .x file so that the
> optimisation works at -O1 (which doesn't include -fstrict-aliasing) and
> #ifndef __OPTIMIZE__ is the only test needed for the dummy version of the
> function that aborts.)
OK,
Would be following testsuite OK?

long long a1, *a=&a1;
short b1, *b=&b1;
main()
{
#ifdef __OPTIMIZE__
  *a = 1;
  *b = 2;
  if (*a != 1)
	  undefined ();
#endif
  return 0;
}

Re: spec2000 regression

[Jan Hubicka]

> This should avoid such problems.
> 
> 2001-08-15  Jason Merrill  <jason_merrill@redhat.com>
> 
> 	* explow.c (set_mem_attributes): Avoid returning a bogus alias set
> 	from a new MEM.
> 
This looks much better to me from robustness point of view.
Thanks,

Honza

Re: spec2000 regression

[Jason Merrill]

This should avoid such problems.

2001-08-15  Jason Merrill  <jason_merrill@redhat.com>

	* explow.c (set_mem_attributes): Avoid returning a bogus alias set
	from a new MEM.

Re: spec2000 regression

[Jan Hubicka]

>     If I understand the purpose of original patch properly, it was hooting
>     for case variable gets unique class, but this class is recomputed
>     several times so it get several unique classes.  For class 0 this does
>     not matter.
> 
> No, that's backwards.  This is a case where the RTL was class 0 and so you
> want to make sure the new class is 0.  Your change will cause it to use
> the alias set of the type, which might be nonzero and hence is wrong.
Uhh, I see, then we probably need to go with Jason's patch, as I don't see any
way how to check case someone call set_memory_attributes after doing
DECL_RTL...

Honza

Re: spec2000 regression

[Richard Kenner]

    If I understand the purpose of original patch properly, it was hooting
    for case variable gets unique class, but this class is recomputed
    several times so it get several unique classes.  For class 0 this does
    not matter.

No, that's backwards.  This is a case where the RTL was class 0 and so you
want to make sure the new class is 0.  Your change will cause it to use
the alias set of the type, which might be nonzero and hence is wrong.

Re: spec2000 regression

[Jan Hubicka]

bad style or not, I believe it would be somewhat dificult to avoid getting it into
gcc. Such bugs can be dificult to find, as they waste only the performance.
Thanks Andreas for his automated tester :)
> 
> On the contrary, it is needed for the case where a variable gets a class
> different from that of its type, so recomputing it would give the wrong
> answer.  How does this work for you?
> 
> 2001-08-15  Jason Merrill  <jason_merrill@redhat.com>
> 
> 	* varasm.c (make_decl_rtl): Don't SET_DECL_RTL until we're done
> 	modifying the rtl.
> 
Yes, I've made the same patch before the one I sent to list and it solves
this particular problem.
Note that function.c contains other similar code - just take a look for
set_memory_attributes calls contains DECL_RTL in the argument.

Honza

Re: spec2000 regression

[Joseph S. Myers]

On Wed, 15 Aug 2001, Jan Hubicka wrote:

> I am now testing following patch. OK to install if it succeeds?

You should add a testcase to the testsuite for this bug.  You can do this
by calling a non-existent function such that the call will be optimised
away if the optimisation succeeds.  (Look e.g. at
gcc.c-torture/execute/builtin-abs-1.c - in this case you'll want to use
'set additional_flags "-fstrict-aliasing"' in the .x file so that the
optimisation works at -O1 (which doesn't include -fstrict-aliasing) and
#ifndef __OPTIMIZE__ is the only test needed for the dummy version of the
function that aborts.)

-- 
Joseph S. Myers
jsm28@cam.ac.uk

Re: spec2000 regression

[Jason Merrill]

>>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:

>> I'd prefer to fix things so they don't set DECL_RTL first.  MEM_ALIAS_SET
>> is not a test to see if the alias set has been set.  The alias set might
>> have been zero in which case we *do* want to use it.

> That what I was trying to do first, but there are number of places this
> is happening and fixing each of it still keeps open door for possible
> new bugs - it is natural to save the RTL to proper place first.

I disagree.  Even without this bug, it's bad style.

> If I understand the purpose of original patch properly, it was hooting for
> case variable gets unique class, but this class is recomputed several times
> so it get several unique classes.
> For class 0 this does not matter.

On the contrary, it is needed for the case where a variable gets a class
different from that of its type, so recomputing it would give the wrong
answer.  How does this work for you?

2001-08-15  Jason Merrill  <jason_merrill@redhat.com>

	* varasm.c (make_decl_rtl): Don't SET_DECL_RTL until we're done
	modifying the rtl.

Re: spec2000 regression

[Jason Merrill]

>>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:

> Hi, the problem is that actually code first sets the DECL_RTL and then
> calls set_mem_attributes, that calls get_alias_set, but it thinks that
> the alias set is already computed, but it isn't.

Makes sense.

> I am now testing following patch. OK to install if it succeeds?

No, we might want alias set 0.  Rather, the code shouldn't set DECL_RTL
until it's done building up the RTL.

Jason

Re: spec2000 regression

[Jan Hubicka]

>     the problem is that actually code first sets the DECL_RTL and then
>     calls set_mem_attributes, that calls get_alias_set, but it thinks that
>     the alias set is already computed, but it isn't.
> 
>     I am now testing following patch. OK to install if it succeeds?
> 
> I'd prefer to fix things so they don't set DECL_RTL first.  MEM_ALIAS_SET
> is not a test to see if the alias set has been set.  The alias set might
> have been zero in which case we *do* want to use it.
That what I was trying to do first, but there are number of places this
is happening and fixing each of it still keeps open door for possible
new bugs - it is natural to save the RTL to proper place first.

If I understand the purpose of original patch properly, it was hooting for
case variable gets unique class, but this class is recomputed several times
so it get several unique classes.
For class 0 this does not matter.

Honza
> 
> So I think your change is wrong.

Re: spec2000 regression

[Richard Kenner]

    the problem is that actually code first sets the DECL_RTL and then
    calls set_mem_attributes, that calls get_alias_set, but it thinks that
    the alias set is already computed, but it isn't.

    I am now testing following patch. OK to install if it succeeds?

I'd prefer to fix things so they don't set DECL_RTL first.  MEM_ALIAS_SET
is not a test to see if the alias set has been set.  The alias set might
have been zero in which case we *do* want to use it.

So I think your change is wrong.

Re: spec2000 regression

[Jan Hubicka]

> >>>>> "Jan" == Jan Hubicka <jh@suse.cz> writes:
> 
> > So remaining patch I can't analyze completely is yours to alias.c
> > I've failed to find the corresponding email at gcc-patches mailing list
> > (even when I believe I've went across it in the pass)
> 
> The subject line is "PATCH to get_alias_set".  There were two messages.
> 
> > so please can you double-check the patch and try to explain me what
> > exactly it is shooting for?
> 
> It's trying to avoid recalculating the alias set for a variable.  I suppose
> it could cause a performance regression if the initial alias set were
> wrong, but I would expect that to cause other problems as well.
Hi,
the problem is that actually code first sets the DECL_RTL and then calls
set_mem_attributes, that calls get_alias_set, but it thinks that the alias set
is already computed, but it isn't.

I am now testing following patch. OK to install if it succeeds?

It fixes my testcase:

long *a;
short *b;
main()
{
  *a = 1;
  *b = 2;
  return *a;
}

Now gets again:

	pushl	%ebp
	movl	%esp, %ebp
	pushl	%eax
	pushl	%eax
	movl	a, %eax
	andl	$-16, %esp
	movl	$1, (%eax)
	movl	b, %eax
	movw	$2, (%eax)
	movl	%ebp, %esp
	movl	$1, %eax
	popl	%ebp
	ret

Honza

Wed Aug 15 14:38:26 CEST 2001  Jan Hubicka  <jh@suse.cz>
	* alias.c (get_alias_set): Check that the alias set is already
	computed.

Index: alias.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/alias.c,v
retrieving revision 1.137
diff -c -3 -p -r1.137 alias.c
*** alias.c	2001/08/08 16:56:50	1.137
--- alias.c	2001/08/15 12:38:13
*************** get_alias_set (t)
*** 530,536 ****
  	 return it.  This is necessary for C++ anonymous unions, whose
  	 component variables don't look like union members (boo!).  */
        if (TREE_CODE (t) == VAR_DECL
! 	  && DECL_RTL_SET_P (t) && GET_CODE (DECL_RTL (t)) == MEM)
  	return MEM_ALIAS_SET (DECL_RTL (t));
  
        /* Give the language another chance to do something special.  */
--- 530,537 ----
  	 return it.  This is necessary for C++ anonymous unions, whose
  	 component variables don't look like union members (boo!).  */
        if (TREE_CODE (t) == VAR_DECL
! 	  && DECL_RTL_SET_P (t) && GET_CODE (DECL_RTL (t)) == MEM
! 	  && MEM_ALIAS_SET (DECL_RTL (t)))
  	return MEM_ALIAS_SET (DECL_RTL (t));
  
        /* Give the language another chance to do something special.  */