From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 85B383858D32 for ; Fri, 13 Jan 2023 11:21:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 85B383858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=foss.arm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=foss.arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 340F7FEC; Fri, 13 Jan 2023 03:21:55 -0800 (PST) Received: from [10.2.78.76] (unknown [10.2.78.76]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 77E4D3F587; Fri, 13 Jan 2023 03:21:12 -0800 (PST) Message-ID: <5e3d222d-4fdc-80e5-39f9-50f9d3ab099a@foss.arm.com> Date: Fri, 13 Jan 2023 11:21:10 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [GCC][PATCH v2] arm: Add cde feature support for Cortex-M55 CPU. Content-Language: en-GB To: Srinath Parvathaneni , "gcc-patches@gcc.gnu.org" Cc: Richard Earnshaw References: <6bc253d0-a642-e89e-0897-e8cb2fc8ce2c@arm.com> From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3495.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_LOTSOFHASH,KAM_SHORT,NICE_REPLY_A,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 31/10/2022 12:38, Srinath Parvathaneni via Gcc-patches wrote: > Hi, > >> -----Original Message----- >> From: Christophe Lyon >> Sent: Monday, October 17, 2022 2:30 PM >> To: Srinath Parvathaneni ; gcc- >> patches@gcc.gnu.org >> Cc: Richard Earnshaw >> 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), 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 >>> >>> * 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 >>> >>> * 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" >>> >>> >>> + auto_sbitmap removable_isa (isa_num_bits); Removable isn't a good description of this variable. I think it would be better to use something closer to the ISA_IGNORE_FOR_MULTILIB macro that gets defined from arm-cpus.in: perhaps 'ignore_multilib_isa'. OK with that change. R.