Reviewers: Jakob,
Message:
I've explored new dimensions of tedium, and invite you to do the same :).
PTAL,
--Michael
https://codereview.chromium.org/356713003/diff/60001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/356713003/diff/60001/src/ia32/full-codegen-ia32.cc#newcode1421
src/ia32/full-codegen-ia32.cc:1421: // Use inline caching. Variable name
is passed in ecx and the global
On 2014/06/25 10:59:12, Jakob wrote:
nit: outdated comment. I think you can just delete it, it doesn't
really provide
insight.
Done.
https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc
File src/ia32/ic-ia32.cc (right):
https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newcode960
src/ia32/ic-ia32.cc:960: ASSERT(edx.is(ReceiverRegister()));
On 2014/06/25 10:59:12, Jakob wrote:
I'm not too happy with these ASSERTs, as opposed to using
ReceiverRegister()
instead of raw "edx" below. But I concede that it's hard to decide
where to draw
the line. I can live with the ASSERT, at least for now.
Okay, yep. I'm cautious because when code starts mentioned 4, 5 or 6
registers within a few lines I feel like the reader is best served by
knowing what those registers are. It's extra cognitive load to remember
"which one is ReceiverRegister()? Oh yeah." That is, the reader will
actually turn it back into a register.
Plus when you consider that one of the IC spec registers plays another
role (say, it's the accumulator and a call is made), then you really
have to be careful against obscuring the plain old register name.
At any rate, I'll be back in this code in order to unify LoadIC and
KeyedLoadIC register specs. I'll pay the price for any failure to fully
push through a useful abstraction! And in doing that work I'll push the
abstraction deeper wherever possible.
https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newcode983
src/ia32/ic-ia32.cc:983: static const Register LoadIC_TempRegister() {
return ebx; }
On 2014/06/25 10:59:12, Jakob wrote:
As discussed, I think medium-term we want to define this globally like
ReceiverRegister() and friends. But for now this is OK.
Okay, thanks. I will keep it this way for now. I'm reluctant to elevate
temp registers as a concept for the platform independent file to be
concerned with. Lets keep playing the refactoring out and the right home
for these will become clearer.
https://codereview.chromium.org/356713003/diff/60001/src/ia32/ic-ia32.cc#newcode988
src/ia32/ic-ia32.cc:988: // -- ReceiverRegister() : receiver
On 2014/06/25 10:59:12, Jakob wrote:
Arguably this entire comment block could now be reduced to:
// Return address is on the stack (esp[0]).
But I'm fine with keeping it.
It's a super-redundant comment, so I'll take it out. :D
Description:
Use IC register definitions in platform files.
Please review this at https://codereview.chromium.org/356713003/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+394, -400 lines):
M src/arm/code-stubs-arm.cc
M src/arm/debug-arm.cc
M src/arm/full-codegen-arm.cc
M src/arm/ic-arm.cc
M src/arm/lithium-arm.cc
M src/arm/lithium-codegen-arm.cc
M src/arm/stub-cache-arm.cc
M src/arm64/code-stubs-arm64.cc
M src/arm64/debug-arm64.cc
M src/arm64/full-codegen-arm64.cc
M src/arm64/ic-arm64.cc
M src/arm64/lithium-arm64.cc
M src/arm64/lithium-codegen-arm64.cc
M src/arm64/stub-cache-arm64.cc
M src/code-stubs.cc
M src/ia32/code-stubs-ia32.cc
M src/ia32/debug-ia32.cc
M src/ia32/full-codegen-ia32.cc
M src/ia32/ic-ia32.cc
M src/ia32/lithium-codegen-ia32.cc
M src/ia32/lithium-ia32.cc
M src/ia32/stub-cache-ia32.cc
M src/x64/code-stubs-x64.cc
M src/x64/debug-x64.cc
M src/x64/full-codegen-x64.cc
M src/x64/ic-x64.cc
M src/x64/lithium-codegen-x64.cc
M src/x64/lithium-x64.cc
M src/x64/stub-cache-x64.cc
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
