Bug 161724

Summary: Add support for a ternary sub32 emitter for ARM64 and 32-bit ARM.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, jfbastien, keith_miller, msaboff, saam
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
fpizlo: review+
proposed patch with test added and feedback applied. fpizlo: review+

Mark Lam
Reported 2016-09-07 17:47:30 PDT
We can use this to fix the FIXME in emitAllocateWithNonNullAllocator().
Attachments
proposed patch. (5.54 KB, patch)
2016-09-07 18:11 PDT, Mark Lam
fpizlo: review+
proposed patch with test added and feedback applied. (9.26 KB, patch)
2016-09-08 13:03 PDT, Mark Lam
fpizlo: review+
Mark Lam
Comment 1 2016-09-07 18:11:46 PDT
Created attachment 288217 [details] proposed patch.
Mark Lam
Comment 2 2016-09-07 18:15:35 PDT
Looking into how to write a test for this now.
Filip Pizlo
Comment 3 2016-09-07 18:17:29 PDT
Comment on attachment 288217 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=288217&action=review You ought to be able to write a test in testb3 that checks that we are now using the 3-operand sub. > Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:568 > + NO_RETURN_DUE_TO_CRASH void sub32(RegisterID, RegisterID, RegisterID) > + { > + RELEASE_ASSERT_NOT_REACHED(); > + } > + Hmm. I think we have an opportunity to make live a lot easier for ourselves if we implement this. Here's a possible impl: sub32(a, b, c) { // Implement c = a - b if (c == b) { neg32(c); add32(a, c); return; } move(a, c); sub32(b, c); }
Mark Lam
Comment 4 2016-09-08 13:03:26 PDT
Created attachment 288310 [details] proposed patch with test added and feedback applied.
Mark Lam
Comment 5 2016-09-08 13:08:05 PDT
Thanks for the review. Landed in r205656: <http://trac.webkit.org/r205656>.
Note You need to log in before you can comment on or make changes to this bug.