...
Created attachment 371069 [details] patch
Comment on attachment 371069 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=371069&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp:66 > + StringPrintStream printStream; > + printStream.print(TypeDumper(*type)); Is this overkill just to get a nice function name? I could make this happen just on debug builds, and use some constant name for release builds.
Comment on attachment 371069 [details] patch Please see my comments in https://bugs.webkit.org/show_bug.cgi?id=195771. Should that bug be marked as a duplicate of this one?
(In reply to Myles C. Maxfield from comment #5) > 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? I will make this debug builds only. > > > 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???) Rvalue references are just a cast. What happens here is a caller must move the argument into this value, calling the move constructor. > > > 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? No. Since there are passes that run after this that create variables without initializers. > > > 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? Sure!
Created attachment 371767 [details] patch for landing
Created attachment 371768 [details] patch for landing
Comment on attachment 371768 [details] patch for landing Clearing flags on attachment: 371768 Committed r246273: <https://trac.webkit.org/changeset/246273>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51593411>