HasIndexedProperty should know about sane chain
Created attachment 394333 [details] Patch
Created attachment 394334 [details] Patch
Comment on attachment 394334 [details] Patch r=me
rdar://problem/59821190
Committed r258901: <https://trac.webkit.org/changeset/258901> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394334 [details].
Comment on attachment 394334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394334&action=review I've found a bug on 32-bits code path and left some additional comments. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13658 > + m_jit.isEmpty(scratchGPR, resultGPR); The name `isEmpty` looks strange to me. `HasIndexedProperty` returns true if index is present and false otherwise. Naming this function as `isEmpty` and placing result on `resultGPR` implies that the semantics of this operation is inverted, where it returns true if index is not present and false otherwise. Maybe it would be better name it as `isNotEmpty`. > Source/JavaScriptCore/jit/AssemblyHelpers.h:964 > + void isEmpty(GPRReg gpr, GPRReg dst) This looks a bit strange. The JSVALUE64 version is testing if value is not empty, while 32-bits is checking if value is empty.
Comment on attachment 394334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394334&action=review >> Source/JavaScriptCore/jit/AssemblyHelpers.h:964 >> + void isEmpty(GPRReg gpr, GPRReg dst) > > This looks a bit strange. The JSVALUE64 version is testing if value is not empty, while 32-bits is checking if value is empty. agreed. Also, your 64-bit implementation can just test against itself for zero/nonzero
(In reply to EWS from comment #5) > Committed r258901: <https://trac.webkit.org/changeset/258901> This seems to have broken 186 jsc tests on jsc-armv7 as indicated by ews (https://ews-build.webkit.org/#/builders/26/builds/12356). Can you please have a look? This is slowing down jsc-armv7 EWS.
Uploaded a patch to address Caio and Saam's comments: https://bugs.webkit.org/show_bug.cgi?id=209507