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

FP16 Extension Document #2647

Closed
wants to merge 8 commits into from
Closed

Conversation

jzm-intel
Copy link
Contributor

@jzm-intel jzm-intel commented Mar 7, 2022

Define the GPUFeatureName fp16-in-shader-and-buffer and describe this extension in WebGPU spec. In WGSL spec, add f16 extension identifier and description, add f16 scalar type and corresponding vector, matrix types and describe their memory layouts, add type constructor expressions, zero value expressions, conversion expressions, reinterpretation of representation expressions, arithmetic expressions, and comparison expressions for f16, and describe accuracy requirement for expressions for f16.

Issue: #2512


EDIT:
This PR currently has a complete markdown document that describe the whole extension.

Define the GPUFeatureName `fp16-in-shader-and-buffer` and describe this
extension in WebGPU spec. In WGSL spec, add `f16` extension identifier
and description, add `f16` scalar type and corresponding vector, matrix
types and describe their memory layouts, add type constructor
expressions, zero value expressions, conversion expressions,
reinterpretation of representation expressions, arithmetic expressions,
and comparison expressions for f16, and describe accuracy requirement
for expressions for f16.

Issue: gpuweb#2512
@jzm-intel
Copy link
Contributor Author

Following the guidance of #2301, this PR add a markdown file describing FP16 extension in the extensions folder. This extension is aimed at being merged into the spec eventually.
This PR contains only a part of the full extension, the remain part will be updated in next PR(s). The full extension draft can be found at https://hackmd.io/912xyyVDR3y-r0NsgglRJw.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2022

Previews, as seen when this build job started (d0ba8df):
WebGPU | IDL
WGSL
Explainer

@jzm-intel
Copy link
Contributor Author

Please take a look, thanks!

@kdashg
Copy link
Contributor

kdashg commented Mar 9, 2022

WGSL meeting minutes 2022-03-08
  • DN: On our side, we wonder if this should be a branch. Like, how should we add this? Branch to be folded in later?
  • AB/KG: Write the PR that adds the feature to the main spec.
  • AB: There’s value in writing separate high level doc to describe the feature, for initial review. But we’re past that now.
  • DN: The PR can be incomplete, and then get added onto. Wait to land it until the whole feature is described in that PR, or “complete”.
  • KG: If a separate doc is useful, then use the separate document. For this one think this one is past that stage.
  • JB: I don’t see a way to write a f16 literal. Deliberate?
  • AB: Conversion operators. We waved them off of trying to add suffixes.
  • MM: So why have suffixes at all?
  • KG: I think we had more of a reason for them historically, though we have less pressure to do so now. Do you want to file an issue?
  • MM: I probably should file an issue.
  • KG to write issue for suffix discussion

@kainino0x
Copy link
Contributor

  • DN: On our side, we wonder if this should be a branch. Like, how should we add this? Branch to be folded in later?

I prefer maintaining documents like this in the extensions/ directory, over a branch, until they're ready. But if it's time to spec this in the actual specs, we can go straight to PRs against the specs.


# FP16 usages support (PR0)

**Roadmap:** This draft extension is **on the standards track**. It is currently intended to be used in Web-compliant contexts, any user with a capable device will be able to use this extension to acquire the FP16 ability.
Copy link
Contributor

Choose a reason for hiding this comment

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

Anything that's not described in the actual specs must not be exposed to the Web. So if we land this document, this should say something like:

Suggested change
**Roadmap:** This draft extension is **on the standards track**. It is currently intended to be used in Web-compliant contexts, any user with a capable device will be able to use this extension to acquire the FP16 ability.
**Roadmap:** This draft extension is **on the standards track**, but is a work in progress. User agents must not implement/expose these features until they are merged into the main specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (56446bc):
WebGPU | IDL
WGSL
Explainer

@jzm-intel
Copy link
Contributor Author

jzm-intel commented Mar 10, 2022

I prefer maintaining documents like this in the extensions/ directory, over a branch, until they're ready. But if it's time to spec this in the actual specs, we can go straight to PRs against the specs.

For this FP16 extension, if we going to maintain a markdown document, I will simply move the content from the draft in HackMD into this document with minor fixes and ask for review. After all content are moved to the markdown document here, another PR against the main spec shall be opened (assumedly by me) and go for another rounds of reviews.

@kdashg
Copy link
Contributor

kdashg commented Mar 14, 2022

I prefer maintaining documents like this in the extensions/ directory, over a branch, until they're ready. But if it's time to spec this in the actual specs, we can go straight to PRs against the specs.

For this FP16 extension, if we going to maintain a markdown document, I will simply move the content from the draft in HackMD into this document with minor fixes and ask for review. After all content are moved to the markdown document here, another PR against the main spec shall be opened (assumedly by me) and go for another rounds of reviews.

This works for me. There's enough consensus in the group to move to making the actual PR to add this to the WGSL spec, though it should be more or less the whole extension, not just parts of it.

@jzm-intel
Copy link
Contributor Author

If I get it right, I will make this PR a complete markdown document, and after this PR is landed I will open a PR against the main spec.

@jzm-intel jzm-intel changed the title FP16 Extension: Part 1 FP16 Extension Document Mar 15, 2022
@github-actions
Copy link
Contributor

Previews, as seen when this build job started (7a3343f):
WebGPU | IDL
WGSL
Explainer

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (e4b8a79):
WebGPU | IDL
WGSL
Explainer

@kdashg
Copy link
Contributor

kdashg commented Mar 15, 2022

WGSL meeting minutes 2022-03-15
  • **KG: **intel wants to make a document, then a PR. We suggested that they just make a PR directly. Does anyone think there should not be a separate document at all, and they should just jump directly to a PR?
  • AB: Does the document get updated to change the contents of the PR? There’s a place for a document describing an overview of the feature. There’s less utility in a document that just describes the diff that would be the PR.
  • KG: I think the former. Then once it’s merged, we can get rid of the document
  • MM: Intel is doing a lot of work, and we should let them work in whatever way is most effective for them
  • AB: Making them write that twice is what we want to avoid, we should help them not do that.
  • KG: It seems like that (write it twice) is what they’ve chosen to do, so we should let them approach it how they want.

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (a9f9a18):
WebGPU | IDL
WGSL
Explainer

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (c158c38):
WebGPU | IDL
WGSL
Explainer

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (d40029c):
WebGPU | IDL
WGSL
Explainer

@github-actions
Copy link
Contributor

Previews, as seen when this build job started (6d8a555):
WebGPU | IDL
WGSL
Explainer

@jzm-intel
Copy link
Contributor Author

Close this PR, as the extension against spec PR #2696 has been merged.

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.

None yet

3 participants