Bug 198433 – [WHLSL] Zero-fill parameters to entry point functions or make any mismatch a compile error

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebGPU

[Saam Barati]

...

[Saam Barati]

Currently, it's a compile error if the JS attributes define fewer things than the WHLSL defined attributes. Maybe we're ok keeping it this way, then we wouldn't need to zero fill

[Saam Barati]

(In reply to Saam Barati from comment #1)
> Currently, it's a compile error if the JS attributes define fewer things
> than the WHLSL defined attributes. Maybe we're ok keeping it this way, then
> we wouldn't need to zero fill

If we loosen this, though, we'd need to zero fill.

Attaching a patch that does that zero filling.

[Saam Barati]

Created attachment 371093 [details]
WIP

[Saam Barati]

We could also make this a compile error. Maybe that's nicer, but I don't think it's a P1

[Saam Barati]

*** Bug 195771 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

Created attachment 372620 [details]
Patch

[Saam Barati]

Comment on attachment 372620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372620&action=review

> LayoutTests/ChangeLog:8
> +        Turns out this bug is already fixed, so this just adds some tests.

Which part is already fixed? Last time I checked we allow multiple parameters to be hooked up to the same attribute index.

[Myles C. Maxfield]

Comment on attachment 372620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372620&action=review

>> LayoutTests/ChangeLog:8
>> +        Turns out this bug is already fixed, so this just adds some tests.
> 
> Which part is already fixed? Last time I checked we allow multiple parameters to be hooked up to the same attribute index.

https://bugs.webkit.org/show_bug.cgi?id=198440 Is for duplicates. This is for attributes that exist in the shader text but aren’t hooked up to any API objects.

[Saam Barati]

(In reply to Myles C. Maxfield from comment #8)
> Comment on attachment 372620 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=372620&action=review
> 
> >> LayoutTests/ChangeLog:8
> >> +        Turns out this bug is already fixed, so this just adds some tests.
> > 
> > Which part is already fixed? Last time I checked we allow multiple parameters to be hooked up to the same attribute index.
> 
> https://bugs.webkit.org/show_bug.cgi?id=198440 Is for duplicates. This is
> for attributes that exist in the shader text but aren’t hooked up to any API
> objects.

Can you make the changelog more descriptive of what we're testing for here? I'm still confused, because I thought we actually didn't allow code like this? I don't remember ever actually implementing zero filling here.

[Saam Barati]

Comment on attachment 372620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372620&action=review

> LayoutTests/webgpu/whlsl-extra-attribute.html:9
> +vertex float4 vertexShader(float4 position : attribute(0), float i : attribute(1)) : SV_Position {

Don't you want to be checking that "i" is zero here since it's not hooked up?

> LayoutTests/webgpu/whlsl-extra-resource.html:9
> +vertex float4 vertexShader(float4 position : attribute(0), device float[] theBuffer : register(u0)) : SV_Position {

Don't you want to be checking that theBuffer is null here?

[Radar WebKit Bug Importer]

<rdar://problem/52699959>

[Myles C. Maxfield]

WHLSL is no longer relevant.