WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
204404
Regression (
r252680
): JSCOnly build broken: no matching function for call to JSC::DFG::SpeculativeJIT::jsValueResult
https://bugs.webkit.org/show_bug.cgi?id=204404
Summary
Regression (r252680): JSCOnly build broken: no matching function for call to ...
Aakash Jain
Reported
2019-11-20 03:22:42 PST
JSCOnly build seems to be broken currently (for ARMv7 and MIPS32el). From
https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/10050
: FAILED: Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/dfg/DFGSpeculativeJIT.cpp.o /home/buildbot/buildroot/buildroot/output.rpi3/host/usr/ccache/arm-buildroot-linux-gnueabihf-g++ ... -c ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compileIncOrDec(JSC::DFG::Node*)’: ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4625:37: error: no matching function for call to ‘JSC::DFG::SpeculativeJIT::jsValueResult(JSC::GPRReg, JSC::DFG::Node*&)’ jsValueResult(result.gpr(), node);
Attachments
Patch
(12.75 KB, patch)
2019-11-20 04:30 PST
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Patch
(14.10 KB, patch)
2019-11-20 04:48 PST
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Patch
(14.76 KB, patch)
2019-11-20 07:39 PST
,
Caio Lima
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Aakash Jain
Comment 1
2019-11-20 03:24:24 PST
Seems to be broken between
r252678
and
r252682
. JSCOnly Linux MIPS32el Release: failed on:
r252685
https://build.webkit.org/builders/JSCOnly%20Linux%20MIPS32el%20Release/builds/4021
passed on:
r252678
https://build.webkit.org/builders/JSCOnly%20Linux%20MIPS32el%20Release/builds/4020
JSCOnly Linux ARMv7 Thumb2 Release failed on:
r252682
https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/10050
passed on:
r252670
https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/10049
Aakash Jain
Comment 2
2019-11-20 03:32:39 PST
Almost certainly
https://trac.webkit.org/changeset/252680/webkit
That commit modified DFGSpeculativeJIT.cpp. More specifically it added jsValueResult() call inside SpeculativeJIT(). The landed patch wasn't run through EWS, and EWS had few complaints about previous patches in that bug (e.g.: JSC EWS complained about failing JSC tests). Unfortunately JSCOnly EWS from 'old EWS' wasn't run on this patch as this queue was behind.
Caio Lima
Comment 3
2019-11-20 04:04:13 PST
There are 2 Patches that broke builds
https://trac.webkit.org/changeset/252684
and
https://trac.webkit.org/changeset/252680
The fix to
https://trac.webkit.org/changeset/252680
is trivial, but I'm working on a proper fix to
https://trac.webkit.org/changeset/252684
in
https://bugs.webkit.org/show_bug.cgi?id=204082
. I'll try to fix both issues there. If it takes longer than expected, I'll create a patch to fix it ASAP.
Caio Lima
Comment 4
2019-11-20 04:30:24 PST
Created
attachment 383955
[details]
Patch It fixes the build issues, but it won't have desired behavior until we fix
https://bugs.webkit.org/show_bug.cgi?id=204082
Caio Lima
Comment 5
2019-11-20 04:34:12 PST
(In reply to Caio Lima from
comment #4
)
> Created
attachment 383955
[details]
> Patch > > It fixes the build issues, but it won't have desired behavior until we fix >
https://bugs.webkit.org/show_bug.cgi?id=204082
I uploaded the wrong diff.
Caio Lima
Comment 6
2019-11-20 04:48:18 PST
Created
attachment 383956
[details]
Patch
Aakash Jain
Comment 7
2019-11-20 05:53:53 PST
rs=me
Caio Lima
Comment 8
2019-11-20 07:39:28 PST
Created
attachment 383963
[details]
Patch
Saam Barati
Comment 9
2019-11-20 08:14:43 PST
Comment on
attachment 383963
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=383963&action=review
r=me. I’ll let you decide if you want to wait on having the baseline use the IC.
> Source/JavaScriptCore/bytecode/AccessCase.cpp:1154 > + jit.loadValue(CCallHelpers::BaseIndex(scratchGPR, scratch2GPR, CCallHelpers::TimesEight), JSValueRegs::payloadOnly(scratchGPR));
This won’t work for 32-bit. Is the goal just to get it to compile?
> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:-149 > - callOperation(operationGetByVal, dst, m_codeBlock->globalObject(), JSValueRegs(regT1, regT0), JSValueRegs(regT3, regT2));
Why not wait to do this until it all works?
Caio Lima
Comment 10
2019-11-20 09:04:08 PST
Comment on
attachment 383963
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=383963&action=review
Thank you very much for the review!
>> Source/JavaScriptCore/bytecode/AccessCase.cpp:1154 >> + jit.loadValue(CCallHelpers::BaseIndex(scratchGPR, scratch2GPR, CCallHelpers::TimesEight), JSValueRegs::payloadOnly(scratchGPR)); > > This won’t work for 32-bit. Is the goal just to get it to compile?
Yes. I already fixed it to 32-bit locally on GetByVal IC changes.
>> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:-149 >> - callOperation(operationGetByVal, dst, m_codeBlock->globalObject(), JSValueRegs(regT1, regT0), JSValueRegs(regT3, regT2)); > > Why not wait to do this until it all works?
This also caused a compilation error, since it requires we move `operationGetByVal` out of "DFGOperations.h" or include this header in this file. I think that falling back to slowpath always is a better solution to make it compile.
WebKit Commit Bot
Comment 11
2019-11-20 09:48:46 PST
Comment on
attachment 383963
[details]
Patch Clearing flags on attachment: 383963 Committed
r252690
: <
https://trac.webkit.org/changeset/252690
>
WebKit Commit Bot
Comment 12
2019-11-20 09:48:48 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 13
2019-11-20 09:49:18 PST
<
rdar://problem/57362597
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug