On 02/22/2016 07:28 AM, Jakub Jelinek wrote:
On Mon, Feb 22, 2016 at 07:09:41AM -0700, Jeff Law wrote:
On 02/22/2016 06:08 AM, Jakub Jelinek wrote:
Hi!

Supposedly my recent invalid shift count expansion changes broke
m68k bootstrap, we now really require that the shift expanders
have some non-VOIDmode, so that we can convert_mode it to that
mode.  But m68k didn't specify mode.  For valid shift counts
the patch makes no difference beyond fixing the ICE, those are
all CONST_INTs which when converted to CONST_INTs valid for
SImode give the same values.

Fixed thusly, Matthias has kindly tested it on m68k-linux, ok for trunk?

2016-02-22  Jakub Jelinek  <ja...@redhat.com>

        PR target/69885
        * config/m68k/m68k.md (ashldi3, ashrdi3, lshrdi3): Use
        SImode for last match_operand.

        * gcc.dg/pr69885.c: New test.
OK for the trunk.  Can you take a peek at other ports and see if any have
the same issue?  It wouldn't surprise me if some of the older ports did.  If
so, similar patches for other ports are pre-approved.

Quick grepping for 'match_operand .*const_int_operand' used in
define_insn/define_expand that could possibly be any of the shift/rotate
expanders didn't reveal anything in any other port.
Excellent.  Thanks for verifying.


It also seems like an update for md.texi is in order.  That can be handled
as a pre-approved follow-up as well.

We already document in md.texi:
Here @var{m} is the mode of
operand 0 and operand 1; operand 2's mode is specified by the
instruction pattern, and the compiler will convert the operand to that
mode before generating the instruction.

Would you like to somehow stress that operand 2's mode must be specified
in the instruction pattern or expander?  As it says that the compiler will
convert it to that mode, I'd kind of say that VOIDmode should not be used
there.
Yea, as it's currently written, it's a bit vague. I think your suggesting of saying that VOIDmode should not be used is what we need. I'll leave the final word-smithing to you.

jeff

Reply via email to