r214222 - PR20473: Don't "deduplicate" string literals with the same value but different

[Richard Smith]

This fixes a wrong-code regression since 3.4, and should go onto the branch.


On Tue, Jul 29, 2014 at 2:20 PM, Richard Smith <richard-llvm at metafoo.co.uk>
wrote:

> Author: rsmith
> Date: Tue Jul 29 16:20:12 2014
> New Revision: 214222
>
> URL: http://llvm.org/viewvc/llvm-project?rev=214222&view=rev
> Log:
> PR20473: Don't "deduplicate" string literals with the same value but
> different
> lengths! In passing, simplify string literal deduplication by relying on
> LLVM
> to deduplicate the underlying constant values.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>     cfe/trunk/lib/CodeGen/CodeGenModule.h
>     cfe/trunk/test/CodeGen/init.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=214222&r1=214221&r2=214222&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Jul 29 16:20:12 2014
> @@ -2773,10 +2773,11 @@ CodeGenModule::GetAddrOfConstantStringFr
>    auto Alignment =
>        getContext().getAlignOfGlobalVarInChars(S->getType()).getQuantity();
>
> -  llvm::StringMapEntry<llvm::GlobalVariable *> *Entry = nullptr;
> +  llvm::Constant *C = GetConstantArrayFromStringLiteral(S);
> +  llvm::GlobalVariable **Entry = nullptr;
>    if (!LangOpts.WritableStrings) {
> -    Entry = getConstantStringMapEntry(S->getBytes(),
> S->getCharByteWidth());
> -    if (auto GV = Entry->getValue()) {
> +    Entry = &ConstantStringMap[C];
> +    if (auto GV = *Entry) {
>        if (Alignment > GV->getAlignment())
>          GV->setAlignment(Alignment);
>        return GV;
> @@ -2803,10 +2804,9 @@ CodeGenModule::GetAddrOfConstantStringFr
>      GlobalVariableName = ".str";
>    }
>
> -  llvm::Constant *C = GetConstantArrayFromStringLiteral(S);
>    auto GV = GenerateStringLiteral(C, LT, *this, GlobalVariableName,
> Alignment);
>    if (Entry)
> -    Entry->setValue(GV);
> +    *Entry = GV;
>
>    reportGlobalToASan(GV, S->getStrTokenLoc(0), "<string literal>");
>    return GV;
> @@ -2822,26 +2822,6 @@ CodeGenModule::GetAddrOfConstantStringFr
>    return GetAddrOfConstantCString(Str);
>  }
>
> -
> -llvm::StringMapEntry<llvm::GlobalVariable *>
> *CodeGenModule::getConstantStringMapEntry(
> -    StringRef Str, int CharByteWidth) {
> -  llvm::StringMap<llvm::GlobalVariable *> *ConstantStringMap = nullptr;
> -  switch (CharByteWidth) {
> -  case 1:
> -    ConstantStringMap = &Constant1ByteStringMap;
> -    break;
> -  case 2:
> -    ConstantStringMap = &Constant2ByteStringMap;
> -    break;
> -  case 4:
> -    ConstantStringMap = &Constant4ByteStringMap;
> -    break;
> -  default:
> -    llvm_unreachable("unhandled byte width!");
> -  }
> -  return &ConstantStringMap->GetOrCreateValue(Str);
> -}
> -
>  /// GetAddrOfConstantCString - Returns a pointer to a character array
> containing
>  /// the literal and a terminating '\0' character.
>  /// The result has pointer to array type.
> @@ -2854,19 +2834,20 @@ llvm::GlobalVariable *CodeGenModule::Get
>                      .getQuantity();
>    }
>
> +  llvm::Constant *C =
> +      llvm::ConstantDataArray::getString(getLLVMContext(), StrWithNull,
> false);
> +
>    // Don't share any string literals if strings aren't constant.
> -  llvm::StringMapEntry<llvm::GlobalVariable *> *Entry = nullptr;
> +  llvm::GlobalVariable **Entry = nullptr;
>    if (!LangOpts.WritableStrings) {
> -    Entry = getConstantStringMapEntry(StrWithNull, 1);
> -    if (auto GV = Entry->getValue()) {
> +    Entry = &ConstantStringMap[C];
> +    if (auto GV = *Entry) {
>        if (Alignment > GV->getAlignment())
>          GV->setAlignment(Alignment);
>        return GV;
>      }
>    }
>
> -  llvm::Constant *C =
> -      llvm::ConstantDataArray::getString(getLLVMContext(), StrWithNull,
> false);
>    // Get the default prefix if a name wasn't specified.
>    if (!GlobalName)
>      GlobalName = ".str";
> @@ -2874,7 +2855,7 @@ llvm::GlobalVariable *CodeGenModule::Get
>    auto GV = GenerateStringLiteral(C, llvm::GlobalValue::PrivateLinkage,
> *this,
>                                    GlobalName, Alignment);
>    if (Entry)
> -    Entry->setValue(GV);
> +    *Entry = GV;
>    return GV;
>  }
>
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=214222&r1=214221&r2=214222&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Tue Jul 29 16:20:12 2014
> @@ -361,9 +361,7 @@ class CodeGenModule : public CodeGenType
>
>    llvm::StringMap<llvm::Constant*> CFConstantStringMap;
>
> -  llvm::StringMap<llvm::GlobalVariable *> Constant1ByteStringMap;
> -  llvm::StringMap<llvm::GlobalVariable *> Constant2ByteStringMap;
> -  llvm::StringMap<llvm::GlobalVariable *> Constant4ByteStringMap;
> +  llvm::DenseMap<llvm::Constant *, llvm::GlobalVariable *>
> ConstantStringMap;
>    llvm::DenseMap<const Decl*, llvm::Constant *> StaticLocalDeclMap;
>    llvm::DenseMap<const Decl*, llvm::GlobalVariable*>
> StaticLocalDeclGuardMap;
>    llvm::DenseMap<const Expr*, llvm::Constant *>
> MaterializedGlobalTemporaryMap;
> @@ -1044,9 +1042,6 @@ private:
>                                          llvm::PointerType *PTy,
>                                          const VarDecl *D);
>
> -  llvm::StringMapEntry<llvm::GlobalVariable *> *
> -  getConstantStringMapEntry(StringRef Str, int CharByteWidth);
> -
>    /// Set attributes which are common to any form of a global definition
> (alias,
>    /// Objective-C method, function, global variable).
>    ///
>
> Modified: cfe/trunk/test/CodeGen/init.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/init.c?rev=214222&r1=214221&r2=214222&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/init.c (original)
> +++ cfe/trunk/test/CodeGen/init.c Tue Jul 29 16:20:12 2014
> @@ -132,3 +132,11 @@ void test13(int x) {
>    // CHECK: @test13
>    // CHECK: and i16 {{.*}}, -1024
>  }
> +
> +// CHECK-LABEL: @PR20473
> +void PR20473() {
> +  // CHECK: memcpy{{.*}}([2 x i8]* @
> +  bar((char[2]) {""});
> +  // CHECK: memcpy{{.*}}([3 x i8]* @
> +  bar((char[3]) {""});
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140729/8d636188/attachment.html>