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

[Roman Lebedev via llvm-commits]

Also, test missing?

On Sat, Aug 15, 2020 at 6:58 PM Luofan Chen via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> 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?
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits