Re: [PATCH rdma-next 1/2] arm64/io: add memcpy_toio_64

[Mark Rutland <mark.rutland@arm.com>]

On Tue, Jan 16, 2024 at 02:51:21PM -0400, Jason Gunthorpe wrote:
> Hey Catalin,
> 
> I'm just revising this and I'm wondering if you know why ARM64 has this:
> 
> #define __raw_writeq __raw_writeq
> static __always_inline void __raw_writeq(u64 val, volatile void __iomem *addr)
> {
> 	asm volatile("str %x0, [%1]" : : "rZ" (val), "r" (addr));
> }
> 
> Instead of
> 
> #define __raw_writeq __raw_writeq
> static __always_inline void __raw_writeq(u64 val, volatile void __iomem *addr)
> {
> 	asm volatile("str %x0, %1" : : "rZ" (val), "m" (*(volatile u64 *)addr));
> }
> 
> ?? Like x86 has.

I believe this is for the same reason as doing so in all of our other IO
accessors.

We've deliberately ensured that our IO accessors use a single base register
with no offset as this is the only form that HW can represent in ESR_ELx.ISS.SRT
when reporting a stage-2 abort, which a hypervisor may use for emulating IO.

Mark.

> 
> The codegen for a 64 byte unrolled copy loop is way better with "m" on gcc:
> 
> "r" constraint (gcc 13.2.0):
> 
> .L3:
>         ldr     x3, [x1]
>         str x3, [x0]
>         ldr     x3, [x1, 8]
>         add     x4, x0, 8
>         str x3, [x4]
>         ldr     x3, [x1, 16]
>         add     x4, x0, 16
>         str x3, [x4]
>         ldr     x3, [x1, 24]
>         add     x4, x0, 24
>         str x3, [x4]
>         ldr     x3, [x1, 32]
>         add     x4, x0, 32
>         str x3, [x4]
>         ldr     x3, [x1, 40]
>         add     x4, x0, 40
>         str x3, [x4]
>         ldr     x3, [x1, 48]
>         add     x4, x0, 48
>         str x3, [x4]
>         ldr     x3, [x1, 56]
>         add     x4, x0, 56
>         str x3, [x4]
>         add     x1, x1, 64
>         add     x0, x0, 64
>         cmp     x2, x1
>         bhi     .L3
> 
> "m" constraint:
> 
> .L3:
>         ldp     x10, x9, [x1]
>         ldp     x8, x7, [x1, 16]
>         ldp     x6, x5, [x1, 32]
>         ldp     x4, x3, [x1, 48]
>         str x10, [x0]
>         str x9, [x0, 8]
>         str x8, [x0, 16]
>         str x7, [x0, 24]
>         str x6, [x0, 32]
>         str x5, [x0, 40]
>         str x4, [x0, 48]
>         str x3, [x0, 56]
>         add     x1, x1, 64
>         add     x0, x0, 64
>         cmp     x2, x1
>         bhi     .L3
> 
> clang 17 doesn't do any better either way, it doesn't seem to do
> anything with 'm', but I guess it could..
> 
> clang 17 (either):
> 
> .LBB0_2:                                // =>This Inner Loop Header: Depth=1
>         ldp     x9, x10, [x1]
>         add     x14, x0, #8
>         add     x18, x0, #40
>         ldp     x11, x12, [x1, #16]
>         add     x2, x0, #48
>         add     x3, x0, #56
>         ldp     x13, x15, [x1, #32]
>         ldp     x16, x17, [x1, #48]
>         str     x9, [x0]
>         str     x10, [x14]
>         add     x9, x0, #16
>         add     x10, x0, #24
>         add     x14, x0, #32
>         str     x11, [x9]
>         str     x12, [x10]
>         str     x13, [x14]
>         str     x15, [x18]
>         str     x16, [x2]
>         str     x17, [x3]
>         add     x1, x1, #64
>         add     x0, x0, #64
>         cmp     x1, x8
>         b.lo    .LBB0_2
> 
> It doesn't matter for this series, but it seems like something ARM64
> might want to look at to improve..
> 
> Jason
>