RecordCallTarget() on x64 needs to be more careful in checking for an ElementsKind in the call cell. (issue 14651002)

2 views
Skip to first unread message

mvst...@chromium.org

unread,
Apr 30, 2013, 9:37:48 AM4/30/13
to da...@chromium.org, v8-...@googlegroups.com
Reviewers: danno,

Message:
Hi Danno, could you have a look? I do a second compare if the low dword is
0.

Description:
RecordCallTarget() on x64 needs to be more careful in checking for an
ElementsKind in the call cell.

BUG=

Please review this at https://codereview.chromium.org/14651002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
M src/x64/code-stubs-x64.cc


Index: src/x64/code-stubs-x64.cc
diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index
3a9a0234e2e8ed1d1f511687c6f26a493aef21f3..77568feb38cad6bd23686d9975865c6510ca8a21
100644
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -3818,8 +3818,16 @@ static void GenerateRecordCallTarget(MacroAssembler*
masm) {
Handle<Object> terminal_kind_sentinel =
TypeFeedbackCells::MonomorphicArraySentinel(isolate,
LAST_FAST_ELEMENTS_KIND);
- __ Cmp(rcx, terminal_kind_sentinel);
+ ASSERT(terminal_kind_sentinel->IsSmi());
+ int terminal_kind_value =
Handle<Smi>::cast(terminal_kind_sentinel)->value();
+ // Because of the way Smis are encoded on x64, we have to be careful in
+ // comparisons.
+ __ cmpl(rcx, Immediate(0));
+ __ j(not_equal, &miss);
+ __ SmiToInteger32(rcx, rcx);
+ __ cmpl(rcx, Immediate(terminal_kind_value));
__ j(above, &miss);
+
// Make sure the function is the Array() function
__ LoadArrayFunction(rcx);
__ cmpq(rdi, rcx);


da...@chromium.org

unread,
May 3, 2013, 9:30:09 AM5/3/13
to mvst...@chromium.org, v8-...@googlegroups.com
lgtm if you address comments.


https://codereview.chromium.org/14651002/diff/1/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/14651002/diff/1/src/x64/code-stubs-x64.cc#newcode3822
src/x64/code-stubs-x64.cc:3822: int terminal_kind_value =
Handle<Smi>::cast(terminal_kind_sentinel)->value();
You don't need this any more.

https://codereview.chromium.org/14651002/diff/1/src/x64/code-stubs-x64.cc#newcode3825
src/x64/code-stubs-x64.cc:3825: __ cmpl(rcx, Immediate(0));
__ JumpIfNotSmi(rcx, &miss);
__ Cmp(rcx, terminal_kind_sentinel);
__ j(above, &miss);

https://codereview.chromium.org/14651002/

mvst...@chromium.org

unread,
May 3, 2013, 11:19:43 AM5/3/13
to da...@chromium.org, v8-...@googlegroups.com
Thanks, folded this in with another CL that Ulan looked at:
https://codereview.chromium.org/14803005/

https://codereview.chromium.org/14651002/
Reply all
Reply to author
Forward
0 new messages