Skip to content

Commit

Permalink
Don't override Tensor, Storage macros defined outside torch/csrc in t… (
Browse files Browse the repository at this point in the history
pytorch#8243)

* Don't override Tensor, Storage macros defined outside torch/csrc in torch/csrc.

This PR does the following:
1) Removes THSTensor macros in torch/csrc, which aren't used.
2) For macros defined outside of torch/csrc (THTensor, THTensor_, THStorage, THStorage_):
a) No longer override them, i.e. previously THTensor could actually be THCTensor if a generic file was included from a file including THCP.h.
b) Instead, introduce new macros THW* (e.g. THWTensor) to represent a (potentially empty) wildcard character.

In addition to making this code easier to read and codemod, this allows us to more freely change TH/THC; for example:
currently in the THC random code, the state is casted to THByteTensor*; this happens to work because the macros don't happen to override THByteTensor.
But if THByteTensor just becomes an alias of THTensor (which is the plan for a single tensor type), then this no longer works.
The whole thing is a bit of a mess previously because you really have to understand which macros and redefined and which aren't.

We could also rename the macros that live in torch/csrc (e.g. the THPTensor macros), but since that is more self contained, I punted for now.

* Don't change the plugin.
  • Loading branch information
gchanan authored Jun 7, 2018
1 parent a466c12 commit 93a9bb9
Show file tree
Hide file tree
Showing 16 changed files with 159 additions and 177 deletions.
5 changes: 5 additions & 0 deletions torch/csrc/THP.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
#define LIBRARY_STATE_TYPE
#define LIBRARY_STATE_TYPE_NOARGS

#define THWStorage THStorage
#define THWStorage_(NAME) THStorage_(NAME)
#define THWTensor THTensor
#define THWTensor_(NAME) THTensor_(NAME)

#include "PtrWrapper.h"
#include "Exceptions.h"
#include "Generator.h"
Expand Down
2 changes: 1 addition & 1 deletion torch/csrc/cuda/Storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <THC/THCGenerateAllTypes.h>

template<>
void THPPointer<THStorage>::free() {
void THPPointer<THCStorage>::free() {
if (ptr)
THCStorage_free(LIBRARY_STATE ptr);
}
15 changes: 6 additions & 9 deletions torch/csrc/cuda/override_macros.h
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#include "undef_macros.h"

#define THStoragePtr THCStoragePtr
#define THWStoragePtr THCStoragePtr
#define THPStoragePtr THCPStoragePtr
#define THTensorPtr THCTensorPtr
#define THWTensorPtr THCTensorPtr
#define THPTensorPtr THCPTensorPtr

#define THStorage THCStorage
#define THStorage_(NAME) THCStorage_(NAME)
#define THTensor THCTensor
#define THTensor_(NAME) THCTensor_(NAME)
#define THWStorage THCStorage
#define THWStorage_(NAME) THCStorage_(NAME)
#define THWTensor THCTensor
#define THWTensor_(NAME) THCTensor_(NAME)

#define THPStorage_(NAME) TH_CONCAT_4(THCP,Real,Storage_,NAME)
#define THPStorage THCPStorage
Expand All @@ -29,10 +29,7 @@
#define THPTensorStateless THCPTensorStateless


#define THSTensorPtr THCSTensorPtr
#define THSPTensorPtr THCSPTensorPtr
#define THSTensor THCSTensor
#define THSTensor_(NAME) THCSTensor_(NAME)

#define THSPTensor_(NAME) TH_CONCAT_4(THCSP,Real,Tensor_,NAME)
#define THSPTensor_stateless_(NAME) TH_CONCAT_4(THCSP,Real,Tensor_stateless_,NAME)
Expand Down
8 changes: 4 additions & 4 deletions torch/csrc/cuda/restore_macros.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

#define THTensor TH_CONCAT_3(TH,Real,Tensor)
#define THTensor_(NAME) TH_CONCAT_4(TH,Real,Tensor_,NAME)
#define THWTensor TH_CONCAT_3(TH,Real,Tensor)
#define THWTensor_(NAME) TH_CONCAT_4(TH,Real,Tensor_,NAME)

#define THPTensor TH_CONCAT_3(THP,Real,Tensor)
#define THPTensorStr TH_CONCAT_STRING_3(torch.,Real,Tensor)
Expand All @@ -13,8 +13,8 @@
#define THPStorage_(NAME) TH_CONCAT_4(THP,Real,Storage_,NAME)

#ifdef _THP_CORE
#define THStoragePtr TH_CONCAT_3(TH,Real,StoragePtr)
#define THTensorPtr TH_CONCAT_3(TH,Real,TensorPtr)
#define THWStoragePtr TH_CONCAT_3(TH,Real,StoragePtr)
#define THWTensorPtr TH_CONCAT_3(TH,Real,TensorPtr)
#define THPStoragePtr TH_CONCAT_3(THP,Real,StoragePtr)
#define THPTensorPtr TH_CONCAT_3(THP,Real,TensorPtr)
#endif
15 changes: 6 additions & 9 deletions torch/csrc/cuda/undef_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
#undef THPStorageClass
#undef THPStorageType

#undef THStorage
#undef THStorage_
#undef THTensor
#undef THTensor_
#undef THWStorage
#undef THWStorage_
#undef THWTensor
#undef THWTensor_

#undef THStoragePtr
#undef THWStoragePtr
#undef THPStoragePtr
#undef THTensorPtr
#undef THWTensorPtr
#undef THPTensorPtr


Expand All @@ -44,9 +44,6 @@
#undef THSPTensorStateless
#undef THSPTensorType

#undef THSTensor
#undef THSTensor_
#undef THSTensorPtr
#undef THSPTensorPtr


Expand Down
12 changes: 6 additions & 6 deletions torch/csrc/distributed/override_macros.h
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
#include "undef_macros.h"

#define THStoragePtr THDStoragePtr
#define THWStoragePtr THDStoragePtr
#define THPStoragePtr THDPStoragePtr
#define THTensorPtr THDTensorPtr
#define THWTensorPtr THDTensorPtr
#define THPTensorPtr THDPTensorPtr

#define THStorage THDStorage
#define THStorage_(NAME) THDStorage_(NAME)
#define THTensor THDTensor
#define THTensor_(NAME) THDTensor_(NAME)
#define THWStorage THDStorage
#define THWStorage_(NAME) THDStorage_(NAME)
#define THWTensor THDTensor
#define THWTensor_(NAME) THDTensor_(NAME)

#define THPStorage_(NAME) TH_CONCAT_4(THDP,Real,Storage_,NAME)
#define THPStorage THDPStorage
Expand Down
12 changes: 6 additions & 6 deletions torch/csrc/distributed/undef_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
#undef THPStorageClass
#undef THPStorageType

#undef THStorage
#undef THStorage_
#undef THTensor
#undef THTensor_
#undef THWStorage
#undef THWStorage_
#undef THWTensor
#undef THWTensor_

#undef THStoragePtr
#undef THWStoragePtr
#undef THPStoragePtr
#undef THTensorPtr
#undef THWTensorPtr
#undef THPTensorPtr

#undef THHostTensor
Expand Down
82 changes: 41 additions & 41 deletions torch/csrc/generic/Storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,32 @@

PyObject *THPStorageClass = NULL;

PyObject * THPStorage_(New)(THStorage *ptr)
PyObject * THPStorage_(New)(THWStorage *ptr)
{
TORCH_ASSERT(ptr);
PyTypeObject *type = (PyTypeObject *)THPStorageClass;
PyObject *obj = type->tp_alloc(type, 0);
if (obj) {
((THPStorage *)obj)->cdata = ptr;
} else {
THStorage_(free)(LIBRARY_STATE ptr);
THWStorage_(free)(LIBRARY_STATE ptr);
}
return obj;
}

static void THPStorage_(dealloc)(THPStorage* self)
{
THStorage_(free)(LIBRARY_STATE self->cdata);
THWStorage_(free)(LIBRARY_STATE self->cdata);
Py_TYPE(self)->tp_free((PyObject*)self);
}

static THStorage* THPStorage_(newWithAllocator)(int64_t size, THAllocator* allocator)
static THWStorage* THPStorage_(newWithAllocator)(int64_t size, THAllocator* allocator)
{
#if defined(THC_GENERIC_FILE) || defined(THD_GENERIC_FILE)
THPUtils_setError(THPStorageStr " does not support custom allocators");
return NULL;
#else
return THStorage_(newWithAllocator)(LIBRARY_STATE size, allocator, NULL);
return THWStorage_(newWithAllocator)(LIBRARY_STATE size, allocator, NULL);
#endif
}

Expand All @@ -55,7 +55,7 @@ static PyObject * THPStorage_(pynew)(PyTypeObject *type, PyObject *args, PyObjec
if (num_args == 0) {
PyObject *cdata_ptr = PyDict_GetItemString(kwargs, "cdata");
if (num_kwargs == 1 && cdata_ptr && THPUtils_checkLong(cdata_ptr)) {
THStorage *ptr = (THStorage*)PyLong_AsVoidPtr(cdata_ptr);
THWStorage *ptr = (THWStorage*)PyLong_AsVoidPtr(cdata_ptr);
self->cdata = ptr;
return (PyObject*)self.release();
}
Expand All @@ -68,7 +68,7 @@ static PyObject * THPStorage_(pynew)(PyTypeObject *type, PyObject *args, PyObjec
if (allocator) {
self->cdata = THPStorage_(newWithAllocator)(0, allocator);
} else {
self->cdata = THStorage_(new)(LIBRARY_STATE_NOARGS);
self->cdata = THWStorage_(new)(LIBRARY_STATE_NOARGS);
}
return (PyObject*)self.release();
}
Expand All @@ -81,7 +81,7 @@ static PyObject * THPStorage_(pynew)(PyTypeObject *type, PyObject *args, PyObjec
if (allocator) {
self->cdata = THPStorage_(newWithAllocator)(size, allocator);
} else {
self->cdata = THStorage_(newWithSize)(LIBRARY_STATE size);
self->cdata = THWStorage_(newWithSize)(LIBRARY_STATE size);
}
return (PyObject*)self.release();
}
Expand Down Expand Up @@ -117,11 +117,11 @@ static PyObject * THPStorage_(pynew)(PyTypeObject *type, PyObject *args, PyObjec
"%" PRId64 ", but the viewed storage has only %" PRId64 " element(s) after offset %" PRId64,
size, numel - offset, offset);

real *data_ptr = THStorage_(data)(LIBRARY_STATE storage_arg->cdata) + offset;
THStoragePtr storage(THStorage_(newWithData)(LIBRARY_STATE data_ptr, size));
real *data_ptr = THWStorage_(data)(LIBRARY_STATE storage_arg->cdata) + offset;
THWStoragePtr storage(THWStorage_(newWithData)(LIBRARY_STATE data_ptr, size));
storage->flag = TH_STORAGE_REFCOUNTED | TH_STORAGE_VIEW;
storage->view = storage_arg->cdata;
THStorage_(retain)(LIBRARY_STATE storage_arg->cdata);
THWStorage_(retain)(LIBRARY_STATE storage_arg->cdata);
self->cdata = storage.release();
return (PyObject*)self.release();
#endif
Expand All @@ -135,7 +135,7 @@ static PyObject * THPStorage_(pynew)(PyTypeObject *type, PyObject *args, PyObjec
Py_ssize_t length = PySequence_Length(first_arg);
THPUtils_assert(length >= 0, "couldn't obtain the length of %s",
THPUtils_typename(first_arg));
self->cdata = THStorage_(newWithSize)(LIBRARY_STATE length);
self->cdata = THWStorage_(newWithSize)(LIBRARY_STATE length);
THPObjectPtr item;
try {
for (Py_ssize_t i = 0; i < length; i++) {
Expand Down Expand Up @@ -177,7 +177,7 @@ static PyObject * THPStorage_(pynew)(PyTypeObject *type, PyObject *args, PyObjec
static Py_ssize_t THPStorage_(length)(THPStorage *self)
{
HANDLE_TH_ERRORS
return THStorage_(size)(LIBRARY_STATE self->cdata);
return THWStorage_(size)(LIBRARY_STATE self->cdata);
END_HANDLE_TH_ERRORS_RET(-1)
}

Expand All @@ -188,13 +188,13 @@ static PyObject * THPStorage_(get)(THPStorage *self, PyObject *index)
if (THPUtils_checkLong(index)) {
int64_t nindex = THPUtils_unpackLong(index);
if (nindex < 0)
nindex += THStorage_(size)(LIBRARY_STATE self->cdata);
nindex += THWStorage_(size)(LIBRARY_STATE self->cdata);
if (nindex < 0 || nindex >= self->cdata->size) {
PyErr_Format(PyExc_IndexError, "index %" PRId64 " out of range for storage of "
"size %" PRId64, (int64_t) nindex, (int64_t) self->cdata->size);
return NULL;
}
real value = THStorage_(get)(LIBRARY_STATE self->cdata, nindex);
real value = THWStorage_(get)(LIBRARY_STATE self->cdata, nindex);
return THPUtils_(newReal)(value);
/* Slice index */
} else if (PySlice_Check(index)) {
Expand All @@ -203,7 +203,7 @@ static PyObject * THPStorage_(get)(THPStorage *self, PyObject *index)
return NULL;
#else
Py_ssize_t start, stop, slicelength, step;
int64_t len = THStorage_(size)(LIBRARY_STATE self->cdata);
int64_t len = THWStorage_(size)(LIBRARY_STATE self->cdata);
if (!THPUtils_parseSlice(index, len, &start, &stop, &step, &slicelength))
return NULL;
if (step != 1) {
Expand All @@ -212,11 +212,11 @@ static PyObject * THPStorage_(get)(THPStorage *self, PyObject *index)
return NULL;
}

real *data = THStorage_(data)(LIBRARY_STATE self->cdata);
THStoragePtr new_storage(THStorage_(newWithData)(LIBRARY_STATE data + start, slicelength));
real *data = THWStorage_(data)(LIBRARY_STATE self->cdata);
THWStoragePtr new_storage(THWStorage_(newWithData)(LIBRARY_STATE data + start, slicelength));
new_storage->flag = TH_STORAGE_REFCOUNTED | TH_STORAGE_VIEW;
new_storage->view = self->cdata;
THStorage_(retain)(LIBRARY_STATE self->cdata);
THWStorage_(retain)(LIBRARY_STATE self->cdata);

PyObject *_ret = THPStorage_(New)(new_storage);
new_storage.release();
Expand All @@ -242,11 +242,11 @@ static int THPStorage_(set)(THPStorage *self, PyObject *index, PyObject *value)
real rvalue = THPUtils_(unpackReal)(value);
if (THPUtils_checkLong(index)) {
int64_t nindex = THPUtils_unpackLong(index);
THStorage_(set)(LIBRARY_STATE self->cdata, nindex, rvalue);
THWStorage_(set)(LIBRARY_STATE self->cdata, nindex, rvalue);
return 0;
} else if (PySlice_Check(index)) {
Py_ssize_t start, stop, slicelength, step;
int64_t len = THStorage_(size)(LIBRARY_STATE self->cdata);
int64_t len = THWStorage_(size)(LIBRARY_STATE self->cdata);
if (!THPUtils_parseSlice(index, len, &start, &stop, &step, &slicelength))
return -1;
if (step != 1) {
Expand All @@ -257,7 +257,7 @@ static int THPStorage_(set)(THPStorage *self, PyObject *index, PyObject *value)
// TODO: check the bounds only once
// TODO: fill?
for (;start < stop; start++)
THStorage_(set)(LIBRARY_STATE self->cdata, start, rvalue);
THWStorage_(set)(LIBRARY_STATE self->cdata, start, rvalue);
return 0;
}
THPUtils_setError("can't index a " THPStorageStr " with %s",
Expand Down Expand Up @@ -319,33 +319,33 @@ static struct PyMemberDef THPStorage_(members)[] = {
{NULL}
};

extern THPCopyList THStorage_(copy_functions);
THPCopyList THStorage_(copy_functions);
extern THPCopyList THWStorage_(copy_functions);
THPCopyList THWStorage_(copy_functions);

void THPStorage_(initCopyMethods)()
{
#ifndef THD_GENERIC_FILE
auto& h = THStorage_(copy_functions);
auto& h = THWStorage_(copy_functions);
// copy from CPU types
THPInsertStorageCopyFunction<THPStorage, THPByteStorage>(&THPByteStorageType, h, &THStorage_(copyByte));
THPInsertStorageCopyFunction<THPStorage, THPCharStorage>(&THPCharStorageType, h, &THStorage_(copyChar));
THPInsertStorageCopyFunction<THPStorage, THPShortStorage>(&THPShortStorageType, h, &THStorage_(copyShort));
THPInsertStorageCopyFunction<THPStorage, THPIntStorage>(&THPIntStorageType, h, &THStorage_(copyInt));
THPInsertStorageCopyFunction<THPStorage, THPLongStorage>(&THPLongStorageType, h, &THStorage_(copyLong));
THPInsertStorageCopyFunction<THPStorage, THPHalfStorage>(&THPHalfStorageType, h, &THStorage_(copyHalf));
THPInsertStorageCopyFunction<THPStorage, THPFloatStorage>(&THPFloatStorageType, h, &THStorage_(copyFloat));
THPInsertStorageCopyFunction<THPStorage, THPDoubleStorage>(&THPDoubleStorageType, h, &THStorage_(copyDouble));
THPInsertStorageCopyFunction<THPStorage, THPByteStorage>(&THPByteStorageType, h, &THWStorage_(copyByte));
THPInsertStorageCopyFunction<THPStorage, THPCharStorage>(&THPCharStorageType, h, &THWStorage_(copyChar));
THPInsertStorageCopyFunction<THPStorage, THPShortStorage>(&THPShortStorageType, h, &THWStorage_(copyShort));
THPInsertStorageCopyFunction<THPStorage, THPIntStorage>(&THPIntStorageType, h, &THWStorage_(copyInt));
THPInsertStorageCopyFunction<THPStorage, THPLongStorage>(&THPLongStorageType, h, &THWStorage_(copyLong));
THPInsertStorageCopyFunction<THPStorage, THPHalfStorage>(&THPHalfStorageType, h, &THWStorage_(copyHalf));
THPInsertStorageCopyFunction<THPStorage, THPFloatStorage>(&THPFloatStorageType, h, &THWStorage_(copyFloat));
THPInsertStorageCopyFunction<THPStorage, THPDoubleStorage>(&THPDoubleStorageType, h, &THWStorage_(copyDouble));
#ifdef THC_GENERIC_FILE
// copy from GPU types
THPInsertStorageCopyFunction<THPStorage, THCPByteStorage>(&THCPByteStorageType, h, &THStorage_(copyCudaByte));
THPInsertStorageCopyFunction<THPStorage, THCPCharStorage>(&THCPCharStorageType, h, &THStorage_(copyCudaChar));
THPInsertStorageCopyFunction<THPStorage, THCPShortStorage>(&THCPShortStorageType, h, &THStorage_(copyCudaShort));
THPInsertStorageCopyFunction<THPStorage, THCPIntStorage>(&THCPIntStorageType, h, &THStorage_(copyCudaInt));
THPInsertStorageCopyFunction<THPStorage, THCPLongStorage>(&THCPLongStorageType, h, &THStorage_(copyCudaLong));
THPInsertStorageCopyFunction<THPStorage, THCPFloatStorage>(&THCPFloatStorageType, h, &THStorage_(copyCudaFloat));
THPInsertStorageCopyFunction<THPStorage, THCPDoubleStorage>(&THCPDoubleStorageType, h, &THStorage_(copyCudaDouble));
THPInsertStorageCopyFunction<THPStorage, THCPByteStorage>(&THCPByteStorageType, h, &THWStorage_(copyCudaByte));
THPInsertStorageCopyFunction<THPStorage, THCPCharStorage>(&THCPCharStorageType, h, &THWStorage_(copyCudaChar));
THPInsertStorageCopyFunction<THPStorage, THCPShortStorage>(&THCPShortStorageType, h, &THWStorage_(copyCudaShort));
THPInsertStorageCopyFunction<THPStorage, THCPIntStorage>(&THCPIntStorageType, h, &THWStorage_(copyCudaInt));
THPInsertStorageCopyFunction<THPStorage, THCPLongStorage>(&THCPLongStorageType, h, &THWStorage_(copyCudaLong));
THPInsertStorageCopyFunction<THPStorage, THCPFloatStorage>(&THCPFloatStorageType, h, &THWStorage_(copyCudaFloat));
THPInsertStorageCopyFunction<THPStorage, THCPDoubleStorage>(&THCPDoubleStorageType, h, &THWStorage_(copyCudaDouble));
#ifdef CUDA_HALF_TENSOR
THPInsertStorageCopyFunction<THPStorage, THCPHalfStorage>(&THCPHalfStorageType, h, &THStorage_(copyCudaHalf));
THPInsertStorageCopyFunction<THPStorage, THCPHalfStorage>(&THCPHalfStorageType, h, &THWStorage_(copyCudaHalf));
#endif
// add CPU <- GPU copies to base type
#define THPCpuStorage TH_CONCAT_3(THP, Real, Storage)
Expand Down
4 changes: 2 additions & 2 deletions torch/csrc/generic/Storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

struct THPStorage {
PyObject_HEAD
THStorage *cdata;
THWStorage *cdata;
};

THP_API PyObject * THPStorage_(New)(THStorage *ptr);
THP_API PyObject * THPStorage_(New)(THWStorage *ptr);
extern PyObject *THPStorageClass;

#ifdef _THP_CORE
Expand Down
Loading

0 comments on commit 93a9bb9

Please sign in to comment.