RE: [GCC][PATCH v2] arm: Add cde feature support for Cortex-M55 CPU.

[Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>]

Ping!!
---------------------------------
From: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com> 
Sent: Tuesday, December 6, 2022 11:32 AM
To: gcc-patches@gcc.gnu.org; Richard Earnshaw <Richard.Earnshaw@arm.com>
Cc: Christophe Lyon <Christophe.Lyon@arm.com>
Subject: Re: [GCC][PATCH v2] arm: Add cde feature support for Cortex-M55 CPU.

Ping!!
________________________________________
From: Srinath Parvathaneni
Sent: 31 October 2022 12:38
To: mailto:gcc-patches@gcc.gnu.org <mailto:gcc-patches@gcc.gnu.org>
Cc: Richard Earnshaw <mailto:Richard.Earnshaw@arm.com>; Christophe Lyon <mailto:Christophe.Lyon@arm.com>
Subject: RE: [GCC][PATCH v2] arm: Add cde feature support for Cortex-M55 CPU. 
 
Hi,

> -----Original Message-----
> From: Christophe Lyon <mailto:Christophe.Lyon@arm.com>
> Sent: Monday, October 17, 2022 2:30 PM
> To: Srinath Parvathaneni <mailto:Srinath.Parvathaneni@arm.com>; gcc-
> mailto:patches@gcc.gnu.org
> Cc: Richard Earnshaw <mailto:Richard.Earnshaw@arm.com>
> Subject: Re: [GCC][PATCH] arm: Add cde feature support for Cortex-M55
> CPU.
> 
> Hi Srinath,
> 
> 
> On 10/10/22 10:20, Srinath Parvathaneni via Gcc-patches wrote:
> > Hi,
> >
> > This patch adds cde feature (optional) support for Cortex-M55 CPU,
> > please refer [1] for more details. To use this feature we need to
> > specify +cdecpN (e.g. -mcpu=cortex-m55+cdecp<N>), where N is the
> coprocessor number 0 to 7.
> >
> > Bootstrapped for arm-none-linux-gnueabihf target, regression tested on
> > arm-none-eabi target and found no regressions.
> >
> > [1] https://developer.arm.com/documentation/101051/0101/?lang=en
> (version: r1p1).
> >
> > Ok for master?
> >
> > Regards,
> > Srinath.
> >
> > gcc/ChangeLog:
> >
> > 2022-10-07  Srinath Parvathaneni  <mailto:srinath.parvathaneni@arm.com>
> >
> >          * common/config/arm/arm-common.cc (arm_canon_arch_option_1):
> Ignore cde
> >          options for mlibarch.
> >          * config/arm/arm-cpus.in (begin cpu cortex-m55): Add cde options.
> >          * doc/invoke.texi (CDE): Document options for Cortex-M55 CPU.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2022-10-07  Srinath Parvathaneni  <mailto:srinath.parvathaneni@arm.com>
> >
> >          * gcc.target/arm/multilib.exp: Add multilib tests for Cortex-M55 CPU.
> >
> >
> > ###############     Attachment also inlined for ease of reply
> ###############
> >
> >
> > diff --git a/gcc/common/config/arm/arm-common.cc
> > b/gcc/common/config/arm/arm-common.cc
> > index
> >
> c38812f1ea6a690cd19b0dc74d963c4f5ae155ca..b6f955b3c012475f398382e72
> c9a
> > 3966412991ec 100644
> > --- a/gcc/common/config/arm/arm-common.cc
> > +++ b/gcc/common/config/arm/arm-common.cc
> > @@ -753,6 +753,15 @@ arm_canon_arch_option_1 (int argc, const char
> **argv, bool arch_for_multilib)
> >         arm_initialize_isa (target_isa, selected_cpu->common.isa_bits);
> >         arm_parse_option_features (target_isa, &selected_cpu->common,
> >                               strchr (cpu, '+'));
> > +      if (arch_for_multilib)
> > +   {
> > +     const enum isa_feature removable_bits[] =
> {ISA_IGNORE_FOR_MULTILIB,
> > +                                                isa_nobit};
> > +     sbitmap isa_bits = sbitmap_alloc (isa_num_bits);
> > +     arm_initialize_isa (isa_bits, removable_bits);
> > +     bitmap_and_compl (target_isa, target_isa, isa_bits);
> > +   }
> > +
> 
> I can see the piece of code you add here is exactly the same as the one a few
> lines above when handling "if (arch)". Can this be moved below and thus be
> common to the two cases, or does it have to be performed before
> bitmap_ior of fpu_isa?

Thanks for pointing out this, I have moved the common code below the arch and cpu
if blocks in the attached patch.
 
> Also, IIUC, CDE was already optional for other CPUs (M33, M35P, star-mc1),
> so the hunk above fixes a latent bug when handling multilibs for these CPUs
> too? If so, maybe worth splitting the patch into two parts since the above is
> not strictly related to M55?
>
Even though CDE is optional for the mentioned CPUs as per the specs, the code to
enable CDE as optional feature is missing in current compiler.
Current GCC compiler supports CDE as optional feature only with -march options and
this pass adds CDE as optional for M55 and so this is not a fix bug.

> But I'm not a maintainer ;-)
> 
> Thanks,
> 
> Christophe
> 
> >         if (fpu && strcmp (fpu, "auto") != 0)
> >      {
> >        /* The easiest and safest way to remove the default fpu diff
> > --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in index
> >
> 5a63bc548e54dbfdce5d1df425bd615d81895d80..aa02c04c4924662f3ddd58e
> 69673
> > 92ba3f4b4a87 100644
> > --- a/gcc/config/arm/arm-cpus.in
> > +++ b/gcc/config/arm/arm-cpus.in
> > @@ -1633,6 +1633,14 @@ begin cpu cortex-m55
> >    option nomve remove mve mve_float
> >    option nofp remove ALL_FP mve_float
> >    option nodsp remove MVE mve_float
> > + option cdecp0 add cdecp0
> > + option cdecp1 add cdecp1
> > + option cdecp2 add cdecp2
> > + option cdecp3 add cdecp3
> > + option cdecp4 add cdecp4
> > + option cdecp5 add cdecp5
> > + option cdecp6 add cdecp6
> > + option cdecp7 add cdecp7
> >    isa quirk_no_asmcpu quirk_vlldm
> >    costs v7m
> >    vendor 41
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index
> >
> aa5655764a0360959f9c1061749d2cc9ebd23489..26857f7a90e42d925bc69086
> 86ac
> > 78138a53c4ad 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -21698,6 +21698,10 @@ floating-point instructions on @samp{cortex-
> m55}.
> >   Disable the M-Profile Vector Extension (MVE) single precision floating-
> point
> >   instructions on @samp{cortex-m55}.
> >
> > +@item +cdecp0, +cdecp1, ... , +cdecp7 Enable the Custom Datapath
> > +Extension (CDE) on selected coprocessors according to the numbers
> > +given in the options in the range 0 to 7 on @samp{cortex-m55}.
> > +
> >   @item  +nofp
> >   Disables the floating-point instructions on @samp{arm9e},
> >   @samp{arm946e-s}, @samp{arm966e-s}, @samp{arm968e-s},
> @samp{arm10e},
> > diff --git a/gcc/testsuite/gcc.target/arm/multilib.exp
> > b/gcc/testsuite/gcc.target/arm/multilib.exp
> > index
> >
> 2fa648c61dafebb663969198bf7849400a7547f6..7a977bff58b7b68bfe9e49d7
> 6029
> > 89a39caa6534 100644
> > --- a/gcc/testsuite/gcc.target/arm/multilib.exp
> > +++ b/gcc/testsuite/gcc.target/arm/multilib.exp
> > @@ -851,6 +851,18 @@ if {[multilib_config "rmprofile"] } {
> >      {-mcpu=cortex-m55+nomve+nofp -mfpu=auto -mfloat-abi=softfp}
> "thumb/v8-m.main/nofp"
> >      {-mcpu=cortex-m55+nodsp+nofp -mfpu=auto -mfloat-abi=soft}
> "thumb/v8-m.main/nofp"
> >      {-mcpu=cortex-m55+nodsp+nofp -mfpu=auto -mfloat-abi=softfp}
> "thumb/v8-m.main/nofp"
> > +   {-mcpu=cortex-m55 -mfloat-abi=hard -mfpu=auto} "thumb/v8-
> m.main+dp/hard"
> > +   {-mcpu=cortex-m55+cdecp0 -mfloat-abi=hard -mfpu=auto}
> "thumb/v8-m.main+dp/hard"
> > +   {-mcpu=cortex-m55+nomve+cdecp0 -mfloat-abi=hard -mfpu=auto}
> "thumb/v8-m.main+dp/hard"
> > +   {-mcpu=cortex-
> m55+cdecp0+cdecp1+cdecp2+cdecp3+cdecp4+cdecp5+cdecp6+cdecp7 -
> mfloat-abi=hard -mfpu=auto} "thumb/v8-m.main+dp/hard"
> > +   {-mcpu=cortex-m55 -mfloat-abi=softfp -mfpu=auto} "thumb/v8-
> m.main+dp/softfp"
> > +   {-mcpu=cortex-m55+cdecp0 -mfloat-abi=softfp -mfpu=auto}
> "thumb/v8-m.main+dp/softfp"
> > +   {-mcpu=cortex-m55+nomve+cdecp0 -mfloat-abi=softfp -mfpu=auto}
> "thumb/v8-m.main+dp/softfp"
> > +   {-mcpu=cortex-
> m55+cdecp0+cdecp1+cdecp2+cdecp3+cdecp4+cdecp5+cdecp6+cdecp7 -
> mfloat-abi=softfp -mfpu=auto} "thumb/v8-m.main+dp/softfp"
> > +   {-mcpu=cortex-m55 -mfloat-abi=soft -mfpu=auto} "thumb/v8-
> m.main/nofp"
> > +   {-mcpu=cortex-m55+cdecp0 -mfloat-abi=soft -mfpu=auto}
> "thumb/v8-m.main/nofp"
> > +   {-mcpu=cortex-m55+nomve+cdecp0 -mfloat-abi=soft -mfpu=auto}
> "thumb/v8-m.main/nofp"
> > +   {-mcpu=cortex-
> m55+cdecp0+cdecp1+cdecp2+cdecp3+cdecp4+cdecp5+cdecp6+cdecp7 -
> mfloat-abi=soft -mfpu=auto} "thumb/v8-m.main/nofp"
> >      {-march=armv8-m.main+cdecp0 -mfpu=auto -mfloat-abi=soft}
> "thumb/v8-m.main/nofp"
> >      {-march=armv8-m.main+fp+cdecp0 -mfpu=auto -mfloat-abi=soft}
> "thumb/v8-m.main/nofp"
> >      {-march=armv8-m.main+fp.dp+cdecp0 -mfpu=auto -mfloat-abi=soft}
> "thumb/v8-m.main/nofp"
> >
> >
> >