[llvm] b7448a3 - [Attributor][NFC] Use indexes instead of iterator

[Luofan Chen via llvm-commits]

Oh I missed that. I will fix the issues you mentioned.

Really sorry about that!

On 8/15/20 11:25 PM, Florian Hahn wrote:
>
>> On Aug 15, 2020, at 16:12, Luofan Chen via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>>
>> Author: Luofan Chen
>> Date: 2020-08-15T23:09:46+08:00
>> New Revision: b7448a348bb863365b3d4652010a29efedd8f2e7
>>
>> URL: https://github.com/llvm/llvm-project/commit/b7448a348bb863365b3d4652010a29efedd8f2e7
>> DIFF: https://github.com/llvm/llvm-project/commit/b7448a348bb863365b3d4652010a29efedd8f2e7.diff
>>
>> LOG: [Attributor][NFC] Use indexes instead of iterator
>>
>> When adding elements when iterating, the iterator will become
>> valid, which could cause errors. This fixes the issue by using
>> indexes instead of iterator.
>>
>> Added:
>>
>>
>> Modified:
>>     llvm/lib/Transforms/IPO/Attributor.cpp
>>
>> Removed:
>>
>>
>>
>> ################################################################################
>> diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
>> index 6599ff6d6246..88e06558d652 100644
>> --- a/llvm/lib/Transforms/IPO/Attributor.cpp
>> +++ b/llvm/lib/Transforms/IPO/Attributor.cpp
>> @@ -2204,8 +2204,9 @@ static bool runAttributorOnFunctions(InformationCache &InfoCache,
>>    // TODO: for now we eagerly internalize functions without calculating the
>>    //       cost, we need a cost interface to determine whether internalizing
>>    //       a function is "benefitial"
>> -  if (AllowDeepWrapper) {
>> -    for (Function *F : Functions)
>> +  if (AllowDeepWrapper)
>> +    for (unsigned u = 0; u < Functions.size(); u ++) {
> The space between `u ++` seems odd. Did you clang-format that?
>
>> +      Function *F = Functions[u];
>>        if (!F->isDeclaration() && !F->isDefinitionExact() && F->getNumUses() &&
>>            !GlobalValue::isInterposableLinkage(F->getLinkage())) {
>>          Function *NewF = internalizeFunction(*F);
>> @@ -2219,7 +2220,7 @@ static bool runAttributorOnFunctions(InformationCache &InfoCache,
>>              CGUpdater.reanalyzeFunction(*CallerF);
>>            }
>>        }
>> -  }
>> +    }
>
> It looks like this will also visit the newly inserted functions. Is that what you want? Wouldn’t it be enough to compute the size up-front and only check the functions that are in Functions initially?