[PATCH]: Fix PR middle-end/56382 -- Only move MODE_COMPLEX_FLOAT by parts if we can create pseudos

[John David Anglin]

On 24-Aug-13, at 6:43 AM, Steven Bosscher wrote:

> On Fri, Aug 23, 2013 at 2:47 AM, John David Anglin wrote:
>> Ping.
>>
>>
>> On 28-Jul-13, at 12:17 PM, John David Anglin wrote:
>>
>>> This patch fixes PR middle-end/56382 on hppa64-hp-hpux11.11.  The  
>>> patch
>>> prevents moving a complex float by parts if we can't
>>> create pseudos.  On a big endian 64-bit target, we need a psuedo  
>>> to move a
>>> complex float and this fails during reload.
>>>
>>> OK for trunk?
>>>
>
> I'm trying to understand how the patch would help...
>
> The code you're patching is:
>
>  /* Move floating point as parts.  */
>  if (GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT
> +    && can_create_pseudo_p ()
>      && optab_handler (mov_optab, GET_MODE_INNER (mode)) !=  
> CODE_FOR_nothing)
>    try_int = false;
>  /* Not possible if the values are inherently not adjacent.  */
>  else if (GET_CODE (x) == CONCAT || GET_CODE (y) == CONCAT)
>    try_int = false;
>  /* Is possible if both are registers (or subregs of registers).  */
>  else if (register_operand (x, mode) && register_operand (y, mode))
>    try_int = true;
>  /* If one of the operands is a memory, and alignment constraints
>     are friendly enough, we may be able to do combined memory  
> operations.
>     We do not attempt this if Y is a constant because that  
> combination is
>     usually better with the by-parts thing below.  */
>  else if ((MEM_P (x) ? !CONSTANT_P (y) : MEM_P (y))
>           && (!STRICT_ALIGNMENT
>               || get_mode_alignment (mode) == BIGGEST_ALIGNMENT))
>    try_int = true;
>  else
>    try_int = false;
>
> With the new test for can_create_pseudo_p, you're trying to make
> "try_int" be false. Apparently your failure happens if one of the
> operands is a MEM? Otherwise the second "else if " test would find x
> and y be registers and "try_int" still ends up being true.

With the patch, I'm trying to prevent "try_int" from being set  
directly to false
when the mode class is MODE_COMPLEX_FLOAT.  The 64-bit PA target
has move instructions to handle the inner mode.  So, "optab_handler  
(mov_optab,
GET_MODE_INNER (mode)) != CODE_FOR_nothing" is always true.

>
> It seems to me that can_create_pseudo_p is not the right test anyway.
> There many be other targets that can take this path just fine without
> needing new registers. In the PR audit trail you say: "The problem is
> SCmode is the same size as DImode on this target, so the subreg can't
> be extracted by a move." Using can_create_pseudo_p is too big a hammer
> to solve this problem. The right test would be to see if you end up
> needing extra registers to perform the move. But emit_move_change_mode
> already handles that, AFAICT, so can you please try and test if the
> following patch solves the PR for you?

As expected, your patch doesn't fix the PR.

The bug lies in using "emit_move_complex_parts" when we can't create  
pseudos.
There are several places in the functions that it calls where  
"gen_reg_rtx" can be
called (e.g., "store_bit_field_using_insv").  In debugging, I found  
that fixing these
didn't help.  In the end, it fails to find a way move the complex parts.

In gcc.c-torture/compile/pr55921.c, we have two register operands so  
try_int
can be true.  "emit_move_via_integer" is successful in this case.   
Your patch
might be correct.

I'm not sure that can_create_pseudo_p is that big a hammer as it appears
emit_move_complex is mainly called prior to reload.  The proposed change
only affects the code when we can't create pseudos.  Even then, we  
fall back
to emit_move_complex_parts.

As you say, some other check might be more appropriate to determine
whether a call to gen_reg_rtx might be needed in  
emit_move_complex_parts.
For the PA, it would be something like "GET_MODE_BITSIZE (mode) >  
BITS_PER_WORD".
But, we still need to check can_create_pseudo_p as we probably still  
want to use
emit_move_complex_parts before reload.

Dave
--
John David Anglin	dave.anglin@bell.net