Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

**BREAKING** Make memory copies data type aware for consistency. #728

Merged
merged 6 commits into from
Dec 15, 2023
Merged

**BREAKING** Make memory copies data type aware for consistency. #728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e630206
Make memory copies data type aware for consistency.
kris-rowe Dec 11, 2023
67547d6
Test memory copies in core unit tests.
kris-rowe Dec 11, 2023
ad93746
Address compiler warnings: explicitly cast types in comparison
kris-rowe Dec 11, 2023
817dda7
Update documentation.
kris-rowe Dec 11, 2023
9cbe1f7
Update Fortran examples.
kris-rowe Dec 11, 2023
1ce394b
Drop dpcpp from ctests when running GitHub actions.
kris-rowe Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
: entries
);

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

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

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

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

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

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

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

Expand Down Expand Up @@ -297,17 +297,13 @@
//---[ 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);

Check warning on line 306 in include/occa/functional/array.hpp

View check run for this annotation

Codecov / codecov/patch

include/occa/functional/array.hpp#L306 

Added line #L306 was not covered by tests
return value;
}

Expand All @@ -319,12 +315,12 @@
}

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
View file
Open in desktop
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
View file
Open in desktop
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