Summary: | [WHLSL] Implement zero-filling of types | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||
Component: | WebGPU | Assignee: | Saam Barati <saam> | ||||
Status: | RESOLVED DUPLICATE | ||||||
Severity: | Normal | CC: | antonydoniya58, jonlee, webkit-bug-importer | ||||
Priority: | P1 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 198426 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Myles C. Maxfield
2019-03-14 15:19:41 PDT
We should also consider how to zero-fill unused inputs/outputs of entry points. Created attachment 371047 [details]
patch
(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. 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? (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? 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? Comment on attachment 371047 [details]
patch
I should have obsoleted this patch
(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. All that's left here is: https://bugs.webkit.org/show_bug.cgi?id=198433 So marking as a dupe *** This bug has been marked as a duplicate of bug 198433 *** |