Re: splitting const_int's

[Richard Sandiford <rdsandiford at googlemail dot com>]

Just a few suggestions for simplfying this...

"Omar Torres" <gcc.omar@gmail.com> writes:
> (define_split
>    [(set (match_operand:HI 0 "nonimmediate_operand" "")
>          (match_operand:HI 1 "general_operand"      ""))]
>    "reload_completed
>     && REG_P (operands[0])"

You might as well make the first operand a register_operand and
avoid the REG_P check.  More importantly:

>        operands[4] = simplify_gen_subreg(QImode,operands[1],HImode,0);
>        operands[5] = simplify_gen_subreg(QImode,operands[1],HImode,1);

...this code is designed to handle REGs and CONST_INTs correctly,
and avoid the problem you were seeing.  (As Eric says, gen_int_mode
is the canonical way of generating a correct CONST_INT in cases where
you do need to create one manually.  In this case it's simpler to use
simplify_gen_subreg though.)

I notice you're generating subregs of symbolic constants,
but I'm not sure that's really supported.

As far as the MEM_P case goes:

>    else if (MEM_P (operands[1]) || CONST == GET_CODE (operands[1])) {
>        int ioff;
>        rtx base, off;
>        base = XEXP (operands[1],0);
>        off  = gen_rtx_CONST_INT (VOIDmode, 0);
>        if (CONST == GET_CODE (base)) {
>            base = XEXP(base,0);
>        }
>        if (PLUS == GET_CODE (base)) {
>         off  = XEXP (base,1);
>         base = XEXP (base,0);
>        }
>        gcc_assert (REG == GET_CODE (base)
>                    && CONST_INT == GET_CODE (off));
>        ioff = INTVAL (off);
>        gcc_assert (254 >= ioff
>                    && 0 <= ioff);
>        operands[4] = gen_rtx_MEM (QImode,
>                                   gen_rtx_PLUS (Pmode,
>                                                 base,
>                                                 gen_rtx_CONST_INT 		
>                                             (QImode, ioff + 1)));
>        operands[5] = gen_rtx_MEM (QImode,
>                                   gen_rtx_PLUS (Pmode,
>                                                 base,
>
> gen_rtx_CONST_INT(QImode, ioff)));
>    }

you can use:

    operands[4] = adjust_address (operands[1], QImode, 1);
    operands[5] = adjust_address (operands[1], QImode, 0);

The CONST handling looks suspicious here.  CONSTs aren't memory references,
but you split them into memory references.

Also, you need to beware of cases in which operands[1] overlaps
operands[0].  The splitter says:

 [(set (match_dup 2) (match_dup 4))
  (set (match_dup 3) (match_dup 5))]

and operands[2] is always the highpart:

   operands[2] = gen_highpart(QImode, operands[0]);

but consider the case in which operands[1] (and thus operands[4])
is a memory reference that uses the high part of operands[0] as
a base register.  In that case, the base register will be modified
by the first split instruction and have the wrong value in the
second split instruction.  See other ports for the canonical way
of handling this.

Richard