[llvm-commits] [llvm] r166435 - /llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp

[Chandler Carruth]

On Mon, Oct 22, 2012 at 1:06 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Mon, Oct 22, 2012 at 12:43 PM, Julien Lerouge <jlerouge at apple.com> wrote:
>> Author: jlerouge
>> Date: Mon Oct 22 14:43:56 2012
>> New Revision: 166435
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=166435&view=rev
>> Log:
>> Iterating over a DenseMap<std::pair<BasicBlock*, unsigned>, PHINode*> is not
>> deterministic, replace it with a DenseMap<std::pair<unsigned, unsigned>,
>> PHINode*> (we already have a map from BasicBlock to unsigned).
>>
>> <rdar://problem/12541389>
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp?rev=166435&r1=166434&r2=166435&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp Mon Oct 22 14:43:56 2012
>> @@ -214,7 +214,7 @@
>>
>>      /// NewPhiNodes - The PhiNodes we're adding.
>>      ///
>> -    DenseMap<std::pair<BasicBlock*, unsigned>, PHINode*> NewPhiNodes;
>> +    DenseMap<std::pair<unsigned, unsigned>, PHINode*> NewPhiNodes;
>>
>>      /// PhiToAllocaMap - For each PHI node, keep track of which entry in Allocas
>>      /// it corresponds to.
>> @@ -588,7 +588,7 @@
>>    while (EliminatedAPHI) {
>>      EliminatedAPHI = false;
>>
>> -    for (DenseMap<std::pair<BasicBlock*, unsigned>, PHINode*>::iterator I =
>> +    for (DenseMap<std::pair<unsigned, unsigned>, PHINode*>::iterator I =
>>             NewPhiNodes.begin(), E = NewPhiNodes.end(); I != E;) {
>>        PHINode *PN = I->second;
>
> It would be much clearer that this is deterministic if you used
> llvm::MapVector instead.

More importantly, we're likely to make DenseMap iteration order
*never* be deterministic specifically to flush out these subtle
contexts. MapVector seems a much better tool here.

>
> -Eli
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits