On 10/16/2012 02:44 AM, Richard Sandiford wrote:
Vladimir Makarov <vmaka...@redhat.com> writes:
On 12-10-15 4:25 PM, Richard Sandiford wrote:
Richard Sandiford <rdsandif...@googlemail.com> writes:
Vladimir Makarov <vmaka...@redhat.com> writes:
After committing a patch yesterday to implement proposals from a
review, I found that GCC crashes on SPEC2000 gap. LRA is trying to find
a mode of operand (const_int 1) in *lea_general_1 insn and can not find
it as the operand and insn template operand has VOIDmode.
There are still cases when context lookup is necessary to find a mode of
the operand. So I am reversing the change I did yesterday.
The patch is committed as rev. 192462.
2012-10-15 Vladimir Makarov <vmaka...@redhat.com>
* lra-int.h (lra_get_mode): Remove.
* lra-constraints.c (find_mode, get_op_mode): New functions.
(match_reload): Use get_op_mode instead of lra_get_mode.
(process_alt_operands, curr_insn_transform): Ditto.
But my objection to this code still stands. It's wrong to assume
that an operand to an rtx has the same mode as the containing rtx.
Please add a testcase that shows the problem.
(...because I was hoping to have a look myself). But if that's too
difficult to reduce, then which operand to *lea_general_1 was the problem?
The pattern looks like:
(define_insn_and_split "*lea_general_1"
[(set (match_operand 0 "register_operand" "=r")
(plus (plus (match_operand 1 "index_register_operand" "l")
(match_operand 2 "register_operand" "r"))
(match_operand 3 "immediate_operand" "i")))]
So operands 0, 1 and 2 should have been registers. Operand 3 never
needs reloading, so its mode shouldn't matter.
In this case the const needs a reload as it was a pseudo substituted by
equiv constant.
But in that case the operand modes we needed were there in the original
instruction operands. If equiv substitution is causing us to lose track
of those operand modes, then that needs to be fixed.
If the pattern had instead been:
(set (match_operand:SI 0 "register_operand" "=r")
(unspec:SI [(match_operand 1 "register_operand" "r")] UNSPEC_FOO))
and equiv substitution replaced operand 1 with a const_int without
the original mode being recorded anywhere, then we'd have no way
of recovering that mode from the pattern itself, because the modes
of unspec parameters are entirely target-specific.
I remember I thought about this. The case is rare, it is better to
calculate than keep mode for each operand of each insn. LRA speed is
achieved by minimizations of keeping data for each operand. We keep
only locations. I believe it is a better approach.
