Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(856)

Unified Diff: src/x64/macro-assembler-x64.cc

Issue 2823004: X64: Change overflow checks to only jump on overflow. (Closed)
Patch Set: Addressed review comments. Created 10 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/x64/macro-assembler-x64.cc
diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc
index 3823cadb54b519f93c9a973302df964d5809bc82..b859264c6e1e2a4d2be2182e699cc405ca94c42f 100644
--- a/src/x64/macro-assembler-x64.cc
+++ b/src/x64/macro-assembler-x64.cc
@@ -696,15 +696,13 @@ void MacroAssembler::SmiAdd(Register dst,
movq(dst, src1);
addq(dst, src2);
}
- Assert(no_overflow, "Smi addition onverflow");
+ Assert(no_overflow, "Smi addition overflow");
} else if (dst.is(src1)) {
- addq(dst, src2);
+ movq(kScratchRegister, src1);
+ addq(kScratchRegister, src2);
Label smi_result;
- j(no_overflow, &smi_result);
- // Restore src1.
- subq(src1, src2);
- jmp(on_not_smi_result);
- bind(&smi_result);
+ j(overflow, on_not_smi_result);
+ movq(dst, kScratchRegister);
} else {
movq(dst, src1);
addq(dst, src2);
@@ -727,15 +725,11 @@ void MacroAssembler::SmiSub(Register dst,
movq(dst, src1);
subq(dst, src2);
}
- Assert(no_overflow, "Smi substraction onverflow");
+ Assert(no_overflow, "Smi subtraction overflow");
} else if (dst.is(src1)) {
+ cmpq(dst, src2);
+ j(overflow, on_not_smi_result);
subq(dst, src2);
- Label smi_result;
- j(no_overflow, &smi_result);
- // Restore src1.
- addq(src1, src2);
- jmp(on_not_smi_result);
- bind(&smi_result);
} else {
movq(dst, src1);
subq(dst, src2);
@@ -757,15 +751,12 @@ void MacroAssembler::SmiSub(Register dst,
movq(dst, src1);
subq(dst, src2);
}
- Assert(no_overflow, "Smi substraction onverflow");
+ Assert(no_overflow, "Smi subtraction overflow");
} else if (dst.is(src1)) {
- subq(dst, src2);
- Label smi_result;
- j(no_overflow, &smi_result);
- // Restore src1.
- addq(src1, src2);
- jmp(on_not_smi_result);
- bind(&smi_result);
+ movq(kScratchRegister, src1);
+ subq(kScratchRegister, src2);
+ j(overflow, on_not_smi_result);
+ movq(src1, kScratchRegister);
} else {
movq(dst, src1);
subq(dst, src2);
@@ -883,12 +874,9 @@ void MacroAssembler::SmiAddConstant(Register dst,
ASSERT(!dst.is(kScratchRegister));
Move(kScratchRegister, constant);
- addq(dst, kScratchRegister);
- Label result_ok;
- j(no_overflow, &result_ok);
- subq(dst, kScratchRegister);
- jmp(on_not_smi_result);
- bind(&result_ok);
+ addq(kScratchRegister, dst);
+ j(overflow, on_not_smi_result);
+ movq(dst, kScratchRegister);
} else {
Move(dst, constant);
addq(dst, src);
@@ -910,10 +898,12 @@ void MacroAssembler::SmiSubConstant(Register dst, Register src, Smi* constant) {
} else {
// Subtract by adding the negative, to do it in two operations.
if (constant->value() == Smi::kMinValue) {
- Move(kScratchRegister, constant);
- movq(dst, src);
- subq(dst, kScratchRegister);
+ Move(dst, constant);
+ // Adding and subtracting the min-value gives the same result, it only
+ // differs on the overflow bit, which we don't check here.
+ addq(dst, src);
} else {
+ // Subtract by adding the negation.
Move(dst, Smi::FromInt(-constant->value()));
addq(dst, src);
}
@@ -931,21 +921,32 @@ void MacroAssembler::SmiSubConstant(Register dst,
}
} else if (dst.is(src)) {
ASSERT(!dst.is(kScratchRegister));
-
- Move(kScratchRegister, constant);
- subq(dst, kScratchRegister);
- Label sub_success;
- j(no_overflow, &sub_success);
- addq(src, kScratchRegister);
- jmp(on_not_smi_result);
- bind(&sub_success);
- } else {
if (constant->value() == Smi::kMinValue) {
+ // Subtracting min-value from any non-negative value will overflow.
+ // We test the non-negativeness before doing the subtraction.
+ testq(src, src);
+ j(not_sign, on_not_smi_result);
Move(kScratchRegister, constant);
- movq(dst, src);
subq(dst, kScratchRegister);
+ } else {
+ // Subtract by adding the negation.
+ Move(kScratchRegister, Smi::FromInt(-constant->value()));
+ addq(kScratchRegister, dst);
j(overflow, on_not_smi_result);
+ movq(dst, kScratchRegister);
+ }
+ } else {
+ if (constant->value() == Smi::kMinValue) {
+ // Subtracting min-value from any non-negative value will overflow.
+ // We test the non-negativeness before doing the subtraction.
+ testq(src, src);
+ j(not_sign, on_not_smi_result);
+ Move(dst, constant);
+ // Adding and subtracting the min-value gives the same result, it only
+ // differs on the overflow bit, which we don't check here.
+ addq(dst, src);
} else {
+ // Subtract by adding the negation.
Move(dst, Smi::FromInt(-(constant->value())));
addq(dst, src);
j(overflow, on_not_smi_result);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698