Reharmonize the implementation with the spec.
In particular, I altered the naming of some instruction format fields
as well as the names of instruction formats themselves, all in an attempt
to make the implementation be a more straightforward match of the spec.
This patch mostly changes comments to reflect the new harmonized
reality. The only "code-like" change is the renaming of kFmt3inline
and kFmt3rinline to kFmt35mi and kFmt3rmi (respectively), which is
what they're called in the spec.
Bonus: Added the new extended opcode instruction formats to
InstrUtils.h, though I left them commented out for now.
Change-Id: I0109f361c1e9b6f0308c45e8cda5320e9ad3060c
diff --git a/dexdump/DexDump.c b/dexdump/DexDump.c
index 506409c..ee7bd80 100644
--- a/dexdump/DexDump.c
+++ b/dexdump/DexDump.c
@@ -878,9 +878,8 @@
case kFmt32x: // op vAAAA, vBBBB
printf(" v%d, v%d", pDecInsn->vA, pDecInsn->vB);
break;
- case kFmt35c: // op vB, {vD, vE, vF, vG, vA}, thing@CCCC
+ case kFmt35c: // op {vC, vD, vE, vF, vG}, thing@BBBB
{
- /* NOTE: decoding of 35c doesn't quite match spec */
fputs(" {", stdout);
for (i = 0; i < (int) pDecInsn->vA; i++) {
if (i == 0)
@@ -960,7 +959,7 @@
printf("}, [%04x] // vtable #%04x", pDecInsn->vB, pDecInsn->vB);
}
break;
- case kFmt3rinline: // [opt] execute-inline/range
+ case kFmt3rmi: // [opt] execute-inline/range
{
fputs(" {", stdout);
for (i = 0; i < (int) pDecInsn->vA; i++) {
@@ -972,7 +971,7 @@
printf("}, [%04x] // inline #%04x", pDecInsn->vB, pDecInsn->vB);
}
break;
- case kFmt3inline: // [opt] inline invoke
+ case kFmt35mi: // [opt] inline invoke
{
#if 0
const InlineOperation* inlineOpsTable = dvmGetInlineOpsTable();
diff --git a/libdex/InstrUtils.c b/libdex/InstrUtils.c
index 2a4dd2c..ae4d7bd 100644
--- a/libdex/InstrUtils.c
+++ b/libdex/InstrUtils.c
@@ -1005,10 +1005,10 @@
fmt = kFmt3rms;
break;
case OP_EXECUTE_INLINE:
- fmt = kFmt3inline;
+ fmt = kFmt35mi;
break;
case OP_EXECUTE_INLINE_RANGE:
- fmt = kFmt3rinline;
+ fmt = kFmt3rmi;
break;
case OP_INVOKE_DIRECT_EMPTY:
fmt = kFmt35c;
@@ -1146,26 +1146,23 @@
pDec->vA = INST_AA(inst);
pDec->vB = FETCH(1) | ((u4) FETCH(2) << 16);
break;
- case kFmt35c: // op vB, {vD..vG,vA}, thing@CCCC
+ case kFmt35c: // op {vC, vD, vE, vF, vG}, thing@BBBB
case kFmt35ms: // [opt] invoke-virtual+super
{
/*
- * The lettering changes that came about when we went from 4 args
- * to 5 made the "range" versions of the calls different from
- * the non-range versions. We have the choice between decoding
- * them the way the spec shows and having lots of conditionals
- * in the verifier, or mapping the values onto their original
- * registers and leaving the verifier intact.
+ * Note that the fields mentioned in the spec don't appear in
+ * their "usual" positions here compared to most formats. This
+ * was done so that the field names for the argument count and
+ * reference index match between this format and the corresponding
+ * range formats (3rc and friends).
*
- * Current plan is to leave the verifier alone. We can fix it
- * later if it's architecturally unbearable.
- *
- * Bottom line: method constant is always in vB.
+ * Bottom line: The argument count is always in vA, and the
+ * method constant is always in vB.
*/
u2 regList;
int i, count;
- pDec->vA = INST_B(inst);
+ pDec->vA = INST_B(inst); // This is labeled A in the spec.
pDec->vB = FETCH(1);
regList = FETCH(2);
@@ -1175,7 +1172,10 @@
}
count = pDec->vA;
if (count == 5) {
- /* 5th arg comes from A field in instruction */
+ /*
+ * Per note above, the 5th arg comes from the A field in the
+ * instruction, but it's labeled G in the spec.
+ */
pDec->arg[4] = INST_A(inst);
count--;
}
@@ -1188,12 +1188,13 @@
pDec->vC = pDec->arg[0];
}
break;
- case kFmt3inline: // [opt] inline invoke
+ case kFmt35mi: // [opt] inline invoke
{
+ /* See note under case 35c, above. */
u2 regList;
int i;
- pDec->vA = INST_B(inst);
+ pDec->vA = INST_B(inst); // This is labeled A in the spec.
pDec->vB = FETCH(1);
regList = FETCH(2);
@@ -1215,7 +1216,7 @@
break;
case kFmt3rc: // op {vCCCC .. v(CCCC+AA-1)}, meth@BBBB
case kFmt3rms: // [opt] invoke-virtual+super/range
- case kFmt3rinline: // [opt] execute-inline/range
+ case kFmt3rmi: // [opt] execute-inline/range
pDec->vA = INST_AA(inst);
pDec->vB = FETCH(1);
pDec->vC = FETCH(2);
diff --git a/libdex/InstrUtils.h b/libdex/InstrUtils.h
index d987ef4..2ed2aa4 100644
--- a/libdex/InstrUtils.h
+++ b/libdex/InstrUtils.h
@@ -61,14 +61,21 @@
kFmt31i, // op vAA, #+BBBBBBBB
kFmt31t, // op vAA, +BBBBBBBB
kFmt31c, // op vAA, string@BBBBBBBB
- kFmt35c, // op {vD, vE, vF, vG, vA}, thing@CCCC (decoded differently)
+ kFmt35c, // op {vC,vD,vE,vF,vG}, thing@BBBB
kFmt35ms, // [opt] invoke-virtual+super
kFmt35fs, // [opt] invoke-interface
kFmt3rc, // op {vCCCC .. v(CCCC+AA-1)}, meth@BBBB
kFmt3rms, // [opt] invoke-virtual+super/range
kFmt51l, // op vAA, #+BBBBBBBBBBBBBBBB
- kFmt3inline, // [opt] inline invoke
- kFmt3rinline, // [opt] inline invoke/range
+ kFmt35mi, // [opt] inline invoke
+ kFmt3rmi, // [opt] inline invoke/range
+ /* coming soon:
+ kFmt33x, // exop vAA, vBB, vCCCC
+ kFmt32s, // exop vAA, vBB, #+CCCC
+ kFmt41c, // exop vAAAA, thing@BBBBBBBB
+ kFmt52c, // exop vAAAA, vCCCC, thing@BBBBBBBB
+ kFmt5rc, // exop {vCCCC .. v(CCCC+AAAA-1)}, meth@BBBBBBBB
+ */
};
/*
diff --git a/vm/compiler/codegen/arm/CodegenDriver.c b/vm/compiler/codegen/arm/CodegenDriver.c
index f0745ae..5cd901e 100644
--- a/vm/compiler/codegen/arm/CodegenDriver.c
+++ b/vm/compiler/codegen/arm/CodegenDriver.c
@@ -4214,8 +4214,8 @@
notHandled = handleFmt35ms_3rms(cUnit, mir,blockList[i],
labelList);
break;
- case kFmt3inline:
- case kFmt3rinline:
+ case kFmt35mi:
+ case kFmt3rmi:
notHandled = handleExecuteInline(cUnit, mir);
break;
case kFmt51l:
