Re: [PATCH] Add loongarch native checksum implementation.

On Fri, Jun 16, 2023 at 8:28 AM YANG Xudong <yangxudong(at)ymatrix(dot)cn> wrote:
> > +# If the intrinsics are supported, sets
pgac_loongarch_crc32c_intrinsics,
> > +# and CFLAGS_CRC.
> >
> > +# Check if __builtin_loongarch_crcc_* intrinsics can be used
> > +# with the default compiler flags.
> > +# CFLAGS_CRC is set if the extra flag is required.
> >
> > Same here -- it seems we don't need to set CFLAGS_CRC at all. Can you
> > confirm?
> >
>
> We don't need to set CFLAGS_CRC as commented. I have updated the
> configure script to make it align with the logic in meson build script.

(Looking again at v2)

The compilation test is found in c-compiler.m4, which still has all logic
for CFLAGS_CRC, including saving and restoring the old CFLAGS. Can this
also be simplified?

I diff'd pg_crc32c_loongarch.c with the current other files, and found it
is structurally the same as the Arm implementation. That's logical if
memory alignment is important.

/*
- * ARMv8 doesn't require alignment, but aligned memory access is
- * significantly faster. Process leading bytes so that the loop below
- * starts with a pointer aligned to eight bytes.
+ * Aligned memory access is significantly faster.
+ * Process leading bytes so that the loop below starts with a pointer
aligned to eight bytes.

Can you confirm the alignment requirement -- it's not clear what the
intention is since "doesn't require" wasn't carried over. Is there any
documentation (or even a report in some other context) about aligned vs
unaligned memory access performance?

--
John Naylor
EDB: http://www.enterprisedb.com