Re: [Regression] [PATCH v5 2/4] vdso: Switch get/put unaligned from packed struct to memcpy

[David Laight <david.laight.linux@gmail.com>]

On Tue, 20 Oct 2026 09:21:47 -0700
Ian Rogers <irogers@google.com> wrote:

> On Mon, Oct 19, 2026 at 3:05 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Mon, Oct 19, 2026, at 23:14, Ian Rogers wrote:  
> > > On Mon, Oct 19, 2026 at 2:00 PM Arnd Bergmann <arnd@arndb.de> wrote:  
> > >> On Mon, Oct 19, 2026, at 19:04, Ian Rogers wrote:  
> > >
> > > Is there a reason that the compiler's instruction selection can't make
> > > the same decision for the memcpy case as the __packed case? We can
> > > work around the compiler bug with a config option.
> > > The intent with using __builtin_memcpy, as opposed to memcpy, is to
> > > avoid a library call.  
> >
> > As far as I understand, the only difference between calling
> > __builtin_memcpy() and memcpy() is the need for a prototype
> > for the latter, I would expect the compiler to behave the exact
> > same for for both otherwise. Whether any of the string.h functions
> > are replaced by an inline alternative is a complex optimizer
> > decision, not something you can enforce by calling the function
> > by one name rather than the other.  
> 
> So to repeat, relying on __packed and assuming __packed won't be
> lowered to a memcpy(), memset(), etc. call by the compiler is false.
> Both GCC and Clang lower __packed accesses and assignments to
> out-of-line memcpy()/memset() calls in a variety of cases on
> -mno-unaligned-access targets:
> 
> https://godbolt.org/z/qKWv6Gdav
> 
> Using the __packed definitions of get_unaligned() and put_unaligned():
> 
>     #define __get_unaligned_t(type, ptr) ({                        \
>         const struct { type x; } __packed *__get_pptr =
> (typeof(__get_pptr))(ptr); \
>         __get_pptr->x;                                \
>     })
> 
>     #define __put_unaligned_t(type, val, ptr) do {                    \
>         struct { type x; } __packed *__put_pptr =
> (typeof(__put_pptr))(ptr);    \
>         __put_pptr->x = (val);                            \
>     } while (0)
> 
> 1. 4-byte aggregate with __packed put_unaligned():
> 
>     struct s4 { short a, b; };
>     void put_s4(struct s4 *p, struct s4 v)
>     {
>         put_unaligned(v, p);
>     }
> 
> Even though struct s4 is 4 bytes and passed in a single register (r1),
> GCC with -O2 -mno-unaligned-access spills r1 to the stack and calls
> memcpy() for 4 bytes:
> 
>     put_s4:
>         str    lr, [sp, #-4]!
>         mov    r2, #4
>         sub    sp, sp, #12
>         str    r1, [sp, #4]
>         add    r1, sp, r2
>         bl    memcpy
>         add    sp, sp, #12
>         ldr    pc, [sp], #4
> 
> 2. Copying a 4-byte __packed struct containing a single int:
> 
>     struct packed_int { int x; } __packed;
>     void copy_packed_int(struct packed_int *dst, const struct packed_int *src)
>     {
>         *dst = *src;
>     }
> 
> Clang's frontend lowers *dst = *src directly to @llvm.memcpy(...,
> align 1), and GCC with -O2 -mno-unaligned-access emits an out-of-line
> call to memcpy(..., 4) (as does riscv64-linux-gnu-gcc -Os).
> 
> 3. struct debug_entry __packed (from tools/perf/util/jitdump.h):
> 
>     struct debug_entry {
>         unsigned long long addr;
>         int lineno;
>         int discrim;
>     } __packed;
> 
>     struct debug_entry get_entry(const void *p)
>     {
>         return get_unaligned((const struct debug_entry *)p);
>     }
> 
>     void copy_entry(struct debug_entry *dst, const struct debug_entry *src)
>     {
>         *dst = *src;
>     }
> 
> Whether accessed via the __packed get_unaligned() macro or via direct
> assignment on a __packed struct, both GCC and Clang emit memcpy()
> calls. In fact, with the __packed statement-expression macro, GCC
> emits two back-to-back memcpy() calls in get_entry() (one into the
> statement expression temporary on the stack, and a second from the
> stack into the return slot).
> 
> 4. Loops of scalar int __packed get_unaligned() / put_unaligned():
> 
>     void copy_8_ints(int * restrict dst, const int * restrict src)
>     {
>         for (int i = 0; i < 8; i++)
>             put_unaligned(get_unaligned(&src[i]), &dst[i]);
>     }
> 
> Even for a scalar int, loop idiom recognition in both GCC and Clang at
> -O2 turns the __packed loads/stores into a call to memcpy(dst, src,
> 32).
> 
> 5. Adjacent scalar __packed zero stores promoted to memset():
> 
>     void zero_2_u64(long long *dst)
>     {
>         put_unaligned(0LL, &dst[0]);
>         put_unaligned(0LL, &dst[1]);
>     }
> 
> Even in straight-line code without a loop, Clang's MemCpyOpt merges
> two adjacent 64-bit (or four 32-bit) __packed zero stores into
> @llvm.memset(..., align 1), which lowers under -O2
> -mno-unaligned-access to an out-of-line memset() call:
> 
>     zero_2_u64:
>         mov    r1, #0
>         mov    r2, #16
>         b    memset
> 
> On the Kconfig patch, I have updated v2 so
> CONFIG_VDSO_UNALIGNED_PACKED defaults to
> !HAVE_EFFICIENT_UNALIGNED_ACCESS to cover all strict-alignment targets
> rather than just CPU_32v5.
> 
> I continue to think that:
> 
> 1) given __packed implicitly can be turned into a memcpy, it is no
> better than a memcpy;

I suspect it happens less often with __packed.
You can probably add volatile to stop the other merging.

It really might be necessary to give up on the C versions and use asm()
(at least for the unaligned access case).

David

> 2) the __builtin_memcpy for efficient unaligned targets avoids type
> punning, is more intention revealing, is clearer for compiler writers,
> and is what compiler writers tell me they want.
> 
> That said, the code with a __packed or with a memcpy can be made to
> work, optimized, etc. I'd like to be pragmatic and address Stefan and
> Marc's problem so a revert works, I just think we can try to do a
> little better.
> 
> Thanks,
> Ian
>