Thanks for the post-commit review!<br><br><div>On Sun Mar 23 2014 at 11:34:25 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Mar 23, 2014 at 10:47 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br>
Date: Sun Mar 23 12:47:16 2014<br>
New Revision: 204562<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=204562&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=204562&view=rev</a><br>
Log:<br>
MS ABI: Eliminate Duplicate Strings<br>
<br>
COFF doesn't have mergeable sections so LLVM/clang's normal tactics for<br>
string deduplication will not have any effect.<br>
<br>
To remedy this we place each string inside it's own section and mark<br>
the section as IMAGE_COMDAT_SELECT_ANY.  However, we can only do this if the<br>
string has an external name that we can generate from it's contents.<br>
<br>
To be compatible with MSVC, we must use their scheme.  Otherwise identical<br>
strings in translation units from clang may not be deduplicated with<br>
translation units in MSVC.<br>
<br>
This fixes PR18248.<br>
<br>
N.B. We will not attempt to do anything with a string literal which is not of<br>
type 'char' or 'wchar_t' because their compiler does not support unicode<br>
string literals as of this date.<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/AST/Mangle.h<br>
    cfe/trunk/lib/AST/ItaniumMangle.cpp<br>
    cfe/trunk/lib/AST/MicrosoftMangle.cpp<br>
    cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
    cfe/trunk/lib/CodeGen/CodeGenModule.h<br>
    cfe/trunk/test/CodeGen/wchar-const.c<br>
<br>
Modified: cfe/trunk/include/clang/AST/Mangle.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Mangle.h?rev=204562&r1=204561&r2=204562&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Mangle.h?rev=204562&r1=204561&r2=204562&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/include/clang/AST/Mangle.h (original)<br>
+++ cfe/trunk/include/clang/AST/Mangle.h Sun Mar 23 12:47:16 2014<br>
@@ -31,9 +31,10 @@ namespace clang {<br>
   class FunctionDecl;<br>
   class NamedDecl;<br>
   class ObjCMethodDecl;<br>
-  class VarDecl;<br>
+  class StringLiteral;<br>
   struct ThisAdjustment;<br>
   struct ThunkInfo;<br>
+  class VarDecl;<br>
<br>
 /// MangleBuffer - a convenient class for storing a name which is<br>
 /// either the result of a mangling or is a constant string with<br>
@@ -117,6 +118,7 @@ public:<br>
<br>
   bool shouldMangleDeclName(const NamedDecl *D);<br>
   virtual bool shouldMangleCXXName(const NamedDecl *D) = 0;<br>
+  virtual bool shouldMangleStringLiteral(const StringLiteral *SL) = 0;<br>
<br>
   // FIXME: consider replacing raw_ostream & with something like SmallString &.<br>
   void mangleName(const NamedDecl *D, raw_ostream &);<br>
@@ -135,6 +137,7 @@ public:<br>
                              raw_ostream &) = 0;<br>
   virtual void mangleCXXDtor(const CXXDestructorDecl *D, CXXDtorType Type,<br>
                              raw_ostream &) = 0;<br>
+  virtual void mangleStringLiteral(const StringLiteral *SL, raw_ostream &) = 0;<br>
<br>
   void mangleGlobalBlock(const BlockDecl *BD,<br>
                          const NamedDecl *ID,<br>
<br>
Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=204562&r1=204561&r2=204562&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=204562&r1=204561&r2=204562&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)<br>
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Sun Mar 23 12:47:16 2014<br>
@@ -21,6 +21,7 @@<br>
 #include "clang/AST/DeclCXX.h"<br>
 #include "clang/AST/DeclObjC.h"<br>
 #include "clang/AST/DeclTemplate.h"<br>
+#include "clang/AST/Expr.h"<br>
 #include "clang/AST/ExprCXX.h"<br>
 #include "clang/AST/ExprObjC.h"<br>
 #include "clang/AST/TypeLoc.h"<br>
@@ -126,6 +127,9 @@ public:<br>
   /// @{<br>
<br>
   bool shouldMangleCXXName(const NamedDecl *D) override;<br>
+  bool shouldMangleStringLiteral(const StringLiteral *) override {<br>
+    return false;<br>
+  }<br>
   void mangleCXXName(const NamedDecl *D, raw_ostream &) override;<br>
   void mangleThunk(const CXXMethodDecl *MD, const ThunkInfo &Thunk,<br>
                    raw_ostream &) override;<br>
@@ -153,6 +157,8 @@ public:<br>
   void mangleItaniumThreadLocalWrapper(const VarDecl *D,<br>
                                        raw_ostream &) override;<br>
<br>
+  void mangleStringLiteral(const StringLiteral *, raw_ostream &) override;<br>
+<br>
   bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) {<br>
     // Lambda closure types are already numbered.<br>
     if (isLambda(ND))<br>
@@ -3774,6 +3780,10 @@ void ItaniumMangleContextImpl::mangleTyp<br>
   mangleCXXRTTIName(Ty, Out);<br>
 }<br>
<br>
+void ItaniumMangleContextImpl::mangleStringLiteral(const StringLiteral *, raw_ostream &) {<br>
+  llvm_unreachable("Can't mangle string literals");<br>
+}<br>
+<br>
 ItaniumMangleContext *<br>
 ItaniumMangleContext::create(ASTContext &Context, DiagnosticsEngine &Diags) {<br>
   return new ItaniumMangleContextImpl(Context, Diags);<br>
<br>
Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=204562&r1=204561&r2=204562&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=204562&r1=204561&r2=204562&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)<br>
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Sun Mar 23 12:47:16 2014<br>
@@ -20,6 +20,7 @@<br>
 #include "clang/AST/DeclCXX.h"<br>
 #include "clang/AST/DeclObjC.h"<br>
 #include "clang/AST/DeclTemplate.h"<br>
+#include "clang/AST/Expr.h"<br>
 #include "clang/AST/ExprCXX.h"<br>
 #include "clang/AST/VTableBuilder.h"<br>
 #include "clang/Basic/ABI.h"<br>
@@ -27,6 +28,9 @@<br>
 #include "clang/Basic/TargetInfo.h"<br>
 #include "llvm/ADT/StringExtras.h"<br>
 #include "llvm/ADT/StringMap.h"<br>
+#include "llvm/Support/MathExtras.h"<br>
+<br>
+#include <algorithm><br>
<br>
 using namespace clang;<br>
<br>
@@ -93,6 +97,7 @@ public:<br>
   MicrosoftMangleContextImpl(ASTContext &Context, DiagnosticsEngine &Diags)<br>
       : MicrosoftMangleContext(Context, Diags) {}<br>
   bool shouldMangleCXXName(const NamedDecl *D) override;<br>
+  bool shouldMangleStringLiteral(const StringLiteral *SL) override;<br>
   void mangleCXXName(const NamedDecl *D, raw_ostream &Out) override;<br>
   void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD, raw_ostream &) override;<br>
   void mangleThunk(const CXXMethodDecl *MD, const ThunkInfo &Thunk,<br>
@@ -118,6 +123,7 @@ public:<br>
   void mangleDynamicInitializer(const VarDecl *D, raw_ostream &Out) override;<br>
   void mangleDynamicAtExitDestructor(const VarDecl *D,<br>
                                      raw_ostream &Out) override;<br>
+  void mangleStringLiteral(const StringLiteral *SL, raw_ostream &Out) override;<br>
   bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) {<br>
     // Lambda closure types are already numbered.<br>
     if (isLambda(ND))<br>
@@ -321,6 +327,13 @@ bool MicrosoftMangleContextImpl::shouldM<br>
   return true;<br>
 }<br>
<br>
+bool<br>
+MicrosoftMangleContextImpl::shouldMangleStringLiteral(const StringLiteral *SL) {<br>
+  return SL->isAscii() || SL->isWide();<br>
+  // TODO: This needs to be updated when MSVC gains support for Unicode<br>
+  // literals.<br>
+}<br>
+<br>
 void MicrosoftCXXNameMangler::mangle(const NamedDecl *D,<br>
                                      StringRef Prefix) {<br>
   // MSVC doesn't mangle C++ names the same way it mangles extern "C" names.<br>
@@ -2315,6 +2328,162 @@ MicrosoftMangleContextImpl::mangleDynami<br>
   mangleInitFiniStub(D, Out, 'F');<br>
 }<br>
<br>
+void MicrosoftMangleContextImpl::mangleStringLiteral(const StringLiteral *SL,<br>
+                                                     raw_ostream &Out) {<br>
+  // <char-type> ::= 0   # char<br>
+  //             ::= 1   # wchar_t<br>
+  //             ::= ??? # char16_t/char32_t will need a mangling too...<br>
+  //<br>
+  // <literal-length> ::= <non-negative integer>  # the length of the literal<br>
+  //<br>
+  // <encoded-crc>    ::= <hex digit>+ @          # crc of the literal including<br>
+  //                                              # null-terminator<br>
+  //<br>
+  // <encoded-string> ::= <simple character>           # uninteresting character<br>
+  //                  ::= '?$' <hex digit> <hex digit> # these two nibbles<br>
+  //                                                   # encode the byte for the<br>
+  //                                                   # character<br>
+  //                  ::= '?' [a-z]                    # \xe1 - \xfa<br>
+  //                  ::= '?' [A-Z]                    # \xc1 - \xda<br>
+  //                  ::= '?' [0-9]                    # [,/\:. \n\t'-]<br>
+  //<br>
+  // <literal> ::= '??_C@_' <char-type> <literal-length> <encoded-crc><br>
+  //               <encoded-string> '@'<br>
+  MicrosoftCXXNameMangler Mangler(*this, Out);<br>
+  Mangler.getStream() << "\01??_C@_";<br>
+<br>
+  // <char-type>: The "kind" of string literal is encoded into the mangled name.<br>
+  // TODO: This needs to be updated when MSVC gains support for unicode<br>
+  // literals.<br>
+  if (SL->isAscii())<br>
+    Mangler.getStream() << '0';<br>
+  else if (SL->isWide())<br>
+    Mangler.getStream() << '1';<br>
+  else<br>
+    llvm_unreachable("unexpected string literal kind!");<br>
+<br>
+  // <literal-length>: The next part of the mangled name consists of the length<br>
+  // of the string.<br>
+  // The StringLiteral does not consider the NUL terminator byte(s) but the<br>
+  // mangling does.<br>
+  // N.B. The length is in terms of bytes, not characters.<br>
+  Mangler.mangleNumber(SL->getByteLength() + SL->getCharByteWidth());<br>
+<br>
+  // We will use the "Rocksoft^tm Model CRC Algorithm" to describe the<br>
+  // properties of our CRC:<br>
+  //   Width  : 32<br>
+  //   Poly   : 04C11DB7<br>
+  //   Init   : FFFFFFFF<br>
+  //   RefIn  : True<br>
+  //   RefOut : True<br>
+  //   XorOut : 00000000<br>
+  //   Check  : 340BC6D9<br>
+  uint32_t CRC = 0xFFFFFFFFU;<br>
+<br>
+  auto UpdateCRC = [&CRC](char Byte) {<br>
+    for (unsigned i = 0; i < 8; ++i) {<br>
+      bool Bit = CRC & 0x80000000U;<br>
+      if (Byte & (1U << i))<br>
+        Bit = !Bit;<br>
+      CRC <<= 1;<br>
+      if (Bit)<br>
+        CRC ^= 0x04C11DB7U;<br>
+    }<br>
+  };<br>
+<br>
+  // CRC all the bytes of the StringLiteral.<br>
+  for (char Byte : SL->getBytes())<br>
+    UpdateCRC(Byte);<br>
+<br>
+  // The NUL terminator byte(s) were not present earlier,<br>
+  // we need to manually process those bytes into the CRC.<br>
+  for (unsigned NullTerminator = 0; NullTerminator < SL->getCharByteWidth();<br>
+       ++NullTerminator)<br>
+    UpdateCRC('\x00');<br>
+<br>
+  // The literature refers to the process of reversing the bits in the final CRC<br>
+  // output as "reflection".<br>
+  CRC = llvm::reverseBits(CRC);<br>
+<br>
+  // <encoded-crc>: The CRC is encoded utilizing the standard number mangling<br>
+  // scheme.<br>
+  Mangler.mangleNumber(CRC);<br>
+<br>
+  // <encoded-crc>: The mangled name also contains the first 32 _characters_<br>
+  // (including null-terminator bytes) of the StringLiteral.<br>
+  // Each character is encoded by splitting them into bytes and then encoding<br>
+  // the constituent bytes.<br>
+  auto MangleCharacter = [&Mangler](char Byte) {<br>
+    // There are five different manglings for characters:<br>
+    // - ?[0-9]: The set of [,/\:. \n\t'-].<br>
+    // - [a-zA-Z0-9_$]: A one-to-one mapping.<br>
+    // - ?[a-z]: The range from \xe1 to \xfa.<br>
+    // - ?[A-Z]: The range from \xc1 to \xda.<br>
+    // - ?$XX: A fallback which maps nibbles.<br>
+    static const char SpecialMap[] = {',', '/',  '\\', ':',  '.',<br>
+                                      ' ', '\n', '\t', '\'', '-'};<br>
+    const char *SpecialMapI =<br>
+        std::find(std::begin(SpecialMap), std::end(SpecialMap), Byte);<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Do we generate good code for this? Maybe move it down into the 'else' block to avoid the linear scan in the common case?</div>
</div></div></div></blockquote><div><br></div><div>I was hoping that we would turn this into a memchr or a simple vector operation, we aren't that lucky though :/ </div><div><br></div><div>I've verified that the new formulation has an IR switch instruction.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    if (SpecialMapI != std::end(SpecialMap)) {<br>
+      Mangler.getStream() << '?' << SpecialMapI - SpecialMap;<br>
+    } else if ((Byte >= 'a' && Byte <= 'z') || (Byte >= 'A' && Byte <= 'Z') ||<br>
+               (Byte >= '0' && Byte <= '9') || Byte == '_' || Byte == '$') {<br>
+      Mangler.getStream() << Byte;<br>
+    } else if (Byte >= '\xe1' && Byte <= '\xfa') {<br>
+      Mangler.getStream() << '?' << (char)('a' + Byte - '\xe1');<br>
+    } else if (Byte >= '\xc1' && Byte <= '\xda') {<br>
+      Mangler.getStream() << '?' << (char)('A' + Byte - '\xc1');<br>
+    } else {<br>
+      Mangler.getStream() << "?$";<br>
+      Mangler.getStream() << (char)('A' + ((Byte >> 4) & 0xf));<br>
+      Mangler.getStream() << (char)('A' + (Byte & 0xf));<br>
+    }<br>
+  };<br>
+<br>
+  // Enforce our 32 character max.<br>
+  unsigned MaxBytes = 32 * SL->getCharByteWidth();<br>
+  StringRef Bytes = SL->getBytes().substr(0, MaxBytes);<br>
+  size_t BytesLength = Bytes.size();<br>
+<br>
+  if (SL->isAscii()) {<br>
+    // A character maps directly to a byte for ASCII StringLiterals.<br>
+    for (char Byte : Bytes)<br>
+      MangleCharacter(Byte);<br>
+  } else if (SL->isWide()) {<br>
+    // The ordering of bytes in a wide StringLiteral is like so:<br>
+    //   A B C D ...<br>
+    // However, they are mangled in the following order:<br>
+    //   B A D C ...<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I don't believe you =)</div><div><br></div><div>The ordering of bytes in a wide StringLiteral depends on the endianness of the host machine. If the strings are mangled in big-endian order, we should do that properly rather than assuming the host is little-endian.</div>
</div></div></div></blockquote><div><br></div><div>Yep, sorry about that.  On the bright side, I now recast this as an operation on code-points instead of byte which simplified the code a bit.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    for (size_t i = 0; i != BytesLength;) {<br>
+      char FirstByte = Bytes[i];<br>
+      ++i;<br>
+      if (i != BytesLength) {<br>
+        char SecondByte = Bytes[i];<br>
+        ++i;<br>
+        MangleCharacter(SecondByte);<br>+      }<br>
+      MangleCharacter(FirstByte);<br>
+    }<br>
+  } else {<br>
+    llvm_unreachable("unexpected string literal kind!");<br>
+    // TODO: This needs to be updated when MSVC gains support for Unicode<br>
+    // literals.<br>
+  }<br>
+<br>
+  // We should also encode the NUL terminator(s) if we encoded less than 32<br>
+  // characters.<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>"less" should be "fewer" =)</div></div></div></div></blockquote>
<div><br></div><div>I ended up replacing the offending sentence with something different.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


+  if (BytesLength < MaxBytes) {<br>
+    size_t PaddingBytes = SL->getCharByteWidth();<br>
+    size_t BytesLeft = MaxBytes - BytesLength;<br>
+    if (BytesLeft < PaddingBytes)<br>
+      PaddingBytes = BytesLeft;<br>
+    for (unsigned i = 0; i < PaddingBytes; ++i)<br>
+      MangleCharacter('\x00');<br>
+  }<br>
+<br>
+  Mangler.getStream() << '@';<br>
+}<br>
+<br>
 MicrosoftMangleContext *<br>
 MicrosoftMangleContext::create(ASTContext &Context, DiagnosticsEngine &Diags) {<br>
   return new MicrosoftMangleContextImpl(Context, Diags);<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=204562&r1=204561&r2=204562&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=204562&r1=204561&r2=204562&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sun Mar 23 12:47:16 2014<br>
@@ -2572,25 +2572,67 @@ CodeGenModule::GetConstantArrayFromStrin<br>
 llvm::Constant *<br>
 CodeGenModule::GetAddrOfConstantStringFromLiteral(const StringLiteral *S) {<br>
   CharUnits Align = getContext().getAlignOfGlobalVarInChars(S->getType());<br>
-  if (S->isAscii() || S->isUTF8()) {<br>
-    SmallString<64> Str(S->getString());<br>
-<br>
-    // Resize the string to the right size, which is indicated by its type.<br>
-    const ConstantArrayType *CAT = Context.getAsConstantArrayType(S->getType());<br>
-    Str.resize(CAT->getSize().getZExtValue());<br>
-    return GetAddrOfConstantString(Str, /*GlobalName*/ 0, Align.getQuantity());<br>
+<br>
+  llvm::StringMapEntry<llvm::GlobalVariable *> *Entry = nullptr;<br>
+  llvm::GlobalVariable *GV = nullptr;<br>
+  if (!LangOpts.WritableStrings) {<br>
+    llvm::StringMap<llvm::GlobalVariable *> *ConstantStringMap = nullptr;<br>
+    switch (S->getCharByteWidth()) {<br>
+    case 1:<br>
+      ConstantStringMap = &Constant1ByteStringMap;<br>
+      break;<br>
+    case 2:<br>
+      ConstantStringMap = &Constant2ByteStringMap;<br>
+      break;<br>
+    case 4:<br>
+      ConstantStringMap = &Constant4ByteStringMap;<br>
+      break;<br>
+    default:<br>
+      llvm_unreachable("unhandled byte width!");<br>
+    }<br>
+    Entry = &ConstantStringMap->GetOrCreateValue(S->getBytes());<br>
+    GV = Entry->getValue();<br>
+  }<br>
+<br>
+  if (!GV) {<br>
+    StringRef GlobalVariableName;<br>
+    llvm::GlobalValue::LinkageTypes LT;<br>
+    if (!LangOpts.WritableStrings &&<br>
+        getCXXABI().getMangleContext().shouldMangleStringLiteral(S)) {<br>
+      LT = llvm::GlobalValue::LinkOnceODRLinkage;<br>
+<br>
+      SmallString<256> Buffer;<br>
+      llvm::raw_svector_ostream Out(Buffer);<br>
+      getCXXABI().getMangleContext().mangleStringLiteral(S, Out);<br>
+      Out.flush();<br>
+<br>
+      size_t Length = Buffer.size();<br>
+      char *Name = MangledNamesAllocator.Allocate<char>(Length);<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Do you really need an allocation and a copy here? Moving 'Buffer' outside the 'if' would seem sufficient -- creating the GlobalVariable will make its own copy.</div>
</div></div></div></blockquote><div><br></div><div>I didn't, it's code that survived from a different way of phrasing the memoization.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+      std::copy(Buffer.begin(), Buffer.end(), Name);<br>
+      GlobalVariableName = StringRef(Name, Length);<br>
+    } else {<br>
+      LT = llvm::GlobalValue::PrivateLinkage;;<br>
+      GlobalVariableName = ".str";<br>
+    }<br>
+<br>
+    // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.<br>
+    unsigned AddrSpace = 0;<br>
+    if (getLangOpts().OpenCL)<br>
+      AddrSpace = getContext().getTargetAddressSpace(LangAS::opencl_constant);<br>
+<br>
+    llvm::Constant *C = GetConstantArrayFromStringLiteral(S);<br>
+    GV = new llvm::GlobalVariable(<br>
+        getModule(), C->getType(), !LangOpts.WritableStrings, LT, C,<br>
+        GlobalVariableName, /*InsertBefore=*/nullptr,<br>
+        llvm::GlobalVariable::NotThreadLocal, AddrSpace);<br>
+    GV->setUnnamedAddr(true);<br>
+    if (Entry)<br>
+      Entry->setValue(GV);<br>
   }<br>
<br>
-  // FIXME: the following does not memoize wide strings.<br>
-  llvm::Constant *C = GetConstantArrayFromStringLiteral(S);<br>
-  llvm::GlobalVariable *GV =<br>
-    new llvm::GlobalVariable(getModule(),C->getType(),<br>
-                             !LangOpts.WritableStrings,<br>
-                             llvm::GlobalValue::PrivateLinkage,<br>
-                             C,".str");<br>
+  if (Align.getQuantity() > GV->getAlignment())<br>
+    GV->setAlignment(Align.getQuantity());<br>
<br>
-  GV->setAlignment(Align.getQuantity());<br>
-  GV->setUnnamedAddr(true);<br>
   return GV;<br>
 }<br>
<br>
@@ -2615,7 +2657,7 @@ static llvm::GlobalVariable *GenerateStr<br>
   llvm::Constant *C =<br>
       llvm::ConstantDataArray::getString(CGM.getLLVMContext(), str, false);<br>
<br>
-  // OpenCL v1.1 s6.5.3: a string literal is in the constant address space.<br>
+  // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.<br>
   unsigned AddrSpace = 0;<br>
   if (CGM.getLangOpts().OpenCL)<br>
     AddrSpace = CGM.getContext().getTargetAddressSpace(LangAS::opencl_constant);<br>
@@ -2654,7 +2696,7 @@ llvm::Constant *CodeGenModule::GetAddrOf<br>
     return GenerateStringLiteral(Str, false, *this, GlobalName, Alignment);<br>
<br>
   llvm::StringMapEntry<llvm::GlobalVariable *> &Entry =<br>
-    ConstantStringMap.GetOrCreateValue(Str);<br>
+    Constant1ByteStringMap.GetOrCreateValue(Str);<br>
<br>
   if (llvm::GlobalVariable *GV = Entry.getValue()) {<br>
     if (Alignment > GV->getAlignment()) {<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=204562&r1=204561&r2=204562&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=204562&r1=204561&r2=204562&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)<br>
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Sun Mar 23 12:47:16 2014<br>
@@ -319,7 +319,10 @@ class CodeGenModule : public CodeGenType<br>
   llvm::StringMap<llvm::Constant*> AnnotationStrings;<br>
<br>
   llvm::StringMap<llvm::Constant*> CFConstantStringMap;<br>
-  llvm::StringMap<llvm::GlobalVariable*> ConstantStringMap;<br>
+<br>
+  llvm::StringMap<llvm::GlobalVariable *> Constant1ByteStringMap;<br>
+  llvm::StringMap<llvm::GlobalVariable *> Constant2ByteStringMap;<br>
+  llvm::StringMap<llvm::GlobalVariable *> Constant4ByteStringMap;<br>
   llvm::DenseMap<const Decl*, llvm::Constant *> StaticLocalDeclMap;<br>
   llvm::DenseMap<const Decl*, llvm::GlobalVariable*> StaticLocalDeclGuardMap;<br>
   llvm::DenseMap<const Expr*, llvm::Constant *> MaterializedGlobalTemporaryMap;<br>
<br>
Modified: cfe/trunk/test/CodeGen/wchar-const.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/wchar-const.c?rev=204562&r1=204561&r2=204562&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/wchar-const.c?rev=204562&r1=204561&r2=204562&view=diff</a><br>



==============================================================================<br>
--- cfe/trunk/test/CodeGen/wchar-const.c (original)<br>
+++ cfe/trunk/test/CodeGen/wchar-const.c Sun Mar 23 12:47:16 2014<br>
@@ -15,7 +15,7 @@ typedef __WCHAR_TYPE__ wchar_t;<br>
<br>
<br>
 // CHECK-DAR: private unnamed_addr constant [18 x i32] [i32 84,<br>
-// CHECK-WIN: private unnamed_addr constant [18 x i16] [i16 84,<br>
+// CHECK-WIN: linkonce_odr unnamed_addr constant [18 x i16] [i16 84,<br>
 extern void foo(const wchar_t* p);<br>
 int main (int argc, const char * argv[])<br>
 {</blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>You seem to be very short on tests; did you forget to svn add a file? </div></div></div></div></blockquote>
<div><br></div><div>Yep, that was indeed the case.  Sorry about that!<br></div><div><br></div><div>These should all be addressed in r204586.</div>