...
patch forthcoming
Created attachment 376571 [details] patch
Attachment 376571 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:169: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 376571 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=376571&action=review > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:140 > + return WTF::get<AST::ConstantExpression>(matrixType.typeArguments()[typeArgumentIndex]).integerLiteral().value(); Can we move this to Intrinsics? I don't think this is the only place where this kind of computation is performed. > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:211 > + if (nativeFunctionDeclaration.parameters().size() == numRows) { > + // operator matrixMxN (vectorN, ..., vectorN) > + for (unsigned i = 0; i < numRows; ++i) { > + for (unsigned j = 0; j < numColumns; ++j) > + stringBuilder.flexibleAppend("x[", j * numRows + i, "] = ", args[i], "[", j, "];\n"); > + } > + > + } else { > + // operator matrixMxN (scalar, ..., scalar) > + unsigned index = 0; > + for (unsigned i = 0; i < numRows; ++i) { > + for (unsigned j = 0; j < numColumns; ++j) { > + stringBuilder.flexibleAppend("x[", j * numRows + i, "] = ", args[index], ";\n"); > + ++index; > + } > + } > + } Is there reason you're not just calling the MSL constructors because MSL and HLSL define their matrices with different major-ness? This is probably worth a comment. > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:327 > + auto& result = downcast<AST::NativeTypeDeclaration>(downcast<AST::TypeReference>(downcast<AST::TypeDefinition>(typeReference.resolvedType()).type()).resolvedType()); Can this be shared with the lambda above?
Comment on attachment 376571 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=376571&action=review >> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:140 >> + return WTF::get<AST::ConstantExpression>(matrixType.typeArguments()[typeArgumentIndex]).integerLiteral().value(); > > Can we move this to Intrinsics? I don't think this is the only place where this kind of computation is performed. We should put it on NativeTypeDeclaration. >> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp:327 >> + auto& result = downcast<AST::NativeTypeDeclaration>(downcast<AST::TypeReference>(downcast<AST::TypeDefinition>(typeReference.resolvedType()).type()).resolvedType()); > > Can this be shared with the lambda above? it could, but I don't think having more functions in this limited scenario is gonna make anything more clear.
Created attachment 376586 [details] patch for landing
Comment on attachment 376586 [details] patch for landing Clearing flags on attachment: 376586 Committed r248814: <https://trac.webkit.org/changeset/248814>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54419405>