Commit 7f92d66f authored by evgeny's avatar evgeny
Browse files

[ThinLTO] Fix bug when importing writeonly variables 

Patch enables import of write-only variables with non-trivial initializers
to fix linker errors. Initializers of imported variables are converted to
'zeroinitializer' to avoid promotion of referenced objects.

Differential revision: https://reviews.llvm.org/D70006
parent caad2170

llvm/lib/IR/ModuleSummaryIndex.cpp

+15 −1
Original line number Diff line number Diff line
@@ -197,7 +197,21 @@ void ModuleSummaryIndex::propagateAttributes( 
bool ModuleSummaryIndex::canImportGlobalVar(GlobalValueSummary *S,

                                            bool AnalyzeRefs) const {

  auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) {

    return !isReadOnly(GVS) && GVS->refs().size();

    // We don't analyze GV references during attribute propagation, so

    // GV with non-trivial initializer can be marked either read or

    // write-only.

    // Importing definiton of readonly GV with non-trivial initializer

    // allows us doing some extra optimizations (like converting indirect

    // calls to direct).

    // Definition of writeonly GV with non-trivial initializer should also

    // be imported. Not doing so will result in:

    // a) GV internalization in source module (because it's writeonly)

    // b) Importing of GV declaration to destination module as a result

    //    of promotion.

    // c) Link error (external declaration with internal definition).

    // However we do not promote objects referenced by writeonly GV

    // initializer by means of converting it to 'zeroinitializer'

    return !isReadOnly(GVS) && !isWriteOnly(GVS) && GVS->refs().size();

  };

  auto *GVS = cast<GlobalVarSummary>(S->getBaseObject());

llvm/lib/Transforms/IPO/FunctionImport.cpp

+8 −4
Original line number Diff line number Diff line
@@ -318,7 +318,11 @@ static void computeImportForReferencedGlobals( 
        if (ILI.second)

          NumImportedGlobalVarsThinLink++;

        MarkExported(VI, RefSummary.get());

        // Promote referenced functions and variables

        // Promote referenced functions and variables. We don't promote

        // objects referenced by writeonly variable initializer, because

        // we convert such variables initializers to "zeroinitializer".

        // See processGlobalForThinLTO.

        if (!Index.isWriteOnly(cast<GlobalVarSummary>(RefSummary.get())))

          for (const auto &VI : RefSummary->refs())

            for (const auto &RefFn : VI.getSummaryList())

              MarkExported(VI, RefFn.get());

llvm/lib/Transforms/Utils/FunctionImportUtils.cpp

+14 −3
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ 
//===----------------------------------------------------------------------===//



#include "llvm/Transforms/Utils/FunctionImportUtils.h"

#include "llvm/IR/Constants.h"

#include "llvm/IR/InstIterator.h"

using namespace llvm;
@@ -251,9 +252,19 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { 
      // matches one in this module (e.g. weak or appending linkage).

      auto* GVS = dyn_cast_or_null<GlobalVarSummary>(

          ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier()));      

      // At this stage "maybe" is "definitely"

      if (GVS && (ImportIndex.isReadOnly(GVS) || ImportIndex.isWriteOnly(GVS)))

      if (GVS &&

          (ImportIndex.isReadOnly(GVS) || ImportIndex.isWriteOnly(GVS))) {

        V->addAttribute("thinlto-internalize");

        // Objects referenced by writeonly GV initializer should not be 

        // promoted, because there is no any kind of read access to them

        // on behalf of this writeonly GV. To avoid promotion we convert

        // GV initializer to 'zeroinitializer'. This effectively drops

        // references in IR module (not in combined index), so we can

        // ignore them when computing import. We do not export references

        // of writeonly object. See computeImportForReferencedGlobals

        if (ImportIndex.isWriteOnly(GVS) && GVS->refs().size())

          V->setInitializer(Constant::getNullValue(V->getValueType()));

      }

    }

  }

llvm/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll

0 → 100644
+17 −0
Original line number Diff line number Diff line 
; ModuleID = 'foo.o'

source_filename = "foo.cpp"

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

target triple = "x86_64-unknown-linux-gnu"



%struct.S = type { i32 }

%struct.Q = type { %struct.S* }



@_ZL3Obj = internal constant %struct.S { i32 42 }, align 4

@outer = dso_local local_unnamed_addr global %struct.Q { %struct.S* @_ZL3Obj }, align 8



; Function Attrs: nofree norecurse nounwind uwtable writeonly

define dso_local void @_Z3foov() local_unnamed_addr {

entry:

  store %struct.S* null, %struct.S** getelementptr inbounds (%struct.Q, %struct.Q* @outer, i64 0, i32 0), align 8

  ret void

}

llvm/test/ThinLTO/X86/writeonly-with-refs.ll

0 → 100644
+26 −0
Original line number Diff line number Diff line 
; RUN: opt -thinlto-bc %s -o %t1

; RUN: opt -thinlto-bc %p/Inputs/writeonly-with-refs.ll -o %t2

; RUN: llvm-lto2 run -save-temps %t1 %t2 -o %t-out \

; RUN:    -r=%t1,main,plx \

; RUN:    -r=%t1,_Z3foov,l \

; RUN:    -r=%t2,_Z3foov,pl \

; RUN:    -r=%t2,outer,pl



; @outer should have been internalized and converted to zeroinitilizer.

; RUN: llvm-dis %t-out.1.5.precodegen.bc -o - | FileCheck %s

; RUN: llvm-dis %t-out.2.5.precodegen.bc -o - | FileCheck %s



; CHECK: @outer = internal unnamed_addr global %struct.Q zeroinitializer



source_filename = "main.cpp"

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

target triple = "x86_64-unknown-linux-gnu"



; Function Attrs: norecurse uwtable

define dso_local i32 @main() local_unnamed_addr {

entry:

  tail call void @_Z3foov()

  ret i32 0

}



declare dso_local void @_Z3foov() local_unnamed_addr