Hi Trupti,
On Fri, Jul 3, 2026 at 4:17 AM Trupti <[email protected]> wrote:
>
> Hi JeffHi Jeff,
One small suggestion for the patch. For:
private:
static ivec set_vector(int i)
{
#ifdef __GNUC__
- return (ivec){i, i, i, i};
+ return (ivec){(unsigned int)i,(unsigned int)i, (unsigned
int)i, (unsigned int)i};
#else
#error compiler not supported
#endif
I believe you can use this, which usually produces faster code. I'm
not sure how important it is since a C++ constructor is probably _not_
being called on a critical path.
+ return (ivec)vec_splat_s32(i);
There's also a vec_splat_u32 for unsigned integers. But the cast
before the return should handle the type conversion from int to
unsigned int vector.
> > This is how we handle it in Crypto++ [0]. I would not worry about
> > __APPLE_ALTIVEC__. __ALTIVEC__ picks up the use case.
> >
> > In an implementation file that needs the definitions:
> >
> > // could use -maltivec, -mcpu=power7, -mcpu=power8, etc
> > #if defined(__ALTIVEC__) || defined(_ARCH_PWR7)
> > # include "ppc_simd.h"
> > #endif
> >
> > And then at the head of ppc_simd.h: [1]
> >
> > #if defined(__ALTIVEC__)
> > # include <altivec.h>
> > # undef vector
> > # undef pixel
> > # undef bool
> > #endif
> >
> > And then in the same ppc_simd.h file:
> >
> > #if defined(__ALTIVEC__) || defined(CRYPTOPP_DOXYGEN_PROCESSING)
> >
> > /// \brief Vector of 8-bit elements
> > typedef __vector unsigned char uint8x16_p;
> >
> > /// \brief Vector of 16-bit elements
> > typedef __vector unsigned short uint16x8_p;
> >
> > /// \brief Vector of 32-bit elements
> > typedef __vector unsigned int uint32x4_p;
> >
> > #if defined(__VSX__) || defined(_ARCH_PWR8) ||
> > defined(CRYPTOPP_DOXYGEN_PROCESSING)
> > /// \brief Vector of 64-bit elements
> > typedef __vector unsigned long long uint64x2_p;
> > #endif // VSX or ARCH_PWR8
> >
> > #endif // __ALTIVEC__ or Doxygen
> >
> > Now use the typedefs like uint8x16_p, uint16x8_p, uint32x4_p, and
> > uint64x2_p.
> >
> >
> > [0] https://cryptopp.com/
> > [1] https://github.com/weidai11/cryptopp/blob/master/ppc_simd.h
> >
> > JeffHi Jeff,
>
>
> Thank you for taking the time to review and for the pointer to
> ppc_simd.h — really helpful reference.
>
> One thing I confirmed: vec_altivec.hpp only ever gets included when
> __ALTIVEC__ is already true (that check happens earlier, in vec.hpp).
> So adding the same guard inside vec_altivec.hpp doesn't change anything
> right now — it's already guaranteed true.
> But as you suggested, and I think that's the correct way to go, I've
> kept the guard anyway so the header stays safe on its own,
> in case someone includes it directly in the future without going through
> vec.hpp first. Thanks for pointing that out.
>
> I have updated my patch to gate the #include <altivec.h> and #undef
> vector/pixel/bool behind #if defined(__ALTIVEC__), instead of relying on
> __APPLE_ALTIVEC__ reasoning.
>
> Rebuilt and tested on my ppc64el machine.
>
> Updated patches attached. Will go ahead and create the Salsa Merge
> Request with this version.
>
> Thanks again,
> Trupti
