Bug 643969 - LIR_jf can generate jump-to-location-zero code on i386 and x64 (r=rreitmai)

--HG--
extra : convert_revision : 3b0667d8dc545c74a903d2b63e2116b407f035d7
This commit is contained in:
Steven Johnson 2011-04-04 12:20:54 -07:00
parent 02e91234a0
commit ed109e1671
13 changed files with 102 additions and 57 deletions

View File

@ -0,0 +1,27 @@
ptr = allocp 8
j start
success: zero = immd 0.0
retd zero
; do a store+load so we don't optimize away the test
start: a = immd 1.0
b = immd 2.0
c = addd a b
std c ptr 0
expect = immd 4.0
actual = ldd ptr 0
; Test is for this code pattern, i386/x64 backend
; could generate bad code for the != case of LIR_jf
; if the branch is backwards. (Note that we compare 3.0
; to 4.0 because we need the test to fail.)
cond = eqd expect actual
jf cond success
bad = immd -1.0
retd bad

View File

@ -0,0 +1 @@
Output is: 0

View File

@ -1329,8 +1329,14 @@ namespace nanojit
// Evict all registers, most conservative approach.
intersectRegisterState(label->regs);
}
NIns *branch = asm_branch(branchOnFalse, cond, 0);
_patches.put(branch,to);
Branches branches = asm_branch(branchOnFalse, cond, 0);
printf("b1=%p %p\n",branches.branch1,branches.branch2);
if (branches.branch1) {
_patches.put(branches.branch1,to);
}
if (branches.branch2) {
_patches.put(branches.branch2,to);
}
}
}

View File

@ -231,6 +231,23 @@ namespace nanojit
LabelState *get(LIns *);
};
/**
* Some architectures (i386, X64) can emit two branches that need patching
* in some situations. This is returned by asm_branch() implementations
* with 0, 1 or 2 of these fields set to a non-NULL value. (If only 1 is set,
* it must be patch1, not patch2.)
*/
struct Branches
{
NIns* const branch1;
NIns* const branch2;
inline explicit Branches(NIns* b1 = NULL, NIns* b2 = NULL)
: branch1(b1)
, branch2(b2)
{
}
};
/** map tracking the register allocation state at each bailout point
* (represented by SideExit*) in a trace fragment. */
typedef HashMap<SideExit*, RegAlloc*> RegAllocMap;
@ -481,7 +498,7 @@ namespace nanojit
void asm_nongp_copy(Register r, Register s);
void asm_call(LIns*);
Register asm_binop_rhs_reg(LIns* ins);
NIns* asm_branch(bool branchOnFalse, LIns* cond, NIns* targ);
Branches asm_branch(bool branchOnFalse, LIns* cond, NIns* targ);
NIns* asm_branch_ov(LOpcode op, NIns* targ);
void asm_jtbl(LIns* ins, NIns** table);
void asm_insert_random_nop();

View File

@ -2291,7 +2291,7 @@ Assembler::asm_cmpd(LIns* ins)
/* Call this with targ set to 0 if the target is not yet known and the branch
* will be patched up later.
*/
NIns*
Branches
Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ)
{
LOpcode condop = cond->opcode();
@ -2351,7 +2351,7 @@ Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ)
asm_cmp(cond);
return at;
return Branches(at);
}
NIns* Assembler::asm_branch_ov(LOpcode op, NIns* target)

View File

@ -1618,7 +1618,7 @@ namespace nanojit
return patch;
}
NIns* Assembler::asm_branch(bool branchOnFalse, LIns *cond, NIns * const targ)
Branches Assembler::asm_branch(bool branchOnFalse, LIns *cond, NIns * const targ)
{
NanoAssert(cond->isCmp());
LOpcode condop = cond->opcode();
@ -1628,7 +1628,7 @@ namespace nanojit
Register ra = findRegFor(a, allow);
Register rb = (b==a) ? ra : findRegFor(b, allow & ~rmask(ra));
return asm_bxx(branchOnFalse, condop, ra, rb, targ);
return Branches(asm_bxx(branchOnFalse, condop, ra, rb, targ));
}
void Assembler::asm_j(NIns * const targ, bool bdelay)

View File

@ -420,7 +420,7 @@ namespace nanojit
return ((d << shift) >> shift) == d;
}
NIns* Assembler::asm_branch(bool onfalse, LIns *cond, NIns * const targ) {
Branches Assembler::asm_branch(bool onfalse, LIns *cond, NIns * const targ) {
LOpcode condop = cond->opcode();
NanoAssert(cond->isCmp());
@ -434,7 +434,7 @@ namespace nanojit
#endif
patch = asm_branch_far(onfalse, cond, targ);
asm_cmp(condop, cond->oprnd1(), cond->oprnd2(), CR7);
return patch;
return Branches(patch);
}
NIns* Assembler::asm_branch_near(bool onfalse, LIns *cond, NIns * const targ) {

View File

@ -1449,7 +1449,7 @@ namespace nanojit
return patch_target;
}
NIns *Assembler::asm_branch(bool branchOnFalse, LIns *condition, NIns *target) {
Branches Assembler::asm_branch(bool branchOnFalse, LIns *condition, NIns *target) {
NanoAssert(condition->isCmp());
LOpcode opcode = condition->opcode();
@ -1459,7 +1459,7 @@ namespace nanojit
asm_cmp(opcode, condition);
return patch_target;
return Branches(patch_target);
}
void Assembler::nBeginAssembly() {

View File

@ -998,14 +998,14 @@ namespace nanojit
LDSW32(rs, ds, t);
}
NIns* Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ)
Branches Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ)
{
NIns* at = 0;
LOpcode condop = cond->opcode();
NanoAssert(cond->isCmp());
if (isCmpDOpcode(condop))
{
return asm_branchd(branchOnFalse, cond, targ);
return Branches(asm_branchd(branchOnFalse, cond, targ));
}
underrunProtect(32);
@ -1064,7 +1064,7 @@ namespace nanojit
BCC(0, tt);
}
asm_cmp(cond);
return at;
return Branches(at);
}
NIns* Assembler::asm_branch_ov(LOpcode op, NIns* targ)

View File

@ -1246,13 +1246,13 @@ namespace nanojit
asm_cmpi(cond);
}
NIns* Assembler::asm_branch(bool onFalse, LIns* cond, NIns* target) {
NIns* patch = asm_branch_helper(onFalse, cond, target);
Branches Assembler::asm_branch(bool onFalse, LIns* cond, NIns* target) {
Branches branches = asm_branch_helper(onFalse, cond, target);
asm_cmp(cond);
return patch;
return branches;
}
NIns* Assembler::asm_branch_helper(bool onFalse, LIns *cond, NIns *target) {
Branches Assembler::asm_branch_helper(bool onFalse, LIns *cond, NIns *target) {
if (target && !isTargetWithinS32(target)) {
// A conditional jump beyond 32-bit range, so invert the
// branch/compare and emit an unconditional jump to the target:
@ -1269,7 +1269,7 @@ namespace nanojit
: asm_branchi_helper(onFalse, cond, target);
}
NIns* Assembler::asm_branchi_helper(bool onFalse, LIns *cond, NIns *target) {
Branches Assembler::asm_branchi_helper(bool onFalse, LIns *cond, NIns *target) {
// We must ensure there's room for the instruction before calculating
// the offset. And the offset determines the opcode (8bit or 32bit).
LOpcode condop = cond->opcode();
@ -1330,7 +1330,7 @@ namespace nanojit
}
}
}
return _nIns; // address of instruction to patch
return Branches(_nIns); // address of instruction to patch
}
NIns* Assembler::asm_branch_ov(LOpcode, NIns* target) {
@ -1412,15 +1412,17 @@ namespace nanojit
// LIR_jt jae ja swap+jae swap+ja jp over je
// LIR_jf jb jbe swap+jb swap+jbe jne+jp
NIns* Assembler::asm_branchd_helper(bool onFalse, LIns *cond, NIns *target) {
Branches Assembler::asm_branchd_helper(bool onFalse, LIns *cond, NIns *target) {
LOpcode condop = cond->opcode();
NIns *patch;
NIns *patch1 = NULL;
NIns *patch2 = NULL;
if (condop == LIR_eqd) {
if (onFalse) {
// branch if unordered or !=
JP(16, target); // underrun of 12 needed, round up for overhang --> 16
patch1 = _nIns;
JNE(0, target); // no underrun needed, previous was enough
patch = _nIns;
patch2 = _nIns;
} else {
// jp skip (2byte)
// jeq target
@ -1428,7 +1430,7 @@ namespace nanojit
underrunProtect(16); // underrun of 7 needed but we write 2 instr --> 16
NIns *skip = _nIns;
JE(0, target); // no underrun needed, previous was enough
patch = _nIns;
patch1 = _nIns;
JP8(0, skip); // ditto
}
}
@ -1443,9 +1445,9 @@ namespace nanojit
case LIR_ged: if (onFalse) JB(8, target); else JAE(8, target); break;
default: NanoAssert(0); break;
}
patch = _nIns;
patch1 = _nIns;
}
return patch;
return Branches(patch1, patch2);
}
void Assembler::asm_condd(LIns *ins) {
@ -2033,17 +2035,6 @@ namespace nanojit
return; // don't patch
}
((int32_t*)next)[-1] = int32_t(target - next);
if (next[0] == 0x0F && next[1] == 0x8A) {
// code is jne<target>,jp<target>, for LIR_jf(feq)
// we just patched the jne, now patch the jp.
next += 6;
NanoAssert(((int32_t*)next)[-1] == 0);
if (!isS32(target - next)) {
setError(BranchTooFar);
return; // don't patch
}
((int32_t*)next)[-1] = int32_t(target - next);
}
}
Register Assembler::nRegisterAllocFromSet(RegisterMask set) {

View File

@ -426,9 +426,9 @@ namespace nanojit
void asm_cmpi(LIns*);\
void asm_cmpi_imm(LIns*);\
void asm_cmpd(LIns*);\
NIns* asm_branch_helper(bool, LIns*, NIns*);\
NIns* asm_branchi_helper(bool, LIns*, NIns*);\
NIns* asm_branchd_helper(bool, LIns*, NIns*);\
Branches asm_branch_helper(bool, LIns*, NIns*);\
Branches asm_branchi_helper(bool, LIns*, NIns*);\
Branches asm_branchd_helper(bool, LIns*, NIns*);\
void asm_div(LIns *ins);\
void asm_div_mod(LIns *ins);\
int max_stk_used;\

View File

@ -1486,14 +1486,14 @@ namespace nanojit
}
}
NIns* Assembler::asm_branch_helper(bool branchOnFalse, LIns* cond, NIns* targ)
Branches Assembler::asm_branch_helper(bool branchOnFalse, LIns* cond, NIns* targ)
{
return isCmpDOpcode(cond->opcode())
? asm_branchd_helper(branchOnFalse, cond, targ)
: asm_branchi_helper(branchOnFalse, cond, targ);
}
NIns* Assembler::asm_branchi_helper(bool branchOnFalse, LIns* cond, NIns* targ)
Branches Assembler::asm_branchi_helper(bool branchOnFalse, LIns* cond, NIns* targ)
{
if (branchOnFalse) {
// op == LIR_xf/LIR_jf
@ -1524,14 +1524,14 @@ namespace nanojit
default: NanoAssert(0); break;
}
}
return _nIns;
return Branches(_nIns);
}
NIns* Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ)
Branches Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ)
{
NIns* at = asm_branch_helper(branchOnFalse, cond, targ);
Branches branches = asm_branch_helper(branchOnFalse, cond, targ);
asm_cmp(cond);
return at;
return branches;
}
NIns* Assembler::asm_branch_ov(LOpcode, NIns* target)
@ -2595,9 +2595,10 @@ namespace nanojit
}
}
NIns* Assembler::asm_branchd_helper(bool branchOnFalse, LIns* cond, NIns *targ)
Branches Assembler::asm_branchd_helper(bool branchOnFalse, LIns* cond, NIns *targ)
{
NIns* at = 0;
NIns* patch1 = NULL;
NIns* patch2 = NULL;
LOpcode opcode = cond->opcode();
if (_config.i386_sse2) {
@ -2611,8 +2612,10 @@ namespace nanojit
if (cond->oprnd1() == cond->oprnd2()) {
JP(targ);
} else {
JP(targ);
JP(targ); // unordered
patch1 = _nIns;
JNE(targ);
patch2 = _nIns;
}
break;
case LIR_ltd:
@ -2634,8 +2637,8 @@ namespace nanojit
underrunProtect(16); // underrun of 7 needed but we write 2 instr --> 16
NIns *skip = _nIns;
JE(targ);
at = _nIns;
JP(skip);
patch1 = _nIns;
JP(skip); // unordered
}
break;
case LIR_ltd:
@ -2652,10 +2655,10 @@ namespace nanojit
JNP(targ);
}
if (!at)
at = _nIns;
if (!patch1)
patch1 = _nIns;
return at;
return Branches(patch1, patch2);
}
// WARNING: This function cannot generate any code that will affect the

View File

@ -202,9 +202,9 @@ namespace nanojit
void asm_cmp(LIns *cond); \
void asm_cmpi(LIns *cond); \
void asm_cmpd(LIns *cond);\
NIns* asm_branch_helper(bool, LIns* cond, NIns*);\
NIns* asm_branchi_helper(bool, LIns* cond, NIns*);\
NIns* asm_branchd_helper(bool, LIns* cond, NIns*);\
Branches asm_branch_helper(bool, LIns* cond, NIns*);\
Branches asm_branchi_helper(bool, LIns* cond, NIns*);\
Branches asm_branchd_helper(bool, LIns* cond, NIns*);\
void asm_div_mod(LIns *cond); \
void asm_load(int d, Register r); \
void asm_immd(Register r, uint64_t q, double d, bool canClobberCCs); \