This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro

From: Prakhar Bahuguna <prakhar dot bahuguna at arm dot com>
To: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
Cc: gcc-patches at gcc dot gnu dot org, nd at arm dot com, Ramana dot Radhakrishnan at arm dot com, Kyrylo dot Tkachov at foss dot arm dot com
Date: Fri, 16 Jun 2017 15:56:51 +0100
Subject: Re: [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
Authentication-results: sourceware.org; auth=none
Authentication-results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com;
Nodisclaimer: True
References: <20170614093535.qcu6x2e5dhphbzyk@e107464-lin.cambridge.arm.com> <7d924fde-aaa9-3489-fc4a-7bedc74cd28c@arm.com> <20170616074800.w5gioskxpxkiy6vr@e107464-lin.cambridge.arm.com> <0c026471-cef3-13cd-dfa8-d727e8d9b13e@arm.com>
Spamdiagnosticmetadata: NSPM
Spamdiagnosticoutput: 1:99

On 16/06/2017 15:37:18, Richard Earnshaw (lists) wrote:
> On 16/06/17 08:48, Prakhar Bahuguna wrote:
> > On 15/06/2017 17:23:43, Richard Earnshaw (lists) wrote:
> >> On 14/06/17 10:35, Prakhar Bahuguna wrote:
> >>> The ARM ACLE defines the __ARM_FEATURE_COPROC macro which indicates which
> >>> coprocessor intrinsics are available for the target. If __ARM_FEATURE_COPROC is
> >>> undefined, the target does not support coprocessor intrinsics. The feature
> >>> levels are defined as follows:
> >>>
> >>> +---------+-----------+--------------------------------------------------+
> >>> | **Bit** | **Value** | **Intrinsics Available**                         |
> >>> +---------+-----------+--------------------------------------------------+
> >>> | 0       | 0x1       | __arm_cdp __arm_ldc, __arm_ldcl, __arm_stc,      |
> >>> |         |           | __arm_stcl, __arm_mcr and __arm_mrc              |
> >>> +---------+-----------+--------------------------------------------------+
> >>> | 1       | 0x2       | __arm_cdp2, __arm_ldc2, __arm_stc2, __arm_ldc2l, |
> >>> |         |           | __arm_stc2l, __arm_mcr2 and __arm_mrc2           |
> >>> +---------+-----------+--------------------------------------------------+
> >>> | 2       | 0x4       | __arm_mcrr and __arm_mrrc                        |
> >>> +---------+-----------+--------------------------------------------------+
> >>> | 3       | 0x8       | __arm_mcrr2 and __arm_mrrc2                      |
> >>> +---------+-----------+--------------------------------------------------+
> >>>
> >>> This patch implements full support for this feature macro as defined in section
> >>> 5.9 of the ACLE
> >>> (https://developer.arm.com/products/software-development-tools/compilers/arm-compiler-5/docs/101028/latest/5-feature-test-macros).
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> 2017-06-14  Prakhar Bahuguna  <prakhar.bahuguna@arm.com>
> >>>
> >>> 	* config/arm/arm-c.c (arm_cpu_builtins): New block to define
> >>> 	 __ARM_FEATURE_COPROC according to support.
> >>>
> >>> 2017-06-14  Prakhar Bahuguna  <prakhar.bahuguna@arm.com>
> >>> 	* gcc/testsuite/gcc.target/arm/acle/cdp.c: Add feature macro bitmap
> >>> 	test.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/cdp2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/ldc.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/ldc2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/ldc2l.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/ldcl.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mcr.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mcr2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mcrr.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mcrr2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mrc.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mrc2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mrrc.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/mrrc2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/stc.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/stc2.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/stc2l.c: Likewise.
> >>> 	* gcc/testsuite/gcc.target/arm/acle/stcl.c: Likewise.
> >>>
> >>> Testing done: ACLE regression tests updated with tests for feature macro bits.
> >>> All regression tests pass.
> >>>
> >>> Okay for trunk?
> >>>
> >>>
> >>> 0001-Implement-__ARM_FEATURE_COPROC-coprocessor-intrinsic.patch
> >>>
> >>>
> >>> From 79d71aec9d2bdee936b240ae49368ff5f8d8fc48 Mon Sep 17 00:00:00 2001
> >>> From: Prakhar Bahuguna <prakhar.bahuguna@arm.com>
> >>> Date: Tue, 2 May 2017 13:43:40 +0100
> >>> Subject: [PATCH] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature
> >>>  macro
> >>>
> >>> ---
> >>>  gcc/config/arm/arm-c.c                    | 19 +++++++++++++++++++
> >>>  gcc/testsuite/gcc.target/arm/acle/cdp.c   |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/cdp2.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/ldc.c   |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/ldc2.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/ldc2l.c |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/ldcl.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mcr.c   |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mcr2.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mcrr.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mcrr2.c |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mrc.c   |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mrc2.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mrrc.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/mrrc2.c |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/stc.c   |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/stc2.c  |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/stc2l.c |  3 +++
> >>>  gcc/testsuite/gcc.target/arm/acle/stcl.c  |  3 +++
> >>>  19 files changed, 73 insertions(+)
> >>>
> >>> diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c
> >>> index 3abe7d1f1f5..3daf4e5e1f3 100644
> >>> --- a/gcc/config/arm/arm-c.c
> >>> +++ b/gcc/config/arm/arm-c.c
> >>> @@ -200,6 +200,25 @@ arm_cpu_builtins (struct cpp_reader* pfile)
> >>>    def_or_undef_macro (pfile, "__ARM_FEATURE_IDIV", TARGET_IDIV);
> >>>  
> >>>    def_or_undef_macro (pfile, "__ARM_ASM_SYNTAX_UNIFIED__", inline_asm_unified);
> >>> +
> >>> +  if ((!TARGET_THUMB || TARGET_THUMB2) && arm_arch4 &&
> >>
> >> (!TARGET_THUMB || TARGET_THUMB2) looks to me to be equivalent to
> >> TARGET_32BIT.  That should be used in preference.
> > TARGET_32BIT is defined to be (TARGET_ARM || arm_arch_thumb2). This is not true
> > for ARMv8-M Baseline which supports neither ARM mode nor the full Thumb2 mode,
> > so this alternative is used. 
> 
> That's irrelevant since that case is caught by the arm_arch_notm test.
> 
> #define TARGET_32BIT                    (TARGET_ARM || arm_arch_thumb2)
> #define TARGET_THUMB2                   (TARGET_THUMB && arm_arch_thumb2)
> #define TARGET_ARM                      (! TARGET_THUMB)
> 
> So (!TARGET_THUMB || TARGET_THUMB2)
>  = (!TARGET_THUMB || (TARGET_THUMB && arm_arch_thumb2))
>   = ((!TARGET_THUMB || TARGET_THUMB) && (!TARGET_THUMB || arm_arch_thumb2))
>    = (1 && (!TARGET_THUMB || arm_arch_thumb2))
>     = (!TARGET_THUMB || arm_arch_thumb2)
>      = (TARGET_ARM || arm_arch_thumb2)
>       = TARGET_32BIT
> 
> Please use the latter.
> 
> > The same conditional is used by arm_acle.h.
> 
> A patch to fix that is pre-approved.
> 
> 
> > 
> >> Why the test for arm_arch4?  Co-processor instructions in ARM state go
> >> back to the dawn of time.
> > In arm_acle.h, the coprocessor intrinsics are only enabled for ARMv4 and above,
> > hence the macro is only available when the intrinsics are available.
> > 
> >> GNU coding style requires operators on multi-line statements to be at
> >> the start of the continuation lines, not at the end of lines.
> >>
> >>> +      !(arm_arch8 && arm_arch_notm))
> >>> +    {
> >>> +      int coproc_level = 0x1;
> >>> +
> >>> +      if (arm_arch5)
> >>> +	coproc_level |= 0x2;
> >>> +      if (arm_arch5e)
> >>> +	coproc_level |= 0x4;
> >>> +      if (arm_arch6)
> >>> +	coproc_level |= 0x8;
> >>> +
> >>> +      builtin_define_with_int_value ("__ARM_FEATURE_COPROC", coproc_level);
> >>> +    }
> >>> +  else
> >>> +    {
> >>> +      cpp_undef (pfile, "__ARM_FEATURE_COPROC");
> >>> +    }
> >>
> >> Redundant braces around single statement.
> >>
> >> R.
> > 
> > Thanks, will fix these.
> > 
> 

Updated to use TARGET_32BIT.

-- 

Prakhar Bahuguna

Attachment: 0001-Implement-__ARM_FEATURE_COPROC-coprocessor-intrinsic.patch
Description: Text document

Follow-Ups:
- Re: [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
  - From: Richard Earnshaw (lists)
- [arm-embedded] [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
  - From: Thomas Preudhomme

References:
- [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
  - From: Prakhar Bahuguna
- Re: [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
  - From: Richard Earnshaw (lists)
- Re: [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
  - From: Prakhar Bahuguna
- Re: [PATCH, ARM] Implement __ARM_FEATURE_COPROC coprocessor intrinsic feature macro
  - From: Richard Earnshaw (lists)

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]