[PATCH] D35148: Use DenseMap instead std::map for GVSummaryMapTy.

[Mehdi AMINI via llvm-commits]

2017-07-10 11:40 GMT-07:00 Teresa Johnson <tejohnson at google.com>:

>
>
> On Mon, Jul 10, 2017 at 11:36 AM, Dehao Chen via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> danielcdh added a comment.
>>
>> How should we proceed with this patch?
>>
>> There are indeed some code that iterates through this type, e.g.:
>>
>
> Right, that's what I was referring to here: "The only place I see an
> iteration is when we are computing imports for a module, so I suppose that
> could affect the debug output ordering, but does that matter (if iteration
> order is even non-deterministic for DenseMap, and from what I can tell from
> the LLVM doc linked above it isn't)."
>
> I don't think this is an issue, and from Mehdi's response it sounds like
> he agrees.
>
> I think the patch is good to go. Mehdi - do you concur?
>

I'm not totally sure: for example we won't call `computeImportForFunction`
in the same order.
Right now I don't see how it should matter though.

However, as long as the iteration order is deterministic, it does not
matter if it is undefined: we still have full reproducibility.
It would be an issue if we have multiple-threads that lock and insert to
this map though, as it would make the iteration order non reproducible. If
it is not case I think we're good!

Hope it makes sense.

-- 
Mehdi




>
>
>>   // Populate the worklist with the import for the functions in the
>> current
>>   // module
>>   for (auto &GVSummary : DefinedGVSummaries) {
>>     if (!Index.isGlobalValueLive(GVSummary.second)) {
>>       DEBUG(dbgs() << "Ignores Dead GUID: " << GVSummary.first << "\n");
>>       continue;
>>     }
>>     auto *Summary = GVSummary.second;
>>     if (auto *AS = dyn_cast<AliasSummary>(Summary))
>>       Summary = &AS->getAliasee();
>>     auto *FuncSummary = dyn_cast<FunctionSummary>(Summary);
>>     if (!FuncSummary)
>>       // Skip import for global variables
>>       continue;
>>     DEBUG(dbgs() << "Initalize import for " << GVSummary.first << "\n");
>>     computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
>>                              DefinedGVSummaries, Worklist, ImportList,
>>                              ExportLists);
>>   }
>>
>>
>> https://reviews.llvm.org/D35148
>>
>>
>>
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413 <(408)%20460-2413>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170710/c45c1a19/attachment.html>