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

Yuqiang Xian
Reported 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".
Attachments
the patch (5.10 KB, patch)
2011-10-09 18:58 PDT, Yuqiang Xian 		darin: review+
darin: commit-queue-
DetailsFormatted DiffDiff
patch addressing Darin's comments (5.14 KB, patch)
2011-10-09 19:43 PDT, Yuqiang Xian 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yuqiang Xian
Comment 1 2011-10-09 18:58:53 PDT
Created attachment 110315 [details] the patch
Darin Adler
Comment 2 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.
Filip Pizlo
Comment 3 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.
Yuqiang Xian
Comment 4 2011-10-09 19:43:02 PDT
Created attachment 110319 [details] patch addressing Darin's comments
WebKit Review Bot
Comment 5 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>
WebKit Review Bot
Comment 6 2011-10-09 21:04:38 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.