ART: aarch64 jni compiler needs to extend small return types
As aarch64 calling convention does not mandate extension on return
values anymore and leaves the upper bits undefined, the jni compiler
needs to sign- or zero-extend the returned values when necessary.
As three architectures need extension now, refactor this fact into a
flag into a virtual method.
Add tests to JniTest that exercise the required extension.
Change-Id: Idebb7c4dedebb852e58ade63e1c2b1eeced23104
diff --git a/compiler/jni/quick/arm/calling_convention_arm.h b/compiler/jni/quick/arm/calling_convention_arm.h
index 00a239b..604ce1c 100644
--- a/compiler/jni/quick/arm/calling_convention_arm.h
+++ b/compiler/jni/quick/arm/calling_convention_arm.h
@@ -71,6 +71,11 @@
ManagedRegister CurrentParamRegister() OVERRIDE;
FrameOffset CurrentParamStackOffset() OVERRIDE;
+ // AAPCS mandates return values are extended.
+ bool RequiresSmallResultTypeExtension() const OVERRIDE {
+ return false;
+ }
+
protected:
size_t NumberOfOutgoingStackArgs() OVERRIDE;
diff --git a/compiler/jni/quick/arm64/calling_convention_arm64.h b/compiler/jni/quick/arm64/calling_convention_arm64.h
index 92f547c..9fd3265 100644
--- a/compiler/jni/quick/arm64/calling_convention_arm64.h
+++ b/compiler/jni/quick/arm64/calling_convention_arm64.h
@@ -68,6 +68,11 @@
ManagedRegister CurrentParamRegister() OVERRIDE;
FrameOffset CurrentParamStackOffset() OVERRIDE;
+ // aarch64 calling convention leaves upper bits undefined.
+ bool RequiresSmallResultTypeExtension() const OVERRIDE {
+ return true;
+ }
+
protected:
size_t NumberOfOutgoingStackArgs() OVERRIDE;
diff --git a/compiler/jni/quick/calling_convention.h b/compiler/jni/quick/calling_convention.h
index 4d25d1c..18afd58 100644
--- a/compiler/jni/quick/calling_convention.h
+++ b/compiler/jni/quick/calling_convention.h
@@ -287,6 +287,8 @@
FrameOffset ReturnValueSaveLocation() const;
// Register that holds result if it is integer.
virtual ManagedRegister IntReturnRegister() = 0;
+ // Whether the compiler needs to ensure zero-/sign-extension of a small result type
+ virtual bool RequiresSmallResultTypeExtension() const = 0;
// Callee save registers to spill prior to native code (which may clobber)
virtual const std::vector<ManagedRegister>& CalleeSaveRegisters() const = 0;
diff --git a/compiler/jni/quick/jni_compiler.cc b/compiler/jni/quick/jni_compiler.cc
index 93b1b5a..9f439eb 100644
--- a/compiler/jni/quick/jni_compiler.cc
+++ b/compiler/jni/quick/jni_compiler.cc
@@ -314,7 +314,7 @@
mr_conv->InterproceduralScratchRegister());
// 10. Fix differences in result widths.
- if (instruction_set == kX86 || instruction_set == kX86_64) {
+ if (main_jni_conv->RequiresSmallResultTypeExtension()) {
if (main_jni_conv->GetReturnType() == Primitive::kPrimByte ||
main_jni_conv->GetReturnType() == Primitive::kPrimShort) {
__ SignExtend(main_jni_conv->ReturnRegister(),
diff --git a/compiler/jni/quick/mips/calling_convention_mips.h b/compiler/jni/quick/mips/calling_convention_mips.h
index e33fbad..8d82dce 100644
--- a/compiler/jni/quick/mips/calling_convention_mips.h
+++ b/compiler/jni/quick/mips/calling_convention_mips.h
@@ -71,6 +71,11 @@
ManagedRegister CurrentParamRegister() OVERRIDE;
FrameOffset CurrentParamStackOffset() OVERRIDE;
+ // Mips does not need to extend small return types.
+ bool RequiresSmallResultTypeExtension() const OVERRIDE {
+ return false;
+ }
+
protected:
size_t NumberOfOutgoingStackArgs() OVERRIDE;
diff --git a/compiler/jni/quick/x86/calling_convention_x86.h b/compiler/jni/quick/x86/calling_convention_x86.h
index 5b9069c..025eb6d 100644
--- a/compiler/jni/quick/x86/calling_convention_x86.h
+++ b/compiler/jni/quick/x86/calling_convention_x86.h
@@ -69,6 +69,11 @@
ManagedRegister CurrentParamRegister() OVERRIDE;
FrameOffset CurrentParamStackOffset() OVERRIDE;
+ // x86 needs to extend small return types.
+ bool RequiresSmallResultTypeExtension() const OVERRIDE {
+ return true;
+ }
+
protected:
size_t NumberOfOutgoingStackArgs() OVERRIDE;
diff --git a/compiler/jni/quick/x86_64/calling_convention_x86_64.h b/compiler/jni/quick/x86_64/calling_convention_x86_64.h
index d545774..1ba5353 100644
--- a/compiler/jni/quick/x86_64/calling_convention_x86_64.h
+++ b/compiler/jni/quick/x86_64/calling_convention_x86_64.h
@@ -69,6 +69,11 @@
ManagedRegister CurrentParamRegister() OVERRIDE;
FrameOffset CurrentParamStackOffset() OVERRIDE;
+ // x86-64 needs to extend small return types.
+ bool RequiresSmallResultTypeExtension() const OVERRIDE {
+ return true;
+ }
+
protected:
size_t NumberOfOutgoingStackArgs() OVERRIDE;
diff --git a/compiler/utils/arm64/assembler_arm64.cc b/compiler/utils/arm64/assembler_arm64.cc
index 1d87eaa..b4bb979 100644
--- a/compiler/utils/arm64/assembler_arm64.cc
+++ b/compiler/utils/arm64/assembler_arm64.cc
@@ -467,12 +467,26 @@
#endif
}
-void Arm64Assembler::SignExtend(ManagedRegister /*mreg*/, size_t /*size*/) {
- UNIMPLEMENTED(FATAL) << "no sign extension necessary for Arm64";
+void Arm64Assembler::SignExtend(ManagedRegister mreg, size_t size) {
+ Arm64ManagedRegister reg = mreg.AsArm64();
+ CHECK(size == 1 || size == 2) << size;
+ CHECK(reg.IsWRegister()) << reg;
+ if (size == 1) {
+ ___ sxtb(reg_w(reg.AsWRegister()), reg_w(reg.AsWRegister()));
+ } else {
+ ___ sxth(reg_w(reg.AsWRegister()), reg_w(reg.AsWRegister()));
+ }
}
-void Arm64Assembler::ZeroExtend(ManagedRegister /*mreg*/, size_t /*size*/) {
- UNIMPLEMENTED(FATAL) << "no zero extension necessary for Arm64";
+void Arm64Assembler::ZeroExtend(ManagedRegister mreg, size_t size) {
+ Arm64ManagedRegister reg = mreg.AsArm64();
+ CHECK(size == 1 || size == 2) << size;
+ CHECK(reg.IsWRegister()) << reg;
+ if (size == 1) {
+ ___ uxtb(reg_w(reg.AsWRegister()), reg_w(reg.AsWRegister()));
+ } else {
+ ___ uxth(reg_w(reg.AsWRegister()), reg_w(reg.AsWRegister()));
+ }
}
void Arm64Assembler::VerifyObject(ManagedRegister /*src*/, bool /*could_be_null*/) {
diff --git a/test/JniTest/JniTest.java b/test/JniTest/JniTest.java
index d53cf5e..3c4ed35 100644
--- a/test/JniTest/JniTest.java
+++ b/test/JniTest/JniTest.java
@@ -24,6 +24,10 @@
testCallStaticVoidMethodOnSubClass();
testGetMirandaMethod();
testZeroLengthByteBuffers();
+ testByteMethod();
+ testShortMethod();
+ testBooleanMethod();
+ testCharMethod();
}
private static native void testFindClassOnAttachedNativeThread();
@@ -79,4 +83,67 @@
private static interface testGetMirandaMethod_MirandaInterface {
public boolean inInterface();
}
+
+ // Test sign-extension for values < 32b
+
+ native static byte byteMethod(byte b1, byte b2, byte b3, byte b4, byte b5, byte b6, byte b7,
+ byte b8, byte b9, byte b10);
+
+ private static void testByteMethod() {
+ byte returns[] = { 0, 1, 2, 127, -1, -2, -128 };
+ for (int i = 0; i < returns.length; i++) {
+ byte result = byteMethod((byte)i, (byte)2, (byte)(-3), (byte)4, (byte)(-5), (byte)6,
+ (byte)(-7), (byte)8, (byte)(-9), (byte)10);
+ if (returns[i] != result) {
+ System.out.println("Run " + i + " with " + returns[i] + " vs " + result);
+ throw new AssertionError();
+ }
+ }
+ }
+
+ native static short shortMethod(short s1, short s2, short s3, short s4, short s5, short s6, short s7,
+ short s8, short s9, short s10);
+
+ private static void testShortMethod() {
+ short returns[] = { 0, 1, 2, 127, 32767, -1, -2, -128, -32768 };
+ for (int i = 0; i < returns.length; i++) {
+ short result = shortMethod((short)i, (short)2, (short)(-3), (short)4, (short)(-5), (short)6,
+ (short)(-7), (short)8, (short)(-9), (short)10);
+ if (returns[i] != result) {
+ System.out.println("Run " + i + " with " + returns[i] + " vs " + result);
+ throw new AssertionError();
+ }
+ }
+ }
+
+ // Test zero-extension for values < 32b
+
+ native static boolean booleanMethod(boolean b1, boolean b2, boolean b3, boolean b4, boolean b5, boolean b6, boolean b7,
+ boolean b8, boolean b9, boolean b10);
+
+ private static void testBooleanMethod() {
+ if (booleanMethod(false, true, false, true, false, true, false, true, false, true)) {
+ throw new AssertionError();
+ }
+
+ if (!booleanMethod(true, true, false, true, false, true, false, true, false, true)) {
+ throw new AssertionError();
+ }
+ }
+
+ native static char charMethod(char c1, char c2, char c3, char c4, char c5, char c6, char c7,
+ char c8, char c9, char c10);
+
+ private static void testCharMethod() {
+ char returns[] = { (char)0, (char)1, (char)2, (char)127, (char)255, (char)256, (char)15000,
+ (char)34000 };
+ for (int i = 0; i < returns.length; i++) {
+ char result = charMethod((char)i, 'a', 'b', 'c', '0', '1', '2', (char)1234, (char)2345,
+ (char)3456);
+ if (returns[i] != result) {
+ System.out.println("Run " + i + " with " + (int)returns[i] + " vs " + (int)result);
+ throw new AssertionError();
+ }
+ }
+ }
}
diff --git a/test/JniTest/jni_test.cc b/test/JniTest/jni_test.cc
index 33af94b..024ba53 100644
--- a/test/JniTest/jni_test.cc
+++ b/test/JniTest/jni_test.cc
@@ -137,3 +137,92 @@
assert(env->GetDirectBufferAddress(byte_buffer) == &buffer[0]);
assert(env->GetDirectBufferCapacity(byte_buffer) == 0);
}
+
+constexpr size_t kByteReturnSize = 7;
+jbyte byte_returns[kByteReturnSize] = { 0, 1, 2, 127, -1, -2, -128 };
+
+extern "C" jbyte JNICALL Java_JniTest_byteMethod(JNIEnv* env, jclass klass, jbyte b1, jbyte b2,
+ jbyte b3, jbyte b4, jbyte b5, jbyte b6,
+ jbyte b7, jbyte b8, jbyte b9, jbyte b10) {
+ // We use b1 to drive the output.
+ assert(b2 == 2);
+ assert(b3 == -3);
+ assert(b4 == 4);
+ assert(b5 == -5);
+ assert(b6 == 6);
+ assert(b7 == -7);
+ assert(b8 == 8);
+ assert(b9 == -9);
+ assert(b10 == 10);
+
+ assert(0 <= b1);
+ assert(b1 < static_cast<jbyte>(kByteReturnSize));
+
+ return byte_returns[b1];
+}
+
+constexpr size_t kShortReturnSize = 9;
+jshort short_returns[kShortReturnSize] = { 0, 1, 2, 127, 32767, -1, -2, -128,
+ static_cast<jshort>(0x8000) };
+// The weird static_cast is because short int is only guaranteed down to -32767, not Java's -32768.
+
+extern "C" jshort JNICALL Java_JniTest_shortMethod(JNIEnv* env, jclass klass, jshort s1, jshort s2,
+ jshort s3, jshort s4, jshort s5, jshort s6,
+ jshort s7, jshort s8, jshort s9, jshort s10) {
+ // We use s1 to drive the output.
+ assert(s2 == 2);
+ assert(s3 == -3);
+ assert(s4 == 4);
+ assert(s5 == -5);
+ assert(s6 == 6);
+ assert(s7 == -7);
+ assert(s8 == 8);
+ assert(s9 == -9);
+ assert(s10 == 10);
+
+ assert(0 <= s1);
+ assert(s1 < static_cast<jshort>(kShortReturnSize));
+
+ return short_returns[s1];
+}
+
+extern "C" jboolean JNICALL Java_JniTest_booleanMethod(JNIEnv* env, jclass klass, jboolean b1,
+ jboolean b2, jboolean b3, jboolean b4,
+ jboolean b5, jboolean b6, jboolean b7,
+ jboolean b8, jboolean b9, jboolean b10) {
+ // We use b1 to drive the output.
+ assert(b2 == JNI_TRUE);
+ assert(b3 == JNI_FALSE);
+ assert(b4 == JNI_TRUE);
+ assert(b5 == JNI_FALSE);
+ assert(b6 == JNI_TRUE);
+ assert(b7 == JNI_FALSE);
+ assert(b8 == JNI_TRUE);
+ assert(b9 == JNI_FALSE);
+ assert(b10 == JNI_TRUE);
+
+ assert(b1 == JNI_TRUE || b1 == JNI_FALSE);
+ return b1;
+}
+
+constexpr size_t kCharReturnSize = 8;
+jchar char_returns[kCharReturnSize] = { 0, 1, 2, 127, 255, 256, 15000, 34000 };
+
+extern "C" jchar JNICALL Java_JniTest_charMethod(JNIEnv* env, jclass klacc, jchar c1, jchar c2,
+ jchar c3, jchar c4, jchar c5, jchar c6,
+ jchar c7, jchar c8, jchar c9, jchar c10) {
+ // We use c1 to drive the output.
+ assert(c2 == 'a');
+ assert(c3 == 'b');
+ assert(c4 == 'c');
+ assert(c5 == '0');
+ assert(c6 == '1');
+ assert(c7 == '2');
+ assert(c8 == 1234);
+ assert(c9 == 2345);
+ assert(c10 == 3456);
+
+ assert(c1 < static_cast<jchar>(kCharReturnSize));
+
+ return char_returns[c1];
+}