Bug 161724 - Add support for a ternary sub32 emitter for ARM64 and 32-bit ARM.
Summary: Add support for a ternary sub32 emitter for ARM64 and 32-bit ARM.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-07 17:47 PDT by Mark Lam
Modified: 2016-09-08 13:08 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch. (5.54 KB, patch)
2016-09-07 18:11 PDT, Mark Lam
fpizlo: review+
Details | Formatted Diff | Diff
proposed patch with test added and feedback applied. (9.26 KB, patch)
2016-09-08 13:03 PDT, Mark Lam
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2016-09-07 17:47:30 PDT
We can use this to fix the FIXME in emitAllocateWithNonNullAllocator().
Comment 1 Mark Lam 2016-09-07 18:11:46 PDT
Created attachment 288217 [details]
proposed patch.
Comment 2 Mark Lam 2016-09-07 18:15:35 PDT
Looking into how to write a test for this now.
Comment 3 Filip Pizlo 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);
}
Comment 4 Mark Lam 2016-09-08 13:03:26 PDT
Created attachment 288310 [details]
proposed patch with test added and feedback applied.
Comment 5 Mark Lam 2016-09-08 13:08:05 PDT
Thanks for the review.  Landed in r205656: <http://trac.webkit.org/r205656>.