Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Clarify out-of-bounds access behavior. #5

Merged
merged 1 commit into from
Nov 28, 2018

Conversation

kenrussell
Copy link
Member

Create a separate section defining out-of-bounds access behavior for
loads, writes, atomics, and pointer creation, based on the previous
text as well as that in the Vulkan 1.0 specification (used with
attribution). Rename section "Out of bounds" to "Opcodes Potentially
Resulting in Out-Of-Bounds Accesses" and refer to this new section.

Attempt to resolve #4.

Create a separate section defining out-of-bounds access behavior for
loads, writes, atomics, and pointer creation, based on the previous
text as well as that in the Vulkan 1.0 specification (used with
attribution). Rename section "Out of bounds" to "Opcodes Potentially
Resulting in Out-Of-Bounds Accesses" and refer to this new section.

Attempt to resolve gpuweb#4.
@kenrussell
Copy link
Member Author

@Kangz @dneto0 in particular please review.

Any other comments welcome.

Copy link
Contributor

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

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

+1 . Would like another reviewer to approve.

Copy link
Collaborator

@Kangz Kangz left a comment

Choose a reason for hiding this comment

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

LGTM overall, small nits.


* Causes a trap. That is, causes the shader invocation to write zero values to all shader stage outputs, and then terminate without other side effects.
** 0, 1, or the maximum representable positive integer value, for
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: markdown syntax is to indent the * instead of doubling it. Same for the next bullet point

* Return zero values for reads or (0, 0, 0, X) with X being 0, 1, -1, or extrema for integers, or -0.0, +0.0, -1.0, +1.0 for floating point values
* Atomics can return undefined values.
* Return values from anywhere within the memory range(s) bound to the
buffer (possibly including bytes of memory past the end of the
Copy link
Collaborator

Choose a reason for hiding this comment

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

For WebGPU we should not allow for reading past the end (or the start) of the buffer. If possible we should also say it returns values from anywhere within the buffer view which can be a subset of the buffer.

Same for other types of operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is expressly what's defined in the Vulkan spec for robust access. I would guess it's to allow for spillage into the trailing bytes in the padding at the end of a buffer.
e.g. if a buffer has 9 bytes of payload, there's going to be at least one more byte (and probably 128-9 more bytes) of padding before the next resource allocation.
So this allowance in the spec lets you write into that 10th byte that, by construction (the behaviour of the trusted part of the API implementation) is not written to by any other agent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I copied this text directly from the Vulkan spec. If we want to be able to use hardware support for robust buffer access to accelerate WebGPU's out-of-bounds handling, then we need this affordance.

* Be discarded.
* Modify values within the memory range(s) bound to the buffer, but
_must_ not modify any other memory.
* Return an undefined value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: maybe we should qualify what kind of value must be returned, but it's a bit hard to know how each hardware works.

@dneto0 dneto0 merged commit 9fa7cfb into gpuweb:master Nov 28, 2018
@kenrussell
Copy link
Member Author

Thanks @dneto0 and @Kangz for your reviews and for accepting this change.

@kenrussell kenrussell deleted the clarify-out-of-bounds-accesses branch November 30, 2018 23:32
kenrussell added a commit to kenrussell/spirv-execution-env that referenced this pull request Nov 30, 2018
kenrussell added a commit that referenced this pull request Dec 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify the phrasing of robust-access rules
3 participants