Re: [PATCH 2/4] RISC-V: Allow unaligned accesses in cpymemsi expansion

[Jeff Law]

On 5/7/24 11:17 PM, Christoph Müllner wrote:
The RISC-V cpymemsi expansion is called, whenever the by-pieces
infrastructure will not take care of the builtin expansion.
The code emitted by the by-pieces infrastructure may emits code,
that includes unaligned accesses if riscv_slow_unaligned_access_p
is false.

The RISC-V cpymemsi expansion is handled via riscv_expand_block_move().
The current implementation of this function does not check
riscv_slow_unaligned_access_p and never emits unaligned accesses.

Since by-pieces emits unaligned accesses, it is reasonable to implement
the same behaviour in the cpymemsi expansion. And that's what this patch
is doing.

The patch checks riscv_slow_unaligned_access_p at the entry and sets
the allowed alignment accordingly. This alignment is then propagated
down to the routines that emit the actual instructions.

The changes introduced by this patch can be seen in the adjustments
of the cpymem tests.

gcc/ChangeLog:

        * config/riscv/riscv-string.cc (riscv_block_move_straight): Add
        parameter align.
        (riscv_adjust_block_mem): Replace parameter length by align.
        (riscv_block_move_loop): Add parameter align.
        (riscv_expand_block_move_scalar): Set alignment properly if the
        target has fast unaligned access.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/cpymem-32-ooo.c: Adjust for unaligned access.
        * gcc.target/riscv/cpymem-64-ooo.c: Likewise.
Mostly ok.  One concern noted below.



Signed-off-by: Christoph Müllner <christoph.muell...@vrull.eu>
---
  gcc/config/riscv/riscv-string.cc              | 53 +++++++++++--------
  .../gcc.target/riscv/cpymem-32-ooo.c          | 20 +++++--
  .../gcc.target/riscv/cpymem-64-ooo.c          | 14 ++++-
  3 files changed, 59 insertions(+), 28 deletions(-)

@@ -730,8 +732,16 @@ riscv_expand_block_move_scalar (rtx dest, rtx src, rtx 
length)
    unsigned HOST_WIDE_INT hwi_length = UINTVAL (length);
    unsigned HOST_WIDE_INT factor, align;
  
-  align = MIN (MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), BITS_PER_WORD);
-  factor = BITS_PER_WORD / align;
+  if (riscv_slow_unaligned_access_p)
+    {
+      align = MIN (MIN (MEM_ALIGN (src), MEM_ALIGN (dest)), BITS_PER_WORD);
+      factor = BITS_PER_WORD / align;
+    }
+  else
+    {
+      align = hwi_length * BITS_PER_UNIT;
+      factor = 1;
+    }
Not sure why you're using hwi_length here.  That's a property of the 
host, not the target.  ISTM you wanted BITS_PER_WORD here to encourage 
word sized moves irrespective of alignment.

OK with that change after a fresh rounding of testing.

jeff