FP16 Extension against spec by jiangzhaoming · Pull Request #2696 · gpuweb/gpuweb

jiangzhaoming · 2022-03-25T14:50:46Z

FP16 extension against the whole spec, merging the proposed changes into WebGPU and WGSL spec.
The related issue is #2512.
The FP16 extension document (PR #2647) is intend to be aligned with this PR.

Preview | Diff

github-actions · 2022-03-25T15:13:01Z

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

github-actions · 2022-03-25T15:25:22Z

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

github-actions · 2022-03-27T14:23:59Z

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

jiangzhaoming · 2022-03-28T06:19:03Z

Please take a look, thanks

Kangz · 2022-03-28T06:59:48Z

That's a lot of reviewers you added. Can you try to limit them a bit, maybe one for the API spec and one for the WGSL spec?

alan-baker · 2022-03-29T19:29:38Z

In the 2022-03-29 meeting, the group arrived at a consensus of adding a literal suffix for f16. See #2664.

Separately, literal unification (#2227) was merged so you'll likely want to rebase and update your PR based on those changes.

kainino0x

(reviewed API spec only)

github-actions · 2022-03-30T07:36:04Z

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

kainino0x

API spec LGTM

alan-baker

WGSL looks good overall. Mostly minor comments.

alan-baker · 2022-04-07T14:01:04Z

 * the 32-bit signed integer value `1i`,
 * the 32-bit unsigned integer value `1u`,
 * the 32-bit floating point value `1.0f`,
+* the 16-bit floating point value `1.0h` if `f16` extension is enabled,


If f16 is not enabled, what is the value of 1.0h? That is, is the h suffix valid with the extension enabled?

I would suggest it should be a shader-creation error if the h suffix is used without the extension enabled.

It is now explicitly stated in the Literals section.

alan-baker · 2022-04-07T15:08:32Z

 <table class='data'>
  <thead>
-    <tr><td>Overload<td>Description
+    <tr><td>Parameterization<td>Overload<td>Description


I'm not a huge fan of the asymmetry here between pack and unpack built-in functions. I'd suggest either removing the f16 versions of packing or rename all the functions similarly to MSL, e.g.:

pack4x8snormf16 pack4x8snormf32 unpack4x8snormf16 unpack4x8snormf32

Remove the f16 version of packing.

github-actions · 2022-04-08T11:58:31Z

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

alan-baker

Thanks for the changes!

dneto0

This looks great!

I pointed out a couple of grammar details that we can fix before or after landing.

dneto0 · 2022-04-09T14:10:34Z

+    |T| is i32, u32, or f32
+    <td class="nowrap">bitcast&lt;vec4&lt;f16&gt;&gt;(|e|): vec4&lt;f16&gt;
+    <td>Reinterpretation of bits as vec4&lt;f16&gt;.<br>
+    The result is the reinterpretation of the 64 bits in |e| as a vec4&lt;f16&gt; value, following the internal layout rules.


This is a really nice way to say it: "following the internal layout rules".

To be clear, I'm not asking for a change. But I think after landing there may be a way to simplify this section by referencing the layout rules for the whole section.

github-actions · 2022-04-11T07:26:09Z

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

kdashg · 2022-04-12T07:49:21Z

WGSL meeting minutes 2022-04-05

Update:
- Proposal of FP16 WGSL extension #2512 (comment)
ZJ: I have slides
ZJ: Do we have any comments on the ranking changes?
DN: We don’t have consensus on that approach yet, so let’s try not to block fp16 on that.
MM: I can give more background. During office hour, there were concerns about the h suffix, and I remembered back to a previous call where we briefly discussed an extension idea to change the default float type. It sounds like that extension is likely to change our decision on h-suffix, but until someone proposes that extension, we should let the h-suffix extension stand.
AB: I expect it is in good shape. Have to review this version.
ZJ: We’re somewhat concerned that there aren’t many comments on the other parts of the PR
AB: Sorry, planning to do a more detailed review, but from what I remember we’re mostly satisfied with the overall shape of this.
AB: Given that we only have these live meetings once a month, are you ok with offline comments, or did you want another round of online comments?
ZJ: Either is good.

jiangzhaoming · 2022-04-12T08:26:53Z

@tidoust
Hi, this PR has been approved by reviewers, but the check "ipr" is still "Waiting for status to be reported" (have been in this status from the PR was submitted). When looking at the W3C repository Manager, clicking "Revalidate" will result in an error message "PR not found: gpuweb/gpuweb/pulls/2696", which is kind of confusing.
Could you please take a look? Thanks!

tidoust · 2022-04-12T09:19:25Z

@tidoust
Hi, this PR has been approved by reviewers, but the check "ipr" is still "Waiting for status to be reported" (have been in this status from the PR was submitted). When looking at the W3C repository Manager, clicking "Revalidate" will result in an error message "PR not found: gpuweb/gpuweb/pulls/2696", which is kind of confusing.
Could you please take a look? Thanks!

Sorry about that. When this happens, it means that the IPR bot crashed when it ran. That does not happen often but it does happen once in a while. Unfortunately, as far as I know, there is no easy way to re-run the bot on the PR when that happens, except by re-creating the PR.

I confirm that the IPR bot would have reported a green flag here, and suggest that spec editors merge the PR as-is (they should have the power to do so from a GitHub permissions perspective).

jiangzhaoming · 2022-04-12T10:33:08Z

@tidoust
Hi, this PR has been approved by reviewers, but the check "ipr" is still "Waiting for status to be reported" (have been in this status from the PR was submitted). When looking at the W3C repository Manager, clicking "Revalidate" will result in an error message "PR not found: gpuweb/gpuweb/pulls/2696", which is kind of confusing.
Could you please take a look? Thanks!

Sorry about that. When this happens, it means that the IPR bot crashed when it ran. That does not happen often but it does happen once in a while. Unfortunately, as far as I know, there is no easy way to re-run the bot on the PR when that happens, except by re-creating the PR.

I confirm that the IPR bot would have reported a green flag here, and suggest that spec editors merge the PR as-is (they should have the power to do so from a GitHub permissions perspective).

Thanks!

@kdashg

SHA: 3e48cab Reason: push, by @kdashg Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@kdashg

SHA: 3e48cab Reason: push, by @kdashg Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@kdashg

SHA: 3e48cab Reason: push, by @kdashg Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

kdashg · 2022-04-13T17:02:40Z

WGSL meeting minutes 2022-04-12

Offline:
- David and Alan reviewed and approve
Alan: API signed off already. Good to land.
Merge

jiangzhaoming · 2022-04-15T02:24:41Z

Shall this PR close issue #658?

* FP16 Extension spec: WIP * Add ID for extension list section. * Fix typo * Fix typo * Changes for WebGPU spec * Fix typo * Fix typo * Fix typo * Update accroding to comments * F16 literals require f16 extension * Remove packing function for f16 * Fix grammar

FP16 Extension spec: WIP

fc9d047

jiangzhaoming marked this pull request as draft March 25, 2022 14:51

jiangzhaoming and others added 3 commits March 25, 2022 22:52

Merge branch 'main' into FP16Spec

02274a4

Add ID for extension list section.

7b7fb15

Fix typo

0595a49

Fix typo

f19625c

jiangzhaoming added 4 commits March 27, 2022 21:51

Changes for WebGPU spec

b9bc47c

Fix typo

48fe2f3

Fix typo

138eb5f

Fix typo

8a519dc

jiangzhaoming changed the title ~~[WIP] FP16 Extension against spec~~ FP16 Extension against spec Mar 28, 2022

jiangzhaoming requested review from Kangz, dneto0, kainino0x, kvark, litherum and toji March 28, 2022 06:17

jiangzhaoming marked this pull request as ready for review March 28, 2022 06:18

jiangzhaoming removed request for kvark, litherum and toji March 28, 2022 07:01

kainino0x reviewed Mar 29, 2022

View reviewed changes

Comment thread spec/index.bs Outdated

Comment thread spec/index.bs Outdated

Comment thread spec/index.bs Outdated

Merge branch 'main' into FP16Spec

e6735a5

kainino0x approved these changes Apr 5, 2022

View reviewed changes

alan-baker requested changes Apr 7, 2022

View reviewed changes

jiangzhaoming and others added 3 commits April 8, 2022 19:34

F16 literals require f16 extension

b353a52

Remove packing function for f16

577ab91

Merge branch 'main' into FP16Spec

9a84ffd

alan-baker approved these changes Apr 8, 2022

View reviewed changes

dneto0 approved these changes Apr 9, 2022

View reviewed changes

Fix grammar

8da687b

kdashg merged commit 3e48cab into gpuweb:main Apr 12, 2022

github-actions bot added a commit that referenced this pull request Apr 12, 2022

FP16 Extension against spec (#2696)

fa63711

SHA: 3e48cab Reason: push, by @kdashg Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot added a commit that referenced this pull request Apr 12, 2022

FP16 Extension against spec (#2696)

b28187c

SHA: 3e48cab Reason: push, by @kdashg Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot added a commit that referenced this pull request Apr 12, 2022

FP16 Extension against spec (#2696)

4771f8a

SHA: 3e48cab Reason: push, by @kdashg Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

jiangzhaoming mentioned this pull request Apr 14, 2022

FP16 Extension Document #2647

Closed

jiangzhaoming mentioned this pull request Apr 21, 2022

FP16 support #658

Closed

TimothyOlsson mentioned this pull request May 14, 2022

Parse shaders with half-precision floats (float16) gfx-rs/naga#1149

Closed

kainino0x mentioned this pull request Aug 25, 2022

Add shader-float16 extension #1275

Closed

dneto0 mentioned this pull request Oct 7, 2022

wgsl: can hex floats explicitly represent infinity and NaN as float literals? #1769

Closed

kushalkolar mentioned this pull request Mar 7, 2023

adding float16 support pygfx/pygfx#475

Closed

LLukas22 mentioned this pull request Jun 1, 2023

Builder API and further removal of ONNX from the IR webonnx/wonnx#170

Draft

9 tasks

teoxoy mentioned this pull request Feb 2, 2026

Can fragment output location be of vecN<f16> type? #5551

Open

Conversation

jiangzhaoming commented Mar 25, 2022 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 25, 2022

Uh oh!

github-actions bot commented Mar 25, 2022

Uh oh!

github-actions bot commented Mar 27, 2022

Uh oh!

jiangzhaoming commented Mar 28, 2022

Uh oh!

Kangz commented Mar 28, 2022

Uh oh!

alan-baker commented Mar 29, 2022

Uh oh!

kainino0x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 30, 2022

Uh oh!

kainino0x left a comment

Choose a reason for hiding this comment

Uh oh!

alan-baker left a comment

Choose a reason for hiding this comment

Uh oh!

alan-baker Apr 7, 2022

Choose a reason for hiding this comment

Uh oh!

jiangzhaoming Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alan-baker Apr 7, 2022

Choose a reason for hiding this comment

Uh oh!

jiangzhaoming Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 8, 2022

Uh oh!

alan-baker left a comment

Choose a reason for hiding this comment

Uh oh!

dneto0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dneto0 Apr 9, 2022

Choose a reason for hiding this comment

Uh oh!

dneto0 Apr 9, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 11, 2022

Uh oh!

kdashg commented Apr 12, 2022

Uh oh!

jiangzhaoming commented Apr 12, 2022

Uh oh!

tidoust commented Apr 12, 2022

Uh oh!

jiangzhaoming commented Apr 12, 2022

Uh oh!

kdashg commented Apr 13, 2022

Uh oh!

jiangzhaoming commented Apr 15, 2022

Uh oh!

Reviewers

jiangzhaoming commented Mar 25, 2022 •

edited by pr-preview bot

Loading