Re: [PATCH] lib/raid6: use vdupq_n_u8 to avoid endianness warnings

From: Nick Desaulniers <ndesaulniers@google.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Mark Brown <broonie@kernel.org>,
	Nathan Chancellor <natechancellor@gmail.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] lib/raid6: use vdupq_n_u8 to avoid endianness warnings
Date: Thu, 28 Feb 2019 10:51:17 -0800	[thread overview]
Message-ID: <CAKwvOdm=y-5XHt1V-99Cu+kMVy9Ah6KdE4h2gY+bHgr4eem15A@mail.gmail.com> (raw)
In-Reply-To: <b3bf85a9-85de-3800-787e-2b00d3edd939@arm.com>

On Thu, Feb 28, 2019 at 10:00 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 26/02/2019 20:44, Nick Desaulniers wrote:
> > On Mon, Feb 25, 2019 at 11:19 PM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> >>
> >> On Tue, 26 Feb 2019 at 05:03, <ndesaulniers@google.com> wrote:
> >>>
> >>> Clang warns: vector initializers are not compatible with NEON intrinsics
> >>> in big endian mode [-Wnonportable-vector-initialization]
> >>>
> >>> While this is usually the case, it's not an issue for this case since
> >>> we're initializing the uint8x16_t (16x uint8_t's) with the same value.
> >>>
> >>> Instead, use vdupq_n_u8 which both compilers lower into a single movi
> >>> instruction: https://godbolt.org/z/vBrgzt
> >>>
> >>> This avoids the static storage for a constant value.
> >>>
> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/214
> >>> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> >>> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> >>
> >> Much better, thanks,
> >>
> >> Did you double check that the intrinsic exists on 32-bit ARM as well?
> >> I assume it does, but please make sure if you haven't yet.
> >
> > Thanks for the review!
> > Looking through Clang's generated arm_neon.h, vdupq_n_u8 seems to have
> > 2 definitions predicated on __LITTLE_ENDIAN__ (not __arch64__ or
> > __ARM_ARCH >= 8 like some of the other types and functions).
> >
> > So NEON got some additions in v8?  Is there a doc that lists them?
> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491g/BABDBBJB.html
> > is where I found vdupq_n_u8, but it doesn't seem to mention
> > compatibility (so I assume it's been around since the introduction of
> > NEON?).
>
> FWIW the most recent 'proper' spec document I know of is this one:
>
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0073b/index.html

Bookmarked, thanks!
Ard, page 171 mentions armv7, armv8 for supported architectures for vdupq_n_u8.

>
>
> Apparently we have a more interactive playground on the new site, too:
>
> https://developer.arm.com/technologies/neon/intrinsics

Also bookmarked! I'm also super happy to see this; I'm familiar with
Intel's equivalent:
https://software.intel.com/sites/landingpage/IntrinsicsGuide/

Interactive sites like these are quite useful.  Reading a post
recently: https://www.sigarch.org/simd-instructions-considered-harmful/

"The IA-32 instruction set has grown from 80 to around 1400
instructions since 1978, largely fueled by SIMD."

reminded me how useful and almost necessary the interactive sites are
for navigating the large swathes of SIMD extensions.  (no comment on
the title of that article)

-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel