[llvm] r216363 - NVPTX: remove raw delete call

[David Blaikie]

On Sun, Aug 24, 2014 at 6:59 PM, Dylan Noblesmith <nobled at dreamwidth.org> wrote:
> Author: nobled
> Date: Sun Aug 24 20:59:29 2014
> New Revision: 216363
>
> URL: http://llvm.org/viewvc/llvm-project?rev=216363&view=rev
> Log:
> NVPTX: remove raw delete call
>
> Also make members that are never accessed outside the class
> private.
>
> Modified:
>     llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.h
>
> Modified: llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.h?rev=216363&r1=216362&r2=216363&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.h (original)
> +++ llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.h Sun Aug 24 20:59:29 2014
> @@ -86,13 +86,13 @@ class LLVM_LIBRARY_VISIBILITY NVPTXAsmPr
>      // Once we have this AggBuffer setup, we can choose how to print
>      // it out.
>    public:
> -    unsigned size;         // size of the buffer in bytes
> -    unsigned char *buffer; // the buffer
>      unsigned numSymbols;   // number of symbol addresses
> -    SmallVector<unsigned, 4> symbolPosInBuffer;
> -    SmallVector<const Value *, 4> Symbols;
>
>    private:

The change from public to private makes this patch a bit harder to
read (since the existing SmallVectors show up as -/+ lines when they
were just moved) - might've been better in a separate patch.

> +    const unsigned size;   // size of the buffer in bytes

Could you just use buffer.size() instead?

> +    std::vector<unsigned char> buffer; // the buffer

While I've been generally advocating in the other patch reviews for
vector/SmallVector over unique_ptr<T[]> - in this and one other patch
you chose vector over unique_ptr<T[]> & I'm not sure why (it seems
inconsistent with your other patches, but I assume I'm missing some
detail that makes unique_ptr<T[]> less suitable here?).

(the comment "// the buffer" is pretty hollow & should probably be removed)

> +    SmallVector<unsigned, 4> symbolPosInBuffer;
> +    SmallVector<const Value *, 4> Symbols;
>      unsigned curpos;
>      raw_ostream &O;
>      NVPTXAsmPrinter &AP;
> @@ -100,14 +100,11 @@ class LLVM_LIBRARY_VISIBILITY NVPTXAsmPr
>
>    public:
>      AggBuffer(unsigned _size, raw_ostream &_O, NVPTXAsmPrinter &_AP)
> -        : O(_O), AP(_AP) {
> -      buffer = new unsigned char[_size];
> -      size = _size;
> +        : size(_size), buffer(_size), O(_O), AP(_AP) {
>        curpos = 0;
>        numSymbols = 0;
>        EmitGeneric = AP.EmitGeneric;
>      }
> -    ~AggBuffer() { delete[] buffer; }
>      unsigned addBytes(unsigned char *Ptr, int Num, int Bytes) {
>        assert((curpos + Num) <= size);
>        assert((curpos + Bytes) <= size);
> @@ -179,9 +176,9 @@ class LLVM_LIBRARY_VISIBILITY NVPTXAsmPr
>              else
>                nextSymbolPos = symbolPosInBuffer[nSym];
>            } else if (nBytes == 4)
> -            O << *(unsigned int *)(buffer + pos);
> +            O << *(unsigned int *)(&buffer[pos]);
>            else
> -            O << *(unsigned long long *)(buffer + pos);
> +            O << *(unsigned long long *)(&buffer[pos]);
>          }
>        }
>      }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits