Bug 195771 – [WHLSL] Implement zero-filling of types

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebGPU

[Myles C. Maxfield]

Implement zero-filling of types

[Radar WebKit Bug Importer]

<rdar://problem/50746291>

[Myles C. Maxfield]

We should also consider how to zero-fill unused inputs/outputs of entry points.

[Saam Barati]

Created attachment 371047 [details]
patch

[Saam Barati]

(In reply to Myles C. Maxfield from comment #2)
> We should also consider how to zero-fill unused inputs/outputs of entry
> points.

I didn't do anything here. I should figure out what needs to be done.

[Myles C. Maxfield]

Comment on attachment 371047 [details]
patch

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

Does the Metal compiler still produce warnings now? GPURenderPipelineMetal.mm has a FIXME with a link to this bug.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp:54
> +        // Skip argument declarations.

Cool.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp:66
> +        printStream.print(TypeDumper(*type));

🤔 This seems unwise, though I do understand how it isn't observable. Are you confident this is a good idea?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:-360
> -        auto* overload = resolveFunctionOverloadImpl(*getterFuncs, argumentTypeReferences, nullptr);

Excellent.

> Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLVariableDeclaration.h:73
> +    void setInitializer(UniqueRef<Expression> expression)

Doesn't this need to take an rvalue reference? UniqueRef is move-only (I think???)

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:453
>          m_stringBuilder.append(makeString(m_typeNamer.mangledNameForType(*variableDeclaration.type()), ' ', variableName, ";\n"));

Is this dead code now?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:110
> +            stringBuilder.append(makeString("    for (size_t i = 0; i < sizeof(", metalReturnName, "); ++i) {\n"));
> +            stringBuilder.append("        ptr[i] = 0;\n");
> +            stringBuilder.append("    }\n");

Can we remove the { } and pull it up into one line?

[Myles C. Maxfield]

(In reply to Saam Barati from comment #4)
> (In reply to Myles C. Maxfield from comment #2)
> > We should also consider how to zero-fill unused inputs/outputs of entry
> > points.
> 
> I didn't do anything here. I should figure out what needs to be done.

Is there another bug filed for this?

[Myles C. Maxfield]

Comment on attachment 371047 [details]
patch

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

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:109
> +            stringBuilder.append("        ptr[i] = 0;\n");

I think this "memset" is going to be needed for the follow-up task (of inputs and outputs). Can we pull this out into a standalone metal function?

[Saam Barati]

Comment on attachment 371047 [details]
patch

I should have obsoleted this patch

[Saam Barati]

(In reply to Myles C. Maxfield from comment #7)
> Comment on attachment 371047 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371047&action=review
> 
> > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:109
> > +            stringBuilder.append("        ptr[i] = 0;\n");
> 
> I think this "memset" is going to be needed for the follow-up task (of
> inputs and outputs). Can we pull this out into a standalone metal function?

I think we should do this when it's needed, as it has added complexity. I also think there is a chance it's not needed, depending on if we define things to be a compile time error or not.

[Saam Barati]

All that's left here is:
https://bugs.webkit.org/show_bug.cgi?id=198433

So marking as a dupe

[Saam Barati]

*** This bug has been marked as a duplicate of bug 198433 ***