Bernd Schmidt <ber...@codesourcery.com> writes:
> On 10/16/2012 04:30 AM, Vladimir Makarov wrote:
>> In insn:
>>
>> (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")))]
>>
>> one operand is (const_int 1). Its mode is VOIDmode. recog_data mode is
>> VOIDmode. The patch takes right mode from PLUS containing it.
>
> Shouldn't this use a mode_iterator to put the right mode on the operand?
That does sound better, and it would be nice to turn the genrecog warning:
/* A modeless MATCH_OPERAND can be handy when we can check for
multiple modes in the c_test. In most other cases, it is a
mistake. Only DEFINE_INSN is eligible, since SPLIT and
PEEP2 can FAIL within the output pattern. Exclude special
predicates, which check the mode themselves. Also exclude
predicates that allow only constants. Exclude the SET_DEST
of a call instruction, as that is a common idiom. */
if (GET_MODE (pattern) == VOIDmode
&& code == MATCH_OPERAND
&& GET_CODE (insn) == DEFINE_INSN
&& pred
&& !pred->special
&& pred->allows_non_const
&& strstr (c_test, "operands") == NULL
&& ! (set
&& GET_CODE (set) == SET
&& GET_CODE (SET_SRC (set)) == CALL))
message_with_line (pattern_lineno,
"warning: operand %d missing mode?",
XINT (pattern, 0));
into an error to flush all these things out.
I'm not sure it would help LRA or reload much though. (You might not
have been implying otherwise, sorry.) We would still want to believe
the mode of the matched operand over the mode of the match_operator for
address operands, and still need to fall back on Pmode for const_int
addresses, so I think the code would basically be the same.
Richard
