199537 2019-07-05 16:39:34 -0700 [WHLSL] The checker does not need to keep a separate m_typeAnnotations map 2019-07-05 17:34:19 -0700 1 1 1 Unclassified WebKit WebGPU WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 rmorisset rmorisset commit-queue mmaxfield webkit-bug-importer oldest_to_newest 1550570 0 rmorisset 2019-07-05 16:39:34 -0700 Nothing is ever deleted from it, and all of its contents are eventually copied into the expressions. We might as well put the type annotations directly into the expressions in the first place. It is about a 5% speedup of the checker. 1550573 1 373554 rmorisset 2019-07-05 16:42:31 -0700 Created attachment 373554 Patch 1550575 2 373554 mmaxfield 2019-07-05 16:44:58 -0700 Comment on attachment 373554 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373554&action=review > Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:-845 > - if (leftInfo->typeAnnotation.isRightValue()) { > - setError(); > - return; > - } Why? This seems important. 1550576 3 rmorisset 2019-07-05 16:46:15 -0700 (In reply to Myles C. Maxfield from comment #2) > Comment on attachment 373554 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373554&action=review > > > Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:-845 > > - if (leftInfo->typeAnnotation.isRightValue()) { > > - setError(); > > - return; > > - } > > Why? This seems important. It comes from auto leftInfo = recurseAndGetInfo(assignmentExpression.left(), true); And the true in that means that we went through: if (requiresLeftValue && typeAnnotation.isRightValue()) { setError(); return WTF::nullopt; } So it is redundant. 1550577 4 373554 mmaxfield 2019-07-05 16:50:42 -0700 Comment on attachment 373554 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373554&action=review >>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:-845 >>> - } >> >> Why? This seems important. > > It comes from auto leftInfo = recurseAndGetInfo(assignmentExpression.left(), true); > And the true in that means that we went through: > if (requiresLeftValue && typeAnnotation.isRightValue()) { > setError(); > return WTF::nullopt; > } > So it is redundant. 👍 1550582 5 373554 commit-queue 2019-07-05 17:33:07 -0700 Comment on attachment 373554 Patch Clearing flags on attachment: 373554 Committed r247189: <https://trac.webkit.org/changeset/247189> 1550583 6 commit-queue 2019-07-05 17:33:09 -0700 All reviewed patches have been landed. Closing bug. 1550586 7 webkit-bug-importer 2019-07-05 17:34:19 -0700 <rdar://problem/52708994> 373554 2019-07-05 16:42:31 -0700 2019-07-05 17:33:07 -0700 Patch patch199537 text/plain 5046 rmorisset ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBiZGMwYjMyYzRiNi4uYjEwNWNjMGY5NWIgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMyBAQAorMjAxOS0wNy0wNSAgUm9iaW4gTW9yaXNzZXQgIDxybW9yaXNzZXRAYXBwbGUu Y29tPgorCisgICAgICAgIFtXSExTTF0gVGhlIGNoZWNrZXIgZG9lcyBub3QgbmVlZCB0byBrZWVw IGEgc2VwYXJhdGUgbV90eXBlQW5ub3RhdGlvbnMgbWFwCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTk1MzcKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBOb3RoaW5nIGlzIGV2ZXIgZGVsZXRlZCBmcm9tIGl0 LCBhbmQgYWxsIG9mIGl0cyBjb250ZW50cyBhcmUgZXZlbnR1YWxseSBjb3BpZWQgaW50byB0aGUg ZXhwcmVzc2lvbnMuCisgICAgICAgIFdlIG1pZ2h0IGFzIHdlbGwgcHV0IHRoZSB0eXBlIGFubm90 YXRpb25zIGRpcmVjdGx5IGludG8gdGhlIGV4cHJlc3Npb25zIGluIHRoZSBmaXJzdCBwbGFjZS4K KyAgICAgICAgSXQgaXMgYWJvdXQgYSA1JSBzcGVlZHVwIG9mIHRoZSBjaGVja2VyLgorCisgICAg ICAgIE5vIG5ldyB0ZXN0IGFzIHRoZXJlIGlzIG5vIGludGVkZWQgZnVuY3Rpb25hbCBjaGFuZ2Uu CisKKyAgICAgICAgKiBNb2R1bGVzL3dlYmdwdS9XSExTTC9XSExTTENoZWNrZXIuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6V0hMU0w6OkNoZWNrZXI6OmFzc2lnblR5cGVzKToKKyAgICAgICAgKFdl YkNvcmU6OldITFNMOjpDaGVja2VyOjpnZXRJbmZvKToKKyAgICAgICAgKFdlYkNvcmU6OldITFNM OjpDaGVja2VyOjphc3NpZ25UeXBlKToKKyAgICAgICAgKFdlYkNvcmU6OldITFNMOjpDaGVja2Vy Ojpmb3J3YXJkVHlwZSk6CisgICAgICAgIChXZWJDb3JlOjpXSExTTDo6Q2hlY2tlcjo6dmlzaXQp OgorCiAyMDE5LTA3LTA1ICBSeW9zdWtlIE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgogCiAgICAg ICAgIFtpT1NdIENyYXNoIGluIFdlYktpdDo6V2ViUGFnZTo6cG9zaXRpb25JbmZvcm1hdGlvbiB2 aWEgUmFuZ2U6OnN0YXJ0UG9zaXRpb24KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL01vZHVs ZXMvd2ViZ3B1L1dITFNML1dITFNMQ2hlY2tlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVz L3dlYmdwdS9XSExTTC9XSExTTENoZWNrZXIuY3BwCmluZGV4IDBlNTA4NjlhM2NmLi5kODA5NDE0 YWY4ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy93ZWJncHUvV0hMU0wvV0hM U0xDaGVja2VyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3dlYmdwdS9XSExTTC9X SExTTENoZWNrZXIuY3BwCkBAIC01MTIsNyArNTEyLDYgQEAgcHJpdmF0ZToKICAgICB2b2lkIGZp bmlzaFZpc2l0aW5nKEFTVDo6UHJvcGVydHlBY2Nlc3NFeHByZXNzaW9uJiwgUmVzb2x2aW5nVHlw ZSogYWRkaXRpb25hbEFyZ3VtZW50VHlwZSA9IG51bGxwdHIpOwogCiAgICAgSGFzaE1hcDxBU1Q6 OkV4cHJlc3Npb24qLCBzdGQ6OnVuaXF1ZV9wdHI8UmVzb2x2aW5nVHlwZT4+IG1fdHlwZU1hcDsK LSAgICBIYXNoTWFwPEFTVDo6RXhwcmVzc2lvbiosIEFTVDo6VHlwZUFubm90YXRpb24+IG1fdHlw ZUFubm90YXRpb25zOwogICAgIEhhc2hTZXQ8U3RyaW5nPiBtX3ZlcnRleEVudHJ5UG9pbnRzOwog ICAgIEhhc2hTZXQ8U3RyaW5nPiBtX2ZyYWdtZW50RW50cnlQb2ludHM7CiAgICAgSGFzaFNldDxT dHJpbmc+IG1fY29tcHV0ZUVudHJ5UG9pbnRzOwpAQCAtNTU2LDggKzU1NSw2IEBAIGJvb2wgQ2hl Y2tlcjo6YXNzaWduVHlwZXMoKQogICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgIH0KIAot ICAgIGZvciAoYXV0byYga2V5VmFsdWVQYWlyIDogbV90eXBlQW5ub3RhdGlvbnMpCi0gICAgICAg IGtleVZhbHVlUGFpci5rZXktPnNldFR5cGVBbm5vdGF0aW9uKFdURk1vdmUoa2V5VmFsdWVQYWly LnZhbHVlKSk7CiAgICAgcmV0dXJuIHRydWU7CiB9CiAKQEAgLTc3OCwxMyArNzc1LDEyIEBAIGF1 dG8gQ2hlY2tlcjo6Z2V0SW5mbyhBU1Q6OkV4cHJlc3Npb24mIGV4cHJlc3Npb24sIGJvb2wgcmVx dWlyZXNMZWZ0VmFsdWUpIC0+IE9wCiAgICAgYXV0byB0eXBlSXRlcmF0b3IgPSBtX3R5cGVNYXAu ZmluZCgmZXhwcmVzc2lvbik7CiAgICAgQVNTRVJUKHR5cGVJdGVyYXRvciAhPSBtX3R5cGVNYXAu ZW5kKCkpOwogCi0gICAgYXV0byB0eXBlQW5ub3RhdGlvbkl0ZXJhdG9yID0gbV90eXBlQW5ub3Rh dGlvbnMuZmluZCgmZXhwcmVzc2lvbik7Ci0gICAgQVNTRVJUKHR5cGVBbm5vdGF0aW9uSXRlcmF0 b3IgIT0gbV90eXBlQW5ub3RhdGlvbnMuZW5kKCkpOwotICAgIGlmIChyZXF1aXJlc0xlZnRWYWx1 ZSAmJiB0eXBlQW5ub3RhdGlvbkl0ZXJhdG9yLT52YWx1ZS5pc1JpZ2h0VmFsdWUoKSkgeworICAg IGNvbnN0IGF1dG8mIHR5cGVBbm5vdGF0aW9uID0gZXhwcmVzc2lvbi50eXBlQW5ub3RhdGlvbigp OworICAgIGlmIChyZXF1aXJlc0xlZnRWYWx1ZSAmJiB0eXBlQW5ub3RhdGlvbi5pc1JpZ2h0VmFs dWUoKSkgewogICAgICAgICBzZXRFcnJvcigpOwogICAgICAgICByZXR1cm4gV1RGOjpudWxsb3B0 OwogICAgIH0KLSAgICByZXR1cm4ge3sgKnR5cGVJdGVyYXRvci0+dmFsdWUsIHR5cGVBbm5vdGF0 aW9uSXRlcmF0b3ItPnZhbHVlIH19OworICAgIHJldHVybiB7eyAqdHlwZUl0ZXJhdG9yLT52YWx1 ZSwgdHlwZUFubm90YXRpb24gfX07CiB9CiAKIHZvaWQgQ2hlY2tlcjo6dmlzaXQoQVNUOjpWYXJp YWJsZURlY2xhcmF0aW9uJiB2YXJpYWJsZURlY2xhcmF0aW9uKQpAQCAtODA4LDE2ICs4MDQsMTQg QEAgdm9pZCBDaGVja2VyOjphc3NpZ25UeXBlKEFTVDo6RXhwcmVzc2lvbiYgZXhwcmVzc2lvbiwg VW5pcXVlUmVmPEFTVDo6VW5uYW1lZFR5cGUKIHsKICAgICBhdXRvIGFkZFJlc3VsdCA9IG1fdHlw ZU1hcC5hZGQoJmV4cHJlc3Npb24sIHN0ZDo6bWFrZV91bmlxdWU8UmVzb2x2aW5nVHlwZT4oV1RG TW92ZSh1bm5hbWVkVHlwZSkpKTsKICAgICBBU1NFUlRfVU5VU0VEKGFkZFJlc3VsdCwgYWRkUmVz dWx0LmlzTmV3RW50cnkpOwotICAgIGF1dG8gdHlwZUFubm90YXRpb25BZGRSZXN1bHQgPSBtX3R5 cGVBbm5vdGF0aW9ucy5hZGQoJmV4cHJlc3Npb24sIFdURk1vdmUodHlwZUFubm90YXRpb24pKTsK LSAgICBBU1NFUlRfVU5VU0VEKHR5cGVBbm5vdGF0aW9uQWRkUmVzdWx0LCB0eXBlQW5ub3RhdGlv bkFkZFJlc3VsdC5pc05ld0VudHJ5KTsKKyAgICBleHByZXNzaW9uLnNldFR5cGVBbm5vdGF0aW9u KFdURk1vdmUodHlwZUFubm90YXRpb24pKTsKIH0KIAogdm9pZCBDaGVja2VyOjphc3NpZ25UeXBl KEFTVDo6RXhwcmVzc2lvbiYgZXhwcmVzc2lvbiwgUmVmUHRyPFJlc29sdmFibGVUeXBlUmVmZXJl bmNlPiYmIHJlc29sdmFibGVUeXBlUmVmZXJlbmNlLCBBU1Q6OlR5cGVBbm5vdGF0aW9uIHR5cGVB bm5vdGF0aW9uID0gQVNUOjpSaWdodFZhbHVlKCkpCiB7CiAgICAgYXV0byBhZGRSZXN1bHQgPSBt X3R5cGVNYXAuYWRkKCZleHByZXNzaW9uLCBzdGQ6Om1ha2VfdW5pcXVlPFJlc29sdmluZ1R5cGU+ KFdURk1vdmUocmVzb2x2YWJsZVR5cGVSZWZlcmVuY2UpKSk7CiAgICAgQVNTRVJUX1VOVVNFRChh ZGRSZXN1bHQsIGFkZFJlc3VsdC5pc05ld0VudHJ5KTsKLSAgICBhdXRvIHR5cGVBbm5vdGF0aW9u QWRkUmVzdWx0ID0gbV90eXBlQW5ub3RhdGlvbnMuYWRkKCZleHByZXNzaW9uLCBXVEZNb3ZlKHR5 cGVBbm5vdGF0aW9uKSk7Ci0gICAgQVNTRVJUX1VOVVNFRCh0eXBlQW5ub3RhdGlvbkFkZFJlc3Vs dCwgdHlwZUFubm90YXRpb25BZGRSZXN1bHQuaXNOZXdFbnRyeSk7CisgICAgZXhwcmVzc2lvbi5z ZXRUeXBlQW5ub3RhdGlvbihXVEZNb3ZlKHR5cGVBbm5vdGF0aW9uKSk7CiB9CiAKIHZvaWQgQ2hl Y2tlcjo6Zm9yd2FyZFR5cGUoQVNUOjpFeHByZXNzaW9uJiBleHByZXNzaW9uLCBSZXNvbHZpbmdU eXBlJiByZXNvbHZpbmdUeXBlLCBBU1Q6OlR5cGVBbm5vdGF0aW9uIHR5cGVBbm5vdGF0aW9uID0g QVNUOjpSaWdodFZhbHVlKCkpCkBAIC04MjksOCArODIzLDcgQEAgdm9pZCBDaGVja2VyOjpmb3J3 YXJkVHlwZShBU1Q6OkV4cHJlc3Npb24mIGV4cHJlc3Npb24sIFJlc29sdmluZ1R5cGUmIHJlc29s dmluZ1QKICAgICAgICAgYXV0byBhZGRSZXN1bHQgPSBtX3R5cGVNYXAuYWRkKCZleHByZXNzaW9u LCBzdGQ6Om1ha2VfdW5pcXVlPFJlc29sdmluZ1R5cGU+KHJlc3VsdC5jb3B5UmVmKCkpKTsKICAg ICAgICAgQVNTRVJUX1VOVVNFRChhZGRSZXN1bHQsIGFkZFJlc3VsdC5pc05ld0VudHJ5KTsKICAg ICB9KSk7Ci0gICAgYXV0byB0eXBlQW5ub3RhdGlvbkFkZFJlc3VsdCA9IG1fdHlwZUFubm90YXRp b25zLmFkZCgmZXhwcmVzc2lvbiwgV1RGTW92ZSh0eXBlQW5ub3RhdGlvbikpOwotICAgIEFTU0VS VF9VTlVTRUQodHlwZUFubm90YXRpb25BZGRSZXN1bHQsIHR5cGVBbm5vdGF0aW9uQWRkUmVzdWx0 LmlzTmV3RW50cnkpOworICAgIGV4cHJlc3Npb24uc2V0VHlwZUFubm90YXRpb24oV1RGTW92ZSh0 eXBlQW5ub3RhdGlvbikpOwogfQogCiB2b2lkIENoZWNrZXI6OnZpc2l0KEFTVDo6QXNzaWdubWVu dEV4cHJlc3Npb24mIGFzc2lnbm1lbnRFeHByZXNzaW9uKQpAQCAtODM5LDExICs4MzIsNiBAQCB2 b2lkIENoZWNrZXI6OnZpc2l0KEFTVDo6QXNzaWdubWVudEV4cHJlc3Npb24mIGFzc2lnbm1lbnRF eHByZXNzaW9uKQogICAgIGlmICghbGVmdEluZm8pCiAgICAgICAgIHJldHVybjsKIAotICAgIGlm IChsZWZ0SW5mby0+dHlwZUFubm90YXRpb24uaXNSaWdodFZhbHVlKCkpIHsKLSAgICAgICAgc2V0 RXJyb3IoKTsKLSAgICAgICAgcmV0dXJuOwotICAgIH0KLQogICAgIGF1dG8gcmlnaHRJbmZvID0g cmVjdXJzZUFuZEdldEluZm8oYXNzaWdubWVudEV4cHJlc3Npb24ucmlnaHQoKSk7CiAgICAgaWYg KCFyaWdodEluZm8pCiAgICAgICAgIHJldHVybjsK