216110 2020-09-02 17:33:26 -0700 Finish comment describing the various *Stack SSA nodes in DFG 2020-09-03 14:44:16 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 keith_miller keith_miller ews-watchlist mark.lam msaboff saam sam tzagallo webkit-bug-importer oldest_to_newest 1685076 0 keith_miller 2020-09-02 17:33:26 -0700 Finish comment describing the various *Stack SSA nodes in DFG 1685077 1 407839 keith_miller 2020-09-02 17:35:29 -0700 Created attachment 407839 Patch 1685336 2 ews-feeder 2020-09-03 10:39:12 -0700 Committed r266532: <https://trac.webkit.org/changeset/266532> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407839. 1685337 3 webkit-bug-importer 2020-09-03 10:40:18 -0700 <rdar://problem/68288007> 1685395 4 407839 saam 2020-09-03 12:29:36 -0700 Comment on attachment 407839 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407839&action=review > Source/JavaScriptCore/dfg/DFGNodeType.h:79 > + /* exits are rare this is preferable to representing availability directly in SSA. */\ why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake. 1685396 5 407839 keith_miller 2020-09-03 12:35:27 -0700 Comment on attachment 407839 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407839&action=review >> Source/JavaScriptCore/dfg/DFGNodeType.h:79 >> + /* exits are rare this is preferable to representing availability directly in SSA. */\ > > why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake. I don't think that's what this comment says but maybe it could be more explicit. 1685415 6 saam 2020-09-03 13:49:57 -0700 (In reply to Keith Miller from comment #5) > Comment on attachment 407839 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407839&action=review > > >> Source/JavaScriptCore/dfg/DFGNodeType.h:79 > >> + /* exits are rare this is preferable to representing availability directly in SSA. */\ > > > > why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake. > > I don't think that's what this comment says but maybe it could be more > explicit. "this is preferable to representing availability directly in SSA" is not right. 1685416 7 saam 2020-09-03 13:50:25 -0700 (In reply to Saam Barati from comment #6) > (In reply to Keith Miller from comment #5) > > Comment on attachment 407839 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=407839&action=review > > > > >> Source/JavaScriptCore/dfg/DFGNodeType.h:79 > > >> + /* exits are rare this is preferable to representing availability directly in SSA. */\ > > > > > > why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake. > > > > I don't think that's what this comment says but maybe it could be more > > explicit. > > "this is preferable to representing availability directly in SSA" is not > right. Maybe you mean if both things were already the case? Like we happened to have a spill for another reason? 1685447 8 407839 keith_miller 2020-09-03 14:44:16 -0700 Comment on attachment 407839 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407839&action=review >>>>> Source/JavaScriptCore/dfg/DFGNodeType.h:79 >>>>> + /* exits are rare this is preferable to representing availability directly in SSA. */\ >>>> >>>> why? Storing to the stack isn't free. If something is in a register, you don't also want to throw it onto the stack just for OSRs sake. >>> >>> I don't think that's what this comment says but maybe it could be more explicit. >> >> "this is preferable to representing availability directly in SSA" is not right. > > Maybe you mean if both things were already the case? Like we happened to have a spill for another reason? Correct, I'm saying that the PutStacks are used to represent shadow data flow. We don't necessarily want B3 spend time register allocating that data if we could get it from the stack. That comment wasn't trying to say it's preferable to PutStack every OSR exit value. 407839 2020-09-02 17:35:29 -0700 2020-09-03 10:39:12 -0700 Patch bug-216110-20200902173528.patch text/plain 1645 keith_miller U3VidmVyc2lvbiBSZXZpc2lvbjogMjY2NDk2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBm NzA1YjdmYjEwZDZlOTk4MWIwZWY4NGM5MmZhNWM1NGYwMGU1YjY1Li5hYTg5MWZhNGE4OTAyOWY5 ZjkzM2IzNGI0NmRmNjBlZDcxYjU1MGQ2IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMiBAQAorMjAyMC0wOS0wMiAgS2VpdGggTWlsbGVyICA8a2VpdGhfbWlsbGVyQGFwcGxl LmNvbT4KKworICAgICAgICBGaW5pc2ggY29tbWVudCBkZXNjcmliaW5nIHRoZSB2YXJpb3VzICpT dGFjayBTU0Egbm9kZXMgaW4gREZHCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0yMTYxMTAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAqIGRmZy9ERkdOb2RlVHlwZS5oOgorCiAyMDIwLTA5LTAyICBNaWNoYWVs IFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgogCiAgICAgICAgIEFTU0VSVElPTiBGQUlMRUQ6 IHZhbHVlLmlzQ2VsbCgpICYmIHZhbHVlLmFzQ2VsbCgpLT50eXBlKCkgPT0gQ3VzdG9tR2V0dGVy U2V0dGVyVHlwZSAuL2J5dGVjb2RlL09iamVjdFByb3BlcnR5Q29uZGl0aW9uU2V0LmNwcApkaWZm IC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdOb2RlVHlwZS5oIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL2RmZy9ERkdOb2RlVHlwZS5oCmluZGV4IGQ2ODgxNWQzMWUzNjAwMzQ0 M2YxMzVkNWIzOWRjYjUyY2Q5MDIyMGUuLjY5NjIxOWM4ZGYxMWQzMzZkYzcwZjlhNGZhODBkOGI4 NTM0MjhiMGEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHTm9kZVR5 cGUuaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR05vZGVUeXBlLmgKQEAgLTc0 LDcgKzc0LDkgQEAgbmFtZXNwYWNlIEpTQyB7IG5hbWVzcGFjZSBERkcgewogICAgIG1hY3JvKEdl dExvY2FsLCBOb2RlUmVzdWx0SlMgfCBOb2RlTXVzdEdlbmVyYXRlKSBcCiAgICAgbWFjcm8oU2V0 TG9jYWwsIDApIFwKICAgICBcCi0gICAgLyogVGhlc2UgYXJlIHVzZWQgaW4gU1NBIGZvcm0gdG8g cmVwcmVzZW50IHRvIHRyYWNrICovXAorICAgIC8qIFRoZXNlIGFyZSB1c2VkIGluIFNTQSBmb3Jt IHRvIHJlcHJlc2VudCBPU1IgYXZhaWxhYmlsaXR5IG9uIHRoZSBzdGFjay4gQXMgbG9uZyBhcyBh IGxvY2FsIGlzICovXAorICAgIC8qIGF2YWlsYWJsZSBvbiB0aGUgc3RhY2sgZnJvbSBhbGwgcGF0 aHMgaW50byBhIG1lcmdlIHBvaW50IHdlIGRvbid0IG5lZWQgdG8gaGF2ZSBhIFBoaS4gU2luY2Ug T1NSICovXAorICAgIC8qIGV4aXRzIGFyZSByYXJlIHRoaXMgaXMgcHJlZmVyYWJsZSB0byByZXBy ZXNlbnRpbmcgYXZhaWxhYmlsaXR5IGRpcmVjdGx5IGluIFNTQS4gKi9cCiAgICAgbWFjcm8oUHV0 U3RhY2ssIE5vZGVNdXN0R2VuZXJhdGUpIFwKICAgICBtYWNybyhLaWxsU3RhY2ssIE5vZGVNdXN0 R2VuZXJhdGUpIFwKICAgICBtYWNybyhHZXRTdGFjaywgTm9kZVJlc3VsdEpTKSBcCg==