On Fri, Jul 09, 2021 at 06:14:49PM -0500, Peter Bergner wrote:
> Ok, I removed the consecutive_mem_locations() function from the previous
> patch and just call adjacent_mem_locations() directly now. I also moved
> rs6000_split_multireg_move() to later in the file to fix the declaration
> issue. However, since rs6000_split_multireg_move() is where the new code
> was added to emit the lxvp's, it can be hard to see what I changed because
> of the move. I'll note that all of my changes are restrictd to within the
>
> if (GET_CODE (src) == UNSPEC)
> {
> gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE);
> ...
> }
>
> ...code section. Does this look better? I'm currently running bootstraps
> and regtests on LE and BE.
It is very hard to see the differences now. Don't fold the changes into
one patch, just have the code movement in a separate trivial patch, and
then the actual changes as a separate patch? That way it is much easier
to review :-)
> + unsigned subreg =
> + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i);
This is not new code, but it caught my eye, so just for the record: the
"=" should start a new line:
unsigned subreg
= WORDS_BIG_ENDIAN ? i : (nregs - reg_mode_nregs - i);
(and don't put parens around random words please :-) ).
> + int nvecs = XVECLEN (src, 0);
> + for (int i = 0; i < nvecs; i++)
> + {
> + rtx opnd;
Just "op" (and "op2") please? If you use long names you might as well
just spell "operand" :-)
> + if (WORDS_BIG_ENDIAN)
> + opnd = XVECEXP (src, 0, i);
> + else
> + opnd = XVECEXP (src, 0, nvecs - i - 1);
Put this together with the case below as well? Probably keep the
WORDS_BIG_ENDIAN test as the outer "if"?
> + /* If we are loading an even VSX register and the memory location
> + is adjacent to the next register's memory location (if any),
> + then we can load them both with one LXVP instruction. */
> + if ((regno & 1) == 0)
> + {
> + if (WORDS_BIG_ENDIAN)
> + {
> + rtx opnd2 = XVECEXP (src, 0, i + 1);
> + if (adjacent_mem_locations (opnd, opnd2) == opnd)
> + {
> + opnd = adjust_address (opnd, OOmode, 0);
> + /* Skip the next register, since we're going to
> + load it together with this register. */
> + i++;
> + }
> + }
> + else
> + {
> + rtx opnd2 = XVECEXP (src, 0, nvecs - i - 2);
> + if (adjacent_mem_locations (opnd2, opnd) == opnd2)
> + {
> + opnd = adjust_address (opnd2, OOmode, 0);
> + /* Skip the next register, since we're going to
> + load it together with this register. */
> + i++;
> + }
> + }
> + }
I think it is fine now, but please factor the patch and repost. Thanks!
Segher
