Summary: | [WHLSL] Remove the phase resolveCallsInFunctions | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robin Morisset <rmorisset> | ||||||||||
Component: | WebGPU | Assignee: | Robin Morisset <rmorisset> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, mmaxfield, ryanhaddad, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Robin Morisset
2019-07-03 15:52:16 PDT
Turns out this pass is also used for some other miscellaneous name resolving, so it cannot be removed altogether. But the vector copying and storing at least should be avoidable. I was confused, this pass is truly not necessary. Created attachment 373438 [details]
Patch
Created attachment 373439 [details]
Patch
Comment on attachment 373439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373439&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:1028 > + getterFunction = resolveFunction(m_program, getterFunctions, getterArgumentTypes, getterName, propertyAccessExpression.origin(), m_intrinsics); nit: if ((getterFunction = resolveFunction(...)) (In reply to Myles C. Maxfield from comment #5) > Comment on attachment 373439 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373439&action=review > > > Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:1028 > > + getterFunction = resolveFunction(m_program, getterFunctions, getterArgumentTypes, getterName, propertyAccessExpression.origin(), m_intrinsics); > > nit: if ((getterFunction = resolveFunction(...)) I tried that at first, but it no longer fit on one line on my screen, and was not really a readability improvement. Comment on attachment 373439 [details] Patch Clearing flags on attachment: 373439 Committed r247127: <https://trac.webkit.org/changeset/247127> All reviewed patches have been landed. Closing bug. Reverted r247127 for reason: Broke the watchOS build. Committed r247144: <https://trac.webkit.org/changeset/247144> Found the problem: we replace in-place a PropertyAccessExpression by a CallExpression. Since I've shrunk PropertyAccessExpression, this is no longer safe on 32-bit platforms. The good news is that there is no bare PropertyAccessExpression, all of them are either DotExpressions or IndexExpressions, both of which have an extra field that make them large enough to replace by a CallExpression in-place. So I just have to add a downcast to the right subclass before the call to replaceWith<CallExpression>. Created attachment 373519 [details]
Patch
Created attachment 373520 [details]
Patch
Comment on attachment 373520 [details] Patch Clearing flags on attachment: 373520 Committed r247170: <https://trac.webkit.org/changeset/247170> All reviewed patches have been landed. Closing bug. |