Re: [PATCH] [ARC] Add basic support for double load and store instructions

[Joern Wolfgang Rennecke <gnu at amylaar dot uk>]

On 19/01/16 17:46, Claudiu Zissulescu wrote:
> Hi,
>
> I've prepared a new patch based on the received review (attached). I 
also added a mod on invoke.texi regarding mll64 documentation. This mod 
was missing in the first patch.
>
> I have tested it with dg.exp for arc700, archs and archs+ll64.
>
> Please let me know if everything is alright.

Oops, I missed this the first time round:
> @@ -7009,14 +7038,23 @@ arc_expand_movmem (rtx *operands)
>    size = INTVAL (operands[2]);
>    /* move_by_pieces_ninsns is static, so we can't use it. */
>    if (align >= 4)
> -    n_pieces = (size + 2) / 4U + (size & 1);
> +    {
> +      if (TARGET_LL64)
> +     n_pieces = (size + 2) / 8U + (size & 1);
> +      else
>
You probably mean something like:

n_pieces = (size + 4) / 8U + ((size >> 1) & 1) + (size & 1);

> -  if (piece > 4)
> +  /* Force 32 bit aligned and larger datum to use 64 bit transfers, if
> +     possible.  */
> +  if (TARGET_LL64 && (piece >= 4))
> +    piece = 8;
This needs another condition size >= 8 .

While looking at this code, I also notice we got a pre-exisitng problem
(read: inefficiency) with the number of pieces we actually make.

if (piece > size)
  piece = size & -size

will pick the smallest power of two in the decomposition of size.. and
that'll be the transfer size for the rest of the loop.
Better would be:

while (piece > size)
  piece >>= 1;


What you do with arc_split_move looks like it'll work for movdi, and the
the problem with movdf_insn+1 is pre-existing (I just noticed that), but
it's rather odd to have the split pattern solely for allocating an a larger
operands array.

I think it would make more sense to remote the assignment to
operands 2..5 in arc_spli_move, and instead use xop[0+swap] / xop[1+swap] /
xop2[swap] / xop[3-swap] directly to emit the insns.