[llvm] r216358 - Analysis: unique_ptr-ify DependenceAnalysis::collectCoeffInfo

[David Blaikie]

On further reflection - I'd consider using std::vector or SmallVector
here, even though these sequences do not change after construction
(with SmallVector we gain the benefit of the small optimization here,
with std::vector we at least gain a somewhat more common/less raw API
- even if the size is known to this code (& would still have issues
with the argument passing - unless we needlessly changed the argument
type to ArrayRef, knowing that the size didn't need to be passed but
doing so anyway - perhaps asserting the constraint that the size
matches the expectation inside the function)... not necessarily wedded
to the idea, though.

On Mon, Aug 25, 2014 at 9:14 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Sun, Aug 24, 2014 at 5:28 PM, Dylan Noblesmith <nobled at dreamwidth.org> wrote:
>> Author: nobled
>> Date: Sun Aug 24 19:28:43 2014
>> New Revision: 216358
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=216358&view=rev
>> Log:
>> Analysis: unique_ptr-ify DependenceAnalysis::collectCoeffInfo
>>
>> Modified:
>>     llvm/trunk/include/llvm/Analysis/DependenceAnalysis.h
>>     llvm/trunk/lib/Analysis/DependenceAnalysis.cpp
>>
>> Modified: llvm/trunk/include/llvm/Analysis/DependenceAnalysis.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/DependenceAnalysis.h?rev=216358&r1=216357&r2=216358&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/DependenceAnalysis.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/DependenceAnalysis.h Sun Aug 24 19:28:43 2014
>> @@ -766,9 +766,10 @@ namespace llvm {
>>      /// collectCoefficientInfo - Walks through the subscript,
>>      /// collecting each coefficient, the associated loop bounds,
>>      /// and recording its positive and negative parts for later use.
>> -    CoefficientInfo *collectCoeffInfo(const SCEV *Subscript,
>> -                                      bool SrcFlag,
>> -                                      const SCEV *&Constant) const;
>> +    std::unique_ptr<CoefficientInfo[]>
>> +    collectCoeffInfo(const SCEV *Subscript,
>> +                     bool SrcFlag,
>> +                     const SCEV *&Constant) const;
>>
>>      /// getPositivePart - X^+ = max(X, 0).
>>      ///
>>
>> Modified: llvm/trunk/lib/Analysis/DependenceAnalysis.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/DependenceAnalysis.cpp?rev=216358&r1=216357&r2=216358&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/DependenceAnalysis.cpp (original)
>> +++ llvm/trunk/lib/Analysis/DependenceAnalysis.cpp Sun Aug 24 19:28:43 2014
>> @@ -2437,11 +2437,14 @@ bool DependenceAnalysis::banerjeeMIVtest
>>    ++BanerjeeApplications;
>>    DEBUG(dbgs() << "    Src = " << *Src << '\n');
>>    const SCEV *A0;
>> -  CoefficientInfo *A = collectCoeffInfo(Src, true, A0);
>> +  auto AOwner = collectCoeffInfo(Src, true, A0);
>> +  auto A = AOwner.get();
>
> auto *A, please.
>
>>    DEBUG(dbgs() << "    Dst = " << *Dst << '\n');
>>    const SCEV *B0;
>> -  CoefficientInfo *B = collectCoeffInfo(Dst, false, B0);
>> -  BoundInfo *Bound = new BoundInfo[MaxLevels + 1];
>> +  auto BOwner = collectCoeffInfo(Dst, false, B0);
>> +  auto B = BOwner.get();
>
> Similarly here.
>
>> +  auto BoundOwner = make_unique<BoundInfo[]>(MaxLevels + 1);
>> +  auto Bound = BoundOwner.get();
>
> And here.
>
> (the idea is if auto is deducing cv qualifiers, pointers, etc - put
> those explicitly and don't rely on auto, as they're already fairly
> terse and good documentation - while still allowing the overall type
> to be shrunk/omitted)
>
> Also: I wonder if it'd be nicer just to use B/Bound (and skip the
> "Owner" variables), use unique_ptr<T[]>'s operator[] in most of the
> places (so no code change required for those), and pass foo.get() in
> the few places where you need to pass the pointer (this would be akin
> to having a std::vector and using [] most of the time, but having to
> pass foo.data() when you need to pass the raw buffer sometimes).
>
>>    const SCEV *Delta = SE->getMinusSCEV(B0, A0);
>>    DEBUG(dbgs() << "\tDelta = " << *Delta << '\n');
>>
>> @@ -2498,9 +2501,6 @@ bool DependenceAnalysis::banerjeeMIVtest
>>      ++BanerjeeIndependence;
>>      Disproved = true;
>>    }
>> -  delete [] Bound;
>> -  delete [] A;
>> -  delete [] B;
>>    return Disproved;
>>  }
>>
>> @@ -2818,12 +2818,12 @@ const SCEV *DependenceAnalysis::getNegat
>>  // Walks through the subscript,
>>  // collecting each coefficient, the associated loop bounds,
>>  // and recording its positive and negative parts for later use.
>> -DependenceAnalysis::CoefficientInfo *
>> +std::unique_ptr<DependenceAnalysis::CoefficientInfo[]>
>>  DependenceAnalysis::collectCoeffInfo(const SCEV *Subscript,
>>                                       bool SrcFlag,
>>                                       const SCEV *&Constant) const {
>>    const SCEV *Zero = SE->getConstant(Subscript->getType(), 0);
>> -  CoefficientInfo *CI = new CoefficientInfo[MaxLevels + 1];
>> +  auto CI = make_unique<CoefficientInfo[]>(MaxLevels + 1);
>>    for (unsigned K = 1; K <= MaxLevels; ++K) {
>>      CI[K].Coeff = Zero;
>>      CI[K].PosPart = Zero;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits