Hi Both,

Discussed this with Alice offline and clarified a few things.

The 05/19/2026 16:30, Alice Carlotti wrote:
> On Wed, May 06, 2026 at 09:49:51AM +0100, Tamar Christina wrote:
> > > -----Original Message-----
> > > From: Alfie Richards <[email protected]>
> > > Sent: 04 November 2025 13:58
> > > To: [email protected]
> > > Cc: Richard Earnshaw <[email protected]>; Tamar Christina
> > > <[email protected]>; [email protected]; Alice Carlotti
> > > <[email protected]>; Alex Coplan <[email protected]>; Wilco
> > > Dijkstra <[email protected]>; [email protected]; Alfie
> > > Richards <[email protected]>
> > > Subject: [PATCH v1] aarch64: Add support for FEAT_SSVE_BitPerm
> > > 
> > > Hi All,
> > > 
> > > Requires my patch updating the AArch64 cli arch options
> > > (https://gcc.gnu.org/pipermail/gcc-patches/2025-November/699488.html).
> > > 
> > > Reg tested on AArch64.
> > > 
> > > Okay for master after prereq lands?
> > > 
> > > Alfie
> > > 
> > > -- >8 --
> > > 
> > > Adds support for the FEAT_SSVE_BitPerm AArch64 extension.
> > > 
> > > FEAT_SSVE_BitPerm makes the FEAT_SVE2_BitPerm instructions available in
> > > streaming mode.
> > > 
> > > gcc/ChangeLog:
> > > 
> > >   * config/aarch64/aarch64-c.cc (aarch64_update_cpp_builtins): Add
> > >   __ARM_FEATURE_SSVE_BITPERM.
> > >   * config/aarch64/aarch64-sve-builtins-sve2.def:
> > >   Make sve-bitperm intrinsics streaming compatible.
> > >   * config/aarch64/aarch64-sve2.md: Change gating of sve-bitperm
> > >   instructions.
> > >   * config/aarch64/aarch64.h (TARGET_SVE_BITPERM): New macro.
> > >   (TARGET_SSVE_BITPERM): New macro.
> > >   (TARGET_STREAMING_SSVE_BITPERM): New macro.
> > > 
> > > gcc/testsuite/ChangeLog:
> ...
> 
> > > 
> > > diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-
> > > c.cc
> > > index c3957c762ef..fd940bdf733 100644
> > > --- a/gcc/config/aarch64/aarch64-c.cc
> > > +++ b/gcc/config/aarch64/aarch64-c.cc
> > > @@ -295,6 +295,8 @@ aarch64_update_cpp_builtins (cpp_reader *pfile)
> > >    aarch64_def_or_undef (AARCH64_HAVE_ISA (SME2p1),
> > >                   "__ARM_FEATURE_SME2p1", pfile);
> > >    aarch64_def_or_undef (TARGET_FAMINMAX,
> > > "__ARM_FEATURE_FAMINMAX", pfile);
> > > +  aarch64_def_or_undef (TARGET_SSVE_BITPERM,
> > > "__ARM_FEATURE_SSVE_BITPERM",
> > > +                 pfile);
> > > 
> > >    // Function multi-versioning defines
> > >    aarch64_def_or_undef (targetm.has_ifunc_p (),
> > > diff --git a/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> > > b/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> > > index b622fe33458..6f1601e6f22 100644
> > > --- a/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> > > +++ b/gcc/config/aarch64/aarch64-sve-builtins-sve2.def
> > > @@ -202,8 +202,7 @@ DEF_SVE_FUNCTION (svpmullb_pair, binary_opt_n,
> > > d_unsigned, none)
> > >  DEF_SVE_FUNCTION (svpmullt_pair, binary_opt_n, d_unsigned, none)
> > >  #undef REQUIRED_EXTENSIONS
> > > 
> > > -#define REQUIRED_EXTENSIONS nonstreaming_sve (AARCH64_FL_SVE2 \
> > > -                                       | AARCH64_FL_SVE2_BITPERM)
> > > +#define REQUIRED_EXTENSIONS streaming_compatible (AARCH64_FL_SVE2
> > > | AARCH64_FL_SVE_BITPERM, AARCH64_FL_SSVE_BITPERM)
> > >  DEF_SVE_FUNCTION (svbdep, binary_opt_n, all_unsigned, none)
> > >  DEF_SVE_FUNCTION (svbext, binary_opt_n, all_unsigned, none)
> > >  DEF_SVE_FUNCTION (svbgrp, binary_opt_n, all_unsigned, none)
> > > diff --git a/gcc/config/aarch64/aarch64-sve2.md
> > > b/gcc/config/aarch64/aarch64-sve2.md
> > > index 91091835182..7c86573d364 100644
> > > --- a/gcc/config/aarch64/aarch64-sve2.md
> > > +++ b/gcc/config/aarch64/aarch64-sve2.md
> > > @@ -4191,7 +4191,7 @@ (define_insn
> > > "@aarch64_sve_<sve_int_op><mode>"
> > >     [(match_operand:SVE_FULL_I 1 "register_operand" "w")
> > >      (match_operand:SVE_FULL_I 2 "register_operand" "w")]
> > >     SVE2_INT_BITPERM))]
> > > -  "TARGET_SVE2_BITPERM"
> > > +  "TARGET_SVE_BITPERM"
> > >    "<sve_int_op>\t%0.<Vetype>, %1.<Vetype>, %2.<Vetype>"
> > >    [(set_attr "sve_type" "sve_int_bit_perm")]
> > >  )
> > > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> > > index 47d8fc09925..b15999b5b37 100644
> > > --- a/gcc/config/aarch64/aarch64.h
> > > +++ b/gcc/config/aarch64/aarch64.h
> > > @@ -295,11 +295,25 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE
> > > ATTRIBUTE_UNUSED
> > >                    && AARCH64_HAVE_ISA (SVE_AES) \
> > >                    && TARGET_NON_STREAMING)
> > > 
> > > -/* SVE2 BITPERM instructions, enabled through +sve2-bitperm.  */
> > > +/* SVE BITPERM instructions, enabled through +sve2-bitperm or
> > > +   +ssve-bitperm.  */
> > > +#define TARGET_SVE_BITPERM (AARCH64_HAVE_ISA (SVE_BITPERM))
> > 
> > I find this name to be very confusing (I'm aware the FEAT is FEAT_SVE_)
> > but It looks like TARGET_SVE2_BITPERM isn't used anywhere else but
> > the CPP defines.
> 
> I'd suggest dropping TARGET_SVE2_BITPERM entirely, and using explicit
> AARCH64_HAVE_ISA calls for preprocessor defines in 
> aarch64_update_cpp_builtins.
> I also think these preprocessor defines shouldn't depend on whether we're
> currently in streaming mode, although that might be a minor break with 
> existing
> behaviour.
> 
> > 
> > So how about instead of defining the above..
> > 
> > > +
> > > +/* Non-streaming support for SVE BITPERM instructions, enabled through
> > > +   +sve2-bitperm.  */
> > >  #define TARGET_SVE2_BITPERM (AARCH64_HAVE_ISA (SVE2) \
> > >                        && AARCH64_HAVE_ISA (SVE_BITPERM) \
> > >                        && TARGET_NON_STREAMING)
> > > 
> > 
> > ..drop TARGET_NON_STREAMING from here and change
> > 
> >   aarch64_def_or_undef (TARGET_SVE2_BITPERM,
> >                     "__ARM_FEATURE_SVE2_BITPERM", pfile);
> > 
> > to
> > 
> >   aarch64_def_or_undef (TARGET_SVE2_BITPERM && TARGET_NON_STREAMING,
> >                     "__ARM_FEATURE_SVE2_BITPERM", pfile);
> > 
> > So we avoid the really confusing macro name.
> 
> I'd instead suggesting having a single TARGET_* macro:
> 
> #define TARGET_SVE_BITPERM (AARCH64_HAVE_ISA (SVE_BITPERM) \
>                           && (AARCH64_HAVE_ISA (SVE2) || TARGET_STREAMING) \
>                           && (AARCH64_HAVE_ISA (SSVE_BITPERM) \
>                               || TARGET_NON_STREAMING))
> 
> The .md file would then simply check TARGET_SVE_BITPERM.
> 
> 
> I made similar comments in my review of Sivan's FEAT_SSVE_AES patch
> (https://gcc.gnu.org/pipermail/gcc-patches/2026-March/710828.html).
> We should at least handle FEAT_SSVE_AES and FEAT_SSVE_BITPERM in a consistent
> manner.
> 
> Alice

Agreed with you both here that this is confusing.

I will instead make a singluar macro which is:

#define TARGET_SVE_BITPERM (AARCH64_HAVE_ISA (SVE_BITPERM) \
                            && (AARCH64_HAVE_ISA (SVE2) || TARGET_STREAMING) \
                            && (AARCH64_HAVE_ISA (SSVE_BITPERM) \
                                || TARGET_NON_STREAMING))

as Alice suggested, and use this to gate the instructions in the md files.

Then, as discussed offline, the __ARM_FEATURE_... macros should not depend
on streaming mode. So I will define those separately using only
AARCH64_HAVE_ISA (SVE_BITPERM), AARCH64_HAVE_ISA (SVE2), and AARCH64_HAVE_ISA 
(SSVE_BITPERM)

I will delete TARGET_SSVE_BITPERM and TARGET_SVE2_BITPERM.

KR,
Alfie
> 
> > 
> > > +/* Streaming support for SVE BITPERM instructions, enabled through
> > > +   +ssve-bitperm (which implies SVE2_BITPERM so that all relevant
> > > instructions
> > > +   are streaming compatible).  */
> > > +#define TARGET_SSVE_BITPERM (AARCH64_HAVE_ISA (SSVE_BITPERM))
> > > +
> > > +#define TARGET_STREAMING_SSVE_BITPERM (AARCH64_HAVE_ISA
> > > (SSVE_BITPERM) \
> > > +                                && TARGET_STREAMING)
> > > +
> > > +
> > >  /* SVE2 SHA3 instructions, enabled through +sve2-sha3.  */
> > >  #define TARGET_SVE2_SHA3 (AARCH64_HAVE_ISA (SVE2) \
> > >                     && AARCH64_HAVE_ISA (SVE_SHA3) \

-- 
Alfie Richards

Reply via email to