[llvm] r216364 - NVPTX: remove another raw delete call

[David Blaikie]

On Mon, Aug 25, 2014 at 10:32 PM, Dylan Noblesmith
<nobled at dreamwidth.org> wrote:
> On Mon, Aug 25, 2014 at 12:33 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Sun, Aug 24, 2014 at 6:59 PM, Dylan Noblesmith <nobled at dreamwidth.org> wrote:
>>> Author: nobled
>>> Date: Sun Aug 24 20:59:32 2014
>>> New Revision: 216364
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=216364&view=rev
>>> Log:
>>> NVPTX: remove another raw delete call
>>>
>>> Modified:
>>>     llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
>>>
>>> Modified: llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.cpp?rev=216364&r1=216363&r2=216364&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.cpp (original)
>>> +++ llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.cpp Sun Aug 24 20:59:32 2014
>>> @@ -1063,7 +1063,7 @@ bool NVPTXAsmPrinter::doFinalization(Mod
>>>
>>>    Module::GlobalListType &global_list = M.getGlobalList();
>>>    int i, n = global_list.size();
>>> -  GlobalVariable **gv_array = new GlobalVariable *[n];
>>> +  std::vector<GlobalVariable *> gv_array(n);
>>
>> Another case of vector where you were favoring unique_ptr<T[]> in
>> other similar cases - in fact this case seems an even easier/better
>> case for unique_ptr<T[]> because you don't need the raw pointer at
>> all. Any particular reason to favor vector here?
> I realized std::vector made more sense in some cases, and didn't amend
> all the patches, oops. Sorry 'bout the noise.

That's OK - just trying to understand the decision/differences as I
couldn't spot it myself.

> I figure, for temporary arrays like this one, use std::vector. For
> member variables, use unique_ptr<T[]> if the pointer is ever supposed
> to be null, and vector otherwise.

As Chandler pointed out on some of these, the difference in changin
raw new T[] to std::vector is that it causes the sequence to be
default initialized which could be a performance problem. So any time
you're changing to std::vector, you'll need to be a bit more careful
and explain in the patch/review why it's the right choice. The change
to unique_ptr<T[]> is more mechanical/simpler/less error-prone, I
believe.

> For return values (like of
> collectCoeffInfo), I'm not sure, though. Leaning towards unique_ptr?

Probably - it's at least the safe bet.

>
>
>>>
>>>    // first, back-up GlobalVariable in gv_array
>>>    i = 0;
>>> @@ -1083,8 +1083,6 @@ bool NVPTXAsmPrinter::doFinalization(Mod
>>>      global_list.insert(global_list.end(), gv_array[i]);
>>>
>>>    clearAnnotationCache(&M);
>>> -
>>> -  delete[] gv_array;
>>>    return ret;
>>>
>>>    //bool Result = AsmPrinter::doFinalization(M);
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>