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.

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

Error: Kernel argument type mismatch vol 2 #160

Closed
Patryk27 opened this issue Jul 7, 2019 · 5 comments
Closed

Error: Kernel argument type mismatch vol 2 #160

Patryk27 opened this issue Jul 7, 2019 · 5 comments

Comments

@Patryk27
Copy link
Contributor

Patryk27 commented Jul 7, 2019

Hi,

I've got following Rust and OpenCL code:

queue.kernel_builder("foo")
    .arg(Uint::new(123u32))
    .build()
__kernel void foo(uint bar) {
  //
}

... which ends up with this, quite unexpected and counter-intuitive, run-time error:

Kernel argument type mismatch. The argument named: 'bar' at index: [0] should be a 'uint' (ArgType { base_type: Uint, cardinality: One, is_ptr: false }).

Seems like the core of this problem lays in the ArgType::matches() method:

// kernel.rs:1754
let card_match = match self.cardinality {
    Cardinality::One => TypeId::of::<cl_uint>() == type_id,
    /* ... */
};

cl_uint is an alias for u32, which has different type-id than Uint - so even though this code is, in fact, correct and works just fine, it refuses to compile with the Intel's driver [1].

I was able to successfully fix this with TypeId::of::<cl_uint>() == type_id || TypeId::of::<Uint>() == type_id and I might be able to prepare a complete fix (for other types) too, if you let me know that I'm doing it the right way (TM) :-)

[1] The issue seems to occur only with the Intel's OpenCL driver - I've got an Nvidia card that runs the original code without any changes.

By the way: I'm aware of the #146 - I've created another one, because even though both problems end up with similar error message, the essence of both problems is different.

@c0gent
Copy link
Member

c0gent commented Jul 10, 2019

Yeah your fix is exactly right and I'd be happy to merge a PR with those changes.

I'm curious to know why you're wrapping the u32 in a Uint (not that there's anything wrong with it).

@Patryk27
Copy link
Contributor Author

Patryk27 commented Jul 11, 2019

Great, I'll try to prepare something later.

About the Uint: I've got a kernel where I'm passing together some Uint2s with Uints, so doing it like this:

queue.kernel_builder("foo")
    .arg(Uint::new(100))
    .arg(Uint2::new(200, 300))
    .arg(Uint2::new(400, 500))
    .build()

... seemed more logical to me than just .arg(100) (actually I've never even tried passing just the integer, until I've stepped on this bug and begin to experiment).

@Patryk27
Copy link
Contributor Author

Hi,

I've come up with this: https://github.com/Patryk27/ocl/commit/03086863e95e43033ae67f1530801cb57032e1a3

It seems to work correctly + using the macro makes the entire method more legible IMO - could you please take a look?

Thanks!

@c0gent
Copy link
Member

c0gent commented Jul 12, 2019

Yeah looks great. Go ahead and submit a pull request :)

@Patryk27
Copy link
Contributor Author

D'oh, I forgot about the PR itself :-) #161

@c0gent c0gent closed this as completed in 0308686 Jul 14, 2019
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

No branches or pull requests

2 participants