|
From: <sv...@va...> - 2005-08-02 23:07:07
|
Author: njn
Date: 2005-08-03 00:07:02 +0100 (Wed, 03 Aug 2005)
New Revision: 4309
Log:
Fixed a bug in Cachegrind: it was adding instrumentation after
conditional jumps, so if those jumps were taken, the instrumentation
wasn't executed. This was causing the I-cache access counts to be
underestimated. =20
This commit puts the instrumentation before the jumps, except for the
odd case of REP instructions, giving the same behaviour as 2.4.0.
Based on a patch from Josef Weidendorfer.
Modified:
trunk/cachegrind/cg_main.c
Modified: trunk/cachegrind/cg_main.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/cachegrind/cg_main.c 2005-08-02 21:05:58 UTC (rev 4308)
+++ trunk/cachegrind/cg_main.c 2005-08-02 23:07:02 UTC (rev 4309)
@@ -390,7 +390,7 @@
}
=20
static=20
-void handleOneStatement(IRTypeEnv* tyenv, IRBB* bbOut, IRStmt* st,
+Bool handleOneStatement(IRTypeEnv* tyenv, IRBB* bbOut, IRStmt* st, IRStm=
t* st2,
Addr* instrAddr, UInt* instrLen,
IRExpr** loadAddrExpr, IRExpr** storeAddrExpr,
UInt* dataSize)
@@ -399,12 +399,56 @@
=20
switch (st->tag) {
case Ist_NoOp:
- break;
-
case Ist_AbiHint:
- /* ABI hints aren't interesting to cachegrind. Ignore. */
+ case Ist_Put:
+ case Ist_PutI:
+ case Ist_MFence:
break;
=20
+ case Ist_Exit: {
+ // This is a conditional jump. Most of the time, we want to add t=
he
+ // instrumentation before it, to ensure it gets executed. Eg, (1)=
if
+ // this conditional jump is just before an IMark:
+ //
+ // t108 =3D Not1(t107)
+ // [add instrumentation here]
+ // if (t108) goto {Boring} 0x3A96637D:I32
+ // ------ IMark(0x3A966370, 7) ------
+ //
+ // or (2) if this conditional jump is the last thing before the
+ // block-ending unconditional jump:
+ //
+ // t111 =3D Not1(t110)
+ // [add instrumentation here]
+ // if (t111) goto {Boring} 0x3A96637D:I32
+ // goto {Boring} 0x3A966370:I32
+ //
+ // One case (3) where we want the instrumentation after the condit=
ional
+ // jump is when the conditional jump is for an x86 REP instruction=
:
+ //
+ // ------ IMark(0x3A967F13, 2) ------
+ // t1 =3D GET:I32(4)
+ // t6 =3D CmpEQ32(t1,0x0:I32)=20
+ // if (t6) goto {Boring} 0x3A967F15:I32 # ignore this cond jm=
p
+ // t7 =3D Sub32(t1,0x1:I32)
+ // PUT(4) =3D t7
+ // ...
+ // t56 =3D Not1(t55)
+ // [add instrumentation here]
+ // if (t56) goto {Boring} 0x3A967F15:I32
+ //
+ // Therefore, we return true if the next statement is an IMark, or=
if
+ // there is no next statement (which matches case (2), as the fina=
l
+ // unconditional jump is not represented in the IRStmt list).
+ //
+ // Note that this approach won't do in the long run for supporting
+ // PPC, but it's good enough for x86/AMD64 for the 3.0.X series.
+ if (NULL =3D=3D st2 || Ist_IMark =3D=3D st2->tag)
+ return True;
+ else
+ return False;
+ }
+
case Ist_IMark:
/* st->Ist.IMark.addr is a 64-bit int. ULong_to_Ptr casts this
to the host's native pointer type; if that is 32 bits then it
@@ -418,7 +462,6 @@
machines. */
*instrAddr =3D (Addr)ULong_to_Ptr(st->Ist.IMark.addr);
*instrLen =3D st->Ist.IMark.len;
- addStmtToIRBB( bbOut, st );
break;
=20
case Ist_Tmp: {
@@ -433,7 +476,6 @@
*loadAddrExpr =3D aexpr;
*dataSize =3D sizeofIRType(data->Iex.Load.ty);
}
- addStmtToIRBB( bbOut, st );
break;
}
=20
@@ -444,7 +486,6 @@
tl_assert( NULL =3D=3D *storeAddrExpr ); // XXX: ???
*storeAddrExpr =3D aexpr;
*dataSize =3D sizeofIRType(typeOfIRExpr(tyenv, data));
- addStmtToIRBB( bbOut, st );
break;
}
=20
@@ -464,23 +505,17 @@
tl_assert(d->mAddr =3D=3D NULL);
tl_assert(d->mSize =3D=3D 0);
}
- addStmtToIRBB( bbOut, st );
break;
}
=20
- case Ist_Put:
- case Ist_PutI:
- case Ist_Exit:
- case Ist_MFence:
- addStmtToIRBB( bbOut, st );
- break;
-
default:
VG_(printf)("\n");
ppIRStmt(st);
VG_(printf)("\n");
VG_(tool_panic)("Cachegrind: unhandled IRStmt");
}
+
+ return False;
}
=20
static
@@ -515,9 +550,9 @@
=20
// Instrumentation for the end of each original instruction.
static
-void endOfInstr(IRBB* bbOut, instr_info* i_node, Bool bbSeenBefore,
- UInt instrAddr, UInt instrLen, UInt dataSize,
- IRExpr* loadAddrExpr, IRExpr* storeAddrExpr)
+void instrumentInstr(IRBB* bbOut, instr_info* i_node, Bool bbSeenBefore,
+ UInt instrAddr, UInt instrLen, UInt dataSize,
+ IRExpr* loadAddrExpr, IRExpr* storeAddrExpr)
{
IRDirty* di;
IRExpr *arg1, *arg2, *arg3, **argv;
@@ -534,7 +569,7 @@
if (sizeof(HWord) =3D=3D 8) {
wordTy =3D Ity_I64;
} else {
- VG_(tool_panic)("endOfInstr: strange word size");
+ VG_(tool_panic)("instrumentInstr: strange word size");
}
=20
if (loadAddrExpr)=20
@@ -620,7 +655,7 @@
IRBB* bbOut;
IRStmt* st;
BB_info* bbInfo;
- Bool bbSeenBefore =3D False;
+ Bool bbSeenBefore =3D False, addedInstrumentation, addInstNow;
Addr instrAddr, origAddr;
UInt instrLen;
IRExpr *loadAddrExpr, *storeAddrExpr;
@@ -655,23 +690,40 @@
// Reset stuff for this original instruction
loadAddrExpr =3D storeAddrExpr =3D NULL;
dataSize =3D 0;
+ addedInstrumentation =3D False;
=20
// Process all the statements for this original instruction (ie. u=
ntil
// the next IMark statement, or the end of the block)
do {
- handleOneStatement(bbIn->tyenv, bbOut, st, &instrAddr, &instrLe=
n,
- &loadAddrExpr, &storeAddrExpr, &dataSize);
+ IRStmt* st2 =3D ( i+1 < bbIn->stmts_used ? bbIn->stmts[i+1] : N=
ULL );
+
+ addInstNow =3D handleOneStatement(bbIn->tyenv, bbOut, st, st2,
+ &instrAddr, &instrLen, &loadAdd=
rExpr,
+ &storeAddrExpr, &dataSize);
+ if (addInstNow) {
+ tl_assert(!addedInstrumentation);
+ addedInstrumentation =3D True;
+ =20
+ // Add instrumentation before this statement.
+ instrumentInstr(bbOut, &bbInfo->instrs[ bbInfo_i ], bbSeenBe=
fore,
+ instrAddr, instrLen, dataSize, loadAddrExpr, store=
AddrExpr);
+ }
+
+ addStmtToIRBB( bbOut, st );
+
i++;
- st =3D ( i < bbIn->stmts_used ? bbIn->stmts[i] : NULL );
+ st =3D st2;
}=20
while (st && Ist_IMark !=3D st->tag);
=20
- // Add instrumentation for this original instruction.
- endOfInstr(bbOut, &bbInfo->instrs[ bbInfo_i ], bbSeenBefore,
- instrAddr, instrLen, dataSize, loadAddrExpr, storeAddrE=
xpr);
+ if (!addedInstrumentation) {
+ // Add instrumentation now, after all the instruction's stateme=
nts.
+ instrumentInstr(bbOut, &bbInfo->instrs[ bbInfo_i ], bbSeenBefor=
e,
+ instrAddr, instrLen, dataSize, loadAddrExpr, st=
oreAddrExpr);
+ }
=20
bbInfo_i++;
- }=20
+ }
while (st);
=20
return bbOut;
|