<br><br><div class="gmail_quote">On Tue, Sep 6, 2011 at 1:37 PM, Jim Grosbach <span dir="ltr"><<a href="mailto:grosbach@apple.com">grosbach@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Side note. Quotes don't seem to be nesting correctly. Don't know why.<br>
<div class="im">On Sep 6, 2011, at 1:32 PM, Jason Kim wrote:<br>
<br>
> Hi Jim!<br>
><br>
> On Tue, Sep 6, 2011 at 10:05 AM, Jim Grosbach <<a href="mailto:grosbach@apple.com">grosbach@apple.com</a>> wrote:<br>
> Hi Nick,<br>
><br>
> 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.<br>
><br>
> That is exactly right - Native Client mode acts like a new "ostype" - which entails changes to the backends - Specifically, instruction sequences deemed "unsafe" are either disallowed or is transformed into their safe equivalents.<br>
><br>
> 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?<br>
><br>
> In order to enable this for multiple OS's and architectures (linux AND darwin, ARM AND x86) we need a switch to turn on NaCl mode as a feature across the supported matrix.<br>
><br>
<br>
</div>Why isn't the target triple sufficient in and of itself? If NaCl is the target OS in the triple, then it should be orthogonal to the actual host OS. That is, the code doesn't specify both Linux and NaCl; it's just NaCl as the target OS in the triple, right?</blockquote>
<div><br></div><div>That is correct. From the outside world, NaCl mode acts like a whole new OSType. However, NaCl mode is "mostly" linux (on linux) and mostly darwin on macos - there is a shared code that does instruction processing which is shared between nacl darwin x86 and nacl linux x86 - but for the most part - nacl ostype exists as a slight modification on the existing ostypes - we felt that having a subfeature switch to activate the required processing was the least invasive way to go.</div>
<div><br></div><div>-Jason</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="HOEnZb"><div class="h5">
> Lastly, speculating here as this patch doesn't go into these details, but be very careful with alignment<br>
> 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 will n!<br>
> eed to be taught how to deal with these changes.<br>
><br>
> Yep - we are in the middle of refactoring our local work in constant islands for eventual upstreaming.<br>
><br>
> Hope this clarifies things!<br>
><br>
> -jason<br>
><br>
> -Jim<br>
><br>
> On Sep 5, 2011, at 2:51 PM, Nick Lewycky wrote:<br>
><br>
> > Author: nicholas<br>
> > Date: Mon Sep 5 16:51:43 2011<br>
> > New Revision: 139125<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=139125&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=139125&view=rev</a><br>
> > Log:<br>
> > Add a new MC bit for NaCl (Native Client) mode. NaCl requires that certain<br>
> > instructions are more aligned than the CPU requires, and adds some additional<br>
> > directives, to follow in future patches. Patch by David Meyer!<br>
> ><br>
> > Modified:<br>
> > llvm/trunk/lib/Target/ARM/ARM.td<br>
> > llvm/trunk/lib/Target/ARM/ARMInstrInfo.td<br>
> > llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp<br>
> > llvm/trunk/lib/Target/ARM/ARMSubtarget.h<br>
> > llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp<br>
> > llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp<br>
> > llvm/trunk/lib/Target/X86/X86.td<br>
> > llvm/trunk/lib/Target/X86/X86InstrInfo.td<br>
> > llvm/trunk/lib/Target/X86/X86Subtarget.cpp<br>
> > llvm/trunk/lib/Target/X86/X86Subtarget.h<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/ARM/ARM.td<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARM.td?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARM.td?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/ARM/ARM.td (original)<br>
> > +++ llvm/trunk/lib/Target/ARM/ARM.td Mon Sep 5 16:51:43 2011<br>
> > @@ -23,6 +23,9 @@<br>
> > def ModeThumb : SubtargetFeature<"thumb-mode", "InThumbMode", "true",<br>
> > "Thumb mode">;<br>
> ><br>
> > +def ModeNaCl : SubtargetFeature<"nacl-mode", "InNaClMode", "true",<br>
> > + "Native client mode">;<br>
> > +<br>
> > //===----------------------------------------------------------------------===//<br>
> > // ARM Subtarget features.<br>
> > //<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/ARM/ARMInstrInfo.td<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.td?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstrInfo.td?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/ARM/ARMInstrInfo.td (original)<br>
> > +++ llvm/trunk/lib/Target/ARM/ARMInstrInfo.td Mon Sep 5 16:51:43 2011<br>
> > @@ -209,6 +209,8 @@<br>
> > AssemblerPredicate<"!ModeThumb">;<br>
> > def IsDarwin : Predicate<"Subtarget->isTargetDarwin()">;<br>
> > def IsNotDarwin : Predicate<"!Subtarget->isTargetDarwin()">;<br>
> > +def IsNaCl : Predicate<"Subtarget->isTargetNaCl()">,<br>
> > + AssemblerPredicate<"ModeNaCl">;<br>
> ><br>
> > // FIXME: Eventually this will be just "hasV6T2Ops".<br>
> > def UseMovt : Predicate<"Subtarget->useMovt()">;<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp (original)<br>
> > +++ llvm/trunk/lib/Target/ARM/ARMSubtarget.cpp Mon Sep 5 16:51:43 2011<br>
> > @@ -53,6 +53,7 @@<br>
> > , HasVMLxForwarding(false)<br>
> > , SlowFPBrcc(false)<br>
> > , InThumbMode(false)<br>
> > + , InNaClMode(false)<br>
> > , HasThumb2(false)<br>
> > , NoARM(false)<br>
> > , PostRAScheduler(false)<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/ARM/ARMSubtarget.h<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMSubtarget.h?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMSubtarget.h?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/ARM/ARMSubtarget.h (original)<br>
> > +++ llvm/trunk/lib/Target/ARM/ARMSubtarget.h Mon Sep 5 16:51:43 2011<br>
> > @@ -70,6 +70,9 @@<br>
> > /// InThumbMode - True if compiling for Thumb, false for ARM.<br>
> > bool InThumbMode;<br>
> ><br>
> > + /// InNaClMode - True if targeting Native Client<br>
> > + bool InNaClMode;<br>
> > +<br>
> > /// HasThumb2 - True if Thumb2 instructions are supported.<br>
> > bool HasThumb2;<br>
> ><br>
> > @@ -209,6 +212,9 @@<br>
> > const Triple &getTargetTriple() const { return TargetTriple; }<br>
> ><br>
> > bool isTargetDarwin() const { return TargetTriple.isOSDarwin(); }<br>
> > + bool isTargetNaCl() const {<br>
> > + return TargetTriple.getOS() == Triple::NativeClient;<br>
> > + }<br>
> > bool isTargetELF() const { return !isTargetDarwin(); }<br>
> ><br>
> > bool isAPCS_ABI() const { return TargetABI == ARM_ABI_APCS; }<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp (original)<br>
> > +++ llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp Mon Sep 5 16:51:43 2011<br>
> > @@ -86,6 +86,14 @@<br>
> > ARMArchFeature += ",+thumb-mode";<br>
> > }<br>
> ><br>
> > + Triple TheTriple(TT);<br>
> > + if (TheTriple.getOS() == Triple::NativeClient) {<br>
> > + if (ARMArchFeature.empty())<br>
> > + ARMArchFeature = "+nacl-mode";<br>
> > + else<br>
> > + ARMArchFeature += ",+nacl-mode";<br>
> > + }<br>
> > +<br>
> > return ARMArchFeature;<br>
> > }<br>
> ><br>
> ><br>
> > Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp (original)<br>
> > +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp Mon Sep 5 16:51:43 2011<br>
> > @@ -40,9 +40,16 @@<br>
> ><br>
> > std::string X86_MC::ParseX86Triple(StringRef TT) {<br>
> > Triple TheTriple(TT);<br>
> > + std::string FS;<br>
> > if (TheTriple.getArch() == Triple::x86_64)<br>
> > - return "+64bit-mode";<br>
> > - return "-64bit-mode";<br>
> > + FS = "+64bit-mode";<br>
> > + else<br>
> > + FS = "-64bit-mode";<br>
> > + if (TheTriple.getOS() == Triple::NativeClient)<br>
> > + FS += ",+nacl-mode";<br>
> > + else<br>
> > + FS += ",-nacl-mode";<br>
> > + return FS;<br>
> > }<br>
> ><br>
> > /// GetCpuIDAndInfo - Execute the specified cpuid and return the 4 values in the<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/X86/X86.td<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86.td?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86.td?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/X86/X86.td (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86.td Mon Sep 5 16:51:43 2011<br>
> > @@ -23,6 +23,9 @@<br>
> > def Mode64Bit : SubtargetFeature<"64bit-mode", "In64BitMode", "true",<br>
> > "64-bit mode (x86_64)">;<br>
> ><br>
> > +def ModeNaCl : SubtargetFeature<"nacl-mode", "InNaClMode", "true",<br>
> > + "Native Client mode">;<br>
> > +<br>
> > //===----------------------------------------------------------------------===//<br>
> > // X86 Subtarget features.<br>
> > //===----------------------------------------------------------------------===//<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.td<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/X86/X86InstrInfo.td (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86InstrInfo.td Mon Sep 5 16:51:43 2011<br>
> > @@ -482,6 +482,14 @@<br>
> > AssemblerPredicate<"Mode64Bit">;<br>
> > def IsWin64 : Predicate<"Subtarget->isTargetWin64()">;<br>
> > def NotWin64 : Predicate<"!Subtarget->isTargetWin64()">;<br>
> > +def IsNaCl : Predicate<"Subtarget->isTargetNaCl()">,<br>
> > + AssemblerPredicate<"ModeNaCl">;<br>
> > +def IsNaCl32 : Predicate<"Subtarget->isTargetNaCl32()">,<br>
> > + AssemblerPredicate<"ModeNaCl,!Mode64Bit">;<br>
> > +def IsNaCl64 : Predicate<"Subtarget->isTargetNaCl64()">,<br>
> > + AssemblerPredicate<"ModeNaCl,Mode64Bit">;<br>
> > +def NotNaCl : Predicate<"!Subtarget->isTargetNaCl()">,<br>
> > + AssemblerPredicate<"!ModeNaCl">;<br>
> > def SmallCode : Predicate<"TM.getCodeModel() == CodeModel::Small">;<br>
> > def KernelCode : Predicate<"TM.getCodeModel() == CodeModel::Kernel">;<br>
> > def FarData : Predicate<"TM.getCodeModel() != CodeModel::Small &&"<br>
> ><br>
> > Modified: llvm/trunk/lib/Target/X86/X86Subtarget.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.cpp?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.cpp?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/X86/X86Subtarget.cpp (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86Subtarget.cpp Mon Sep 5 16:51:43 2011<br>
> > @@ -260,7 +260,8 @@<br>
> > // FIXME: this is a known good value for Yonah. How about others?<br>
> > , MaxInlineSizeThreshold(128)<br>
> > , TargetTriple(TT)<br>
> > - , In64BitMode(is64Bit) {<br>
> > + , In64BitMode(is64Bit)<br>
> > + , InNaClMode(false) {<br>
> > // Determine default and user specified characteristics<br>
> > if (!FS.empty() || !CPU.empty()) {<br>
> > std::string CPUName = CPU;<br>
> > @@ -306,6 +307,11 @@<br>
> > if (In64BitMode)<br>
> > ToggleFeature(X86::Mode64Bit);<br>
> ><br>
> > + if (isTargetNaCl()) {<br>
> > + InNaClMode = true;<br>
> > + ToggleFeature(X86::ModeNaCl);<br>
> > + }<br>
> > +<br>
> > if (HasAVX)<br>
> > X86SSELevel = NoMMXSSE;<br>
> ><br>
> ><br>
> > Modified: llvm/trunk/lib/Target/X86/X86Subtarget.h<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.h?rev=139125&r1=139124&r2=139125&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Subtarget.h?rev=139125&r1=139124&r2=139125&view=diff</a><br>
> > ==============================================================================<br>
> > --- llvm/trunk/lib/Target/X86/X86Subtarget.h (original)<br>
> > +++ llvm/trunk/lib/Target/X86/X86Subtarget.h Mon Sep 5 16:51:43 2011<br>
> > @@ -119,6 +119,9 @@<br>
> > /// In64BitMode - True if compiling for 64-bit, false for 32-bit.<br>
> > bool In64BitMode;<br>
> ><br>
> > + /// InNaClMode - True if compiling for Native Client target.<br>
> > + bool InNaClMode;<br>
> > +<br>
> > public:<br>
> ><br>
> > /// This constructor initializes the data members to match that<br>
> > @@ -190,6 +193,11 @@<br>
> > return !isTargetDarwin() && !isTargetWindows() && !isTargetCygMing();<br>
> > }<br>
> > bool isTargetLinux() const { return TargetTriple.getOS() == Triple::Linux; }<br>
> > + bool isTargetNaCl() const {<br>
> > + return TargetTriple.getOS() == Triple::NativeClient;<br>
> > + }<br>
> > + bool isTargetNaCl32() const { return isTargetNaCl() && !is64Bit(); }<br>
> > + bool isTargetNaCl64() const { return isTargetNaCl() && is64Bit(); }<br>
> ><br>
> > bool isTargetWindows() const { return TargetTriple.getOS() == Triple::Win32; }<br>
> > bool isTargetMingw() const { return TargetTriple.getOS() == Triple::MinGW32; }<br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div></blockquote></div><br>