Bug 69734

Summary: Improve Null or Undefined test in 32_64 DFG
Product: WebKit Reporter: Yuqiang Xian <yuqiang.xian>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, barraclough, fpizlo, nagar28496, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch
darin: review+, darin: commit-queue-
patch addressing Darin's comments none

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.