On 03/29/2011 06:21 AM, Richard Sandiford wrote:
> - enum machine_mode mode0 = insn_data[(int) icode].operand[1].mode;
> - enum machine_mode mode1 = insn_data[(int) icode].operand[2].mode;
> - enum machine_mode tmp_mode;
> + enum machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
> + enum machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
> + enum machine_mode mode0, mode1, tmp_mode;
...
> - if (GET_MODE (xop0) != mode0 && mode0 != VOIDmode)
> - xop0 = convert_modes (mode0,
> - GET_MODE (xop0) != VOIDmode
> - ? GET_MODE (xop0)
> - : mode,
> - xop0, unsignedp);
> -
> - if (GET_MODE (xop1) != mode1 && mode1 != VOIDmode)
> - xop1 = convert_modes (mode1,
> - GET_MODE (xop1) != VOIDmode
> - ? GET_MODE (xop1)
> - : mode,
> - xop1, unsignedp);
> + mode0 = GET_MODE (xop0) != VOIDmode ? GET_MODE (xop0) : mode;
> + if (xmode0 != VOIDmode && xmode0 != mode0)
> + {
> + xop0 = convert_modes (xmode0, mode0, xop0, unsignedp);
> + mode0 = xmode0;
> + }
> +
> + mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> + if (xmode1 != VOIDmode && xmode1 != mode1)
> + {
> + xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
> + mode1 = xmode1;
> + }
This smells like a target bug, but I can't quite put my finger
on exactly what's wrong, and I haven't debugged the original.
The backend has said xmode[01] is what it expects. The only
reason you wouldn't write xmode[01] in the create_input_operand
line is if xmode[01] are VOIDmode.
That seems like mistake number one, particularly for a binop,
but I'll accept that for the nonce. Which pattern triggered
the problem in the target?
Then we've got the conditionalization in the init of mode[01],
which is presumably to handle CONST_INT as an input.
What about something like
xmode0 = insn_data...
if (xmode0 == VOIDmode)
xmode0 = mode;
mode0 = GET_MODE (xop0);
if (mode0 == VOIDmode)
mode0 = mode;
if (xmode0 != mode0)
convert_modes
create_input_operand(..., xmode0)
? That seems more obvious than what you have. And I *think*
it should produce the same results. If it doesn't, then this
whole block of code needs a lot more explanation.
r~