Committed, thx for the review!
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-codegen-ia32.cc#newcode370
src/ia32/lithium-codegen-ia32.cc:370: if
(!CpuFeatures::IsSupported(SSE2)) {
On 2013/04/09 07:37:07, danno wrote:
nit: How about putting the following 7 lines in a utility function
called
"FlushX87StackIfNecessary?"
Done.
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-codegen-ia32.cc#newcode386
src/ia32/lithium-codegen-ia32.cc:386: // Make sure the floating point
stack is either empty or has one item,
On 2013/04/09 07:37:07, danno wrote:
This code might be useful elsewhere. How about putting it into the
macro-assembler in a function called "CheckX87StackIsEmpty()"
Done with a variant: I also check if the stack has one item, so I
generalized it to VerifyX87StackDepth(uint depth), that way we can
verify any expected depth.
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-gap-resolver-ia32.cc
File src/ia32/lithium-gap-resolver-ia32.cc (right):
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-gap-resolver-ia32.cc#newcode378
src/ia32/lithium-gap-resolver-ia32.cc:378: cgen_->PopX87();
On 2013/04/09 07:37:07, danno wrote:
Slightly faster might be to just store rather that pop and store with
a push...
but it's probably not important.
Hmm, I'm looking at the x87 instructions, and all variants of the fld
instruction do something like a push, so I'd end up with two items on
the stack. I don't see an "overwrite" instruction or mode.
But agreed, this area will look different with N slots to play with
instead of just 1.
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):
https://codereview.chromium.org/13426006/diff/26001/src/ia32/lithium-ia32.cc#newcode2270
src/ia32/lithium-ia32.cc:2270: } else {
On 2013/04/09 07:37:07, danno wrote:
nit: no need for the additional scope, just do a "else if"
Done.
https://codereview.chromium.org/13426006/
--
--
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/groups/opt_out.
