[llvm] r183532 - R600: Fix a potential iterator invalidation issue.

[Benjamin Kramer]

On 07.06.2013, at 18:53, Vincent Lejeune <vljn at ovi.com> wrote:

> Ok, I though vector::erase(Iterator) left vector untouched if Iterator was equal to vector::end()

Ah. I updated the file with the std::find code I posted in r183541 and added an assert to make sure Chan is really only once in the vector.

- Ben
> 
> 
> ----- Mail original -----
>> De : Benjamin Kramer <benny.kra at gmail.com>
>> À : Vincent Lejeune <vljn at ovi.com>
>> Cc : llvm-commits at cs.uiuc.edu
>> Envoyé le : Vendredi 7 juin 2013 18h37
>> Objet : Re: [llvm] r183532 - R600: Fix a potential iterator invalidation issue.
>> 
>> 
>> On 07.06.2013, at 18:27, Vincent Lejeune <vljn at ovi.com> wrote:
>> 
>>> Hi,
>>> 
>>> I initially wrote the code almost like this way, using std::find instead of
>>> std::remove (because there is at most one element that is equal to chan in 
>> UpdatedUndef) but
>>> I got memory corruption error in some llvm IR.
>> 
>> If there's at most one equal element in the vector something like
>> 
>>     std::vector<unsigned>::iterator ChanPos =
>>         std::find(UpdatedUndef.begin(), UpdatedUndef.end(), Chan);
>>     if (ChanPos != UpdatedUndef.end())
>>       UpdatedUndef.erase(ChanPos);
>> 
>> should work. It would be a little bit faster because std::find exit early. The 
>> std::remove sequence is equivalent to the previous code but avoids the iterator 
>> invalidation issue.
>> 
>> - Ben
>> 
>>> 
>>> Vincent
>>> 
>>> ----- Mail original -----
>>>> De : Benjamin Kramer <benny.kra at googlemail.com>
>>>> À : llvm-commits at cs.uiuc.edu
>>>> Cc : 
>>>> Envoyé le : Vendredi 7 juin 2013 18h13
>>>> Objet : [llvm] r183532 - R600: Fix a potential iterator invalidation 
>> issue.
>>>> 
>>>> Author: d0k
>>>> Date: Fri Jun  7 11:13:49 2013
>>>> New Revision: 183532
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=183532&view=rev
>>>> Log:
>>>> R600: Fix a potential iterator invalidation issue.
>>>> 
>>>> As a bonus this reduces the loop from O(n^2) to O(n).
>>>> 
>>>> Modified:
>>>>      llvm/trunk/lib/Target/R600/R600OptimizeVectorRegisters.cpp
>>>> 
>>>> Modified: llvm/trunk/lib/Target/R600/R600OptimizeVectorRegisters.cpp
>>>> URL: 
>>>> 
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/R600/R600OptimizeVectorRegisters.cpp?rev=183532&r1=183531&r2=183532&view=diff
>>>> 
>> ==============================================================================
>>>> --- llvm/trunk/lib/Target/R600/R600OptimizeVectorRegisters.cpp 
>> (original)
>>>> +++ llvm/trunk/lib/Target/R600/R600OptimizeVectorRegisters.cpp Fri Jun  
>> 7 
>>>> 11:13:49 2013
>>>> @@ -198,11 +198,9 @@ MachineInstr *R600VectorRegMerger::Rebui
>>>>           .addReg(SubReg)
>>>>           .addImm(Chan);
>>>>       UpdatedRegToChan[SubReg] = Chan;
>>>> -    for (std::vector<unsigned>::iterator RemoveIt = 
>> UpdatedUndef.begin(),
>>>> -        RemoveE = UpdatedUndef.end(); RemoveIt != RemoveE; ++ 
>> RemoveIt) {
>>>> -      if (*RemoveIt == Chan)
>>>> -        UpdatedUndef.erase(RemoveIt);
>>>> -    }
>>>> +    UpdatedUndef.erase(
>>>> +        std::remove(UpdatedUndef.begin(), UpdatedUndef.end(), Chan),
>>>> +        UpdatedUndef.end());
>>>>       DEBUG(dbgs() << "    ->"; Tmp->dump(););
>>>>       (void)Tmp;
>>>>       SrcVec = DstReg;
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>