[llvm] r206211 - [ARM64][MC] Set the default CPU to cyclone when initilizating the MC layer.

[Andrew Trick]

On Apr 14, 2014, at 5:37 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> I’ve changed the default to generic in r206228.

FWIW, I don’t think this makes any sense. I don’t think we should generate “generic” arm64 code unless someone explicitly requests it. Otherwise, we’re just hiding a user error or driver bug.

-Andy

> 
> Thanks,
> -Quentin
> 
> On Apr 14, 2014, at 3:16 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
>> Let's go with the "generic" x86 model. It looks like ARM64 does have a
>> definition for that.
>> 
>> -eric
>> 
>> On Mon, Apr 14, 2014 at 3:13 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>>> Well, in fact, I am not sure what is the best approach here.
>>> 
>>> ARM and X86 already seem to duplicate this logic somehow, see below.
>>> 
>>> So if we are about to duplicate something I’d prefer to keep it minimal like
>>> in the current patch.
>>> 
>>> What do you think?
>>> 
>>> ** X86 **
>>> 
>>> — X86Subtarget.cpp:
>>> void X86Subtarget::resetSubtargetFeatures(StringRef CPU, StringRef FS) {
>>>  std::string CPUName = CPU;
>>>  if (CPUName.empty())
>>>    CPUName = "generic";
>>> 
>>> — X86MCTargetDesc.cpp:
>>> MCSubtargetInfo *X86_MC::createX86MCSubtargetInfo(StringRef TT, StringRef
>>> CPU,
>>>                                                  StringRef FS) {
>>> […]
>>>  std::string CPUName = CPU;
>>>  if (CPUName.empty()) {
>>> #if defined(i386) || defined(__i386__) || defined(__x86__) ||
>>> defined(_M_IX86)\
>>>    || defined(__x86_64__) || defined(_M_AMD64) || defined (_M_X64)
>>>    CPUName = sys::getHostCPUName();
>>> #else
>>>    CPUName = "generic";
>>> #endif
>>>  }
>>> 
>>> 
>>> ** ARM **
>>> 
>>> — ARMSubtarget.cpp:
>>> void ARMSubtarget::resetSubtargetFeatures(StringRef CPU, StringRef FS) {
>>>  if (CPUString.empty()) {
>>>    if (isTargetIOS() && TargetTriple.getArchName().endswith("v7s"))
>>>      // Default to the Swift CPU when targeting armv7s/thumbv7s.
>>>      CPUString = "swift";
>>>    else
>>>      CPUString = "generic";
>>>  }
>>> 
>>> — ARMMCTargetDesc.cpp:
>>> std::string ARM_MC::ParseARMTriple(StringRef TT, StringRef CPU) {
>>> [...]
>>>  bool NoCPU = CPU == "generic" || CPU.empty();
>>>  std::string ARMArchFeature;
>>>  if (Idx) {
>>>    unsigned SubVer = TT[Idx];
>>>    if (SubVer == '8') {
>>>      if (NoCPU)
>>>        // v8a: FeatureDB, FeatureFPARMv8, FeatureNEON, FeatureDSPThumb2,
>>>        //      FeatureMP, FeatureHWDiv, FeatureHWDivARM, FeatureTrustZone,
>>>        //      FeatureT2XtPk, FeatureCrypto, FeatureCRC
>>>        ARMArchFeature =
>>> "+v8,+db,+fp-armv8,+neon,+t2dsp,+mp,+hwdiv,+hwdiv-arm,"
>>>                         "+trustzone,+t2xtpk,+crypto,+crc";
>>>      else
>>>        // Use CPU to figure out the exact features
>>>        ARMArchFeature = "+v8”;
>>> […]
>>> 
>>> -Quentin
>>> 
>>> On Apr 14, 2014, at 3:02 PM, Eric Christopher <echristo at gmail.com> wrote:
>>> 
>>> Thanks!
>>> 
>>> I'd probably see the ARM one as closer... it doesn't cope well if
>>> there's no cpu given it doesn't look like :)
>>> 
>>> -eric
>>> 
>>> On Mon, Apr 14, 2014 at 3:00 PM, Quentin Colombet <qcolombet at apple.com>
>>> wrote:
>>> 
>>> Sorry, I thought I was doing the same thing as X86 here.
>>> I’ve been misled by this sequence:
>>> MCSubtargetInfo *X86_MC::createX86MCSubtargetInfo(StringRef TT, StringRef
>>> CPU,
>>>                                                 StringRef FS) {
>>> [...]
>>> std::string CPUName = CPU;
>>> if (CPUName.empty()) {
>>> #if defined(i386) || defined(__i386__) || defined(__x86__) ||
>>> defined(_M_IX86)\
>>>   || defined(__x86_64__) || defined(_M_AMD64) || defined (_M_X64)
>>>   CPUName = sys::getHostCPUName();
>>> #else
>>>   CPUName = "generic";
>>> #endif
>>> }
>>> 
>>> Let me rework my copy then!
>>> 
>>> -Quentin
>>> 
>>> On Apr 14, 2014, at 2:42 PM, Eric Christopher <echristo at gmail.com> wrote:
>>> 
>>> Uh, what?
>>> 
>>> How about taking the same code from the ARM or X86 backends? Or something
>>> else?
>>> 
>>> -eric
>>> 
>>> On Mon, Apr 14, 2014 at 2:25 PM, Quentin Colombet <qcolombet at apple.com>
>>> wrote:
>>> 
>>> Author: qcolombet
>>> Date: Mon Apr 14 16:25:53 2014
>>> New Revision: 206211
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=206211&view=rev
>>> Log:
>>> [ARM64][MC] Set the default CPU to cyclone when initilizating the MC layer.
>>> This matches that ARM64Subtarget does for now.
>>> 
>>> This is related to <rdar://problem/16573920>
>>> 
>>> Modified:
>>>  llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp
>>> 
>>> Modified: llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp?rev=206211&r1=206210&r2=206211&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp
>>> (original)
>>> +++ llvm/trunk/lib/Target/ARM64/MCTargetDesc/ARM64MCTargetDesc.cpp Mon Apr
>>> 14 16:25:53 2014
>>> @@ -43,6 +43,12 @@ static MCInstrInfo *createARM64MCInstrIn
>>> static MCSubtargetInfo *createARM64MCSubtargetInfo(StringRef TT, StringRef
>>> CPU,
>>>                                                  StringRef FS) {
>>> MCSubtargetInfo *X = new MCSubtargetInfo();
>>> +
>>> +  // FIXME: Make this darwin-only.
>>> +  if (CPU.empty())
>>> +    // We default to Cyclone for now, on Darwin.
>>> +    CPU = "cyclone";
>>> +
>>> InitARM64MCSubtargetInfo(X, TT, CPU, FS);
>>> return X;
>>> }
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> 
>>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140415/9c809d48/attachment.html>