[lra] patch to revert a code from previous patch.

[Richard Sandiford]

Vladimir Makarov <vmakarov@redhat.com> writes:
> On 12-10-15 4:25 PM, Richard Sandiford wrote:
>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>> Vladimir Makarov <vmakarov@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  <vmakarov@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.

Richard