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

commonfns&vectors&basic:initialize the output buffer to zeros #1894

Closed
wants to merge 1 commit into from

Conversation

banan328
Copy link
Contributor

@banan328 banan328 commented Feb 13, 2024

To ensure bit exactness when reading output buffers using clEnqueueReadBuffer, it's crucial to initialize all output buffers to zero. This ensures they start with a predictable state, preventing potential undefined behavior. The reason for this initialization is that in all tests, the size of the buffers exceeds the number of work items in the kernel. Without initialization, the last portion of the output buffer might contain unpredictable data ("junk").

@banan328 banan328 changed the title commonfns:initialize the output buffer to zeros commonfns&vectors:initialize the output buffer to zeros Feb 13, 2024
@banan328 banan328 force-pushed the init_commonfns branch 5 times, most recently from 8e9139a to d495670 Compare February 14, 2024 11:43
@banan328 banan328 changed the title commonfns&vectors:initialize the output buffer to zeros commonfns&vectors&basic:initialize the output buffer to zeros Feb 14, 2024
@aharon-abramson
Copy link
Contributor

In our testing environment, we check the bit-exactness of output buffers between our compiler and a reference one. Since our framework isn't aware of what part of the output buffer is actually written by a certain kernel, it compares the entire buffer; if that buffer isn't properly initialized and the kernel doesn't write all of it, some of its content is undefined.

@bashbaug
Copy link
Contributor

Discussed in the June 4th teleconference (I think, may have been the week prior). Concerns are that this "buffers must be initialized" property is not enforced by the current CTS tests, so even if we merge this PR there is no guarantee that these buffers will continue to be initialized, nor is there a way to have confidence that there are not other tests that would need to be updated to initialize buffers.

Given these concerns, would a better solution be for whatever testing framework that is requiring bit exactness to do its own buffer intialization, vs. doing the buffer intialization explicitly in the tests?

Removing "focused review" while evaluating whether these changes are required.

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.

4 participants