This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] SSA expand: Convert mode when copying between partition base vars


Hi,

On Tue, 28 Apr 2009, Andreas Krebbel wrote:

> > Rather try this one.  It fixes the cris issue (and perhaps the sparc 
> > thingy too), which is also about promoting PARM_DECLs.
> 
> This does only have an effect on partitions which do not contain the 
> default definition of the variable. expand_used_vars does not invoke 
> expand_one_var for a PARM_DECL as long as the respective bit in the 
> partition_has_default_def bitmap is set.

Yes, the PARM_DECL partition with the default def gets the RTX from the 
function setup, that's a promoted value.  So the _other_ partitions based 
on the same PARM_DECL (but without the default def) also need to be of a 
promoted mode, so that we can copy freely between those variables.  Or at 
least so my reasoning until your testcase ...

> And anyway I don't see how this prevents the problem which I'm seeing on 
> S/390 where a parameter is copied to a local variable:
> 
> int
> foo (int a)
> {
>   int w = a;
>   if (a < 0)
>     w = -(unsigned int) a;
>   return w;
> }
> 
> The problem is the partition copy which will be introduced between 
> partition 0 and 1. The local variable 'w' is SImode while 'a' has been 
> promoted to DImode:

... where that reasoning indeed doesn't apply.  I failed to consider 
partition copies between different base variables, sorry.  So yes, 
something like your patch is necessary, see below for comments.

> @@ -149,10 +150,17 @@ insert_partition_copy_on_edge (edge e, i
>  
>    set_location_for_edge (e);
>  
> -  /* Partition copy between same base variables only, so it's the same mode,
> -     hence we can use emit_move_insn.  */
> +  /* Partition copy between same base variables only, but the src
> +     might be a parameter promoted to a different type.  */
>    start_sequence ();
> -  emit_move_insn (SA.partition_to_pseudo[dest], SA.partition_to_pseudo[src]);
> +  mode = GET_MODE (SA.partition_to_pseudo[dest]);
> +  x = SA.partition_to_pseudo[src];
> +  if (GET_MODE (x) != mode)
> +    x = convert_to_mode (mode, x,
> +			 TYPE_UNSIGNED (TREE_TYPE (
> +			   partition_to_var (SA.map, src))));
> +  if (x != SA.partition_to_pseudo[dest])
> +    emit_move_insn (SA.partition_to_pseudo[dest], x);

No need for the conditional, x can't be SA.partition_to_pseudo[dest] here.
Can you factor this pattern into a small helper taking
  (rtx dest, rtx src, int unsignedsrcp)
?

> @@ -215,8 +223,14 @@ insert_rtx_to_part_on_edge (edge e, int 
>    set_location_for_edge (e);
>  
>    start_sequence ();
> -  gcc_assert (GET_MODE (src) == GET_MODE (SA.partition_to_pseudo[dest]));
> -  emit_move_insn (SA.partition_to_pseudo[dest], src);
> +  x = SA.partition_to_pseudo[dest];
> +  if (GET_MODE (src) != GET_MODE (x))
> +    x = convert_to_mode (GET_MODE (x), src,
> +			 TYPE_UNSIGNED (TREE_TYPE (
> +			   partition_to_var (SA.map, dest))));
> +
> +  emit_move_insn (SA.partition_to_pseudo[dest], x);

Here something is confused.  x starts as dest, then possibly is the 
converted source, and then moved into dest, so either this moves dest into 
dest or the converted source into dest.  You also take the signedness from 
the destination, where you want to convert the source.  I fear you have to 
pass the signedness from the caller :-/


Ciao,
Michael.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]