[llvm] ef89409 - Fix 'unused-lambda-capture' gcc warning. NFCI.

[David Blaikie via llvm-commits]

On Mon, Oct 17, 2022 at 5:46 PM Alexey Bataev <a.bataev at hotmail.com> wrote:
>
>
>
> Best regards,
> Alexey Bataev
>
> > 17 окт. 2022 г., в 8:36 PM, David Blaikie <dblaikie at gmail.com> написал(а):
> >
> > On Mon, Oct 17, 2022 at 4:58 PM Alexey Bataev <a.bataev at hotmail.com> wrote:
> >>
> >>
> >>
> >> Best regards,
> >> Alexey Bataev
> >>
> >>>> 17 окт. 2022 г., в 7:26 PM, David Blaikie <dblaikie at gmail.com> написал(а):
> >>>
> >>> ping on this
> >>>
> >>>> On Mon, Oct 3, 2022 at 12:44 PM David Blaikie <dblaikie at gmail.com> wrote:
> >>>>
> >>>>> On Tue, Sep 27, 2022 at 7:16 AM Simon Pilgrim via llvm-commits
> >>>>> <llvm-commits at lists.llvm.org> wrote:
> >>>>>
> >>>>>
> >>>>> Author: Simon Pilgrim
> >>>>> Date: 2022-09-27T15:15:43+01:00
> >>>>> New Revision: ef89409a59f3b79ae143b33b7d8e6ee6285aa42f
> >>>>>
> >>>>> URL: https://github.com/llvm/llvm-project/commit/ef89409a59f3b79ae143b33b7d8e6ee6285aa42f
> >>>>> DIFF: https://github.com/llvm/llvm-project/commit/ef89409a59f3b79ae143b33b7d8e6ee6285aa42f.diff
> >>>>>
> >>>>> LOG: Fix 'unused-lambda-capture' gcc warning. NFCI.
> >>>>>
> >>>>> Added:
> >>>>>
> >>>>>
> >>>>> Modified:
> >>>>>   llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> >>>>>
> >>>>> Removed:
> >>>>>
> >>>>>
> >>>>>
> >>>>> ################################################################################
> >>>>> diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> >>>>> index b5c73819335e..9584c421bcca 100644
> >>>>> --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> >>>>> +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> >>>>> @@ -6075,7 +6075,7 @@ InstructionCost BoUpSLP::getEntryCost(const TreeEntry *E,
> >>>>>  bool NeedToShuffleReuses = !E->ReuseShuffleIndices.empty();
> >>>>>  // FIXME: it tries to fix a problem with MSVC buildbots.
> >>>>>  TargetTransformInfo &TTIRef = *TTI;
> >>>>> -  auto &&AdjustExtractsCost = [this, &TTIRef, CostKind, VL, VecTy,
> >>>>> +  auto &&AdjustExtractsCost = [this, &TTIRef, CostKind, VL,
> >>>>>                               VectorizedVals, E](InstructionCost &Cost) {
> >>>>
> >>>> Generally if a lambda's only going to be used in the current scope,
> >>>> I'd suggest using default capture "[&]" so the list doesn't need to be
> >>>> maintained.
> >>
> >> Thera are no strict rules about it, I prefer to keep explicit list where possible to be sure what's used or can be dropped.
> >
> > It's a bit of a maintenance burden, though - in the same way that we
> > don't enumerate what variables are used in a while loop or other scope
> > - I think it's suitable for local lambdas not to enumerate them so
> > it's easier to update/modify the code without having to maintain the
> > list.
> >
> >>>> Also rvalue reference lifetime extension's probably not helpful here,
> >>>> and just plain `auto AdjustExtractsCost` could be used.
> >>
> >> It saves us from one move constructor call in AST (not in codegen, it is elided then).
> >
> > Yeah, nothing to save there - as you say, it doesn't get codegen'd
> > that way. Using `auto` non-reference is used in almost all cases - be
> > nice to change this for consistency, so it doesn't raise questions of
> > "why is this different? is there something importantly odd/unique
> > going on here?"
> >
>
> Some time before I was asked to do different thing.
> Actually we save some memory on this.

Got a link? Not sure how we'd save memory here - Generally I'd expect
we'd use memory if we explicitly enumerate captures compared to
default ref capture. (oh, I guess if the objects are smaller than a
pointer, then capture by value could save memory) - but I really don't
think it's worthwhile for a locally scoped lambda in terms of
maintenance costs when modifying code - unless there's some
particularly compelling use case/data in specific uses that shows it
needs to be further tuned/hand-optimized like that.

- Dave

>
> > - Dave
> >
> >>>>>    ScalarizationOverheadBuilder ScalarizationCost;
> >>>>>    SmallPtrSet<Value *, 4> CheckedExtracts;
> >>>>>
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> llvm-commits mailing list
> >>>>> llvm-commits at lists.llvm.org
> >>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits