Reviewers: Rodolph Perfetta (ARM), Alexandre Rames,
Description:
A64: Implement Smi support in MathAbs
BUG=
Please review this at https://codereview.chromium.org/141593005/
SVN Base: https://v8.googlecode.com/svn/branches/experimental/a64
Affected files (+18, -40 lines):
M src/a64/lithium-a64.cc
M src/a64/lithium-codegen-a64.cc
M src/a64/macro-assembler-a64.h
M src/a64/macro-assembler-a64.cc
M src/a64/stub-cache-a64.cc
M test/cctest/test-assembler-a64.cc
Index: src/a64/lithium-a64.cc
diff --git a/src/a64/lithium-a64.cc b/src/a64/lithium-a64.cc
index
d344b130159b01502124930f5b187952e88d5ca7..1b82b33962deec3a4b7dae5ed5ed6d43df1e1dbb
100644
--- a/src/a64/lithium-a64.cc
+++ b/src/a64/lithium-a64.cc
@@ -2377,8 +2377,7 @@ LInstruction*
LChunkBuilder::DoUnaryMathOperation(HUnaryMathOperation* instr) {
switch (instr->op()) {
case kMathAbs: {
Representation r = instr->representation();
- // TODO(jbramley): Support smis in LMathAbs and use that.
- if (r.IsTagged() || r.IsSmi()) {
+ if (r.IsTagged()) {
// The tagged case might need to allocate a HeapNumber for the
result,
// so it is handled by a separate LInstruction.
LOperand* input = UseRegister(instr->value());
@@ -2395,9 +2394,9 @@ LInstruction*
LChunkBuilder::DoUnaryMathOperation(HUnaryMathOperation* instr) {
// The Double case can never fail so it doesn't need an
environment.
return DefineAsRegister(result);
} else {
- ASSERT(r.IsInteger32());
- // The Integer32 case needs an environment because it can
deoptimize
- // on INT_MIN.
+ ASSERT(r.IsInteger32() || r.IsSmi());
+ // The Integer32 and Smi cases need an environment because they
can
+ // deoptimize on minimum representable number.
return AssignEnvironment(DefineAsRegister(result));
}
}
Index: src/a64/lithium-codegen-a64.cc
diff --git a/src/a64/lithium-codegen-a64.cc b/src/a64/lithium-codegen-a64.cc
index
48a0e902cb61538ac2e688f120880fb1a613b5a6..c85de4d5f2ee734d2c917fa724ae0f8a5fab309d
100644
--- a/src/a64/lithium-codegen-a64.cc
+++ b/src/a64/lithium-codegen-a64.cc
@@ -3492,10 +3492,11 @@ void LCodeGen::DoMathAbs(LMathAbs* instr) {
DoubleRegister input = ToDoubleRegister(instr->value());
DoubleRegister result = ToDoubleRegister(instr->result());
__ Fabs(result, input);
- } else {
- ASSERT(r.IsInteger32());
- Register input = ToRegister32(instr->value());
- Register result = ToRegister32(instr->result());
+ } else if (r.IsSmi() || r.IsInteger32()) {
+ Register input = r.IsSmi() ? ToRegister(instr->value())
+ : ToRegister32(instr->value());
+ Register result = r.IsSmi() ? ToRegister(instr->result())
+ : ToRegister32(instr->result());
Label done;
__ Abs(result, input, NULL, &done);
Deoptimize(instr->environment());
Index: src/a64/macro-assembler-a64.cc
diff --git a/src/a64/macro-assembler-a64.cc b/src/a64/macro-assembler-a64.cc
index
ed61a4681b356bb4287dab27e00bc4bc3f3b4197..eac900503e4ae28e3ee2e34e0bcf250a1e06fffb
100644
--- a/src/a64/macro-assembler-a64.cc
+++ b/src/a64/macro-assembler-a64.cc
@@ -522,15 +522,15 @@ void MacroAssembler::LoadStoreMacro(const
CPURegister& rt,
void MacroAssembler::Abs(const Register& rd, const Register& rm,
- Label * is_not_representable,
- Label * is_representable) {
+ Label* is_not_representable,
+ Label* is_representable) {
ASSERT(allow_macro_instructions_);
ASSERT(AreSameSizeAndType(rd, rm));
Cmp(rm, 1);
Cneg(rd, rm, lt);
- // If the comparison set the v flag, the input was the smallest value
+ // If the comparison sets the v flag, the input was the smallest value
// representable by rm, and the mathematical result of abs(rm) is not
// representable using two's complement.
if ((is_not_representable != NULL) && (is_representable != NULL)) {
@@ -1125,31 +1125,9 @@ void MacroAssembler::ThrowUncatchable(Register value,
}
-void MacroAssembler::SmiAbs(Register smi, Register scratch, Label *slow) {
- // TODO(all): There is another possible implementation of this function
- // which would consist of:
- // * Comparing the smi with 0.
- // * Performing a conditional negate (cneg).
- // * Testing if the result is still negative.
- //
- // This other implementation uses 1 more instruction but uses one of the
new
- // A64 conditional instruction and doesn't use shifted registers.
- //
- // This two versions should be profiled on real hardware as we have no
idea
- // which one will be the fastest.
- ASSERT(!AreAliased(smi, scratch));
-
- STATIC_ASSERT(kSmiTag == 0);
- STATIC_ASSERT(kSmiShift == 32);
-
- // Do bitwise not or do nothing depending on the sign of the argument.
- __ Eor(scratch, smi, Operand(smi, ASR, kXRegSize - 1));
- // Add 1 or do nothing depending on the sign of the argument.
- __ Adds(smi, scratch, Operand(smi, LSR, kXRegSize - 1));
-
- // If the result is still negative, go to the slow case.
- // This only happens for the most negative smi.
- __ B(mi, slow);
+void MacroAssembler::SmiAbs(const Register& smi, Label* slow) {
+ ASSERT(smi.Is64Bits());
+ Abs(smi, smi, slow);
}
Index: src/a64/macro-assembler-a64.h
diff --git a/src/a64/macro-assembler-a64.h b/src/a64/macro-assembler-a64.h
index
397d7bcd2ee995a78a0665fec1f03085daa01e7a..dd45e268013864cd04445b26eb33baf535b33f7f
100644
--- a/src/a64/macro-assembler-a64.h
+++ b/src/a64/macro-assembler-a64.h
@@ -753,7 +753,7 @@ class MacroAssembler : public Assembler {
// Compute the absolute value of 'smi' and leave the result in 'smi'
// register. If 'smi' is the most negative SMI, the absolute value cannot
// be represented as a SMI and a jump to 'slow' is done.
- void SmiAbs(Register smi, Register scratch, Label *slow);
+ void SmiAbs(const Register& smi, Label* slow);
inline void JumpIfSmi(Register value,
Label* smi_label,
Index: src/a64/stub-cache-a64.cc
diff --git a/src/a64/stub-cache-a64.cc b/src/a64/stub-cache-a64.cc
index
3684a5346652dd17a6a0b01a133421170e86eba7..adf2f94b0770be27bef4f31bbc4efe10c96a2f56
100644
--- a/src/a64/stub-cache-a64.cc
+++ b/src/a64/stub-cache-a64.cc
@@ -2541,7 +2541,7 @@ Handle<Code> CallStubCompiler::CompileMathAbsCall(
Label not_smi;
__ JumpIfNotSmi(arg, ¬_smi);
- __ SmiAbs(arg, x1, &slow);
+ __ SmiAbs(arg, &slow);
// Smi case done.
__ Drop(argc + 1);
__ Ret();
Index: test/cctest/test-assembler-a64.cc
diff --git a/test/cctest/test-assembler-a64.cc
b/test/cctest/test-assembler-a64.cc
index
a41c8a53d6570c516c00363b42ea55192ec1e085..954ae7b2caf94016e251e06a7b8a01e3c2441fd7
100644
--- a/test/cctest/test-assembler-a64.cc
+++ b/test/cctest/test-assembler-a64.cc
@@ -9164,7 +9164,7 @@ static void DoSmiAbsTest(int32_t value, bool
must_fail = false) {
__ Mov(x2, 0xc001c0de);
__ Mov(x1, value);
__ SmiTag(x1);
- __ SmiAbs(x1, x0, &slow);
+ __ SmiAbs(x1, &slow);
__ SmiUntag(x1);
__ B(&end);
--
--
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.
