Fix special method codegen
Tightened up the conditions under which we can generate frameless
methods. Added and renamed test case. Added special handling for
identity functions.
Change-Id: I5b04ea222becefc151ef7ff6b255e58922ccd6f2
diff --git a/src/compiler/CompilerIR.h b/src/compiler/CompilerIR.h
index afddf07..429772a 100644
--- a/src/compiler/CompilerIR.h
+++ b/src/compiler/CompilerIR.h
@@ -45,6 +45,7 @@
kLocDalvikFrame = 0, // Normal Dalvik register
kLocPhysReg,
kLocCompilerTemp,
+ kLocInvalid
};
struct PromotionMap {
@@ -553,6 +554,7 @@
kIPutChar,
kIPutShort,
kIPutWide,
+ kIdentity,
};
struct CodePattern {
@@ -580,6 +582,9 @@
{{Instruction::IPUT_CHAR, Instruction::RETURN_VOID}, kIPutChar},
{{Instruction::IPUT_SHORT, Instruction::RETURN_VOID}, kIPutShort},
{{Instruction::IPUT_WIDE, Instruction::RETURN_VOID}, kIPutWide},
+ {{Instruction::RETURN}, kIdentity},
+ {{Instruction::RETURN_OBJECT}, kIdentity},
+ {{Instruction::RETURN_WIDE}, kIdentity},
};
BasicBlock* oatNewBB(CompilationUnit* cUnit, BBType blockType, int blockId);
diff --git a/src/compiler/codegen/arm/Thumb2/Gen.cc b/src/compiler/codegen/arm/Thumb2/Gen.cc
index 21c995f..1938960 100644
--- a/src/compiler/codegen/arm/Thumb2/Gen.cc
+++ b/src/compiler/codegen/arm/Thumb2/Gen.cc
@@ -26,24 +26,78 @@
namespace art {
-/* Return a RegLocation that describes an in-register argument */
-RegLocation argLoc(CompilationUnit* cUnit, RegLocation loc, int sReg)
+
+/* Return the position of an ssa name within the argument list */
+int inPosition(CompilationUnit* cUnit, int sReg)
{
- loc.location = kLocPhysReg;
- int base = SRegToVReg(cUnit, sReg) - cUnit->numRegs;
- loc.sRegLow = sReg;
- loc.lowReg = rARG1 + base;
- loc.home = true;
+ int vReg = SRegToVReg(cUnit, sReg);
+ return vReg - cUnit->numRegs;
+}
+
+/*
+ * Describe an argument. If it's already in an arg register, just leave it
+ * there. NOTE: all live arg registers must be locked prior to this call
+ * to avoid having them allocated as a temp by downstream utilities.
+ */
+RegLocation argLoc(CompilationUnit* cUnit, RegLocation loc)
+{
+ int argNum = inPosition(cUnit, loc.sRegLow);
if (loc.wide) {
- loc.highReg = loc.lowReg + 1;
- oatLockTemp(cUnit, loc.lowReg);
- oatLockTemp(cUnit, loc.highReg);
+ if (argNum == 2) {
+ // Bad case - half in register, half in frame. Just punt
+ loc.location = kLocInvalid;
+ } else if (argNum < 2) {
+ loc.lowReg = rARG1 + argNum;
+ loc.highReg = loc.lowReg + 1;
+ loc.location = kLocPhysReg;
+ } else {
+ loc.location = kLocDalvikFrame;
+ }
} else {
- oatLockTemp(cUnit, loc.lowReg);
+ if (argNum < 3) {
+ loc.lowReg = rARG1 + argNum;
+ loc.location = kLocPhysReg;
+ } else {
+ loc.location = kLocDalvikFrame;
+ }
}
return loc;
}
+/*
+ * Load an argument. If already in a register, just return. If in
+ * the frame, we can't use the normal loadValue() because it assumed
+ * a proper frame - and we're frameless.
+ */
+RegLocation loadArg(CompilationUnit* cUnit, RegLocation loc)
+{
+ if (loc.location == kLocDalvikFrame) {
+ int start = (inPosition(cUnit, loc.sRegLow) + 1) * sizeof(uint32_t);
+ loc.lowReg = oatAllocTemp(cUnit);
+ loadWordDisp(cUnit, rSP, start, loc.lowReg);
+ if (loc.wide) {
+ loc.highReg = oatAllocTemp(cUnit);
+ loadWordDisp(cUnit, rSP, start + sizeof(uint32_t), loc.highReg);
+ }
+ loc.location = kLocPhysReg;
+ }
+ return loc;
+}
+
+/* Lock any referenced arguments that arrive in registers */
+void lockLiveArgs(CompilationUnit* cUnit, MIR* mir)
+{
+ int firstIn = cUnit->numRegs;
+ const int numArgRegs = 3; // TODO: generalize & move to RegUtil.cc
+ for (int i = 0; i < mir->ssaRep->numUses; i++) {
+ int vReg = SRegToVReg(cUnit, mir->ssaRep->uses[i]);
+ int inPosition = vReg - firstIn;
+ if (inPosition < numArgRegs) {
+ oatLockTemp(cUnit, rARG1 + inPosition);
+ }
+ }
+}
+
/* Find the next MIR, which may be in a following basic block */
MIR* getNextMir(CompilationUnit* cUnit, BasicBlock** pBb, MIR* mir)
{
@@ -98,16 +152,25 @@
if (!fastPath) {
return NULL;
}
- mir->optimizationFlags |= MIR_IGNORE_NULL_CHECK;
- genPrintLabel(cUnit, mir);
RegLocation rlObj = oatGetSrc(cUnit, mir, 0);
- rlObj = argLoc(cUnit, rlObj, mir->ssaRep->uses[0]);
+ lockLiveArgs(cUnit, mir);
+ rlObj = argLoc(cUnit, rlObj);
+ // Reject if object reference is not "this"
+ if ((rlObj.location == kLocInvalid) ||
+ (inPosition(cUnit, rlObj.sRegLow) != 0)) {
+ oatResetRegPool(cUnit);
+ return NULL;
+ }
RegLocation rlDest;
if (longOrDouble) {
rlDest = oatGetReturnWide(cUnit, false);
} else {
rlDest = oatGetReturn(cUnit, false);
}
+ // Point of no return - no aborts after this
+ mir->optimizationFlags |= MIR_IGNORE_NULL_CHECK;
+ genPrintLabel(cUnit, mir);
+ rlObj = loadArg(cUnit, rlObj);
genIGet(cUnit, mir, size, rlDest, rlObj, longOrDouble, isObject);
return getNextMir(cUnit, bb, mir);
}
@@ -118,35 +181,68 @@
int fieldOffset;
bool isVolatile;
uint32_t fieldIdx = mir->dalvikInsn.vC;
- if (cUnit->numIns > 3) {
- return NULL; // TODO: fix register allocation for many in arguments
- }
bool fastPath = fastInstance(cUnit, fieldIdx, fieldOffset, isVolatile,
false);
if (!fastPath) {
return NULL;
}
- mir->optimizationFlags |= MIR_IGNORE_NULL_CHECK;
- genPrintLabel(cUnit, mir);
RegLocation rlSrc;
RegLocation rlObj;
- int sSreg = mir->ssaRep->uses[0];
- int oSreg;
+ lockLiveArgs(cUnit, mir);
if (longOrDouble) {
rlSrc = oatGetSrcWide(cUnit, mir, 0, 1);
rlObj = oatGetSrc(cUnit, mir, 2);
- oSreg = mir->ssaRep->uses[2];
} else {
rlSrc = oatGetSrc(cUnit, mir, 0);
rlObj = oatGetSrc(cUnit, mir, 1);
- oSreg = mir->ssaRep->uses[1];
}
- rlSrc = argLoc(cUnit, rlSrc, sSreg);
- rlObj = argLoc(cUnit, rlObj, oSreg);
+ rlSrc = argLoc(cUnit, rlSrc);
+ rlObj = argLoc(cUnit, rlObj);
+ // Reject if object reference is not "this"
+ if ((rlObj.location == kLocInvalid) ||
+ (inPosition(cUnit, rlObj.sRegLow) != 0) ||
+ (rlSrc.location == kLocInvalid)) {
+ oatResetRegPool(cUnit);
+ return NULL;
+ }
+ // Point of no return - no aborts after this
+ mir->optimizationFlags |= MIR_IGNORE_NULL_CHECK;
+ genPrintLabel(cUnit, mir);
+ rlObj = loadArg(cUnit, rlObj);
+ rlSrc = loadArg(cUnit, rlSrc);
genIPut(cUnit, mir, size, rlSrc, rlObj, longOrDouble, isObject);
return getNextMir(cUnit, bb, mir);
}
+MIR* specialIdentity(CompilationUnit* cUnit, MIR* mir)
+{
+ RegLocation rlSrc;
+ RegLocation rlDest;
+ bool wide = (mir->ssaRep->numUses == 2);
+ if (wide) {
+ rlSrc = oatGetSrcWide(cUnit, mir, 0, 1);
+ rlDest = oatGetReturnWide(cUnit, false);
+ } else {
+ rlSrc = oatGetSrc(cUnit, mir, 0);
+ rlDest = oatGetReturn(cUnit, false);
+ }
+ lockLiveArgs(cUnit, mir);
+ rlSrc = argLoc(cUnit, rlSrc);
+ if (rlSrc.location == kLocInvalid) {
+ oatResetRegPool(cUnit);
+ return NULL;
+ }
+ // Point of no return - no aborts after this
+ genPrintLabel(cUnit, mir);
+ rlSrc = loadArg(cUnit, rlSrc);
+ if (wide) {
+ storeValueWide(cUnit, rlDest, rlSrc);
+ } else {
+ storeValue(cUnit, rlDest, rlSrc);
+ }
+ return mir;
+}
+
/*
* Special-case code genration for simple non-throwing leaf methods.
*/
@@ -167,48 +263,53 @@
break;
case kIGet:
nextMir = specialIGet(cUnit, &bb, mir, kWord, false, false);
- break;;
+ break;
case kIGetBoolean:
case kIGetByte:
nextMir = specialIGet(cUnit, &bb, mir, kUnsignedByte, false, false);
- break;;
+ break;
case kIGetObject:
nextMir = specialIGet(cUnit, &bb, mir, kWord, false, true);
- break;;
+ break;
case kIGetChar:
nextMir = specialIGet(cUnit, &bb, mir, kUnsignedHalf, false, false);
- break;;
+ break;
case kIGetShort:
nextMir = specialIGet(cUnit, &bb, mir, kSignedHalf, false, false);
- break;;
+ break;
case kIGetWide:
nextMir = specialIGet(cUnit, &bb, mir, kLong, true, false);
- break;;
+ break;
case kIPut:
nextMir = specialIPut(cUnit, &bb, mir, kWord, false, false);
- break;;
+ break;
case kIPutBoolean:
case kIPutByte:
nextMir = specialIPut(cUnit, &bb, mir, kUnsignedByte, false, false);
- break;;
+ break;
case kIPutObject:
nextMir = specialIPut(cUnit, &bb, mir, kWord, false, true);
- break;;
+ break;
case kIPutChar:
nextMir = specialIPut(cUnit, &bb, mir, kUnsignedHalf, false, false);
- break;;
+ break;
case kIPutShort:
nextMir = specialIPut(cUnit, &bb, mir, kSignedHalf, false, false);
- break;;
+ break;
case kIPutWide:
nextMir = specialIPut(cUnit, &bb, mir, kLong, true, false);
- break;;
+ break;
+ case kIdentity:
+ nextMir = specialIdentity(cUnit, mir);
+ break;
default:
return;
}
if (nextMir != NULL) {
cUnit->currentDalvikOffset = nextMir->offset;
- genPrintLabel(cUnit, nextMir);
+ if (specialCase != kIdentity) {
+ genPrintLabel(cUnit, nextMir);
+ }
newLIR1(cUnit, kThumbBx, rLR);
cUnit->coreSpillMask = 0;
cUnit->numCoreSpills = 0;
diff --git a/test/083-compiler-regressions/expected.txt b/test/083-compiler-regressions/expected.txt
new file mode 100644
index 0000000..abfd60f
--- /dev/null
+++ b/test/083-compiler-regressions/expected.txt
@@ -0,0 +1,11 @@
+b2296099 passes
+b2302318 passes
+b2487514 passes
+b5884080 passes
+largeFrame passes
+largeFrameFloat passes
+getterSetterTest passes
+identityTest passes
+wideGetterSetterTest passes
+wideIdentityTest passes
+returnConstantTest passes
diff --git a/test/083-jit-regressions/info.txt b/test/083-compiler-regressions/info.txt
similarity index 100%
rename from test/083-jit-regressions/info.txt
rename to test/083-compiler-regressions/info.txt
diff --git a/test/083-jit-regressions/src/Main.java b/test/083-compiler-regressions/src/Main.java
similarity index 96%
rename from test/083-jit-regressions/src/Main.java
rename to test/083-compiler-regressions/src/Main.java
index e6080c5..cf082e1 100644
--- a/test/083-jit-regressions/src/Main.java
+++ b/test/083-compiler-regressions/src/Main.java
@@ -20,6 +20,14 @@
* Test for Jit regressions.
*/
public class Main {
+ public static int const0x1234() {
+ return 0x1234;
+ }
+
+ public static long const0x123443211234() {
+ return 0x123443211234L;
+ }
+
public static void main(String args[]) throws Exception {
b2296099Test();
b2302318Test();
@@ -27,6 +35,106 @@
b5884080Test();
largeFrameTest();
largeFrameTestFloat();
+ getterSetterTest();
+ identityTest();
+ wideGetterSetterTest();
+ wideIdentityTest();
+ returnConstantTest();
+ }
+
+ public static void returnConstantTest() {
+ long res = const0x1234();
+ res += const0x123443211234();
+ Foo foo = new Foo();
+ res += foo.iConst0x1234();
+ res += foo.iConst0x123443211234();
+ if (res == 40031347689680L) {
+ System.out.println("returnConstantTest passes");
+ }
+ else {
+ System.out.println("returnConstantTest fails: " + res +
+ " (expecting 40031347689680)");
+ }
+ }
+
+ static void wideIdentityTest() {
+ Foo foo = new Foo();
+ long i = 1;
+ i += foo.wideIdent0(i);
+ i += foo.wideIdent1(0,i);
+ i += foo.wideIdent2(0,0,i);
+ i += foo.wideIdent3(0,0,0,i);
+ i += foo.wideIdent4(0,0,0,0,i);
+ i += foo.wideIdent5(0,0,0,0,0,i);
+ if (i == 64) {
+ System.out.println("wideIdentityTest passes");
+ }
+ else {
+ System.out.println("wideIdentityTest fails: " + i +
+ " (expecting 64)");
+ }
+ }
+
+ static void wideGetterSetterTest() {
+ Foo foo = new Foo();
+ long sum = foo.wideGetBar0();
+ sum += foo.wideGetBar1(1);
+ foo.wideSetBar1(sum);
+ sum += foo.wideGetBar2(1,2);
+ foo.wideSetBar2(0,sum);
+ sum += foo.wideGetBar3(1,2,3);
+ foo.wideSetBar3(0,0,sum);
+ sum += foo.wideGetBar4(1,2,3,4);
+ foo.wideSetBar4(0,0,0,sum);
+ sum += foo.wideGetBar5(1,2,3,4,5);
+ foo.wideSetBar5(0,0,0,0,sum);
+ if (foo.wideGetBar0() == 39488) {
+ System.out.println("wideGetterSetterTest passes");
+ }
+ else {
+ System.out.println("wideGetterSetterTest fails: " +
+ foo.wideGetBar0() + " (expecting 39488)");
+ }
+ }
+
+ static void identityTest() {
+ Foo foo = new Foo();
+ int i = 1;
+ i += foo.ident0(i);
+ i += foo.ident1(0,i);
+ i += foo.ident2(0,0,i);
+ i += foo.ident3(0,0,0,i);
+ i += foo.ident4(0,0,0,0,i);
+ i += foo.ident5(0,0,0,0,0,i);
+ if (i == 64) {
+ System.out.println("identityTest passes");
+ }
+ else {
+ System.out.println("identityTest fails: " + i +
+ " (expecting 64)");
+ }
+ }
+
+ static void getterSetterTest() {
+ Foo foo = new Foo();
+ int sum = foo.getBar0();
+ sum += foo.getBar1(1);
+ foo.setBar1(sum);
+ sum += foo.getBar2(1,2);
+ foo.setBar2(0,sum);
+ sum += foo.getBar3(1,2,3);
+ foo.setBar3(0,0,sum);
+ sum += foo.getBar4(1,2,3,4);
+ foo.setBar4(0,0,0,sum);
+ sum += foo.getBar5(1,2,3,4,5);
+ foo.setBar5(0,0,0,0,sum);
+ if (foo.getBar0() == 39488) {
+ System.out.println("getterSetterTest passes");
+ }
+ else {
+ System.out.println("getterSetterTest fails: " + foo.getBar0() +
+ " (expecting 39488)");
+ }
}
static void b2296099Test() throws Exception {
@@ -8165,3 +8273,134 @@
while (true) {}
}
}
+
+class Foo {
+ private int bar = 1234;
+ private long lbar = 1234;
+
+ public int iConst0x1234() {
+ return 0x1234;
+ }
+
+ public long iConst0x123443211234() {
+ return 0x123443211234L;
+ }
+
+ public void setBar1(int a1) {
+ bar = a1;
+ }
+ public void setBar2(int a1, int a2) {
+ bar = a2;
+ }
+ public void setBar3(int a1, int a2, int a3) {
+ bar = a3;
+ }
+ public void setBar4(int a1, int a2, int a3, int a4) {
+ bar = a4;
+ }
+ public void setBar5(int a1, int a2, int a3, int a4, int a5) {
+ bar = a5;
+ }
+ public int getBar0() {
+ return bar;
+ }
+ public int getBar1(int a1) {
+ return bar;
+ }
+ public int getBar2(int a1, int a2) {
+ return bar;
+ }
+ public int getBar3(int a1, int a2, int a3) {
+ return bar;
+ }
+ public int getBar4(int a1, int a2, int a3, int a4) {
+ return bar;
+ }
+ public int getBar5(int a1, int a2, int a3, int a4, int a5) {
+ return bar;
+ }
+
+ public int ident0(int a1) {
+ return a1;
+ }
+
+ public int ident1(int a2, int a1) {
+ return a1;
+ }
+
+ public int ident2(int a3, int a2, int a1) {
+ return a1;
+ }
+
+ public int ident3(int a4, int a3, int a2, int a1) {
+ return a1;
+ }
+
+ public int ident4(int a5, int a4, int a3, int a2, int a1) {
+ return a1;
+ }
+
+ public int ident5(int a6, int a5, int a4, int a3, int a2, int a1) {
+ return a1;
+ }
+
+
+ public void wideSetBar1(long a1) {
+ lbar = a1;
+ }
+ public void wideSetBar2(long a1, long a2) {
+ lbar = a2;
+ }
+ public void wideSetBar3(long a1, long a2, long a3) {
+ lbar = a3;
+ }
+ public void wideSetBar4(long a1, long a2, long a3, long a4) {
+ lbar = a4;
+ }
+ public void wideSetBar5(long a1, long a2, long a3, long a4, long a5) {
+ lbar = a5;
+ }
+ public long wideGetBar0() {
+ return lbar;
+ }
+ public long wideGetBar1(long a1) {
+ return lbar;
+ }
+ public long wideGetBar2(long a1, long a2) {
+ return lbar;
+ }
+ public long wideGetBar3(long a1, long a2, long a3) {
+ return lbar;
+ }
+ public long wideGetBar4(long a1, long a2, long a3, long a4) {
+ return lbar;
+ }
+ public long wideGetBar5(long a1, long a2, long a3, long a4, long a5) {
+ return lbar;
+ }
+
+ public long wideIdent0(long a1) {
+ return a1;
+ }
+
+ public long wideIdent1(int a2, long a1) {
+ return a1;
+ }
+
+ public long wideIdent2(int a3, int a2, long a1) {
+ return a1;
+ }
+
+ public long wideIdent3(int a4, int a3, int a2, long a1) {
+ return a1;
+ }
+
+ public long wideIdent4(int a5, int a4, int a3, int a2, long a1) {
+ return a1;
+ }
+
+ public long wideIdent5(int a6, int a5, int a4, int a3, int a2, long a1) {
+ return a1;
+ }
+
+}
diff --git a/test/083-jit-regressions/expected.txt b/test/083-jit-regressions/expected.txt
deleted file mode 100644
index 13f61ca..0000000
--- a/test/083-jit-regressions/expected.txt
+++ /dev/null
@@ -1,6 +0,0 @@
-b2296099 passes
-b2302318 passes
-b2487514 passes
-b5884080 passes
-largeFrame passes
-largeFrameFloat passes
