Skip to content

Type script support#2

Open
lu4 wants to merge 4 commits into
trxcllnt:nextfrom
lu4:TypeScriptSupport
Open

Type script support#2
lu4 wants to merge 4 commits into
trxcllnt:nextfrom
lu4:TypeScriptSupport

Conversation

@lu4

@lu4 lu4 commented Jun 9, 2019

Copy link
Copy Markdown

Hey @trxcllnt ! Checkout my changes with TypeScript support. I was able to cover whole OpenCL 1.0-2.0 spec typings and introduced several bug fixes, maybe it'll be useful for you.

Be aware there are minor interface changes that are not backward compatible...

Cheers!

@trxcllnt

Copy link
Copy Markdown
Owner

@lu4 thanks for the PR! I'd be happy to merge this into my fork's master branch, but I'm not sure how much good that'll do you. I'm starting a new job at nvidia tomorrow, and I don't think I'll be doing much OpenCL here soon... but hit me up if you want to collaborate on a native node-cuda module :-)

@trxcllnt trxcllnt changed the base branch from master to next August 6, 2019 23:20
Comment thread src/addon.cpp
Nan::Set(target, JS_STR("v12"), Nan::True());
#else
Nan::Set(target, JS_STR("CL_VERSION_1_2" ), Nan::False());
Nan::Set(target, JS_STR("v12"), Nan::False());

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@lu4 any particular reason these were renamed? In general I'd be happy to merge this PR, but I think we should avoid changing things in the public API unless there's a good reason.

Comment thread src/device.cpp
Nan::Set(arr, 0, JS_INT((uint32_t) (param_value>>32))); // hi
Nan::Set(arr, 1, JS_INT((uint32_t) (param_value & 0xffffffff))); // lo
info.GetReturnValue().Set(arr);
info.GetReturnValue().Set(JS_INT((uint32_t) (param_value >> 10)));

@trxcllnt trxcllnt Aug 14, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Changing the unit means this API doesn't conform to the OpenCL spec, and I worry the right shift will lead to precision loss on certain devices. What's wrong with returning the long as a pair of lo,hi 32-bit numbers? They can very easily be turned into a bigint in JS now days.

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