Hi Vladimir,
Thank you for explain it.
I have a few comments inlined.
On 26/02/16 23:54, Vladimir Makarov wrote:
Thanks for working on this and providing a good description of the
problem. Could you fill a PR and provide a test even if you can not
reduce it.
I will fill a PR. Try to reduce a test case. As it's triggered by my
local change to gcc, I cannot guarantee it.
Anyway, I am quite happy to test your fix when you have one.
As for the scratch. As I understand the scratch was introduced for
operands which will not require any resources (memory or a new
register) for some insn alternatives. If we use pseudo for this, it
will always need memory or a register. The typical constraint for
scratch is "r,X" or "0r". So I guess using just "&r" for scratch is a
bad practice. Still for compatibility I think we should implement the
same reload behaviour for this case too.
Actually (clobber (match_scratch:MODE x "=r")) also triggers this ICE.
The early clobber modifier here doesn't really matter.
the purpose of this pattern is to reserve a pseudo register for use as a
temporary.
The "=" modifier is required for MATCH_SCRATCH expression. Otherwise, it
will error "missing output reload"
That why (set scratch, RXX) is generated.
I believe we should use the same technique -- changing scratches to
pseudo and back at the end of LRA if they don't need a register. It
will solve also a possible problem for correct scratch generation
during LRA.
I am going to work on this problem on the next week. A test case
would be a help for me.
gcc/ChangeLog:
2016-02-26 Renlin Li<renlin...@arm.com>
* lra-constraints.c (curr_insn_transform): Don't generate reload for
output scratch operand.
Sorry, I can not accept the patch as I'd like to provide a better
solution I described above. The patch is also wrong for unused
non-scratch operands. They still should be reloaded if they do not
satisfy their constraints even if they are not used later.
I think still it will be reload according to the code logic here:
if (get_reload_reg (type, mode, old, goal_alt[i],
loc != curr_id->operand_loc[i], "", &new_reg)
&& type != OP_OUT)
{
push_to_sequence (before);
lra_emit_move (new_reg, old);
before = get_insns ();
end_sequence ();
}
*loc = new_reg; ------------->>>>>>>>> the original operand will
be replaced by a reload reg.
if (type != OP_IN
/* Don't generate reload for output scratch operand. */
&& GET_CODE (old) != SCRATCH
&& find_reg_note (curr_insn, REG_UNUSED, old) == NULL_RTX)
a reload register will be generated to replace the old operand in the
original rtx pattern to satisfy their constraints.
Later, it will check, if this operand is an ouput operand which will be
used later, another insn will be generated to
move newly generate pseudo into old operand.
The patch is to add one more condition to this final insn generation.
Regards,
Renlin