ToThis is rarely overridden. And toThis functions are very limited. We can check toThis function in AI and perform appropriate folding.
Created attachment 321048 [details] Patch
Comment on attachment 321048 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321048&action=review r=me Can you also add a test where we change globalThisValue after we've already compiled the code, that way we ensure that we're not trying to completely constant fold the result of GetGlobalThis? > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:174 > + bool allStructureIsScope = !valueForNode.m_structure.isClear(); let's name this: allStructuresAreScope or allStructuresAreJSScope > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:188 > + if (!(type.isObject() && type.overridesToThis() && structure->classInfo()->methodTable.toThis == JSScope::info()->methodTable.toThis)) Why the three conditions? Doesn't (structure->classInfo()->methodTable.toThis == JSScope::info()->methodTable.toThis) suffice for your needs? I'd maybe just write this: ``allStructuresAreScope &= structure->classInfo()->methodTable.toThis == JSScope::info()->methodTable.toThis``
Comment on attachment 321048 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321048&action=review I think testing it is difficult... Currently, we have two chances to see this issue, 1. JSC APIs JSC APIs allows us to resetPrototype, which changes GlobalThis. 2. Touching JSGlobalObject in DFG before calling finishCreation() If JSGlobalObject's initialization / JSDOMGlobalObject (and other childrens) invoke DFG in its initialization sequence, we may see that globalThis with nullptr. I think testing them is difficult. I think (1) should be removed because JSGlobalObject is now immutable prototype exotic object. And seconds thing relies on initialization of JSGlobalObject (and children). I don't think we should restrict DFG invocation in JSGlobalObject initialization: it is useful to use JS things if some critical part of JSGlobalObject is initialized. Currently, globalThis is set after all the initialization finishes. We eventually want to remove this replacing globalThis feature from JSC. >> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:174 >> + bool allStructureIsScope = !valueForNode.m_structure.isClear(); > > let's name this: allStructuresAreScope or allStructuresAreJSScope Changed it to allStructuresAreJSScope. >> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:188 >> + if (!(type.isObject() && type.overridesToThis() && structure->classInfo()->methodTable.toThis == JSScope::info()->methodTable.toThis)) > > Why the three conditions? Doesn't (structure->classInfo()->methodTable.toThis == JSScope::info()->methodTable.toThis) suffice for your needs? > I'd maybe just write this: > ``allStructuresAreScope &= structure->classInfo()->methodTable.toThis == JSScope::info()->methodTable.toThis`` Oh, nice. Fixed.
Committed r222143: <http://trac.webkit.org/changeset/222143>
<rdar://problem/34693252>