Every variant of cluBuildSource*() should have a compile_options parameter about clu HOT 7 CLOSED

Just found clu_initialize_params::compile_options which seems to be the intended way of setting compile options - but why has cluBuildSourceArray an extra parameter then?

Nevertheless, setting these options once at initialisation is insufficient for some use cases as described in the first comment. Another use case are performance studies of the same kernel with different options.

from clu.

Sure, I’d be happy to integrate!

From: noma [mailto:[email protected]]
Sent: Tuesday, January 27, 2015 6:13 AM
To: Computing-Language-Utility/CLU
Subject: [CLU] Every variant of cluBuildSource*() should have a compile_options parameter (#23)

Only cluBuildSourceArray() offers a compile_options parameter, which forces the user to manually read a .cl-file in case she wants to specify compiler options. To my experience, the latter is very common to pass defines for code configuration and compile-time arguments.

If whoever is responsible for that decision agrees, I would happily deliver the changes as a pull request.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/23.

from clu.

Here is a link to my fork, where I added an option parameter to all the build functions that did not already have one. While this seemed the cleanest solution, given the 'extern "C"'-constraints, it might break compatibility with existing code like the examples.

Link

Another option would be to add differently named variants of the functions, but this would be inconsistent with those functions that already have the compile_options parameter.

What do you think?

from clu.

I'm a little worried this makes the "very easy" API a little less easy. Maybe in this case it's justified?

Probably should integrate. Send a pull request?

from clu.

Ok, I tried to do this myself. Changed a few things internally to simplify. Hope I didn't break anything.

from clu.

Thanks for the effort. I've reviewed the commit and added a few notes 3c598c5

from clu.

I've tested the changes with my project and it works for me. Thanks!

from clu.