Skip to content

Conversation

KsanaKozlova
Copy link
Contributor

No description provided.

@KsanaKozlova KsanaKozlova requested a review from densmirn May 16, 2021 19:29
Copy link
Contributor

@densmirn densmirn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for green CI.


cl::sycl::range<1> gws(result_size);
auto kernel_parallel_for_func = [=](cl::sycl::id<1> global_id) {
const size_t i = global_id[0]; /* for (size_t i = 0; i < result_size; ++i) */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const size_t i = global_id[0]; /* for (size_t i = 0; i < result_size; ++i) */
const size_t i = global_id[0];

std::is_same<_DataType_input2, _DataType_input1>::value)
{
event = oneapi::mkl::vm::fmod(DPNP_QUEUE, input1_size, input1_data, input2_data, result);
event.wait();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use dependent event. Do not wait for each.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use dependent event. Do not wait for each.

const size_t i = global_id[0]; /* for (size_t i = 0; i < result_size; ++i) */
const _DataType_output input1_elem = (*input1_it)[i];
const _DataType_output input2_elem = (*input2_it)[i];
double fmod = cl::sycl::fmod((double)input1_elem, (double)input2_elem);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, use another name for fmod variable to avoid potential naming collision/ naming conflicts.


DPNPC_id<_DataType_input1>* input1_it;
const size_t input1_it_size_in_bytes = sizeof(DPNPC_id<_DataType_input1>);
input1_it = reinterpret_cast<DPNPC_id<_DataType_input1>*>(dpnp_memory_alloc_c(input1_it_size_in_bytes));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can call mem allocator at once here.

std::is_same<_DataType_input2, _DataType_input1>::value)
{
event = oneapi::mkl::vm::fmod(DPNP_QUEUE, input1_size, input1_data, input2_data, result);
event.wait();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use dependent event. Do not wait for each.

Comment on lines +420 to +421
input1_it->~DPNPC_id();
input2_it->~DPNPC_id();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we need to redesign this class, if explicitly call of the destructor is required for its interface.

@shssf shssf merged commit f6dd61a into master Aug 17, 2021
@shssf shssf deleted the remainder_func branch August 17, 2021 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants