Bug 172383

Summary: DFG::SpeculativeJIT::pickCanTrample() is wrongly ignoring result registers.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, fpizlo, jfbastien, keith_miller, msaboff, ryanhaddad, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
fpizlo: review+
Patch for re-landing. none

Mark Lam
Reported 2017-05-19 14:15:38 PDT
pickCanTrample() is wrongly assuming that one of regT0 and regT1 is always available as a scratch register. This assumption is wrong if this canTrample register is used for a silentFill() after an operation that returns a result in regT0 or regT1. Turns out the only reason we need the canTrample register is for SetDoubleConstant. We can remove the need for this canTrample register by introducing a moveDouble() pseudo instruction in the MacroAssembler to do the job using the scratchRegister() on X86_64 or the dataMemoryTempRegister() on ARM64. In so doing, we can simplify the silentFill() code and eliminate the bug. <rdar://problem/31418651>
Attachments
proposed patch. (30.01 KB, patch)
2017-05-19 14:26 PDT, Mark Lam 		fpizlo: review+
DetailsFormatted DiffDiff
Patch for re-landing. (29.99 KB, patch)
2017-05-19 17:49 PDT, Mark Lam 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2017-05-19 14:26:41 PDT
Created attachment 310700 [details] proposed patch.
Mark Lam
Comment 2 2017-05-19 15:25:54 PDT
Thanks for the review. Landed in r217156: <http://trac.webkit.org/r217156>.
Ryan Haddad
Comment 3 2017-05-19 16:25:15 PDT
Reverted r217156 for reason: This change broke the iOS build. Committed r217169: <http://trac.webkit.org/changeset/217169>
Mark Lam
Comment 4 2017-05-19 17:49:11 PDT
Created attachment 310735 [details] Patch for re-landing.
Mark Lam
Comment 5 2017-05-19 17:59:05 PDT
Comment on attachment 310735 [details] Patch for re-landing. View in context: https://bugs.webkit.org/attachment.cgi?id=310735&action=review > Source/JavaScriptCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + Will remove this before landing.
Mark Lam
Comment 6 2017-05-19 18:00:17 PDT
Landed in r217179: <http://trac.webkit.org/r217179>.
Note You need to log in before you can comment on or make changes to this bug.