[llvm] r257181 - [ThinLTO] Use new in-place symbol changes for exporting module

[Teresa Johnson via llvm-commits]

On Fri, Jan 8, 2016 at 9:15 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>> On Jan 8, 2016, at 9:06 AM, Teresa Johnson via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: tejohnson
>> Date: Fri Jan  8 11:06:29 2016
>> New Revision: 257181
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=257181&view=rev
>> Log:
>> [ThinLTO] Use new in-place symbol changes for exporting module
>>
>> Due to the new in-place ThinLTO symbol handling support added in
>> r257174, we now invoke renameModuleForThinLTO on the current
>> module from within the FunctionImport pass.
>
> While this is simpler, this make the FunctionImporter doing more than just importing. It couples two steps.
> Might not be that important, but does not seem straightforward to me either.

Point taken. Open to suggestions on where to move it. I wanted to move
it out of clang though as it isn't needed there anymore. Should it be
a separate (new) pass?

Teresa

>
> —
> Mehdi
>
>
>
>
>>
>> Additionally, renameModuleForThinLTO no longer needs to return the
>> Module as it is performing the renaming in place on the one provided.
>>
>> This commit will be immediately preceeded by a companion clang patch to
>> remove its invocation of renameModuleForThinLTO.
>>
>> Modified:
>>    llvm/trunk/include/llvm/Linker/Linker.h
>>    llvm/trunk/lib/Linker/LinkModules.cpp
>>    llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
>>
>> Modified: llvm/trunk/include/llvm/Linker/Linker.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Linker/Linker.h?rev=257181&r1=257180&r2=257181&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Linker/Linker.h (original)
>> +++ llvm/trunk/include/llvm/Linker/Linker.h Fri Jan  8 11:06:29 2016
>> @@ -69,8 +69,7 @@ public:
>>
>> /// Perform in-place global value handling on the given Module for
>> /// exported local functions renamed and promoted for ThinLTO.
>> -std::unique_ptr<Module> renameModuleForThinLTO(std::unique_ptr<Module> M,
>> -                                               const FunctionInfoIndex *Index);
>> +bool renameModuleForThinLTO(Module &M, const FunctionInfoIndex *Index);
>>
>> } // End llvm namespace
>>
>>
>> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=257181&r1=257180&r2=257181&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
>> +++ llvm/trunk/lib/Linker/LinkModules.cpp Fri Jan  8 11:06:29 2016
>> @@ -863,13 +863,9 @@ bool Linker::linkModules(Module &Dest, s
>>   return L.linkInModule(std::move(Src), Flags);
>> }
>>
>> -std::unique_ptr<Module>
>> -llvm::renameModuleForThinLTO(std::unique_ptr<Module> M,
>> -                             const FunctionInfoIndex *Index) {
>> -  ThinLTOGlobalProcessing ThinLTOProcessing(*M, Index);
>> -  if (ThinLTOProcessing.run())
>> -    return nullptr;
>> -  return M;
>> +bool llvm::renameModuleForThinLTO(Module &M, const FunctionInfoIndex *Index) {
>> +  ThinLTOGlobalProcessing ThinLTOProcessing(M, Index);
>> +  return ThinLTOProcessing.run();
>> }
>>
>> //===----------------------------------------------------------------------===//
>>
>> Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=257181&r1=257180&r2=257181&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Fri Jan  8 11:06:29 2016
>> @@ -413,14 +413,19 @@ public:
>>       Index = IndexPtr.get();
>>     }
>>
>> +    // First we need to promote to global scope and rename any local values that
>> +    // are potentially exported to other modules.
>> +    if (renameModuleForThinLTO(M, Index)) {
>> +      errs() << "Error renaming module\n";
>> +      return false;
>> +    }
>> +
>>     // Perform the import now.
>>     auto ModuleLoader = [&M](StringRef Identifier) {
>>       return loadFile(Identifier, M.getContext());
>>     };
>>     FunctionImporter Importer(*Index, ModuleLoader);
>>     return Importer.importFunctions(M);
>> -
>> -    return false;
>>   }
>> };
>> } // anonymous namespace
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>



-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413