[llvm] r269638 - ThinLTO: fix non-determinism in bitcode writing

[Mehdi Amini via llvm-commits]

> On May 16, 2016, at 1:46 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>> 
>> On 2016-May-16, at 02:04, Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> Author: mehdi_amini
>> Date: Mon May 16 04:04:55 2016
>> New Revision: 269638
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=269638&view=rev
>> Log:
>> ThinLTO: fix non-determinism in bitcode writing
>> 
>> Calls are initialized from a DenseMap. We can sort them using the
>> value id to recover some determinism during serialization.
>> 
>> From: mehdi_amini <mehdi_amini at 91177308-0d34-0410-b5e6-96231b3b80d8>
>> 
>> Modified:
>>   llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
>> 
>> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=269638&r1=269637&r2=269638&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
>> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Mon May 16 04:04:55 2016
>> @@ -3177,8 +3177,15 @@ void ModuleBitcodeWriter::writePerModule
>> 
>>  NameVals.insert(NameVals.end(), Refs.begin(), Refs.end());
>> 
>> +  std::vector<FunctionSummary::EdgeTy> Calls = FS->calls();
> 
> Is there a case for a SmallVector here?  Is there some number that would avoid an allocation in most cases (without blowing out the stack)?
> 
> Regardless, you can move this auxiliary vector outside the loop and clear() it between uses.  This will avoid allocating unnecessarily on every iteration.

A bit intrusive though :(

Actually I'd like to avoid this sorting at bitcode writing time, it is not clear to me that we can't get the in-memory summary in a deterministic way. 
This series of patch was more a band-aid to reach a correct behavior while I take time to study the actual construction (from memory or from deserialization) for the summary graph.

-- 
Mehdi



> 
>> +  std::sort(Calls.begin(), Calls.end(),
>> +            [this](const FunctionSummary::EdgeTy &L,
>> +                   const FunctionSummary::EdgeTy &R) {
>> +              return VE.getValueID(L.first.getValue()) <
>> +                     VE.getValueID(R.first.getValue());
>> +            });
>>  bool HasProfileData = F.getEntryCount().hasValue();
>> -  for (auto &ECI : FS->calls()) {
>> +  for (auto &ECI : Calls) {
>>    NameVals.push_back(VE.getValueID(ECI.first.getValue()));
>>    assert(ECI.second.CallsiteCount > 0 && "Expected at least one callsite");
>>    NameVals.push_back(ECI.second.CallsiteCount);
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits