Skip to content

Conversation

@Kotomi-Du
Copy link
Collaborator

No description provided.

@Kotomi-Du Kotomi-Du requested a review from smarcink April 2, 2024 10:23
Copy link
Owner

@smarcink smarcink left a comment

Choose a reason for hiding this comment

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

Some comments. Also please add dumping data for reference OCL path so it can be used for comparison

ConformanceResult validate_conformance(ID3D12CommandQueue* command_queue,
ID3D12CommandAllocator* command_allocator, ID3D12GraphicsCommandList* command_list, bool print_mismatche) override
{
auto dump_buffer_to_file = [&](const auto& buffer, const auto& file_name)
Copy link
Owner

Choose a reason for hiding this comment

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

Similar lambda is in convolution umd dispatcher - could it be moved to be separate function?

readback_buffer->Unmap(0, nullptr);

// Assuming data_out now contains the float data
float* float_ptr = reinterpret_cast<float*>(data_out.data());
Copy link
Owner

Choose a reason for hiding this comment

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

const


// Assuming data_out now contains the float data
float* float_ptr = reinterpret_cast<float*>(data_out.data());
size_t num_floats = data_out.size() / sizeof(float);
Copy link
Owner

Choose a reason for hiding this comment

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

const

readback_buffer->Unmap(0, nullptr);

// Assuming data_out now contains the float data
float* float_ptr = reinterpret_cast<float*>(data_out.data());
Copy link
Owner

Choose a reason for hiding this comment

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

Float assumption is fine, but probably not always valid as scratchpad can have arbitrary data type <- you may want to add such extended comment so it will be clear for code reader in future.

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.

2 participants