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.ang...@bell.net
