Hi!
On Fri, Aug 14, 2020 at 05:59:05PM -0500, Aaron Sawdey via Gcc-patches wrote:
> +static rtx
> +gen_lxvl_stxvl_move (rtx dest, rtx src, int length)
> +{
> + gcc_assert (MEM_P (dest) ^ MEM_P (src));
Maybe just "!="?
> + gcc_assert (GET_MODE (dest) == V16QImode && GET_MODE (src) == V16QImode);
> + gcc_assert (length <= 16);
> +
> + bool is_store = MEM_P (dest);
> +
> + /* If the address form is not a simple register, make it so. */
> + if (is_store)
> + {
> + dest = XEXP (dest, 0);
> + if (!REG_P (dest))
> + dest = force_reg (Pmode, dest);
So this changes what "dest" means.
Maybe it is clearer if you have a separate variable "addr"? That you
can use for dest and src as well, whichever is memory.
> + if (is_store)
> + return gen_stxvl (src, dest, len);
> + else
> + return gen_lxvl (dest, src, len);
(doubled space -- well I guess you wanted to align the code)
> + /* If we can't succeed in doing it in one pass, we can't do it in the
> + might_overlap case. Bail out and return failure. */
> + if (might_overlap && (num_reg+1) >= MAX_MOVE_REG
> + && bytes > move_bytes)
> + return 0;
The "num_reg+1" isn't obvious, and the comment doesn't say (we usually
write is as "num_reg + 1" fwiw, and the parens are superfluous).
Looks good, thanks! Okay for trunk with or without such changes.
Segher
