Skip to content

Commit

Permalink
**BREAKING** Make memory copies data type aware for consistency. (#728)
Browse files Browse the repository at this point in the history
  • Loading branch information
kris-rowe committed Jan 24, 2024
1 parent 19f17a7 commit 6084f64
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 71 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ jobs:
run: |
source /opt/intel/oneapi/setvars.sh
export ONEAPI_DEVICE_SELECTOR=*:cpu
ctest --test-dir build --progress --output-on-failure --parallel 8 --schedule-random -E "opencl-*|examples_cpp_for_loops-dpcpp|examples_cpp_arrays-dpcpp|examples_cpp_generic_inline_kernel-dpcpp|examples_cpp_nonblocking_streams-dpcpp"
ctest --test-dir build --progress --output-on-failure --parallel 8 --schedule-random -E "opencl-*|dpcpp-*"
- name: Upload code coverage
if: ${{ matrix.OCCA_COVERAGE }}
Expand Down
2 changes: 1 addition & 1 deletion examples/fortran/01_add_vectors/main.f90
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ program main
props)

! Copy memory to the device
call occaCopyPtrToMem(o_a, C_loc(a), entries*C_float, 0_occaUDim_t, occaDefault)
call occaCopyPtrToMem(o_a, C_loc(a), entries, 0_occaUDim_t, occaDefault)
call occaCopyPtrToMem(o_b, C_loc(b), occaAllBytes , 0_occaUDim_t, occaDefault)

! Launch device kernel
Expand Down
2 changes: 1 addition & 1 deletion examples/fortran/03_static_compilation/main.f90
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ program main
props)

! Copy memory to the device
call occaCopyPtrToMem(o_a, C_loc(a), entries*C_float, 0_occaUDim_t, occaDefault)
call occaCopyPtrToMem(o_a, C_loc(a), entries, 0_occaUDim_t, occaDefault)
call occaCopyPtrToMem(o_b, C_loc(b), occaAllBytes , 0_occaUDim_t, occaDefault)

! Launch device kernel
Expand Down
2 changes: 1 addition & 1 deletion examples/fortran/09_streams/main.f90
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ program main
occaDefault)

! Copy memory to the device
call occaCopyPtrToMem(o_a, C_loc(a), entries*C_float, 0_occaUDim_t, occaDefault)
call occaCopyPtrToMem(o_a, C_loc(a), entries, 0_occaUDim_t, occaDefault)
call occaCopyPtrToMem(o_b, C_loc(b), occaAllBytes , 0_occaUDim_t, occaDefault)

! Set stream and launch device kernel
Expand Down
26 changes: 14 additions & 12 deletions include/occa/core/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,17 +307,18 @@ namespace occa {
* @startDoc{copyFrom[0]}
*
* Description:
* Copies data from the input `src` to the caller [[memory]] object
* Copies `count` elements from `src` into caller's data buffer, beginning at `offset`.
*
* Arguments:
* src:
* Data source.
*
* bytes:
* How many bytes to copy.
* count:
* The number of elements of type [[dtype_t]] to copy.
*
* offset:
* The [[memory]] offset where data transfer will start.
* The number of elements from beginning of the caller's
* data buffer the destination range is shifted.
*
* props:
* Any backend-specific properties for memory transfer.
Expand All @@ -326,7 +327,7 @@ namespace occa {
* @endDoc
*/
void copyFrom(const void *src,
const dim_t bytes = -1,
const dim_t count = -1,
const dim_t offset = 0,
const occa::json &props = occa::json());

Expand All @@ -352,7 +353,7 @@ namespace occa {
* @endDoc
*/
void copyFrom(const memory src,
const dim_t bytes = -1,
const dim_t count = -1,
const dim_t destOffset = 0,
const dim_t srcOffset = 0,
const occa::json &props = occa::json());
Expand All @@ -367,17 +368,18 @@ namespace occa {
* @startDoc{copyTo[0]}
*
* Description:
* Copies data from the input `src` to the caller [[memory]] object
* Copies `count` elements to `dest` from caller's data buffer, beginning at `offset`.
*
* Arguments:
* dest:
* Where to copy the [[memory]] data to.
*
* bytes:
* How many bytes to copy
* count:
* The number of elements of type [[dtype_t]] to copy
*
* offset:
* The [[memory]] offset where data transfer will start.
* The number of elements from beginning of the caller's
* data buffer the source range is shifted.
*
* props:
* Any backend-specific properties for memory transfer.
Expand All @@ -386,7 +388,7 @@ namespace occa {
* @endDoc
*/
void copyTo(void *dest,
const dim_t bytes = -1,
const dim_t count = -1,
const dim_t offset = 0,
const occa::json &props = occa::json()) const;

Expand All @@ -412,7 +414,7 @@ namespace occa {
* @endDoc
*/
void copyTo(const memory dest,
const dim_t bytes = -1,
const dim_t count = -1,
const dim_t destOffset = 0,
const dim_t srcOffset = 0,
const occa::json &props = occa::json()) const;
Expand Down
26 changes: 11 additions & 15 deletions include/occa/functional/array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ namespace occa {
: entries
);

memory_.copyFrom(src, safeEntries * sizeof(T));
memory_.copyFrom(src, safeEntries);
}

void copyFrom(const occa::memory src,
Expand All @@ -140,7 +140,7 @@ namespace occa {
: entries
);

memory_.copyFrom(src, safeEntries * sizeof(T));
memory_.copyFrom(src, safeEntries);
}

void copyTo(T *dest,
Expand All @@ -151,7 +151,7 @@ namespace occa {
: entries
);

memory_.copyTo(dest, safeEntries * sizeof(T));
memory_.copyTo(dest, safeEntries);
}

void copyTo(occa::memory dest,
Expand All @@ -162,7 +162,7 @@ namespace occa {
: entries
);

memory_.copyTo(dest, safeEntries * sizeof(T));
memory_.copyTo(dest, safeEntries);
}
//==================================

Expand Down Expand Up @@ -297,17 +297,13 @@ namespace occa {
//---[ Utility methods ]------------
T& operator [] (const dim_t index) {
static T value;
memory_.copyTo(&value,
sizeof(T),
index * sizeof(T));
memory_.copyTo(&value,1,index);
return value;
}

T& operator [] (const dim_t index) const {
static T value;
memory_.copyTo(&value,
sizeof(T),
index * sizeof(T));
memory_.copyTo(&value,1,index);
return value;
}

Expand All @@ -319,12 +315,12 @@ namespace occa {
}

array concat(const array &other) const {
const udim_t bytes1 = memory_.size();
const udim_t bytes2 = other.memory_.size();
const udim_t entries = length();
const udim_t other_entries = other.length();

occa::memory ret = getDevice().template malloc<T>(length() + other.length());
ret.copyFrom(memory_, bytes1, 0);
ret.copyFrom(other.memory_, bytes2, bytes1);
occa::memory ret = getDevice().template malloc<T>(entries + other_entries);
ret.copyFrom(memory_, entries, 0);
ret.copyFrom(other.memory_, other_entries, entries);

return array(ret);
}
Expand Down
5 changes: 2 additions & 3 deletions include/occa/functional/typelessArray.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace occa {
template <class ReturnType>
void setupReturnMemory(const ReturnType &value) const {
setupReturnMemoryArray<ReturnType>(1);
returnMemory.copyFrom(&value, sizeof(ReturnType));
returnMemory.copyFrom(&value, 1);
}

template <class ReturnType>
Expand All @@ -39,8 +39,7 @@ namespace occa {

template <class ReturnType>
void setReturnValue(ReturnType &value) const {
size_t bytes = sizeof(ReturnType);
returnMemory.copyTo(&value, bytes);
returnMemory.copyTo(&value, 1);
}

virtual occa::scope getMapArrayScopeOverrides() const {
Expand Down
84 changes: 47 additions & 37 deletions src/core/memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,105 +186,115 @@ namespace occa {
}

void memory::copyFrom(const void *src,
const dim_t bytes,
const dim_t count,
const dim_t offset,
const occa::json &props) {
if (!isInitialized()) return;

udim_t bytes_ = ((bytes == -1) ? modeMemory->size : bytes);
const int dtypeSize = modeMemory->dtype_->bytes();
const dim_t bytes = dtypeSize * ((count == -1) ? length() : count);
const dim_t offset_ = dtypeSize * offset;

OCCA_ERROR("Trying to allocate negative bytes (" << bytes << ")",
bytes >= -1);

OCCA_ERROR("Cannot have a negative offset (" << offset << ")",
offset >= 0);
OCCA_ERROR("Cannot have a negative offset (" << offset_ << ")",
offset_ >= 0);

OCCA_ERROR("Destination memory has size [" << modeMemory->size << "],"
<< " trying to access [" << offset << ", " << (offset + bytes_) << "]",
(bytes_ + offset) <= modeMemory->size);
<< " trying to access [" << offset_ << ", " << (offset_ + bytes) << "]",
udim_t(bytes + offset_) <= modeMemory->size);

modeMemory->copyFrom(src, bytes_, offset, props);
modeMemory->copyFrom(src, bytes, offset_, props);
}

void memory::copyFrom(const memory src,
const dim_t bytes,
const dim_t count,
const dim_t destOffset,
const dim_t srcOffset,
const occa::json &props) {
if (!isInitialized() && !src.isInitialized()) return;
if (!isInitialized() && !src.isInitialized()) return;
assertInitialized();

udim_t bytes_ = ((bytes == -1) ? modeMemory->size : bytes);
const int dtypeSize = modeMemory->dtype_->bytes();
const dim_t bytes = dtypeSize * ((count == -1) ? length() : count);
const dim_t destOffset_ = dtypeSize * destOffset;
const dim_t srcOffset_ = src.modeMemory->dtype_->bytes() * srcOffset;

OCCA_ERROR("Trying to allocate negative bytes (" << bytes << ")",
bytes >= -1);

OCCA_ERROR("Cannot have a negative offset (" << destOffset << ")",
destOffset >= 0);
OCCA_ERROR("Cannot have a negative offset (" << destOffset_ << ")",
destOffset_ >= 0);

OCCA_ERROR("Cannot have a negative offset (" << srcOffset << ")",
srcOffset >= 0);
OCCA_ERROR("Cannot have a negative offset (" << srcOffset_ << ")",
srcOffset_ >= 0);

OCCA_ERROR("Source memory has size [" << src.modeMemory->size << "],"
<< " trying to access [" << srcOffset << ", " << (srcOffset + bytes_) << "]",
(bytes_ + srcOffset) <= src.modeMemory->size);
<< " trying to access [" << srcOffset_ << ", " << (srcOffset_ + bytes) << "]",
udim_t(bytes + srcOffset_) <= src.modeMemory->size);

OCCA_ERROR("Destination memory has size [" << modeMemory->size << "],"
<< " trying to access [" << destOffset << ", " << (destOffset + bytes_) << "]",
(bytes_ + destOffset) <= modeMemory->size);
<< " trying to access [" << destOffset_ << ", " << (destOffset_ + bytes) << "]",
udim_t(bytes + destOffset_) <= modeMemory->size);

modeMemory->copyFrom(src.modeMemory, bytes_, destOffset, srcOffset, props);
modeMemory->copyFrom(src.modeMemory, bytes, destOffset_, srcOffset_, props);
}

void memory::copyTo(void *dest,
const dim_t bytes,
const dim_t count,
const dim_t offset,
const occa::json &props) const {
if (!isInitialized()) return;

udim_t bytes_ = ((bytes == -1) ? modeMemory->size : bytes);
const int dtypeSize = modeMemory->dtype_->bytes();
const dim_t bytes = dtypeSize * ((count == -1) ? length() : count);
const dim_t offset_ = dtypeSize * offset;

OCCA_ERROR("Trying to allocate negative bytes (" << bytes << ")",
bytes >= -1);

OCCA_ERROR("Cannot have a negative offset (" << offset << ")",
offset >= 0);
OCCA_ERROR("Cannot have a negative offset (" << offset_ << ")",
offset_ >= 0);

OCCA_ERROR("Source memory has size [" << modeMemory->size << "],"
<< " trying to access [" << offset << ", " << (offset + bytes_) << "]",
(bytes_ + offset) <= modeMemory->size);
<< " trying to access [" << offset_ << ", " << (offset_ + bytes) << "]",
udim_t(bytes + offset_) <= modeMemory->size);

modeMemory->copyTo(dest, bytes_, offset, props);
modeMemory->copyTo(dest, bytes, offset_, props);
}

void memory::copyTo(memory dest,
const dim_t bytes,
const dim_t count,
const dim_t destOffset,
const dim_t srcOffset,
const occa::json &props) const {
if (!isInitialized() && !dest.isInitialized()) return;
assertInitialized();

udim_t bytes_ = ((bytes == -1) ? modeMemory->size : bytes);
const int dtypeSize = modeMemory->dtype_->bytes();
const dim_t bytes = dtypeSize * ((count == -1) ? length() : count);
const dim_t destOffset_ = dest.modeMemory->dtype_->bytes() * destOffset;
const dim_t srcOffset_ = dtypeSize * srcOffset;

OCCA_ERROR("Trying to allocate negative bytes (" << bytes << ")",
bytes >= -1);

OCCA_ERROR("Cannot have a negative offset (" << destOffset << ")",
destOffset >= 0);
OCCA_ERROR("Cannot have a negative offset (" << destOffset_ << ")",
destOffset_ >= 0);

OCCA_ERROR("Cannot have a negative offset (" << srcOffset << ")",
srcOffset >= 0);
OCCA_ERROR("Cannot have a negative offset (" << srcOffset_ << ")",
srcOffset_ >= 0);

OCCA_ERROR("Source memory has size [" << modeMemory->size << "],"
<< " trying to access [" << srcOffset << ", " << (srcOffset + bytes_) << "]",
(bytes_ + srcOffset) <= modeMemory->size);
<< " trying to access [" << srcOffset_ << ", " << (srcOffset_ + bytes) << "]",
udim_t(bytes + srcOffset_) <= modeMemory->size);

OCCA_ERROR("Destination memory has size [" << dest.modeMemory->size << "],"
<< " trying to access [" << destOffset << ", " << (destOffset + bytes_) << "]",
(bytes_ + destOffset) <= dest.modeMemory->size);
<< " trying to access [" << destOffset_ << ", " << (destOffset_ + bytes) << "]",
udim_t(bytes + destOffset_) <= dest.modeMemory->size);

dest.modeMemory->copyFrom(modeMemory, bytes_, destOffset, srcOffset, props);
dest.modeMemory->copyFrom(modeMemory, bytes, destOffset_, srcOffset_, props);
}

void memory::copyFrom(const void *src,
Expand Down
Loading

0 comments on commit 6084f64

Please sign in to comment.