[PATCH] Enable the __ARM_ARCH_EXT_IDIV__ predefine generation on ARM

[Silviu Baranga]

Hi Bernie. Thanks for the review.

> > (init.c) I guess that we don't want the predefine in these tests
> > because the target is Linux, rather than anything to do with the
> float
> > ABIs? If so, is the right place for that test?
No, this test isn't related to the ABIs, but to the fact that the
triple starts with arm- (so we don't know if we have hardware
division available). Since the check is relatively inexpensive,
I think it's worth doing (although the ARMEABI* checks seem to test
the ABI).

> > (Targets.cpp): I'm a bit edgy about the check for triples starting
> with
> > armv8. What if some future profile does not have idiv in some
> > instrution set? Will it still work if you change that to check for
> > armv8a?
My reasoning was that the two are currently the same, but I would like
to avoid changing the behaviour in the future (so I'll change it to check
for armv8a, at least for now).

> > (arm-target-features.c): Is it worth adding tests that the option is
> a
> > nop when it should be (e.g. -target arm -mthumb -mhwdiv=thumb)?
The option shouldn't be a nop there. The -target arm -mthumb should be
equivalent to "-cc1 -triple thumb" and it won't have hardware division
on by default.

Thanks,
- Silviu



> -----Original Message-----
> From: Bernard Ogden
> Sent: 18 October 2013 13:23
> To: Silviu Baranga; cfe-commits at cs.uiuc.edu
> Subject: RE: [PATCH] Enable the __ARM_ARCH_EXT_IDIV__ predefine
> generation on ARM
> 
> Actually, ignore my first comment. There is a scenario (v7A without
> virtualization) where we could have idiv in ARM but not Thumb.
> 
> > -----Original Message-----
> > From: Bernie Ogden [mailto:bogden at arm.com]
> > Sent: 18 October 2013 13:07
> > To: Silviu Baranga; cfe-commits at cs.uiuc.edu
> > Subject: RE: [PATCH] Enable the __ARM_ARCH_EXT_IDIV__ predefine
> > generation on ARM
> >
> > Hi Siviu,
> >
> > clang_ext.diff:
> >
> > In principle there should be no targets where we have idiv in ARM but
> > not Thumb - but I'm happy with the option as-is because it lets the
> > user choose to not generate the instructions per-instruction-set
> should
> > he ever want to. Who are we to declare what users should do? ;)
> >
> > (init.c) I guess that we don't want the predefine in these tests
> > because the target is Linux, rather than anything to do with the
> float
> > ABIs? If so, is the right place for that test?
> >
> >
> > clang_defaults.diff:
> >
> > (Targets.cpp): I'm a bit edgy about the check for triples starting
> with
> > armv8. What if some future profile does not have idiv in some
> > instrution set? Will it still work if you change that to check for
> > armv8a?
> >
> > (arm-target-features.c): Is it worth adding tests that the option is
> a
> > nop when it should be (e.g. -target arm -mthumb -mhwdiv=thumb)?
> >
> >
> > Regards,
> >
> > Bernie
> >
> >
> > > -----Original Message-----
> > > From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
> > > bounces at cs.uiuc.edu] On Behalf Of Silviu Baranga
> > > Sent: 17 October 2013 16:02
> > > To: cfe-commits at cs.uiuc.edu
> > > Subject: [PATCH] Enable the __ARM_ARCH_EXT_IDIV__ predefine
> > generation
> > > on ARM
> > >
> > > Hi,
> > >
> > > The following set of patches enable the generation of the
> > > __ARM_ARCH_EXT_IDIV__ predefine
> > > for ARM targets which support division instructions. This will fix
> > > PR17555.
> > >
> > > clang_ext.diff:
> > > Targets having the hwdiv or hwdiv-arm feature will also the
> > > __ARM_ARCH_EXT_IDIV__ predefine.
> > >
> > > Also adds the -mhwdiv option to modify these features. The
> arguments
> > to
> > > this option
> > > can be arm/thumb/arm,thumb and indicate in which mode the hardware
> > > division instructions
> > > are supported (for example passing -mhwdiv=arm means the target
> > support
> > > hardware division
> > > instructions in arm mode but not in thumb mode).
> > >
> > > clang_defaults.diff:
> > > Depends on clang_ext.diff.
> > >
> > > Sets the default hwdiv/hwdiv-arm features for different targets.
> The
> > > feature is added by
> > > default for all AArch32 v8 targets, Cortex-A7, Cortex-A15, swift,
> > > Cortex-A53, Cortex-A57,
> > > Cortex-R5, Cortex-M4 and Cortex-M3.
> > >
> > > Please review!
> > >
> > > Thanks,
> > > Silviu
> > >
> > > -- IMPORTANT NOTICE: The contents of this email and any attachments
> > are
> > > confidential and may also be privileged. If you are not the
> intended
> > > recipient, please notify the sender immediately and do not disclose
> > the
> > > contents to any other person, use it for any purpose, or store or
> > copy
> > > the information in any medium.  Thank you.
> > >
> > > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1
> 9NJ,
> > > Registered in England & Wales, Company No:  2557590
> > > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge
> CB1
> > > 9NJ, Registered in England & Wales, Company No:  2548782
>