[Patch]: Splitter for Altivec moves
Aldy Hernandez
aldyh@redhat.com
Thu Jun 26 16:52:00 GMT 2003
[rth: questions below]
> I reworked patch and tested it with a cross powerpc-eabialtivec
> configuration.
Did you test with the little endian multilibs? Just curious, because
I think there are endianness issues with your patch.
Also, could you please come up with a testcase that triggers at least
one of the conditions, ideally all of them?
> * config/rs6000/rs6000.c (rs6000_split_altivec_in_gprs): New function.
> (altivec_in_gprs_p): New function.
> * config/rs6000/rs6000-protos (rs6000_split_altivec_in_gprs): New
> prototype.
> (altivec_in_gprs_p): New prototype.
> * config/rs6000/altivec.md (*movv4si_internal): Change
> multi-assembler alternative to '#'. Add postreload splitter to
> handle this cases.
> (*movv4hi_internal): Likewise.
> (*movv4qi_internal): Likewise.
> (*movv4sf_internal): Likewise.
You need to add a space after each file ChangeLog entry. Ala:
* some_file: blah
* some_other_file: blah
> + extern void rs6000_split_altivec_in_gprs (rtx*);
Space before '*'.
> + /* Return 1 for all valid move insn operand combination involving altivec
> + vectors in gprs. */
> +
> + int
> + altivec_in_gprs_p (rtx op0, rtx op1)
> + {
> + if (REG_P (op0) && !ALTIVEC_REGNO_P (REGNO (op0)))
> + return 1;
> +
> + if (REG_P (op1) && !ALTIVEC_REGNO_P (REGNO (op1)))
> + return 1;
> + return 0;
How about an FPR? An FPR is a register, but it's not a AltiVec
register. The constraint in the original insn should make guarantee
we get either a GPR or an AltiVec register by the time we get here,
but it's better to do things right. We may want to use
altivec_in_gprs_p() for something else later. You want something like:
if (REG_P (op0) && REGNO_REG_CLASS (REGNO (op0)) == GENERAL_REGS)
> + This is done, if a gprs is at least one of the operands. In this case
"This is done if GPRs are one of the operands..."
> + a sequence of simple move insns has to be issued. The sequence of this
A couple syntax problems, plus you need a 2 spaces after a period:
"a sequence of simple move insns have to be issued. The sequence of
these...".
> + nregs = HARD_REGNO_NREGS(reg, mode);
Need a space before "(".
> + else
> + {
> + j = -1;
> +
> + if (GET_CODE (operands[1]) == MEM)
> + {
> + rtx breg;
> + /* We have offsettable addresses only. If we use one of the
> + registers to address memory, we have change that register last. */
> + breg = GET_CODE (XEXP (operands[1], 0)) == PLUS ?
> + XEXP (XEXP (operands[1], 0), 0) :
> + XEXP (operands[1], 0);
> +
> + if (REGNO (breg) >= REGNO (operands[0])
> + && REGNO (breg) < REGNO (operands[0]) + nregs)
> + j = REGNO (breg) - REGNO (operands[0]);
> + }
> +
> + for (i = 0; i < nregs; i++)
> + {
> + /* Calculate index to next subword. */
> + j++;
> + if (j == nregs)
> + j = 0;
> +
> + operands[i + 2] = operand_subword (operands[0], j, 0, mode);
> + operands[i + 2 + nregs] =
> + operand_subword (operands[1], j, 0, mode);
> +
> + }
> + }
> + }
I'm almost sure your operand_subword() call will have endianness
issues when we do MEM->GPRs or MEM<-GPRs.
Rth, can you comment on this?
Aldy
More information about the Gcc-patches
mailing list