nrc / derive-new

derive simple constructor functions for Rust structs
MIT License
525 stars 35 forks source link

Struct with #[repr(C)] are not necessarily compatible with OpenCL layouts #55

Closed MoritzKn closed 2 years ago

MoritzKn commented 2 years ago

The docs say that

[OclPrm] Can also be implemented for custom types as long as layout and alignment are conserved between Rust and OpenCL (repr “C”).

I spend quite some time chasing a bug caused by assuming this is always true. I'm not sure if this is due to some edge case in my structs or my driver (M1 MacBook) or if this simply always the case.

I have two (assumed to be) identical structs declared in OpenCl and in Rust. The one in OpenCL C has a sizeof 48 and the one in Rust has size_of 36:

struct Light {
  float3 pos;
  float3 color;
  float intensity;
};

// sizeof(struct Light) is 48
use ocl::{prm::Float3, OclPrm};

#[repr(C)]
#[derive(Debug, PartialEq, Default, Clone, Copy, new)]
pub struct Light {
    pub pos: Float3,
    pub color: Float3,
    pub intensity: f32,
}

unsafe impl OclPrm for Light {}

// std::mem::size_of::<Light>() is 36

This causes all kinds of issues.

Including this error when using Buffer::builder().len(1).fill_val(Light::default()):

Error executing function: clEnqueueFillBuffer

Status error code: CL_INVALID_VALUE (-30)

Proposed solution

I assume there isn't much this crate can do about this but maybe it's possible to add a note about this to the docs.

My Platform

Platform { Profile: Ok(FULL_PROFILE), Version: Ok(OpenCL 1.2 (Apr 19 2022 18:44:44)), Name: Ok(Apple), Vendor: Ok(Apple), Extensions: Ok(cl_APPLE_SetMemObjectDestructor cl_APPLE_ContextLoggingFunctions cl_APPLE_clut cl_APPLE_query_kernel_names cl_APPLE_gl_sharing cl_khr_gl_event) } { Total Device Count: 1 }
nrc commented 2 years ago

Is this an issue with deriving new? Seems like it might be an issue whether or not that is used, in which case this issue would be better on the rust repo?

MoritzKn commented 2 years ago

My bad! I opened the issue in the entirely wrong repo. I seem to have messed up browser tabs.