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

[js/webgpu] more fixes for access above 2GB #19065

Merged
merged 5 commits into from
Jan 13, 2024
Merged

[js/webgpu] more fixes for access above 2GB #19065

merged 5 commits into from
Jan 13, 2024

Conversation

guschmue
Copy link
Contributor

@guschmue guschmue commented Jan 9, 2024

when jsep calls javascript with an index to HEAP8 or HEAP32 the index is negative when the heap is above 2GB, even if we pass it as uint32_t it remains negative. So in javascript use >>> 0 to make it unsigned.

@guschmue guschmue added the ep:WebGPU ort-web webgpu provider label Jan 9, 2024
@guschmue guschmue changed the title more fixes for access above 2GB [js/webgpu] more fixes for access above 2GB Jan 9, 2024
@fs-eire
Copy link
Contributor

fs-eire commented Jan 11, 2024

I think it makes no difference between passing int32_t, uint32_t or whatever_ptr. Emscripten simply uses int32 to pass them. I think maybe use a more generic name for the macro is better, for example, JSEP_HEAP_SIZE_TYPE. Considering we still use right shift and sum on this value, maybe using int32_t is better. if you still want to use pointer, then use uint8 ptr, otherwise a sum may generate unexpected result.

@fs-eire
Copy link
Contributor

fs-eire commented Jan 11, 2024

/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline

@fs-eire
Copy link
Contributor

fs-eire commented Jan 11, 2024

/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-python-checks-ci-pipeline,onnxruntime-binary-size-checks-ci-pipeline,Android CI Pipeline

@fs-eire
Copy link
Contributor

fs-eire commented Jan 11, 2024

/azp run iOS CI Pipeline,ONNX Runtime React Native CI Pipeline

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

2 similar comments
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@guschmue guschmue merged commit a756017 into main Jan 13, 2024
47 of 53 checks passed
@guschmue guschmue deleted the gs/more_gt_2GB branch January 13, 2024 01:47
mszhanyi pushed a commit that referenced this pull request Jan 15, 2024
when jsep calls javascript with an index to HEAP8 or HEAP32 the index is
negative when the heap is above 2GB, even if we pass it as uint32_t it
remains negative. So in javascript use >>> 0 to make it unsigned.
siweic0 pushed a commit to siweic0/onnxruntime-web that referenced this pull request May 9, 2024
when jsep calls javascript with an index to HEAP8 or HEAP32 the index is
negative when the heap is above 2GB, even if we pass it as uint32_t it
remains negative. So in javascript use >>> 0 to make it unsigned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ep:WebGPU ort-web webgpu provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants