[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 13 10:40:52 PDT 2011
Ping?
On Oct 10, 2011, at 10:15 AM, Jim Grosbach wrote:
>
> On Oct 6, 2011, at 6:07 PM, David Meyer wrote:
>
>> Jim,
>>
>> Sorry, I seem to have lost track of this thread.
>>
>> The IsNaCl32/64 aliases are of course for convenience, similar to IsWin64. There are 30 uses of these keys throughout the .td files in our repository (which, unfortunately, we haven't sent patches for yet).
>>
>> Is it crucial to get rid of these aliases?
>>
>
> It should just be a global search/replace on the strings to convert them. Is there something more complicated than that going on? As currently written, it's possible to write nonsensical predicates like "Requires<[IsNaCl32, Is64Bit]>". That's best avoided.
>
> -Jim
>
>> - pdox
>>
>> On Thu, Oct 6, 2011 at 3:44 PM, Jim Grosbach <grosbach at apple.com> wrote:
>> 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