[llvm-commits] [llvm] r139125 - in /llvm/trunk/lib/Target: ARM/ARM.td ARM/ARMInstrInfo.td ARM/ARMSubtarget.cpp ARM/ARMSubtarget.h ARM/MCTargetDesc/ARMMCTargetDesc.cpp X86/MCTargetDesc/X86MCTargetDesc.cpp X86/X86.td X86/X86InstrInfo.td X86/X86Subt

Jim Grosbach grosbach at apple.com
Thu Oct 6 15:44:51 PDT 2011


Ping?


On Sep 30, 2011, at 9:11 AM, Jim Grosbach wrote:

> 
> On Sep 27, 2011, at 6:36 PM, David Meyer wrote:
> 
>> Hello,
>> 
>> I'm sorry, I did not see this thread earlier.
>> 
>> The original reason for adding "NaClMode" was because we needed an AssemblerPredicate so that we could disable some MnemonicAlias's in the assembler for NaCl.
>> 
>> Because NaCl on X86-64 implies 32-bit pointers, certain aliases no longer make sense. (like call -> callq)
>> 
>> In retrospect, this is not terribly important. I think we can live without it.
>> 
>> Attached is a patch to remove it. Let me know if it's ok to commit.
>> 
>> - pdox
> 
> Hi David,
> 
> This is much closer, thank you. A few comments. OK to commit with the below changes.
> 
>> Index: lib/Target/ARM/ARMSubtarget.cpp
>> ===================================================================
>> --- lib/Target/ARM/ARMSubtarget.cpp	(revision 140669)
>> +++ lib/Target/ARM/ARMSubtarget.cpp	(working copy)
>> @@ -53,7 +53,6 @@
>>   , HasVMLxForwarding(false)
>>   , SlowFPBrcc(false)
>>   , InThumbMode(false)
>> -  , InNaClMode(false)
>>   , HasThumb2(false)
>>   , NoARM(false)
>>   , PostRAScheduler(false)
>> Index: lib/Target/ARM/ARMInstrInfo.td
>> ===================================================================
>> --- lib/Target/ARM/ARMInstrInfo.td	(revision 140669)
>> +++ lib/Target/ARM/ARMInstrInfo.td	(working copy)
>> @@ -209,8 +209,7 @@
>>                                  AssemblerPredicate<"!ModeThumb">;
>> def IsDarwin         : Predicate<"Subtarget->isTargetDarwin()">;
>> def IsNotDarwin      : Predicate<"!Subtarget->isTargetDarwin()">;
>> -def IsNaCl           : Predicate<"Subtarget->isTargetNaCl()">,
>> -                                 AssemblerPredicate<"ModeNaCl">;
>> +def IsNaCl           : Predicate<"Subtarget->isTargetNaCl()">;
>> 
>> // FIXME: Eventually this will be just "hasV6T2Ops".
>> def UseMovt          : Predicate<"Subtarget->useMovt()">;
>> Index: lib/Target/ARM/ARMSubtarget.h
>> ===================================================================
>> --- lib/Target/ARM/ARMSubtarget.h	(revision 140669)
>> +++ lib/Target/ARM/ARMSubtarget.h	(working copy)
>> @@ -70,9 +70,6 @@
>>   /// InThumbMode - True if compiling for Thumb, false for ARM.
>>   bool InThumbMode;
>> 
>> -  /// InNaClMode - True if targeting Native Client
>> -  bool InNaClMode;
>> -
>>   /// HasThumb2 - True if Thumb2 instructions are supported.
>>   bool HasThumb2;
>> 
>> Index: lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
>> ===================================================================
>> --- lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp	(revision 140669)
>> +++ lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp	(working copy)
>> @@ -86,14 +86,6 @@
>>       ARMArchFeature += ",+thumb-mode";
>>   }
>> 
>> -  Triple TheTriple(TT);
>> -  if (TheTriple.getOS() == Triple::NativeClient) {
>> -    if (ARMArchFeature.empty())
>> -      ARMArchFeature = "+nacl-mode";
>> -    else
>> -      ARMArchFeature += ",+nacl-mode";
>> -  }
>> -
>>   return ARMArchFeature;
>> }
>> 
>> Index: lib/Target/ARM/ARM.td
>> ===================================================================
>> --- lib/Target/ARM/ARM.td	(revision 140669)
>> +++ lib/Target/ARM/ARM.td	(working copy)
>> @@ -23,9 +23,6 @@
>> def ModeThumb  : SubtargetFeature<"thumb-mode", "InThumbMode", "true",
>>                                   "Thumb mode">;
>> 
>> -def ModeNaCl   : SubtargetFeature<"nacl-mode", "InNaClMode", "true",
>> -                                  "Native client mode">;
>> -
>> //===----------------------------------------------------------------------===//
>> // ARM Subtarget features.
>> //
>> Index: lib/Target/X86/X86InstrInfo.td
>> ===================================================================
>> --- lib/Target/X86/X86InstrInfo.td	(revision 140669)
>> +++ lib/Target/X86/X86InstrInfo.td	(working copy)
>> @@ -482,14 +482,10 @@
>>                              AssemblerPredicate<"Mode64Bit">;
>> def IsWin64      : Predicate<"Subtarget->isTargetWin64()">;
>> def NotWin64     : Predicate<"!Subtarget->isTargetWin64()">;
>> -def IsNaCl       : Predicate<"Subtarget->isTargetNaCl()">,
>> -                             AssemblerPredicate<"ModeNaCl">;
>> -def IsNaCl32     : Predicate<"Subtarget->isTargetNaCl32()">,
>> -                             AssemblerPredicate<"ModeNaCl,!Mode64Bit">;
>> -def IsNaCl64     : Predicate<"Subtarget->isTargetNaCl64()">,
>> -                             AssemblerPredicate<"ModeNaCl,Mode64Bit">;
>> -def NotNaCl      : Predicate<"!Subtarget->isTargetNaCl()">,
>> -                             AssemblerPredicate<"!ModeNaCl">;
>> +def IsNaCl       : Predicate<"Subtarget->isTargetNaCl()">;
>> +def IsNaCl32     : Predicate<"Subtarget->isTargetNaCl32()">;
>> +def IsNaCl64     : Predicate<"Subtarget->isTargetNaCl64()">;
> 
> There shouldn't be a need to have the 32 and 64 versions here. You can use both IsNaCl and Is64Bit in a pattern predicate, for example, to get the same effect. For example,"Requires<[In64BitMode, IsNaCl]>".
> 
>> +def NotNaCl      : Predicate<"!Subtarget->isTargetNaCl()">;
> 
> It strikes me as odd that both the positive and negative variant is needed, but I see there's precedent for that. No biggie, just a bit of a "huh. Tablegen should be expressive enough that's not necessary," observation.
> 
>> def SmallCode    : Predicate<"TM.getCodeModel() == CodeModel::Small">;
>> def KernelCode   : Predicate<"TM.getCodeModel() == CodeModel::Kernel">;
>> def FarData      : Predicate<"TM.getCodeModel() != CodeModel::Small &&"
>> Index: lib/Target/X86/X86.td
>> ===================================================================
>> --- lib/Target/X86/X86.td	(revision 140669)
>> +++ lib/Target/X86/X86.td	(working copy)
>> @@ -23,9 +23,6 @@
>> def Mode64Bit : SubtargetFeature<"64bit-mode", "In64BitMode", "true",
>>                                   "64-bit mode (x86_64)">;
>> 
>> -def ModeNaCl  : SubtargetFeature<"nacl-mode", "InNaClMode", "true",
>> -                                 "Native Client mode">;
>> -
>> //===----------------------------------------------------------------------===//
>> // X86 Subtarget features.
>> //===----------------------------------------------------------------------===//
>> Index: lib/Target/X86/X86Subtarget.cpp
>> ===================================================================
>> --- lib/Target/X86/X86Subtarget.cpp	(revision 140669)
>> +++ lib/Target/X86/X86Subtarget.cpp	(working copy)
>> @@ -262,8 +262,7 @@
>>   // FIXME: this is a known good value for Yonah. How about others?
>>   , MaxInlineSizeThreshold(128)
>>   , TargetTriple(TT)
>> -  , In64BitMode(is64Bit)
>> -  , InNaClMode(false) {
>> +  , In64BitMode(is64Bit) {
>>   // Determine default and user specified characteristics
>>   if (!FS.empty() || !CPU.empty()) {
>>     std::string CPUName = CPU;
>> @@ -309,11 +308,6 @@
>>   if (In64BitMode)
>>     ToggleFeature(X86::Mode64Bit);
>> 
>> -  if (isTargetNaCl()) {
>> -    InNaClMode = true;
>> -    ToggleFeature(X86::ModeNaCl);
>> -  }
>> -
>>   if (HasAVX)
>>     X86SSELevel = NoMMXSSE;
>> 
>> Index: lib/Target/X86/X86Subtarget.h
>> ===================================================================
>> --- lib/Target/X86/X86Subtarget.h	(revision 140669)
>> +++ lib/Target/X86/X86Subtarget.h	(working copy)
>> @@ -119,9 +119,6 @@
>>   /// In64BitMode - True if compiling for 64-bit, false for 32-bit.
>>   bool In64BitMode;
>> 
>> -  /// InNaClMode - True if compiling for Native Client target.
>> -  bool InNaClMode;
>> -
>> public:
>> 
>>   /// This constructor initializes the data members to match that
>> Index: lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
>> ===================================================================
>> --- lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp	(revision 140669)
>> +++ lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp	(working copy)
>> @@ -45,10 +45,6 @@
>>     FS = "+64bit-mode";
>>   else
>>     FS = "-64bit-mode";
>> -  if (TheTriple.getOS() == Triple::NativeClient)
>> -    FS += ",+nacl-mode";
>> -  else
>> -    FS += ",-nacl-mode";
>>   return FS;
>> }
>> 
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list