[llvm-commits] [llvm] r141665 - /llvm/trunk/lib/Target/CellSPU/SPUInstrInfo.cpp

[David Blaikie]

On Tue, Oct 11, 2011 at 8:08 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Oct 11, 2011 at 5:55 AM, Kalle Raiskila <kalle.raiskila at nokia.com>wrote:
>
>> Fix a iterator out of bounds error, that triggers rarely.
>>
>>  for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
>>
>>     if (I->getOpcode() == SPU::HBRA ||
>>         I->getOpcode() == SPU::HBR_LABEL){
>>       I=MBB.erase(I);
>> +      if (I == MBB.end())
>> +        break;
>>     }
>>   }
>>  }
>>
>
> Is it OK that this algorithm skips any element just after the one it
> removed? Or is it possible that two elements that satisfy the removal
> condition could be adjacent & should both be removed?
>
> The way to make this work if that's how it should behave is:
>
> for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ) {
>   if (...)
>     I = MBB.erase(I);
>   else
>     ++I;
> }
>

Oh, and one other thing - this is an O(N^2) algorithm for sequential
containers like std::vector - the erase/remove idiom is technically the
right way to go but a bit more work to implement (& may or may not be worth
it, depending on the number of items in the container & the number expected
to be removed, etc). The easiest way is to write your predicate as a
function (which is llvm's recommended coding style anyway) and then use:
MBB.erase(std::remove_if(MBB.begin(), MBB.end(), pred), MBB.end());

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111011/8f09b614/attachment.html>