alpha haifa regression

alpha haifa regression

[Richard Henderson]

With the haifa scheduler, alpha fails 920908-1 with -O2.

What happens is, the insn that loads from the va_list gets moved
before the stores from the register variables.  It does this
because alias decides that a variable struct memory cannot alias
a non-variable non-struct memory.

It seems a bit hoaky to me, but then the C alias rules are a bit
perverse, so I'll assume this is correct.

The solution then, since we don't want to slow down normal code
where we _can_ make these assumptions, is to emit blockage after
dumping the varargs regs to memory.  


r~


Index: alpha.h
===================================================================
RCS file: /cvs/cvsfiles/egcs/gcc/config/alpha/alpha.h,v
retrieving revision 1.3
diff -u -p -d -r1.3 alpha.h
--- alpha.h	1997/09/02 19:37:05	1.3
+++ alpha.h	1997/09/29 05:38:13
@@ -1052,6 +1052,7 @@ enum reg_class { NO_REGS, GENERAL_REGS, 
 		      plus_constant (virtual_incoming_args_rtx,		\
 				     (CUM) * UNITS_PER_WORD)),		\
 	     6 - (CUM), (6 - (CUM)) * UNITS_PER_WORD);			\
+	   emit_insn (gen_blockage ());					\
 	 }								\
       PRETEND_SIZE = 12 * UNITS_PER_WORD;				\
     }									\

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199709290539.WAA12011@dot.cygnus.com >you write:
  > What happens is, the insn that loads from the va_list gets moved
  > before the stores from the register variables.  It does this
  > because alias decides that a variable struct memory cannot alias
  > a non-variable non-struct memory.
  > 
  > It seems a bit hoaky to me, but then the C alias rules are a bit
  > perverse, so I'll assume this is correct.
  > 
  > The solution then, since we don't want to slow down normal code
  > where we _can_ make these assumptions, is to emit blockage after
  > dumping the varargs regs to memory.  
Yup.  Same problem, same solution for the PA.

I've installed your patch.

However, in the future, please send ChangeLog entries too!

jeff

Re: alpha haifa regression

[Jim Wilson]

I think the correct fix here is to set the MEM_IN_STRUCT_P bit on the
MEM rtx created by SETUP_INCOMING_VARARGS.  This should make the sched
blockage insn unnecessary.

The MEM_IN_STRUCT_P bit should be set because these are not scalar stores.

Jim

Re: alpha haifa regression

[Richard Henderson]

> I think the correct fix here is to set the MEM_IN_STRUCT_P bit on the
> MEM rtx created by SETUP_INCOMING_VARARGS.  This should make the sched
> blockage insn unnecessary.

But since the tests in true_dependance are symmetric, that will
then cause problems for va_arg(list, int), no?


r~

Re: alpha haifa regression

[Jim Wilson]

	But since the tests in true_dependance are symmetric, that will
	then cause problems for va_arg(list, int), no?

I would suspect no, because va_arg(list, int) expands into complicated code
that does an actual aggregate (array and/or structure) memory access, and
hence va_arg references are always aggregate references, even if a scalar
type is specified.

It all depends on how va_arg was defined though.

It is easy enough to check by looking at the RTL generated by a va_arg
call using a scalar.  I suspect that it will have MEM_IN_STRUCT_P set.

Jim

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199709292200.PAA08832@cygnus.com >you write:
  > 	But since the tests in true_dependance are symmetric, that will
  > 	then cause problems for va_arg(list, int), no?
  > 
  > I would suspect no, because va_arg(list, int) expands into complicated code
  > that does an actual aggregate (array and/or structure) memory access, and
  > hence va_arg references are always aggregate references, even if a scalar
  > type is specified.
I suspect yes -- I've already seen it on the PA.

The new alias code keeps much better track of this kind of stuff
and haifa is much more aggressive about moving stuff around.

It's a problem we can't just ignore.

  > It is easy enough to check by looking at the RTL generated by a va_arg
  > call using a scalar.  I suspect that it will have MEM_IN_STRUCT_P set.
Depends on the target, but most end up doing something like this if
you remove all the casts, address computations & rounding:


  blah = *(type*)address

jeff

Re: alpha haifa regression

[Joern Rennecke]

>   In message <199709292200.PAA08832@cygnus.com>you write:
[Richard Henderson:]
>   > 	But since the tests in true_dependance are symmetric, that will
>   > 	then cause problems for va_arg(list, int), no?
>   > 
[Jim Wilson:]
>   > I would suspect no, because va_arg(list, int) expands into complicated code
>   > that does an actual aggregate (array and/or structure) memory access, and
>   > hence va_arg references are always aggregate references, even if a scalar
>   > type is specified.
> I suspect yes -- I've already seen it on the PA.

I just checked it for alpha. pa and sh.  For a TYPE argument, va_arg
basically casts (or assigns) a void* to a TYPE*, and then dereferences
the TYPE*.  The result is a memory access that has MEM_IN_STRUCT_P set
iff TYPE is a struct.

I propose to cast to a (struct { TYPE t;} *) instead, and dereference that
and select the t member (can of course both be done with the -> operator).

If TYPE is smaller than STRUCTURE_SIZE_BOUNDARY, and the target is
big endian, we have to adjust the alignment padding.
care.  We have then to 
the t member of this structure.  

>   > It is easy enough to check by looking at the RTL generated by a va_arg
>   > call using a scalar.  I suspect that it will have MEM_IN_STRUCT_P set.
> Depends on the target, but most end up doing something like this if
> you remove all the casts, address computations & rounding:
> 
> 
>   blah = *(type*)address

It is actually has an impact what form address takes.  If it happens to be
a a (type *) might happen for type == char if some port uses char *
for address) and the actual operation is a cast with a following dereference,
without an intervening assignment to a variable, the actual form of address
matters - i.e. if there is some non-zero integer added to a pointer, the
MEM_IN_STRUCT_P of the resulting mem gets set.

However, what I have seen so far either uses an ntervening assignment,
or an intervening cast to (void*), thus resulting in a MEM without
MEM_IN_STRUCT_P.

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199710072012.VAA15311@phal.cygnus.co.uk >you write:
  > I just checked it for alpha. pa and sh.  For a TYPE argument, va_arg
  > basically casts (or assigns) a void* to a TYPE*, and then dereferences
  > the TYPE*.  The result is a memory access that has MEM_IN_STRUCT_P set
  > iff TYPE is a struct.
  > 
  > I propose to cast to a (struct { TYPE t;} *) instead, and dereference that
  > and select the t member (can of course both be done with the -> operator).
I think this is more complicated than we need.

Emitting the blockage is easy and is unlikely to have any noticable impact
on performance.

jeff

Re: alpha haifa regression

[Joern Rennecke]

>   > I propose to cast to a (struct { TYPE t;} *) instead, and dereference that
>   > and select the t member (can of course both be done with the -> operator).
> I think this is more complicated than we need.
> 
> Emitting the blockage is easy and is unlikely to have any noticable impact
> on performance.

But for ports where the arguments are set up with __builtin_saveregs -
expanded from va_start - this would require two blockage insns - one
in front and one at the end - which could end up in an advanced position
in the function too.

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199710072139.WAA15411@phal.cygnus.co.uk >you write:
  > But for ports where the arguments are set up with __builtin_saveregs -
  > expanded from va_start - this would require two blockage insns - one
  > in front and one at the end - which could end up in an advanced position
  > in the function too.
No.  You only need once since the prologue itself won't load/store in
the same area as the varargs flushback.  And if there is some el-bizzaro
target that does, then emit two blockages.  No big deal.

jeff

Re: alpha haifa regression

[Joern Rennecke]

>   > But for ports where the arguments are set up with __builtin_saveregs -
>   > expanded from va_start - this would require two blockage insns - one
>   > in front and one at the end - which could end up in an advanced position
>   > in the function too.
> No.  You only need once since the prologue itself won't load/store in
> the same area as the varargs flushback.  And if there is some el-bizzaro
> target that does, then emit two blockages.  No big deal.

For __builtin_saveregs, we are talking about a stack slot, not a
designated varargs flushback area.
And in front of va_start, any C code might be placed.  Hence, we may not
make any assumptions that only the prologue is in front.

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199710072146.WAA15421@phal.cygnus.co.uk >you write:
  > >   > But for ports where the arguments are set up with __builtin_saveregs 
  > -
  > >   > expanded from va_start - this would require two blockage insns - one
  > >   > in front and one at the end - which could end up in an advanced posit
  > ion
  > >   > in the function too.
  > > No.  You only need once since the prologue itself won't load/store in
  > > the same area as the varargs flushback.  And if there is some el-bizzaro
  > > target that does, then emit two blockages.  No big deal.
  > 
  > For __builtin_saveregs, we are talking about a stack slot, not a
  > designated varargs flushback area.
  > And in front of va_start, any C code might be placed.  Hence, we may not
  > make any assumptions that only the prologue is in front.
Registers are flushed at the end of the prologue regardless of where
the va_start is.

jeff

Re: alpha haifa regression

[Joern Rennecke]

> Registers are flushed at the end of the prologue regardless of where
> the va_start is.

Only for ports that use SETUP_INCOMING_VARARGS.  However, that was not what
I was talking about.

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199710072157.WAA15442@phal.cygnus.co.uk >you write:
  > > Registers are flushed at the end of the prologue regardless of where
  > > the va_start is.
  > 
  > Only for ports that use SETUP_INCOMING_VARARGS.  However, that was not what
  > I was talking about.
No.  The PA doesn't use SETUP_INCOMING_VARARGS for example.

va_start calls __builtin_saveregs which causes a register flush to happen
in the prologue.


As long as va_start references __builtin_saveregs we're Ok.

jeff

Re: alpha haifa regression

[Jeffrey A Law]

  In message < 199709292023.NAA04543@cygnus.com >you write:
  > I think the correct fix here is to set the MEM_IN_STRUCT_P bit on the
  > MEM rtx created by SETUP_INCOMING_VARARGS.  This should make the sched
  > blockage insn unnecessary.
  > 
  > The MEM_IN_STRUCT_P bit should be set because these are not scalar stores.
You're probably right.  However the blockage will still be needed.

If you set MEM_IN_STRUCT_P, then the alias code will assume that
the flushback can't alias scalars.  This loses when you extract
scalars off a varargs list.

If you don't set MEM_IN_STRUCT_P, then the alias code will assume
that the flushback can't alias structs.  THis loses when you
extract structs off a varargs list.

Basically you can't win.  The way we do flushbacks creates an
aliasing situation that's going to cause problems somewhere.

Thus the easiest solution is a blockage insn.


jeff

Re: alpha haifa regression

[Jim Wilson]

	If you set MEM_IN_STRUCT_P, then the alias code will assume that
	the flushback can't alias scalars.  This loses when you extract
	scalars off a varargs list.

	If you don't set MEM_IN_STRUCT_P, then the alias code will assume
	that the flushback can't alias structs.  THis loses when you
	extract structs off a varargs list.

You are assuming that it is possible to do both scalar and structure
references to the saved register block.  However, there is only one way to
access this data, and this is via va_arg.  It depends on how va_arg is
defined, but for some targets, it will always result in a MEM with
MEM_IN_STRUCT_P set, because va_arg always does an array and/or structure
reference.  And if this is what va_arg does, then the insns storing into
the saved register block must always have MEM_IN_STRUCT_P set also.

I suggest you look at the RTL before deciding whether a blockage insn is
needed.

Jim

Re: alpha haifa regression

[meissner]

| For __builtin_saveregs, we are talking about a stack slot, not a
| designated varargs flushback area.
| And in front of va_start, any C code might be placed.  Hence, we may not
| make any assumptions that only the prologue is in front.

That is not correct.  Expr.c has the following code in it to guarantee that
__builtin_saveregs is executed first (assuming there is some sort of blockage
in EXPAND_BUILTIN_SAVEREGS to preventt he scheduler from moving things around):

	/* Put the sequence after the NOTE that starts the function.
	   If this is inside a SEQUENCE, make the outer-level insn
	   chain current, so the code is placed at the start of the
	   function.  */
	push_topmost_sequence ();
	emit_insns_before (seq, NEXT_INSN (get_insns ()));
	pop_topmost_sequence ();
	return temp;