|
From: <sv...@va...> - 2009-07-12 12:57:00
|
Author: sewardj
Date: 2009-07-12 13:56:53 +0100 (Sun, 12 Jul 2009)
New Revision: 1907
Log:
Add new integer comparison primitives Iop_CasCmp{EQ,NE}{8,16,32,64},
which are semantically identical to Iop_Cmp{EQ,NE}{8,16,32,64}. Use
these new primitives instead of the normal ones, in the tests
following IR-level compare-and-swap operations, which establish
whether or not the CAS succeeded. This is all for Memcheck's benefit,
as it really needs to be able to identify which comparisons are
CAS-success tests and which aren't. This is all described in great
detail in memcheck/mc_translate.c in the comment
"COMMENT_ON_CasCmpEQ".
Modified:
trunk/priv/guest_amd64_toIR.c
trunk/priv/guest_x86_toIR.c
trunk/priv/host_amd64_isel.c
trunk/priv/host_x86_isel.c
trunk/priv/ir_defs.c
trunk/pub/libvex_ir.h
Modified: trunk/priv/guest_amd64_toIR.c
===================================================================
--- trunk/priv/guest_amd64_toIR.c 2009-07-04 13:07:30 UTC (rev 1906)
+++ trunk/priv/guest_amd64_toIR.c 2009-07-12 12:56:53 UTC (rev 1907)
@@ -148,6 +148,13 @@
jump. It's not such a big deal with casLE since the side exit is
only taken if the CAS fails, that is, the location is contended,
which is relatively unlikely.
+
+ Note also, the test for CAS success vs failure is done using
+ Iop_CasCmp{EQ,NE}{8,16,32,64} rather than the ordinary
+ Iop_Cmp{EQ,NE} equivalents. This is so as to tell Memcheck that it
+ shouldn't definedness-check these comparisons. See
+ COMMENT_ON_CasCmpEQ in memcheck/mc_translate.c for
+ background/rationale.
*/
/* LOCK prefixed instructions. These are translated using IR-level
@@ -320,6 +327,7 @@
|| op8 == Iop_Or8 || op8 == Iop_And8 || op8 == Iop_Xor8
|| op8 == Iop_Shl8 || op8 == Iop_Shr8 || op8 == Iop_Sar8
|| op8 == Iop_CmpEQ8 || op8 == Iop_CmpNE8
+ || op8 == Iop_CasCmpNE8
|| op8 == Iop_Not8 );
switch (ty) {
case Ity_I8: return 0 +op8;
@@ -1436,7 +1444,8 @@
NULL, mkexpr(expTmp), NULL, newVal );
stmt( IRStmt_CAS(cas) );
stmt( IRStmt_Exit(
- binop( mkSizedOp(tyE,Iop_CmpNE8), mkexpr(oldTmp), mkexpr(expTmp) ),
+ binop( mkSizedOp(tyE,Iop_CasCmpNE8),
+ mkexpr(oldTmp), mkexpr(expTmp) ),
Ijk_Boring, /*Ijk_NoRedir*/
IRConst_U64( restart_point )
));
@@ -15499,22 +15508,22 @@
}
case 0xC7: { /* CMPXCHG8B Ev, CMPXCHG16B Ev */
- IRType elemTy = sz==4 ? Ity_I32 : Ity_I64;
- IRTemp expdHi = newTemp(elemTy);
- IRTemp expdLo = newTemp(elemTy);
- IRTemp dataHi = newTemp(elemTy);
- IRTemp dataLo = newTemp(elemTy);
- IRTemp oldHi = newTemp(elemTy);
- IRTemp oldLo = newTemp(elemTy);
- IRTemp flags_old = newTemp(Ity_I64);
- IRTemp flags_new = newTemp(Ity_I64);
- IRTemp success = newTemp(Ity_I1);
- IROp opOR = sz==4 ? Iop_Or32 : Iop_Or64;
- IROp opXOR = sz==4 ? Iop_Xor32 : Iop_Xor64;
- IROp opCmpEQ = sz==4 ? Iop_CmpEQ32 : Iop_CmpEQ64;
- IRExpr* zero = sz==4 ? mkU32(0) : mkU64(0);
- IRTemp expdHi64 = newTemp(Ity_I64);
- IRTemp expdLo64 = newTemp(Ity_I64);
+ IRType elemTy = sz==4 ? Ity_I32 : Ity_I64;
+ IRTemp expdHi = newTemp(elemTy);
+ IRTemp expdLo = newTemp(elemTy);
+ IRTemp dataHi = newTemp(elemTy);
+ IRTemp dataLo = newTemp(elemTy);
+ IRTemp oldHi = newTemp(elemTy);
+ IRTemp oldLo = newTemp(elemTy);
+ IRTemp flags_old = newTemp(Ity_I64);
+ IRTemp flags_new = newTemp(Ity_I64);
+ IRTemp success = newTemp(Ity_I1);
+ IROp opOR = sz==4 ? Iop_Or32 : Iop_Or64;
+ IROp opXOR = sz==4 ? Iop_Xor32 : Iop_Xor64;
+ IROp opCasCmpEQ = sz==4 ? Iop_CasCmpEQ32 : Iop_CasCmpEQ64;
+ IRExpr* zero = sz==4 ? mkU32(0) : mkU64(0);
+ IRTemp expdHi64 = newTemp(Ity_I64);
+ IRTemp expdLo64 = newTemp(Ity_I64);
/* Translate this using a DCAS, even if there is no LOCK
prefix. Life is too short to bother with generating two
@@ -15562,7 +15571,7 @@
/* success when oldHi:oldLo == expdHi:expdLo */
assign( success,
- binop(opCmpEQ,
+ binop(opCasCmpEQ,
binop(opOR,
binop(opXOR, mkexpr(oldHi), mkexpr(expdHi)),
binop(opXOR, mkexpr(oldLo), mkexpr(expdLo))
Modified: trunk/priv/guest_x86_toIR.c
===================================================================
--- trunk/priv/guest_x86_toIR.c 2009-07-04 13:07:30 UTC (rev 1906)
+++ trunk/priv/guest_x86_toIR.c 2009-07-12 12:56:53 UTC (rev 1907)
@@ -118,6 +118,13 @@
jump. It's not such a big deal with casLE since the side exit is
only taken if the CAS fails, that is, the location is contended,
which is relatively unlikely.
+
+ Note also, the test for CAS success vs failure is done using
+ Iop_CasCmp{EQ,NE}{8,16,32,64} rather than the ordinary
+ Iop_Cmp{EQ,NE} equivalents. This is so as to tell Memcheck that it
+ shouldn't definedness-check these comparisons. See
+ COMMENT_ON_CasCmpEQ in memcheck/mc_translate.c for
+ background/rationale.
*/
/* Performance holes:
@@ -715,6 +722,7 @@
|| op8 == Iop_Or8 || op8 == Iop_And8 || op8 == Iop_Xor8
|| op8 == Iop_Shl8 || op8 == Iop_Shr8 || op8 == Iop_Sar8
|| op8 == Iop_CmpEQ8 || op8 == Iop_CmpNE8
+ || op8 == Iop_CasCmpNE8
|| op8 == Iop_Not8);
adj = ty==Ity_I8 ? 0 : (ty==Ity_I16 ? 1 : 2);
return adj + op8;
@@ -765,7 +773,8 @@
NULL, mkexpr(expTmp), NULL, newVal );
stmt( IRStmt_CAS(cas) );
stmt( IRStmt_Exit(
- binop( mkSizedOp(tyE,Iop_CmpNE8), mkexpr(oldTmp), mkexpr(expTmp) ),
+ binop( mkSizedOp(tyE,Iop_CasCmpNE8),
+ mkexpr(oldTmp), mkexpr(expTmp) ),
Ijk_Boring, /*Ijk_NoRedir*/
IRConst_U32( restart_point )
));
@@ -13763,11 +13772,8 @@
/* ------------------------ XCHG ----------------------- */
/* XCHG reg,mem automatically asserts LOCK# even without a LOCK
- prefix. Therefore, surround it with a IRStmt_MBE(Imbe_BusLock)
- and IRStmt_MBE(Imbe_BusUnlock) pair. But be careful; if it is
- used with an explicit LOCK prefix, we don't want to end up with
- two IRStmt_MBE(Imbe_BusLock)s -- one made here and one made by
- the generic LOCK logic at the top of disInstr. */
+ prefix; hence it must be translated with an IRCAS (at least, the
+ memory variant). */
case 0x86: /* XCHG Gb,Eb */
sz = 1;
/* Fall through ... */
@@ -14216,7 +14222,7 @@
/* success when oldHi:oldLo == expdHi:expdLo */
assign( success,
- binop(Iop_CmpEQ32,
+ binop(Iop_CasCmpEQ32,
binop(Iop_Or32,
binop(Iop_Xor32, mkexpr(oldHi), mkexpr(expdHi)),
binop(Iop_Xor32, mkexpr(oldLo), mkexpr(expdLo))
Modified: trunk/priv/host_amd64_isel.c
===================================================================
--- trunk/priv/host_amd64_isel.c 2009-07-04 13:07:30 UTC (rev 1906)
+++ trunk/priv/host_amd64_isel.c 2009-07-12 12:56:53 UTC (rev 1907)
@@ -2197,7 +2197,9 @@
/* CmpEQ8 / CmpNE8 */
if (e->tag == Iex_Binop
&& (e->Iex.Binop.op == Iop_CmpEQ8
- || e->Iex.Binop.op == Iop_CmpNE8)) {
+ || e->Iex.Binop.op == Iop_CmpNE8
+ || e->Iex.Binop.op == Iop_CasCmpEQ8
+ || e->Iex.Binop.op == Iop_CasCmpNE8)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
AMD64RMI* rmi2 = iselIntExpr_RMI(env, e->Iex.Binop.arg2);
HReg r = newVRegI(env);
@@ -2205,8 +2207,8 @@
addInstr(env, AMD64Instr_Alu64R(Aalu_XOR,rmi2,r));
addInstr(env, AMD64Instr_Alu64R(Aalu_AND,AMD64RMI_Imm(0xFF),r));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ8: return Acc_Z;
- case Iop_CmpNE8: return Acc_NZ;
+ case Iop_CmpEQ8: case Iop_CasCmpEQ8: return Acc_Z;
+ case Iop_CmpNE8: case Iop_CasCmpNE8: return Acc_NZ;
default: vpanic("iselCondCode(amd64): CmpXX8");
}
}
@@ -2214,7 +2216,9 @@
/* CmpEQ16 / CmpNE16 */
if (e->tag == Iex_Binop
&& (e->Iex.Binop.op == Iop_CmpEQ16
- || e->Iex.Binop.op == Iop_CmpNE16)) {
+ || e->Iex.Binop.op == Iop_CmpNE16
+ || e->Iex.Binop.op == Iop_CasCmpEQ16
+ || e->Iex.Binop.op == Iop_CasCmpNE16)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
AMD64RMI* rmi2 = iselIntExpr_RMI(env, e->Iex.Binop.arg2);
HReg r = newVRegI(env);
@@ -2222,8 +2226,8 @@
addInstr(env, AMD64Instr_Alu64R(Aalu_XOR,rmi2,r));
addInstr(env, AMD64Instr_Alu64R(Aalu_AND,AMD64RMI_Imm(0xFFFF),r));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ16: return Acc_Z;
- case Iop_CmpNE16: return Acc_NZ;
+ case Iop_CmpEQ16: case Iop_CasCmpEQ16: return Acc_Z;
+ case Iop_CmpNE16: case Iop_CasCmpNE16: return Acc_NZ;
default: vpanic("iselCondCode(amd64): CmpXX16");
}
}
@@ -2231,7 +2235,9 @@
/* CmpEQ32 / CmpNE32 */
if (e->tag == Iex_Binop
&& (e->Iex.Binop.op == Iop_CmpEQ32
- || e->Iex.Binop.op == Iop_CmpNE32)) {
+ || e->Iex.Binop.op == Iop_CmpNE32
+ || e->Iex.Binop.op == Iop_CasCmpEQ32
+ || e->Iex.Binop.op == Iop_CasCmpNE32)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
AMD64RMI* rmi2 = iselIntExpr_RMI(env, e->Iex.Binop.arg2);
HReg r = newVRegI(env);
@@ -2239,8 +2245,8 @@
addInstr(env, AMD64Instr_Alu64R(Aalu_XOR,rmi2,r));
addInstr(env, AMD64Instr_Sh64(Ash_SHL, 32, r));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ32: return Acc_Z;
- case Iop_CmpNE32: return Acc_NZ;
+ case Iop_CmpEQ32: case Iop_CasCmpEQ32: return Acc_Z;
+ case Iop_CmpNE32: case Iop_CasCmpNE32: return Acc_NZ;
default: vpanic("iselCondCode(amd64): CmpXX32");
}
}
@@ -2253,13 +2259,14 @@
|| e->Iex.Binop.op == Iop_CmpLT64U
|| e->Iex.Binop.op == Iop_CmpLE64S
|| e->Iex.Binop.op == Iop_CmpLE64U
- )) {
+ || e->Iex.Binop.op == Iop_CasCmpEQ64
+ || e->Iex.Binop.op == Iop_CasCmpNE64)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
AMD64RMI* rmi2 = iselIntExpr_RMI(env, e->Iex.Binop.arg2);
addInstr(env, AMD64Instr_Alu64R(Aalu_CMP,rmi2,r1));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ64: return Acc_Z;
- case Iop_CmpNE64: return Acc_NZ;
+ case Iop_CmpEQ64: case Iop_CasCmpEQ64: return Acc_Z;
+ case Iop_CmpNE64: case Iop_CasCmpNE64: return Acc_NZ;
case Iop_CmpLT64S: return Acc_L;
case Iop_CmpLT64U: return Acc_B;
case Iop_CmpLE64S: return Acc_LE;
Modified: trunk/priv/host_x86_isel.c
===================================================================
--- trunk/priv/host_x86_isel.c 2009-07-04 13:07:30 UTC (rev 1906)
+++ trunk/priv/host_x86_isel.c 2009-07-12 12:56:53 UTC (rev 1907)
@@ -1802,13 +1802,15 @@
/* CmpEQ8 / CmpNE8 */
if (e->tag == Iex_Binop
&& (e->Iex.Binop.op == Iop_CmpEQ8
- || e->Iex.Binop.op == Iop_CmpNE8)) {
+ || e->Iex.Binop.op == Iop_CmpNE8
+ || e->Iex.Binop.op == Iop_CasCmpEQ8
+ || e->Iex.Binop.op == Iop_CasCmpNE8)) {
if (isZeroU8(e->Iex.Binop.arg2)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
addInstr(env, X86Instr_Test32(0xFF,X86RM_Reg(r1)));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ8: return Xcc_Z;
- case Iop_CmpNE8: return Xcc_NZ;
+ case Iop_CmpEQ8: case Iop_CasCmpEQ8: return Xcc_Z;
+ case Iop_CmpNE8: case Iop_CasCmpNE8: return Xcc_NZ;
default: vpanic("iselCondCode(x86): CmpXX8(expr,0:I8)");
}
} else {
@@ -1819,8 +1821,8 @@
addInstr(env, X86Instr_Alu32R(Xalu_XOR,rmi2,r));
addInstr(env, X86Instr_Test32(0xFF,X86RM_Reg(r)));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ8: return Xcc_Z;
- case Iop_CmpNE8: return Xcc_NZ;
+ case Iop_CmpEQ8: case Iop_CasCmpEQ8: return Xcc_Z;
+ case Iop_CmpNE8: case Iop_CasCmpNE8: return Xcc_NZ;
default: vpanic("iselCondCode(x86): CmpXX8(expr,expr)");
}
}
@@ -1829,7 +1831,9 @@
/* CmpEQ16 / CmpNE16 */
if (e->tag == Iex_Binop
&& (e->Iex.Binop.op == Iop_CmpEQ16
- || e->Iex.Binop.op == Iop_CmpNE16)) {
+ || e->Iex.Binop.op == Iop_CmpNE16
+ || e->Iex.Binop.op == Iop_CasCmpEQ16
+ || e->Iex.Binop.op == Iop_CasCmpNE16)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
X86RMI* rmi2 = iselIntExpr_RMI(env, e->Iex.Binop.arg2);
HReg r = newVRegI(env);
@@ -1837,8 +1841,8 @@
addInstr(env, X86Instr_Alu32R(Xalu_XOR,rmi2,r));
addInstr(env, X86Instr_Test32(0xFFFF,X86RM_Reg(r)));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ16: return Xcc_Z;
- case Iop_CmpNE16: return Xcc_NZ;
+ case Iop_CmpEQ16: case Iop_CasCmpEQ16: return Xcc_Z;
+ case Iop_CmpNE16: case Iop_CasCmpNE16: return Xcc_NZ;
default: vpanic("iselCondCode(x86): CmpXX16");
}
}
@@ -1850,13 +1854,15 @@
|| e->Iex.Binop.op == Iop_CmpLT32S
|| e->Iex.Binop.op == Iop_CmpLT32U
|| e->Iex.Binop.op == Iop_CmpLE32S
- || e->Iex.Binop.op == Iop_CmpLE32U)) {
+ || e->Iex.Binop.op == Iop_CmpLE32U
+ || e->Iex.Binop.op == Iop_CasCmpEQ32
+ || e->Iex.Binop.op == Iop_CasCmpNE32)) {
HReg r1 = iselIntExpr_R(env, e->Iex.Binop.arg1);
X86RMI* rmi2 = iselIntExpr_RMI(env, e->Iex.Binop.arg2);
addInstr(env, X86Instr_Alu32R(Xalu_CMP,rmi2,r1));
switch (e->Iex.Binop.op) {
- case Iop_CmpEQ32: return Xcc_Z;
- case Iop_CmpNE32: return Xcc_NZ;
+ case Iop_CmpEQ32: case Iop_CasCmpEQ32: return Xcc_Z;
+ case Iop_CmpNE32: case Iop_CasCmpNE32: return Xcc_NZ;
case Iop_CmpLT32S: return Xcc_L;
case Iop_CmpLT32U: return Xcc_B;
case Iop_CmpLE32S: return Xcc_LE;
@@ -1880,8 +1886,8 @@
addInstr(env, X86Instr_Alu32R(Xalu_XOR,X86RMI_Reg(lo2), tLo));
addInstr(env, X86Instr_Alu32R(Xalu_OR,X86RMI_Reg(tHi), tLo));
switch (e->Iex.Binop.op) {
- case Iop_CmpNE64: return Xcc_NZ;
- case Iop_CmpEQ64: return Xcc_Z;
+ case Iop_CmpNE64: return Xcc_NZ;
+ case Iop_CmpEQ64: return Xcc_Z;
default: vpanic("iselCondCode(x86): CmpXX64");
}
}
Modified: trunk/priv/ir_defs.c
===================================================================
--- trunk/priv/ir_defs.c 2009-07-04 13:07:30 UTC (rev 1906)
+++ trunk/priv/ir_defs.c 2009-07-12 12:56:53 UTC (rev 1907)
@@ -144,6 +144,10 @@
str = "CmpEQ"; base = Iop_CmpEQ8; break;
case Iop_CmpNE8 ... Iop_CmpNE64:
str = "CmpNE"; base = Iop_CmpNE8; break;
+ case Iop_CasCmpEQ8 ... Iop_CasCmpEQ64:
+ str = "CasCmpEQ"; base = Iop_CasCmpEQ8; break;
+ case Iop_CasCmpNE8 ... Iop_CasCmpNE64:
+ str = "CasCmpNE"; base = Iop_CasCmpNE8; break;
case Iop_Not8 ... Iop_Not64:
str = "Not"; base = Iop_Not8; break;
/* other cases must explicitly "return;" */
@@ -574,7 +578,8 @@
default: vpanic("ppIROp(1)");
}
-
+
+ vassert(str);
switch (op - base) {
case 0: vex_printf("%s",str); vex_printf("8"); break;
case 1: vex_printf("%s",str); vex_printf("16"); break;
@@ -1642,14 +1647,18 @@
UNARY(Ity_I64, Ity_I64);
case Iop_CmpEQ8: case Iop_CmpNE8:
+ case Iop_CasCmpEQ8: case Iop_CasCmpNE8:
COMPARISON(Ity_I8);
case Iop_CmpEQ16: case Iop_CmpNE16:
+ case Iop_CasCmpEQ16: case Iop_CasCmpNE16:
COMPARISON(Ity_I16);
case Iop_CmpEQ32: case Iop_CmpNE32:
+ case Iop_CasCmpEQ32: case Iop_CasCmpNE32:
case Iop_CmpLT32S: case Iop_CmpLE32S:
case Iop_CmpLT32U: case Iop_CmpLE32U:
COMPARISON(Ity_I32);
case Iop_CmpEQ64: case Iop_CmpNE64:
+ case Iop_CasCmpEQ64: case Iop_CasCmpNE64:
case Iop_CmpLT64S: case Iop_CmpLE64S:
case Iop_CmpLT64U: case Iop_CmpLE64U:
COMPARISON(Ity_I64);
Modified: trunk/pub/libvex_ir.h
===================================================================
--- trunk/pub/libvex_ir.h 2009-07-04 13:07:30 UTC (rev 1906)
+++ trunk/pub/libvex_ir.h 2009-07-12 12:56:53 UTC (rev 1907)
@@ -423,6 +423,14 @@
/* Tags for unary ops */
Iop_Not8, Iop_Not16, Iop_Not32, Iop_Not64,
+ /* Exactly like CmpEQ8/16/32/64, but carrying the additional
+ hint that these compute the success/failure of a CAS
+ operation, and hence are almost certainly applied to two
+ copies of the same value, which in turn has implications for
+ Memcheck's instrumentation. */
+ Iop_CasCmpEQ8, Iop_CasCmpEQ16, Iop_CasCmpEQ32, Iop_CasCmpEQ64,
+ Iop_CasCmpNE8, Iop_CasCmpNE16, Iop_CasCmpNE32, Iop_CasCmpNE64,
+
/* -- Ordering not important after here. -- */
/* Widening multiplies */
|