RFR: 8372980: [VectorAPI] AArch64: Add intrinsic support for unsigned min/max reduction operations [v3]

[Xiaohong Gong]

On Thu, 22 Jan 2026 17:27:58 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Eric Fang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Extract some helper functions for better readability
>
> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.hpp line 50:
> 
>> 48:   void sve_maxv(bool is_unsigned, FloatRegister dst, SIMD_RegVariant size,
>> 49:                 PRegister pg, FloatRegister src);
>> 50: 
> 
> Using separate definitions here is adding unnecessary complexity.
> 
> I'd do something like this in the header file:
> 
> 
>   // Typedefs used to disambiguate overloaded member functions.
>   typedef void (Assembler::*neon_reduction2)
>     (FloatRegister, Assembler::SIMD_Arrangement, FloatRegister);
>   typedef void (Assembler::*sve_reduction3)
>     (FloatRegister, Assembler::SIMD_RegVariant, PRegister, FloatRegister);
> 
>   // Helper functions for min/max reduction operations
>   void neon_minp(bool is_unsigned, FloatRegister dst, SIMD_Arrangement size,
>                  FloatRegister src1, FloatRegister src2) {
>     auto m = is_unsigned ? &Assembler::uminp : &Assembler::sminp;
>     (this->*m)(dst, size, src1, src2);
>   }
>   void neon_maxp(bool is_unsigned, FloatRegister dst, SIMD_Arrangement size,
>                    FloatRegister src1, FloatRegister src2) {
>     auto m = is_unsigned ? &Assembler::umaxp : &Assembler::smaxp;
>     (this->*m)(dst, size, src1, src2);
>   }
>   void neon_minv(bool is_unsigned, FloatRegister dst, SIMD_Arrangement size,
>                  FloatRegister src) {
>     auto m = is_unsigned ? (neon_reduction2)&Assembler::uminv : &Assembler::sminv;
>     (this->*m)(dst, size, src);
>   }
>   void neon_maxv(bool is_unsigned, FloatRegister dst, SIMD_Arrangement size,
>                  FloatRegister src) {
>     auto m = is_unsigned ? (neon_reduction2)&Assembler::umaxv : &Assembler::smaxv;
>     (this->*m)(dst, size, src);
>   }
>   void sve_minv(bool is_unsigned, FloatRegister dst, SIMD_RegVariant size,
>                 PRegister pg, FloatRegister src) {
>     auto m = is_unsigned ? (sve_reduction3)&Assembler::sve_uminv : &Assembler::sve_sminv;
>     (this->*m)(dst, size, pg, src);
>   }
>   void sve_maxv(bool is_unsigned, FloatRegister dst, SIMD_RegVariant size,
>                 PRegister pg, FloatRegister src) {
>     auto m = is_unsigned ? (sve_reduction3)&Assembler::sve_umaxv : &Assembler::sve_smaxv;
>     (this->*m)(dst, size, pg, src);
>   }
> 
> 
> 
> To some extent it's a matter of taste, but please try not to use much more repetitive and boilerplate code than you need to.

I’m not sure, but would the code below be better? It might make the macro assembler a bit tidier.

  void neon_minmaxp(bool is_unsigned, bool is_max, FloatRegister dst,
                    SIMD_Arrangement size, FloatRegister src1, FloatRegister src2) {
    auto m = is_unsigned ? (is_max ? &Assembler::umaxp : &Assembler::uminp)
                         : (is_max ? &Assembler::smaxp : &Assembler::sminp);
    (this->*m)(dst, size, src1, src2);
  }

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28693#discussion_r2753257260