Bug 69734 - Improve Null or Undefined test in 32_64 DFG
Summary: Improve Null or Undefined test in 32_64 DFG
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-09 18:54 PDT by Yuqiang Xian
Modified: 2019-05-02 16:23 PDT (History)
5 users (show)

See Also:

Attachments
the patch (5.10 KB, patch)
2011-10-09 18:58 PDT, Yuqiang Xian 		darin: review+
darin: commit-queue-
Details | Formatted Diff | Diff
patch addressing Darin's comments (5.14 KB, patch)
2011-10-09 19:43 PDT, Yuqiang Xian 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yuqiang Xian 2011-10-09 18:54:32 PDT 
Currently Null or Undefined value test in 32_64 DFG will check Null and Undefined tag separately and introduce one more branch. It can be improved in the way how the baseline JIT is doing - by relying on the fact that "UndefinedTag + 1 == NullTag and NullTag & 1".
Comment 1 Yuqiang Xian 2011-10-09 18:58:53 PDT 
Created attachment 110315 [details]
the patch
Comment 2 Darin Adler 2011-10-09 19:27:25 PDT 
Comment on attachment 110315 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=110315&action=review

> Source/JavaScriptCore/dfg/DFGJITCodeGenerator32_64.cpp:1178
> +        ASSERT((JSValue::UndefinedTag + 1 == JSValue::NullTag) && (JSValue::NullTag & 0x1));

This assertion is written in an oblique way and could instead be written to directly mirror what the code relies on:

    COMPILE_ASSERT((JSValue::UndefinedTag | 1) == JSValue::NullTag);

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:460
> +    ASSERT((JSValue::UndefinedTag + 1 == JSValue::NullTag) && (JSValue::NullTag & 0x1));

Ditto.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:560
> +    ASSERT((JSValue::UndefinedTag + 1 == JSValue::NullTag) && (JSValue::NullTag & 0x1));

Ditto.
Comment 3 Filip Pizlo 2011-10-09 19:35:15 PDT 
Comment on attachment 110315 [details]
the patch

Agree with Darin's comments, r=me otherwise. I'll be happy to cq+ if you can just fix that assertion.
Comment 4 Yuqiang Xian 2011-10-09 19:43:02 PDT 
Created attachment 110319 [details]
patch addressing Darin's comments
Comment 5 WebKit Review Bot 2011-10-09 21:04:34 PDT 
Comment on attachment 110319 [details]
patch addressing Darin's comments

Clearing flags on attachment: 110319

Committed r97039: <http://trac.webkit.org/changeset/97039>
Comment 6 WebKit Review Bot 2011-10-09 21:04:38 PDT 
All reviewed patches have been landed.  Closing bug.