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: [email protected] [mailto:[email protected]] > Sent: 20 January 2014 01:03 > To: Bernard Ogden > Cc: [email protected]; [email protected] > 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:[email protected]] > > Sent: 15 January 2014 22:49 > > To: Bernard Ogden; [email protected] > > 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:[email protected]] > > Sent: Wednesday, January 15, 2014 4:22 AM > > To: [email protected]; [email protected] > > 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: [email protected] > > [mailto:[email protected]] On Behalf Of Weiming Zhao > > Sent: 14 January 2014 01:03 > > To: [email protected] > > 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 > > > > > > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
