[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

Nick Lewycky nlewycky at google.com
Tue Sep 27 13:57:08 PDT 2011


On 14 September 2011 21:31, Nick Lewycky <nicholas at mxc.ca> wrote:

> Jim Grosbach wrote:
> >
> > On Sep 8, 2011, at 10:44 PM, Chris Lattner wrote:
> >
> >>
> >> On Sep 6, 2011, at 10:05 AM, Jim Grosbach wrote:
> >>
> >>> Hi Nick,
> >>>
> >>> I have a few concerns about this that hopefully won't be too hard to
> alleviate. On the more trite side, we generally try to keep the nomenclature
> descriptive of purpose rather than of use. This patch names the sub-target
> feature according to purpose (native client) and doesn't give any indication
> of what it actually does. Is this really a sub-target feature at all?
> Honestly, my general impression is that this is more accurately a target
> platform in the triple akin to Linux or Darwin. For example,
> thumbv7-unknown-nacl. Assuming so (and it looks like that is indeed how it's
> specified?), why isn't querying the triple directly a-la
> ARMSubtarget->isTargetDarwin(), sufficient? Lastly, speculating here as this
> patch doesn't go into these details, but be very careful with alignment
> changes in code sections, as the ARM backend is very sensitive to small
> changes. Specifically, the constant island pass tracks instruction alignment
> and relative distances as exactly as it can, and wil
> l
> >    n!
> >>> eed to be taught how to deal with these changes.
> >>
> >> I agree in principle, but I can see it go both ways.  Note that 64-bit
> support is also a "subtarget feature" which is also very odd, but
> convenient.
> >
> > The key distinction is that 64-bit mode is orthogonal to the target and
> NaCl is not. By definition, NaCl is a target OS like  Darwin or Linux, as
> that's where it shows up in the triple already. By making NaCl a true sub
> target feature, we're duplicating information that already exists in the
> triple.
> >
> >> That said, if "NaCL mode" is really just normal codegen plus a few
> tweaks, it would be much better to add subtarget features for the tweaks,
> and nacl mode could enable those tweaks.  This respect is very different
> than 64-bit support.
> >
> > Exactly. ARM does this sort of thing in the ARMSubtarget() constructor,
> for example, by enabling/disabling certain features by default (reserving R9
> and use of movw/movt in v7) based on the target OS.
> >
> > I have no problem with there being an isTargetNaCl() accessor in the
> ARMSubtarget/X86Subtarget classes. That part makes sense, and is akin to how
> we have isTargetDarwin() already. It's only the implementation underneath
> that which I object to.
>
> That makes sense to me. Thanks for the review!
>
> David, could you make the changes per Jim's review?
>

David? Ping?


> Nick
>
> >
> > -Jim
> >
> >
> >>>
> >>> On Sep 5, 2011, at 2:51 PM, Nick Lewycky wrote:
> >>>
> >>>> Author: nicholas
> >>>> Date: Mon Sep  5 16:51:43 2011
> >>>> New Revision: 139125
> >>>>
> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=139125&view=rev
> >>>> Log:
> >>>> Add a new MC bit for NaCl (Native Client) mode. NaCl requires that
> certain
> >>>> instructions are more aligned than the CPU requires, and adds some
> additional
> >>>> directives, to follow in future patches. Patch by David Meyer!
> >>>>
> >>>> Modified:
> >>>>   llvm/trunk/lib/Target/ARM/ARM.td
> >>>>   llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
> >>>>   llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp
> >>>>   llvm/trunk/lib/Target/ARM/ARMSubtarget.h
> >>>>   llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> >>>>   llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
> >>>>   llvm/trunk/lib/Target/X86/X86.td
> >>>>   llvm/trunk/lib/Target/X86/X86InstrInfo.td
> >>>>   llvm/trunk/lib/Target/X86/X86Subtarget.cpp
> >>>>   llvm/trunk/lib/Target/X86/X86Subtarget.h
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/ARM/ARM.td
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARM.td?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/ARM/ARM.td (original)
> >>>> +++ llvm/trunk/lib/Target/ARM/ARM.td Mon Sep  5 16:51:43 2011
> >>>> @@ -23,6 +23,9 @@
> >>>> def ModeThumb  : SubtargetFeature<"thumb-mode", "InThumbMode", "true",
> >>>>                                  "Thumb mode">;
> >>>>
> >>>> +def ModeNaCl   : SubtargetFeature<"nacl-mode", "InNaClMode", "true",
> >>>> +                                  "Native client mode">;
> >>>> +
> >>>>
> //===----------------------------------------------------------------------===//
> >>>> // ARM Subtarget features.
> >>>> //
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/ARM/ARMInstrInfo.td
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.td?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/ARM/ARMInstrInfo.td (original)
> >>>> +++ llvm/trunk/lib/Target/ARM/ARMInstrInfo.td Mon Sep  5 16:51:43 2011
> >>>> @@ -209,6 +209,8 @@
> >>>>                                 AssemblerPredicate<"!ModeThumb">;
> >>>> def IsDarwin         : Predicate<"Subtarget->isTargetDarwin()">;
> >>>> def IsNotDarwin      : Predicate<"!Subtarget->isTargetDarwin()">;
> >>>> +def IsNaCl           : Predicate<"Subtarget->isTargetNaCl()">,
> >>>> +                                 AssemblerPredicate<"ModeNaCl">;
> >>>>
> >>>> // FIXME: Eventually this will be just "hasV6T2Ops".
> >>>> def UseMovt          : Predicate<"Subtarget->useMovt()">;
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp (original)
> >>>> +++ llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp Mon Sep  5 16:51:43
> 2011
> >>>> @@ -53,6 +53,7 @@
> >>>> , HasVMLxForwarding(false)
> >>>> , SlowFPBrcc(false)
> >>>> , InThumbMode(false)
> >>>> +  , InNaClMode(false)
> >>>> , HasThumb2(false)
> >>>> , NoARM(false)
> >>>> , PostRAScheduler(false)
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/ARM/ARMSubtarget.h
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMSubtarget.h?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/ARM/ARMSubtarget.h (original)
> >>>> +++ llvm/trunk/lib/Target/ARM/ARMSubtarget.h Mon Sep  5 16:51:43 2011
> >>>> @@ -70,6 +70,9 @@
> >>>> /// 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;
> >>>>
> >>>> @@ -209,6 +212,9 @@
> >>>> const Triple&getTargetTriple() const { return TargetTriple; }
> >>>>
> >>>> bool isTargetDarwin() const { return TargetTriple.isOSDarwin(); }
> >>>> +  bool isTargetNaCl() const {
> >>>> +    return TargetTriple.getOS() == Triple::NativeClient;
> >>>> +  }
> >>>> bool isTargetELF() const { return !isTargetDarwin(); }
> >>>>
> >>>> bool isAPCS_ABI() const { return TargetABI == ARM_ABI_APCS; }
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> (original)
> >>>> +++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp Mon Sep
>  5 16:51:43 2011
> >>>> @@ -86,6 +86,14 @@
> >>>>      ARMArchFeature += ",+thumb-mode";
> >>>> }
> >>>>
> >>>> +  Triple TheTriple(TT);
> >>>> +  if (TheTriple.getOS() == Triple::NativeClient) {
> >>>> +    if (ARMArchFeature.empty())
> >>>> +      ARMArchFeature = "+nacl-mode";
> >>>> +    else
> >>>> +      ARMArchFeature += ",+nacl-mode";
> >>>> +  }
> >>>> +
> >>>> return ARMArchFeature;
> >>>> }
> >>>>
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
> (original)
> >>>> +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp Mon Sep
>  5 16:51:43 2011
> >>>> @@ -40,9 +40,16 @@
> >>>>
> >>>> std::string X86_MC::ParseX86Triple(StringRef TT) {
> >>>> Triple TheTriple(TT);
> >>>> +  std::string FS;
> >>>> if (TheTriple.getArch() == Triple::x86_64)
> >>>> -    return "+64bit-mode";
> >>>> -  return "-64bit-mode";
> >>>> +    FS = "+64bit-mode";
> >>>> +  else
> >>>> +    FS = "-64bit-mode";
> >>>> +  if (TheTriple.getOS() == Triple::NativeClient)
> >>>> +    FS += ",+nacl-mode";
> >>>> +  else
> >>>> +    FS += ",-nacl-mode";
> >>>> +  return FS;
> >>>> }
> >>>>
> >>>> /// GetCpuIDAndInfo - Execute the specified cpuid and return the 4
> values in the
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/X86/X86.td
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86.td?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/X86/X86.td (original)
> >>>> +++ llvm/trunk/lib/Target/X86/X86.td Mon Sep  5 16:51:43 2011
> >>>> @@ -23,6 +23,9 @@
> >>>> 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.
> >>>>
> //===----------------------------------------------------------------------===//
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.td
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/X86/X86InstrInfo.td (original)
> >>>> +++ llvm/trunk/lib/Target/X86/X86InstrInfo.td Mon Sep  5 16:51:43 2011
> >>>> @@ -482,6 +482,14 @@
> >>>>                             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 SmallCode    : Predicate<"TM.getCodeModel() == CodeModel::Small">;
> >>>> def KernelCode   : Predicate<"TM.getCodeModel() ==
> CodeModel::Kernel">;
> >>>> def FarData      : Predicate<"TM.getCodeModel() != CodeModel::Small&&"
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/X86/X86Subtarget.cpp
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.cpp?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/X86/X86Subtarget.cpp (original)
> >>>> +++ llvm/trunk/lib/Target/X86/X86Subtarget.cpp Mon Sep  5 16:51:43
> 2011
> >>>> @@ -260,7 +260,8 @@
> >>>> // FIXME: this is a known good value for Yonah. How about others?
> >>>> , MaxInlineSizeThreshold(128)
> >>>> , TargetTriple(TT)
> >>>> -  , In64BitMode(is64Bit) {
> >>>> +  , In64BitMode(is64Bit)
> >>>> +  , InNaClMode(false) {
> >>>> // Determine default and user specified characteristics
> >>>> if (!FS.empty() || !CPU.empty()) {
> >>>>    std::string CPUName = CPU;
> >>>> @@ -306,6 +307,11 @@
> >>>> if (In64BitMode)
> >>>>    ToggleFeature(X86::Mode64Bit);
> >>>>
> >>>> +  if (isTargetNaCl()) {
> >>>> +    InNaClMode = true;
> >>>> +    ToggleFeature(X86::ModeNaCl);
> >>>> +  }
> >>>> +
> >>>> if (HasAVX)
> >>>>    X86SSELevel = NoMMXSSE;
> >>>>
> >>>>
> >>>> Modified: llvm/trunk/lib/Target/X86/X86Subtarget.h
> >>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.h?rev=139125&r1=139124&r2=139125&view=diff
> >>>>
> ==============================================================================
> >>>> --- llvm/trunk/lib/Target/X86/X86Subtarget.h (original)
> >>>> +++ llvm/trunk/lib/Target/X86/X86Subtarget.h Mon Sep  5 16:51:43 2011
> >>>> @@ -119,6 +119,9 @@
> >>>> /// 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
> >>>> @@ -190,6 +193,11 @@
> >>>>    return !isTargetDarwin()&&  !isTargetWindows()&&
>  !isTargetCygMing();
> >>>> }
> >>>> bool isTargetLinux() const { return TargetTriple.getOS() ==
> Triple::Linux; }
> >>>> +  bool isTargetNaCl() const {
> >>>> +    return TargetTriple.getOS() == Triple::NativeClient;
> >>>> +  }
> >>>> +  bool isTargetNaCl32() const { return isTargetNaCl()&&  !is64Bit();
> }
> >>>> +  bool isTargetNaCl64() const { return isTargetNaCl()&&  is64Bit(); }
> >>>>
> >>>> bool isTargetWindows() const { return TargetTriple.getOS() ==
> Triple::Win32; }
> >>>> bool isTargetMingw() const { return TargetTriple.getOS() ==
> Triple::MinGW32; }
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> 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
> >>
> >
> >
>
> _______________________________________________
> 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/20110927/203347d9/attachment.html>


More information about the llvm-commits mailing list