Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions

[Thomas Preudhomme <thomas.preudhomme@foss.arm.com>]

On Thursday 19 May 2016 17:42:26 Kyrill Tkachov wrote:
> Hi Thomas,
> 
> I'm not very familiar with the libgcc machinery, but I have a comment on an
> arm.h hunk inline.
> On 17/05/16 10:58, Thomas Preudhomme wrote:
> > Ping?
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
> >          __ARM_ARCH_ISA_ARM to
> >          decide whether to prevent some libgcc routines being included for
> >          some
> >          multilibs rather than __ARM_ARCH_6M__ and add comment to indicate
> >          the
> >          link between this condition and the one in
> >          libgcc/config/arm/lib1func.S.
> >          * config/arm/arm.h (TARGET_ARM_V6M): Add check to
> >          TARGET_ARM_ARCH.
> >          (TARGET_ARM_V7M): Likewise.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * lib/target-supports.exp (check_effective_target_arm_cortex_m):
> >          Use
> >          __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
> > 
> > *** libgcc/ChangeLog ***
> > 
> > 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1
> >          rather
> >          than ARMv6-M.
> >          * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
> >          cases
> >          for all Thumb-1 only targets.
> >          (__only_thumb1__): Define for all Thumb-1 only targets.
> >          (THUMB_LDIV0): Test for __only_thumb1__ rather than
> >          __ARM_ARCH_6M__.
> >          (EQUIV): Likewise.
> >          (ARM_FUNC_ALIAS): Likewise.
> >          (umodsi3): Add check to __only_thumb1__ to guard the idiv
> >          version.
> >          (modsi3): Likewise.
> >          (HAVE_ARM_CLZ): Remove block defining it.
> >          (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__
> >          and
> >          check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
> >          (clzdi2): Likewise.
> >          (ctzsi2): Likewise.
> >          (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather
> >          than
> >          __ARM_ARCH_6M__ in guard for checking whether it is defined.
> >          (final includes): Test for __only_thumb1__ rather than
> >          __ARM_ARCH_6M__ and add comment to indicate the connection
> >          between
> >          this condition and the one in gcc/config/arm/elf.h.
> >          * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
> >          __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
> >          * config/arm/t-softfp: Likewise.
> > 
> > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> > index
> > 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9fe
> > 66611c19 100644
> > --- a/gcc/config/arm/arm.h
> > +++ b/gcc/config/arm/arm.h
> > @@ -2188,8 +2188,10 @@ extern int making_const_table;
> > 
> >   #define TARGET_ARM_ARCH	\
> >   
> >     (arm_base_arch)	\
> > 
> > -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> > -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
> > +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm
> > \ +			&& !arm_arch_thumb2)
> > +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm
> > \ +			&& arm_arch_thumb2)
> 
> I think now that you're checking TARGET_ARM_ARCH you don't need
> the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm &&
> arm_arch_thumb2".

% git --no-pager grep TARGET_ARM_V.M config/arm
config/arm/arm.h:#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
config/arm/arm.h:#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)

What about just removing those? I kept them because I wasn't sure of their 
purpose but I think we should just remove them.

Best regards,

Thomas