danno, thanks a lot for the review. I appreciate it. I know you are quite busy
preparing Chrome 30.

I will think about the best approach to separate policy and mechanism to address
the problem that SMI macro instructions could be called by Lithium and
FullCodeGen/Stubs and make 31-bit SMI incrementally reviewed and committed.


https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/full-codegen-x64.cc
File src/x64/full-codegen-x64.cc (right):

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/full-codegen-x64.cc#newcode4454
src/x64/full-codegen-x64.cc:4454: __ SmiAddConstant(rax, rax,
Smi::FromInt(1));
I will address this in https://codereview.chromium.org/22935005/.

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.cc#newcode990
src/x64/macro-assembler-x64.cc:990: static inline Immediate
SmiToImmediate(Smi* src) {
This is what I have done originally at
https://codereview.chromium.org/21014003/patch/1/1005.

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.cc#newcode1705
src/x64/macro-assembler-x64.cc:1705: if
(wrapper.NeedsKeepSourceOperandsIntact()) {
Thanks for the recommendation. I will use that trick to avoid the
kScratchRegister usage.

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.cc#newcode1955
src/x64/macro-assembler-x64.cc:1955: const SmiInstructionWrapper&
wrapper) {
I will do that.

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.h
File src/x64/macro-assembler-x64.h (right):

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.h#newcode383
src/x64/macro-assembler-x64.h:383: virtual bool NeedsCheckMinusZero()
const = 0;
I have planned to use it for SmiMul. If the caller is in lithium, we
could avoid check for minus zero if HValue::kBailoutOnMinusZero flag is
false.

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.h#newcode388
src/x64/macro-assembler-x64.h:388: class SafeSmiInstructionWrapper :
public SmiInstructionWrapper {
I have planned to unify the interface. So the current SMI functions
which do not check overflow/minusZero could use a singleton
SafeSmiInstrucitonWrapper.

https://chromiumcodereview.appspot.com/21014003/diff/45001/src/x64/macro-assembler-x64.h#newcode428
src/x64/macro-assembler-x64.h:428: bool IsUnsafeInt(const int x);
I will change this when I gather all the SMI functions together.

https://chromiumcodereview.appspot.com/21014003/

--
--
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.

Reply via email to