[llvm] r352768 - Revert "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader"

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 31 08:46:14 PST 2019


Author: tejohnson
Date: Thu Jan 31 08:46:14 2019
New Revision: 352768

URL: http://llvm.org/viewvc/llvm-project?rev=352768&view=rev
Log:
Revert "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader"

This reverts commit r352763.

Causing a couple bot failures, root cause pointed to by sanitizer bot:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/28909/steps/annotate/logs/stdio

Use after free. I understand the issue but will revert and test with fix
before recommitting.

Removed:
    llvm/trunk/test/Transforms/FunctionImport/Inputs/comdat.ll
    llvm/trunk/test/Transforms/FunctionImport/comdat.ll
Modified:
    llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h
    llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp

Modified: llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h?rev=352768&r1=352767&r2=352768&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h Thu Jan 31 08:46:14 2019
@@ -43,11 +43,6 @@ class FunctionImportGlobalProcessing {
   /// to promote any non-renamable values.
   SmallPtrSet<GlobalValue *, 8> Used;
 
-  /// Keep track of any COMDATs that require renaming (because COMDAT
-  /// leader was promoted and renamed). Maps from original COMDAT to one
-  /// with new name.
-  DenseMap<const Comdat *, Comdat *> RenamedComdats;
-
   /// Check if we should promote the given local value to global scope.
   bool shouldPromoteLocalToGlobal(const GlobalValue *SGV);
 

Modified: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp?rev=352768&r1=352767&r2=352768&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp Thu Jan 31 08:46:14 2019
@@ -248,7 +248,6 @@ void FunctionImportGlobalProcessing::pro
   bool DoPromote = false;
   if (GV.hasLocalLinkage() &&
       ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) {
-    auto Name = GV.getName();
     // Once we change the name or linkage it is difficult to determine
     // again whether we should promote since shouldPromoteLocalToGlobal needs
     // to locate the summary (based on GUID from name and linkage). Therefore,
@@ -257,12 +256,6 @@ void FunctionImportGlobalProcessing::pro
     GV.setLinkage(getLinkage(&GV, DoPromote));
     if (!GV.hasLocalLinkage())
       GV.setVisibility(GlobalValue::HiddenVisibility);
-
-    // If we are renaming a COMDAT leader, ensure that we record the COMDAT
-    // for later renaming as well. This is required for COFF.
-    if (const auto *C = GV.getComdat())
-      if (C->getName() == Name)
-        RenamedComdats.try_emplace(C, M.getOrInsertComdat(GV.getName()));
   } else
     GV.setLinkage(getLinkage(&GV, /* DoPromote */ false));
 
@@ -287,16 +280,6 @@ void FunctionImportGlobalProcessing::pro
     processGlobalForThinLTO(SF);
   for (GlobalAlias &GA : M.aliases())
     processGlobalForThinLTO(GA);
-
-  // Replace any COMDATS that required renaming (because the COMDAT leader was
-  // promoted and renamed).
-  if (!RenamedComdats.empty())
-    for (auto &GO : M.global_objects())
-      if (auto *C = GO.getComdat()) {
-        auto Replacement = RenamedComdats.find(C);
-        if (Replacement != RenamedComdats.end())
-          GO.setComdat(Replacement->second);
-      }
 }
 
 bool FunctionImportGlobalProcessing::run() {

Removed: llvm/trunk/test/Transforms/FunctionImport/Inputs/comdat.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/Inputs/comdat.ll?rev=352767&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/Inputs/comdat.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/Inputs/comdat.ll (removed)
@@ -1,10 +0,0 @@
-target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.0.24215"
-
-define void @main() {
-entry:
-  call i8* @lwt_fun()
-  ret void
-}
-
-declare i8* @lwt_fun()

Removed: llvm/trunk/test/Transforms/FunctionImport/comdat.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/comdat.ll?rev=352767&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/comdat.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/comdat.ll (removed)
@@ -1,32 +0,0 @@
-; Test to ensure that comdat is renamed consistently when comdat leader is
-; promoted and renamed due to an import. Required by COFF.
-
-; REQUIRES: x86-registered-target
-
-; RUN: opt -thinlto-bc -o %t1.bc %s
-; RUN: opt -thinlto-bc -o %t2.bc %S/Inputs/comdat.ll
-; RUN: llvm-lto2 run -save-temps -o %t3 %t1.bc %t2.bc \
-; RUN:          -r %t1.bc,lwt_fun,plx \
-; RUN:          -r %t2.bc,main,plx \
-; RUN:          -r %t2.bc,lwt_fun,
-; RUN: llvm-dis -o - %t3.1.3.import.bc | FileCheck %s
-
-target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.0.24215"
-
-; CHECK: $lwt.llvm.[[HASH:[0-9]+]] = comdat any
-$lwt = comdat any
-
-; CHECK: @lwt_aliasee = private unnamed_addr global {{.*}}, comdat($lwt.llvm.[[HASH]])
- at lwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($lwt)
-
-; CHECK: @lwt.llvm.[[HASH]] = hidden unnamed_addr alias
- at lwt = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @lwt_aliasee
-
-; Below function should get imported into other module, resulting in @lwt being
-; promoted and renamed.
-define i8* @lwt_fun() {
-  %1 = getelementptr inbounds [1 x i8*], [1 x i8*]* @lwt, i32 0, i32 0
-  %2 = load i8*, i8** %1
-  ret i8* %2
-}




More information about the llvm-commits mailing list