On Sun, Aug 21, 2022 at 12:47 PM Nathan Bossart <nathandboss...@gmail.com> wrote: > > I spent some more time looking at this one, and I had a few ideas that I > thought I'd share. 0001 is your v6 patch with a few additional changes, > including simplying the assertions for readability, splitting out the > Vector type into Vector8 and Vector32 (needed for ARM), and adjusting > pg_lfind32() to use the new tools in simd.h. 0002 adds ARM versions of > everything, which obsoletes the other thread I started [0]. This is still > a little rough around the edges (e.g., this should probably be more than 2 > patches), but I think it helps demonstrate a more comprehensive design than > what I've proposed in the pg_lfind32-for-ARM thread [0]. > > Apologies if I'm stepping on your toes a bit here.
Not at all! However, the 32-bit-element changes are irrelevant for json, and make review more difficult. I would suggest keeping those in the other thread starting with whatever refactoring is needed. I can always rebase over that. Not a full review, but on a brief look: - I like the idea of simplifying the assertions, but I can't get behind using platform lfind to do it, since it has a different API, requires new functions we don't need, and possibly has portability issues. A simple for-loop is better for assertions. - A runtime elog is not appropriate for a compile time check -- use #error instead. -- John Naylor EDB: http://www.enterprisedb.com
