Fix various JNI compiler bugs/unimplementeds.
For both x86 and arm we were under computing the outgoing argument size.
For ARM the managed double/long passing had been assumed to be following AAPCS,
however, currently we split long/doubles across R1_R2 and R3 and the stack.
Add support for this in the managed register and jni compiler code.
Add test and various other clean ups to jni compiler code.
Change-Id: I4129076d052a8bce42304f5331b71aa3ac50210f
diff --git a/src/assembler_arm.cc b/src/assembler_arm.cc
index 472d000..9f4e03f 100644
--- a/src/assembler_arm.cc
+++ b/src/assembler_arm.cc
@@ -1486,6 +1486,13 @@
StoreToOffset(kStoreWord, src.AsCoreRegister(), SP, dest.Int32Value());
}
+void Assembler::StoreSpanning(FrameOffset dest, ManagedRegister src,
+ FrameOffset in_off, ManagedRegister scratch) {
+ StoreToOffset(kStoreWord, src.AsCoreRegister(), SP, dest.Int32Value());
+ LoadFromOffset(kLoadWord, scratch.AsCoreRegister(), SP, in_off.Int32Value());
+ StoreToOffset(kStoreWord, scratch.AsCoreRegister(), SP, dest.Int32Value() + 4);
+}
+
void Assembler::CopyRef(FrameOffset dest, FrameOffset src,
ManagedRegister scratch) {
LoadFromOffset(kLoadWord, scratch.AsCoreRegister(), SP, src.Int32Value());
@@ -1581,9 +1588,23 @@
if (dest.IsCoreRegister()) {
CHECK(src.IsCoreRegister());
mov(dest.AsCoreRegister(), ShifterOperand(src.AsCoreRegister()));
+ } else if (dest.IsDRegister()) {
+ CHECK(src.IsDRegister());
+ vmovd(dest.AsDRegister(), src.AsDRegister());
+ } else if (dest.IsSRegister()) {
+ CHECK(src.IsSRegister());
+ vmovs(dest.AsSRegister(), src.AsSRegister());
} else {
- // TODO: VFP
- UNIMPLEMENTED(FATAL) << ": VFP";
+ CHECK(dest.IsRegisterPair());
+ CHECK(src.IsRegisterPair());
+ // Ensure that the first move doesn't clobber the input of the second
+ if (src.AsRegisterPairHigh() != dest.AsRegisterPairLow()) {
+ mov(dest.AsRegisterPairLow(), ShifterOperand(src.AsRegisterPairLow()));
+ mov(dest.AsRegisterPairHigh(), ShifterOperand(src.AsRegisterPairHigh()));
+ } else {
+ mov(dest.AsRegisterPairHigh(), ShifterOperand(src.AsRegisterPairHigh()));
+ mov(dest.AsRegisterPairLow(), ShifterOperand(src.AsRegisterPairLow()));
+ }
}
}
}
diff --git a/src/assembler_arm.h b/src/assembler_arm.h
index af3ce40..0eafb5f 100644
--- a/src/assembler_arm.h
+++ b/src/assembler_arm.h
@@ -432,6 +432,8 @@
void Store(FrameOffset dest, ManagedRegister src, size_t size);
void StoreRef(FrameOffset dest, ManagedRegister src);
void StoreRawPtr(FrameOffset dest, ManagedRegister src);
+ void StoreSpanning(FrameOffset dest, ManagedRegister src, FrameOffset in_off,
+ ManagedRegister scratch);
void CopyRef(FrameOffset dest, FrameOffset src, ManagedRegister scratch);
void LoadRef(ManagedRegister dest, ManagedRegister base, MemberOffset offs);
diff --git a/src/assembler_x86.h b/src/assembler_x86.h
index 1b882e0..372c61b 100644
--- a/src/assembler_x86.h
+++ b/src/assembler_x86.h
@@ -435,6 +435,10 @@
void Store(FrameOffset offs, ManagedRegister src, size_t size);
void StoreRef(FrameOffset dest, ManagedRegister src);
void StoreRawPtr(FrameOffset dest, ManagedRegister src);
+ void StoreSpanning(FrameOffset dest, ManagedRegister src, FrameOffset in_off,
+ ManagedRegister scratch) {
+ UNIMPLEMENTED(FATAL); // this case only currently exists for ARM
+ }
void CopyRef(FrameOffset dest, FrameOffset src, ManagedRegister scratch);
diff --git a/src/calling_convention.cc b/src/calling_convention.cc
index 83f7a00..0cd653b 100644
--- a/src/calling_convention.cc
+++ b/src/calling_convention.cc
@@ -24,7 +24,7 @@
void ManagedRuntimeCallingConvention::Next() {
CHECK(HasNext());
- if (IsCurrentUserArg() &&
+ if (IsCurrentArgExplicit() && // don't query parameter type of implicit args
GetMethod()->IsParamALongOrDouble(itr_args_)) {
itr_longs_and_doubles_++;
itr_slots_++;
@@ -36,12 +36,13 @@
itr_slots_++;
}
-bool ManagedRuntimeCallingConvention::IsCurrentUserArg() {
- if (GetMethod()->IsStatic()) {
- return true;
- }
- // For a virtual method, "this" should never be NULL.
- return (itr_args_ != 0);
+bool ManagedRuntimeCallingConvention::IsCurrentArgExplicit() {
+ // Static methods have no implicit arguments, others implicitly pass this
+ return GetMethod()->IsStatic() || (itr_args_ != 0);
+}
+
+bool ManagedRuntimeCallingConvention::IsCurrentArgPossiblyNull() {
+ return IsCurrentArgExplicit(); // any user parameter may be null
}
size_t ManagedRuntimeCallingConvention::CurrentParamSize() {
@@ -54,10 +55,6 @@
// JNI calling convention
-size_t JniCallingConvention::OutArgSize() {
- return RoundUp(NumberOfOutgoingStackArgs() * kPointerSize, kStackAlignment);
-}
-
size_t JniCallingConvention::ReferenceCount() {
const Method* method = GetMethod();
return method->NumReferenceArgs() + (method->IsStatic() ? 1 : 0);
diff --git a/src/calling_convention.h b/src/calling_convention.h
index 44ce93f..3b773d6 100644
--- a/src/calling_convention.h
+++ b/src/calling_convention.h
@@ -52,8 +52,9 @@
// The slot number for current calling_convention argument.
// Note that each slot is 32-bit. When the current argument is bigger
// than 32 bits, return the first slot number for this argument.
- unsigned int itr_refs_;
unsigned int itr_slots_;
+ // The number of references iterated past
+ unsigned int itr_refs_;
// The argument number along argument list for current argument
unsigned int itr_args_;
// Number of longs and doubles seen along argument list
@@ -79,7 +80,8 @@
bool IsCurrentParamAReference();
bool IsCurrentParamInRegister();
bool IsCurrentParamOnStack();
- bool IsCurrentUserArg();
+ bool IsCurrentArgExplicit(); // ie a non-implict argument such as this
+ bool IsCurrentArgPossiblyNull();
size_t CurrentParamSize();
ManagedRegister CurrentParamRegister();
FrameOffset CurrentParamStackOffset();
diff --git a/src/calling_convention_arm.cc b/src/calling_convention_arm.cc
index 78f0c6c..527ce5f 100644
--- a/src/calling_convention_arm.cc
+++ b/src/calling_convention_arm.cc
@@ -39,20 +39,25 @@
}
bool ManagedRuntimeCallingConvention::IsCurrentParamOnStack() {
- return !IsCurrentParamInRegister();
+ if (itr_slots_ < 2) {
+ return false;
+ } else if (itr_slots_ > 2) {
+ return true;
+ } else {
+ // handle funny case of a long/double straddling registers and the stack
+ return GetMethod()->IsParamALongOrDouble(itr_args_);
+ }
}
static const Register kManagedArgumentRegisters[] = {
R1, R2, R3
};
ManagedRegister ManagedRuntimeCallingConvention::CurrentParamRegister() {
- CHECK_LT(itr_slots_, 3u); // Otherwise, should have gone through stack
+ CHECK(IsCurrentParamInRegister());
const Method* method = GetMethod();
if (method->IsParamALongOrDouble(itr_args_)) {
if (itr_slots_ == 0) {
- // return R1 to denote R1_R2
- return ManagedRegister::FromCoreRegister(kManagedArgumentRegisters
- [itr_slots_]);
+ return ManagedRegister::FromRegisterPair(R1_R2);
} else if (itr_slots_ == 1) {
return ManagedRegister::FromRegisterPair(R2_R3);
} else {
@@ -67,10 +72,18 @@
}
FrameOffset ManagedRuntimeCallingConvention::CurrentParamStackOffset() {
- CHECK_GE(itr_slots_, 3u);
- return FrameOffset(displacement_.Int32Value() + // displacement
- kPointerSize + // Method*
- (itr_slots_ * kPointerSize)); // offset into in args
+ CHECK(IsCurrentParamOnStack());
+ FrameOffset result =
+ FrameOffset(displacement_.Int32Value() + // displacement
+ kPointerSize + // Method*
+ (itr_slots_ * kPointerSize)); // offset into in args
+ if (itr_slots_ == 2) {
+ // the odd spanning case, bump the offset to skip the first half of the
+ // input which is in a register
+ CHECK(IsCurrentParamInRegister());
+ result = FrameOffset(result.Int32Value() + 4);
+ }
+ return result;
}
// JNI calling convention
@@ -85,6 +98,20 @@
kStackAlignment);
}
+size_t JniCallingConvention::OutArgSize() {
+ const Method* method = GetMethod();
+ size_t padding; // padding to ensure longs and doubles are not split in AAPCS
+ if (method->IsStatic()) {
+ padding = (method->NumArgs() > 1) && !method->IsParamALongOrDouble(0) &&
+ method->IsParamALongOrDouble(1) ? 4 : 0;
+ } else {
+ padding = (method->NumArgs() > 2) && !method->IsParamALongOrDouble(1) &&
+ method->IsParamALongOrDouble(2) ? 4 : 0;
+ }
+ return RoundUp(NumberOfOutgoingStackArgs() * kPointerSize + padding,
+ kStackAlignment);
+}
+
size_t JniCallingConvention::ReturnPcOffset() {
// Link register is always the last value spilled, skip forward one word for
// the Method* then skip back one word to get the link register (ie +0)
@@ -160,7 +187,13 @@
}
size_t JniCallingConvention::NumberOfOutgoingStackArgs() {
- return GetMethod()->NumArgs() + GetMethod()->NumLongOrDoubleArgs() - 2;
+ const Method* method = GetMethod();
+ size_t static_args = method->IsStatic() ? 1 : 0; // count jclass
+ // regular argument parameters and this
+ size_t param_args = method->NumArgs() +
+ method->NumLongOrDoubleArgs();
+ // count JNIEnv* less arguments in registers
+ return static_args + param_args + 1 - 4;
}
} // namespace art
diff --git a/src/calling_convention_x86.cc b/src/calling_convention_x86.cc
index 7257ef6..92ca780 100644
--- a/src/calling_convention_x86.cc
+++ b/src/calling_convention_x86.cc
@@ -60,6 +60,10 @@
kStackAlignment);
}
+size_t JniCallingConvention::OutArgSize() {
+ return RoundUp(NumberOfOutgoingStackArgs() * kPointerSize, kStackAlignment);
+}
+
size_t JniCallingConvention::ReturnPcOffset() {
// Return PC is pushed at the top of the frame by the call into the method
return FrameSize() - kPointerSize;
@@ -99,7 +103,11 @@
}
size_t JniCallingConvention::NumberOfOutgoingStackArgs() {
- return GetMethod()->NumArgs() + GetMethod()->NumLongOrDoubleArgs();
+ size_t static_args = GetMethod()->IsStatic() ? 1 : 0; // count jclass
+ // regular argument parameters and this
+ size_t param_args = GetMethod()->NumArgs() +
+ GetMethod()->NumLongOrDoubleArgs();
+ return static_args + param_args + 1; // count JNIEnv*
}
} // namespace art
diff --git a/src/jni_compiler.cc b/src/jni_compiler.cc
index 0a11268..3524974 100644
--- a/src/jni_compiler.cc
+++ b/src/jni_compiler.cc
@@ -86,11 +86,11 @@
if (input_in_reg) {
ManagedRegister in_reg = mr_conv.CurrentParamRegister();
- jni_asm->VerifyObject(in_reg, mr_conv.IsCurrentUserArg());
+ jni_asm->VerifyObject(in_reg, mr_conv.IsCurrentArgPossiblyNull());
jni_asm->StoreRef(sirt_offset, in_reg);
} else if (input_on_stack) {
FrameOffset in_off = mr_conv.CurrentParamStackOffset();
- jni_asm->VerifyObject(in_off, mr_conv.IsCurrentUserArg());
+ jni_asm->VerifyObject(in_off, mr_conv.IsCurrentArgPossiblyNull());
jni_asm->CopyRef(sirt_offset, in_off,
mr_conv.InterproceduralScratchRegister());
}
@@ -278,14 +278,15 @@
// 14. Place result in correct register possibly loading from indirect
// reference table
if (jni_conv.IsReturnAReference()) {
- jni_asm->IncreaseFrameSize(kStackAlignment);
- jni_conv.ResetIterator(FrameOffset(kStackAlignment));
+ jni_asm->IncreaseFrameSize(out_arg_size);
+ jni_conv.ResetIterator(FrameOffset(out_arg_size));
- jni_conv.Next();
+ jni_conv.Next(); // Skip Thread* argument
+ // Pass result as arg2
SetNativeParameter(jni_asm, &jni_conv, jni_conv.ReturnRegister());
- jni_conv.ResetIterator(FrameOffset(kStackAlignment));
-
+ // Pass Thread*
+ jni_conv.ResetIterator(FrameOffset(out_arg_size));
if (jni_conv.IsCurrentParamInRegister()) {
jni_asm->GetCurrentThread(jni_conv.CurrentParamRegister());
} else {
@@ -296,7 +297,7 @@
jni_asm->Call(reinterpret_cast<uintptr_t>(DecodeJObjectInThread),
jni_conv.InterproceduralScratchRegister());
- jni_asm->DecreaseFrameSize(kStackAlignment);
+ jni_asm->DecreaseFrameSize(out_arg_size);
jni_conv.ResetIterator(FrameOffset(0));
}
jni_asm->Move(mr_conv.ReturnRegister(), jni_conv.ReturnRegister());
@@ -344,11 +345,16 @@
bool null_allowed = false;
bool ref_param = jni_conv->IsCurrentParamAReference();
CHECK(!ref_param || mr_conv->IsCurrentParamAReference());
+ // input may be in register, on stack or both - but not none!
CHECK(input_in_reg || mr_conv->IsCurrentParamOnStack());
- CHECK(output_in_reg || jni_conv->IsCurrentParamOnStack());
+ if (output_in_reg) { // output shouldn't straddle registers and stack
+ CHECK(!jni_conv->IsCurrentParamOnStack());
+ } else {
+ CHECK(jni_conv->IsCurrentParamOnStack());
+ }
// References need placing in SIRT and the entry address passing
if (ref_param) {
- null_allowed = mr_conv->IsCurrentUserArg();
+ null_allowed = mr_conv->IsCurrentArgPossiblyNull();
// Compute SIRT offset. Note null is placed in the SIRT but the jobject
// passed to the native code must be null (not a pointer into the SIRT
// as with regular references).
@@ -362,7 +368,12 @@
if (ref_param) {
jni_asm->CreateSirtEntry(out_reg, sirt_offset, in_reg, null_allowed);
} else {
- jni_asm->Move(out_reg, in_reg);
+ if (!mr_conv->IsCurrentParamOnStack()) {
+ // regular non-straddling move
+ jni_asm->Move(out_reg, in_reg);
+ } else {
+ UNIMPLEMENTED(FATAL); // we currently don't expect to see this case
+ }
}
} else if (!input_in_reg && !output_in_reg) {
FrameOffset out_off = jni_conv->CurrentParamStackOffset();
@@ -404,7 +415,16 @@
} else {
size_t param_size = mr_conv->CurrentParamSize();
CHECK_EQ(param_size, jni_conv->CurrentParamSize());
- jni_asm->Store(out_off, in_reg, param_size);
+ if (!mr_conv->IsCurrentParamOnStack()) {
+ // regular non-straddling store
+ jni_asm->Store(out_off, in_reg, param_size);
+ } else {
+ // store where input straddles registers and stack
+ CHECK_EQ(param_size, 8u);
+ FrameOffset in_off = mr_conv->CurrentParamStackOffset();
+ jni_asm->StoreSpanning(out_off, in_reg, in_off,
+ mr_conv->InterproceduralScratchRegister());
+ }
}
}
}
diff --git a/src/jni_compiler_test.cc b/src/jni_compiler_test.cc
index ca8f23a..026c826 100644
--- a/src/jni_compiler_test.cc
+++ b/src/jni_compiler_test.cc
@@ -257,7 +257,6 @@
return x + y;
}
-
TEST_F(JniCompilerTest, CompileAndRunStaticIntIntMethod) {
SetupForTest(true, "fooSII",
"(II)I",
@@ -269,6 +268,32 @@
EXPECT_EQ(1, gJava_MyClass_fooSII_calls);
}
+int gJava_MyClass_fooSDD_calls = 0;
+jdouble Java_MyClass_fooSDD(JNIEnv* env, jclass klass, jdouble x, jdouble y) {
+ EXPECT_EQ(1u, Thread::Current()->NumSirtReferences());
+ EXPECT_EQ(Thread::kNative, Thread::Current()->GetState());
+ EXPECT_EQ(Thread::Current()->GetJniEnv(), env);
+ EXPECT_TRUE(klass != NULL);
+ EXPECT_TRUE(env->IsInstanceOf(JniCompilerTest::jobj_, klass));
+ gJava_MyClass_fooSDD_calls++;
+ return x - y; // non-commutative operator
+}
+
+TEST_F(JniCompilerTest, CompileAndRunStaticDoubleDoubleMethod) {
+ SetupForTest(true, "fooSDD", "(DD)D",
+ reinterpret_cast<void*>(&Java_MyClass_fooSDD));
+
+ EXPECT_EQ(0, gJava_MyClass_fooSDD_calls);
+ jdouble result = env_->CallStaticDoubleMethod(jklass_, jmethod_, 99.0, 10.0);
+ EXPECT_EQ(99.0 - 10.0, result);
+ EXPECT_EQ(1, gJava_MyClass_fooSDD_calls);
+ jdouble a = 3.14159265358979323846;
+ jdouble b = 0.69314718055994530942;
+ result = env_->CallStaticDoubleMethod(jklass_, jmethod_, a, b);
+ EXPECT_EQ(a - b, result);
+ EXPECT_EQ(2, gJava_MyClass_fooSDD_calls);
+}
+
int gJava_MyClass_fooSIOO_calls = 0;
jobject Java_MyClass_fooSIOO(JNIEnv* env, jclass klass, jint x, jobject y,
jobject z) {
diff --git a/src/managed_register_arm.cc b/src/managed_register_arm.cc
index 227467e..5973be1 100644
--- a/src/managed_register_arm.cc
+++ b/src/managed_register_arm.cc
@@ -53,6 +53,10 @@
} else {
CHECK(IsRegisterPair());
low = (r - kNumberOfDRegIds) * 2; // Return a Register.
+ if (low > 6) {
+ // we didn't got a pair higher than R6_R7, must be the dalvik special case
+ low = 1;
+ }
}
return low;
}
diff --git a/src/managed_register_arm.h b/src/managed_register_arm.h
index a132fa9..93cbc1b 100644
--- a/src/managed_register_arm.h
+++ b/src/managed_register_arm.h
@@ -14,7 +14,8 @@
R2_R3 = 1,
R4_R5 = 2,
R6_R7 = 3,
- kNumberOfRegisterPairs = 4,
+ R1_R2 = 4, // Dalvik style passing
+ kNumberOfRegisterPairs = 5,
kNoRegisterPair = -1,
};
@@ -107,7 +108,11 @@
RegisterPair AsRegisterPair() const {
CHECK(IsRegisterPair());
Register reg_low = AsRegisterPairLow();
- return static_cast<RegisterPair>(reg_low / 2);
+ if (reg_low == R1) {
+ return R1_R2;
+ } else {
+ return static_cast<RegisterPair>(reg_low / 2);
+ }
}
Register AsRegisterPairLow() const {
@@ -205,11 +210,15 @@
// Return a RegisterPair consisting of Register r_low and r_low + 1.
static ManagedRegister FromCoreRegisterPair(Register r_low) {
- CHECK_NE(r_low, kNoRegister);
- CHECK_EQ(0, (r_low % 2));
- const int r = r_low / 2;
- CHECK_LT(r, kNumberOfPairRegIds);
- return FromRegisterPair(static_cast<RegisterPair>(r));
+ if (r_low != R1) { // not the dalvik special case
+ CHECK_NE(r_low, kNoRegister);
+ CHECK_EQ(0, (r_low % 2));
+ const int r = r_low / 2;
+ CHECK_LT(r, kNumberOfPairRegIds);
+ return FromRegisterPair(static_cast<RegisterPair>(r));
+ } else {
+ return FromRegisterPair(R1_R2);
+ }
}
// Return a DRegister overlapping SRegister r_low and r_low + 1.
diff --git a/src/managed_register_arm_test.cc b/src/managed_register_arm_test.cc
index 238291f..66c7811 100644
--- a/src/managed_register_arm_test.cc
+++ b/src/managed_register_arm_test.cc
@@ -221,6 +221,18 @@
EXPECT_EQ(R1, reg.AsRegisterPairHigh());
EXPECT_TRUE(reg.Equals(ManagedRegister::FromCoreRegisterPair(R0)));
+ reg = ManagedRegister::FromRegisterPair(R1_R2);
+ EXPECT_TRUE(!reg.IsNoRegister());
+ EXPECT_TRUE(!reg.IsCoreRegister());
+ EXPECT_TRUE(!reg.IsSRegister());
+ EXPECT_TRUE(!reg.IsDRegister());
+ EXPECT_TRUE(!reg.IsOverlappingDRegister());
+ EXPECT_TRUE(reg.IsRegisterPair());
+ EXPECT_EQ(R1_R2, reg.AsRegisterPair());
+ EXPECT_EQ(R1, reg.AsRegisterPairLow());
+ EXPECT_EQ(R2, reg.AsRegisterPairHigh());
+ EXPECT_TRUE(reg.Equals(ManagedRegister::FromCoreRegisterPair(R1)));
+
reg = ManagedRegister::FromRegisterPair(R2_R3);
EXPECT_TRUE(!reg.IsNoRegister());
EXPECT_TRUE(!reg.IsCoreRegister());