[PATCH] add predefined macros for thumbv8

[Bernie Ogden]

Ah, ARMTargetInfo's constructor sets the CPU to 1136. Even if the arch in
the triple is < v6. Hmm.

This seems to trace back to the introduction of setCPU in
https://llvm.org/svn/llvm-project/cfe/trunk@91700 - which at the time had a
FIXME attached to it, but the FIXME is now gone and setCPU seems to be used
across many architectures.

I still think that's a bug, but someone with more knowledge than me may
think differently. I think it could be fixed a few ways:

1) Stop deriving the arch from the CPU. We've got the triple, derive the
arch from that. But that overrides the general rule that the target arch
just sets a default CPU and could open up a can of worms - what if I set
-target arm, or -target armv8 and then a pre-v8 CPU?
2) Set predefines based on subtarget features. This can't cover all cases,
though - e.g. interworking.
3) Pull the mapping in getARMCPUForMArch out to somewhere it's accessible
from cc1, as well as the driver. Then make the ARMTargetInfo constructor set
a default CPU based on the arch, rather than hard-coding 1136.

All of which could take a while to sort out... so I think you should go
ahead and commit your patch, but modify it to set __thumb2__ for triples
ending in v6t2 and v7 as well, and perhaps add a comment explaining why this
is necessary.

And thanks for looking into the problem :)

Regards,

Bernie

> -----Original Message-----
> From: weimingz at codeaurora.org [mailto:weimingz at codeaurora.org]
> Sent: 20 January 2014 01:03
> To: Bernard Ogden
> Cc: weimingz at codeaurora.org; cfe-commits at cs.uiuc.edu
> Subject: RE: [PATCH] add predefined macros for thumbv8
> 
> Hi Bernie,
> 
> It doesn't seem to be a bug.
> when passing "-cc1 -E -dM -ffreestanding -triple=thumbv8" to clang, the
> default cpu is arm1136j-s, so CPUArchVer is 6. Sometime, the triple and
> mcpu flag may be insconsistent.
> 
> 
> Thanks,
> Weiming
> 
> 
> > (Failed to send to list - apologies to Weiming for the double reply.)
> >
> >
> >
> > That sounds like a  further bug - thumbv8 really shouldn't result in
> the
> > arch ver appearing to be 6. I think the right way to deal with this
> is to
> > fix the underlying bug. Would you mind looking into it?
> >
> >
> >
> > Thanks,
> >
> >
> >
> > Bernie
> >
> >
> >
> >
> >
> > From: Weiming Zhao [mailto:weimingz at codeaurora.org]
> > Sent: 15 January 2014 22:49
> > To: Bernard Ogden; cfe-commits at cs.uiuc.edu
> > Subject: RE: [PATCH] add predefined macros for thumbv8
> >
> >
> >
> > Hi Bernie,
> >
> >
> >
> > Thanks for reviewing. I changed that check to "CPUArchVer >=7"
> >
> >
> >
> > The reason for checking of ArchName is if I just pass triple=thumbv8
> (e.g.
> > -cc1 -E -dM -ffreestanding -triple=thumbv8), the CPUArchVer is still
> 6
> > unless mcpu is set to cortex-a53.
> >
> >
> >
> > Thanks,
> >
> > Weiming
> >
> >
> >
> >
> >
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted
> > by
> > The Linux Foundation
> >
> >
> >
> > From: Bernie Ogden [mailto:bogden at arm.com]
> > Sent: Wednesday, January 15, 2014 4:22 AM
> > To: weimingz at codeaurora.org; cfe-commits at cs.uiuc.edu
> > Subject: RE: [PATCH] add predefined macros for thumbv8
> >
> >
> >
> > Hi Weiming,
> >
> >
> >
> > I think __thumb2_ should be set regardless of whether we're
> generating ARM
> > or Thumb code - and I think this is what your patch does. Isn't the
> check
> > of
> > the ArchName string redundant with the check for CPUArchVer == 8? If
> I'm
> > right then the tests need a corresponding adjustment.
> >
> >
> >
> > I'd also slightly prefer that you check for CPUArchVer >= 7, rather
> than
> > checking for == 7 and ==8, but I don't feel all that strongly about
> it.
> >
> >
> >
> > Looks like we should consider moving the tests in
> > Preprocessor/arm-target-features.c into Preprocessor/init.c, but
> that's
> > not
> > your problem :)
> >
> >
> >
> > Regards,
> >
> >
> >
> > Bernie
> >
> >
> >
> > From: cfe-commits-bounces at cs.uiuc.edu
> > [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Weiming Zhao
> > Sent: 14 January 2014 01:03
> > To: cfe-commits at cs.uiuc.edu
> > Subject: [PATCH] add predefined macros for thumbv8
> >
> >
> >
> > Hi,
> >
> >
> >
> > Attached patch adds 2 predefined macros to thumbv8 target:
> >
> > __thumb2__
> >
> > __ __THUMB_INTERWORK__
> >
> >
> >
> > See http://llvm.org/bugs/show_bug.cgi?id=18465
> >
> >
> >
> > Please help to review it.
> >
> >
> >
> > Thanks,
> >
> > Weiming
> >
> >
> >
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted
> > by
> > The Linux Foundation
> >
> >
> >
> 
>