Skip to content

Commit

Permalink
Rollback CS#1603899 that led to a JIT assert ngen'ing System.Windows.…
Browse files Browse the repository at this point in the history 
…Forms.dll

====================
007551: Merge pull request dotnet#1241 from mikedn/modopt

Extend the DIV/MOD dividend into RDX:RAX only if needed
====================

Assert failure(PID 33656 [0x00008378], Thread: 17792 [0x4580]): Assertion failed 'addr->OperIsAddrMode() || (addr->IsCnsIntOrI() && addr->isContained()) || !addr->isContained()' in 'System.Windows.Forms.CheckedListBox:OnDrawItem(ref):this' (IL size 1216)

    File: e:\dd\projectk\src\ndp\clr\src\jit\emitxarch.cpp Line: 2698
    Image: C:\Windows\Microsoft.NET\Framework64\v4.0.rb1605209\mscorsvw.exe

The tree:

***** BB41, stmt 82 (embedded)
     (  6,  8) [003723] ------------             *  stmtExpr  void  (embedded) (IL 0x109...  ???)
N1045 (  3,  2) [000115] ------------             |        /--*  lclVar    ref    V00 this         u:2 REG rcx $80
N1047 (  1,  4) [002642] ------------             |        +--*  const     long   344 field offset Fseq[idealCheckSize] REG NA $10b
N1049 (  4,  6) [002643] -------N----             |     /--*  +         byref  REG NA $356
N1051 (  6,  8) [000116] ----GO------             |  /--*  indir     int    REG rcx <l:$685, c:$2ef>
N1053 (  6,  8) [003669] DA--GO------             \--*  st.lclVar int    V172 cse1         rcx REG rcx RV

During codegen:

Generating BB41, stmt 71        Holding variables: [rbx rsi rdi r12-r15]

Generating: N1043 (  3,  2) [000114] ------------             *  lclVar    int    V05 loc3         u:3 r12 (last use) REG r12 RV $31a
Generating: N1045 (  3,  2) [000115] ------------             *  lclVar    ref    V00 this         u:2 REG rcx $80
IN00db:        mov      rcx, gword ptr [V00 rbp+10H]
                            GC regs: 00000040 {rsi} => 00000042 {rcx rsi}
Generating: N1047 (  1,  4) [002642] ------------             *  const     long   344 field offset Fseq[idealCheckSize] REG NA $10b
Generating: N1049 (  4,  6) [002643] -------N----             *  +         byref  REG NA $356
Generating: N1051 (  6,  8) [000116] ----GO------             *  indir     int    REG rcx <l:$685, c:$2ef>
... assert ...

(This is rollback dotnet#2: the TFS/GitHub mirror unfortunately undid rollback CS#1605814 with CS#1605840. This change should avoid that problem.)

[tfs-changeset: 1605917]
  • Loading branch information
@BruceForstall
BruceForstall committed May 18, 2016
1 parent 7d78bb2 commit ce8e7e3
Show file tree
Hide file tree
Showing 16 changed files with 285 additions and 1,689 deletions.
112 changes: 112 additions & 0 deletions src/jit/codegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3365,6 +3365,118 @@ CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
}
}


// Generate code for division (or mod) by power of two
// or negative powers of two. (meaning -1 * a power of two, not 2^(-1))
// Op2 must be a contained integer constant.
void
CodeGen::genCodeForPow2Div(GenTreeOp* tree)
{
#if 0
GenTree *dividend = tree->gtOp.gtOp1;
GenTree *divisor = tree->gtOp.gtOp2;
genTreeOps oper = tree->OperGet();
emitAttr size = emitTypeSize(tree);
emitter *emit = getEmitter();
regNumber targetReg = tree->gtRegNum;
var_types targetType = tree->TypeGet();

bool isSigned = oper == GT_MOD || oper == GT_DIV;

// precondition: extended dividend is in RDX:RAX
// which means it is either all zeros or all ones

noway_assert(divisor->isContained());
GenTreeIntConCommon* divImm = divisor->AsIntConCommon();
int64_t imm = divImm->IconValue();
ssize_t abs_imm = abs(imm);
noway_assert(isPow2(abs_imm));


if (isSigned)
{
if (imm == 1)
{
if (targetReg != REG_RAX)
inst_RV_RV(INS_mov, targetReg, REG_RAX, targetType);

return;
}

if (abs_imm == 2)
{
if (oper == GT_MOD)
{
emit->emitIns_R_I(INS_and, size, REG_RAX, 1); // result is 0 or 1
// xor with rdx will flip all bits if negative
emit->emitIns_R_R(INS_xor, size, REG_RAX, REG_RDX); // 111.11110 or 0
}
else
{
assert(oper == GT_DIV);
// add 1 if it's negative
emit->emitIns_R_R(INS_sub, size, REG_RAX, REG_RDX);
}
}
else
{
// add imm-1 if negative
emit->emitIns_R_I(INS_and, size, REG_RDX, abs_imm - 1);
emit->emitIns_R_R(INS_add, size, REG_RAX, REG_RDX);
}

if (oper == GT_DIV)
{
unsigned shiftAmount = genLog2(unsigned(abs_imm));
inst_RV_SH(INS_sar, size, REG_RAX, shiftAmount);

if (imm < 0)
{
emit->emitIns_R(INS_neg, size, REG_RAX);
}
}
else
{
assert(oper == GT_MOD);
if (abs_imm > 2)
{
emit->emitIns_R_I(INS_and, size, REG_RAX, abs_imm - 1);
}
// RDX contains 'imm-1' if negative
emit->emitIns_R_R(INS_sub, size, REG_RAX, REG_RDX);
}

if (targetReg != REG_RAX)
{
inst_RV_RV(INS_mov, targetReg, REG_RAX, targetType);
}
}
else
{
assert (imm > 0);

if (targetReg != dividend->gtRegNum)
{
inst_RV_RV(INS_mov, targetReg, dividend->gtRegNum, targetType);
}

if (oper == GT_UDIV)
{
inst_RV_SH(INS_shr, size, targetReg, genLog2(unsigned(imm)));
}
else
{
assert(oper == GT_UMOD);

emit->emitIns_R_I(INS_and, size, targetReg, imm -1);
}
}
#else // !0
NYI("genCodeForPow2Div");
#endif // !0
}


/***********************************************************************************************
* Generate code for localloc
*/
Expand Down
2 changes: 2 additions & 0 deletions src/jit/codegenlinear.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

void genCodeForMulHi(GenTreeOp* treeNode);

void genCodeForPow2Div(GenTreeOp* treeNode);

void genLeaInstruction(GenTreeAddrMode *lea);

void genSetRegToCond(regNumber dstReg, GenTreePtr tree);
Expand Down
163 changes: 144 additions & 19 deletions src/jit/codegenxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1284,31 +1284,42 @@ void CodeGen::genCodeForDivMod(GenTreeOp* treeNode)
gcInfo.gcMarkRegSetNpt(RBM_RDX);
}

// Perform the 'targetType' (64-bit or 32-bit) divide instruction
instruction ins;
if (oper == GT_UMOD || oper == GT_UDIV)
ins = INS_div;
if (divisor->isContainedIntOrIImmed())
{
GenTreeIntConCommon* divImm = divisor->AsIntConCommon();
assert(divImm->IsIntCnsFitsInI32());
ssize_t imm = divImm->IconValue();
assert(isPow2(abs(imm)));
genCodeForPow2Div(treeNode->AsOp());
}
else
ins = INS_idiv;
{
// Perform the 'targetType' (64-bit or 32-bit) divide instruction
instruction ins;
if (oper == GT_UMOD || oper == GT_UDIV)
ins = INS_div;
else
ins = INS_idiv;

emit->emitInsBinary(ins, size, treeNode, divisor);
emit->emitInsBinary(ins, size, treeNode, divisor);

// Signed divide RDX:RAX by r/m64, with result
// stored in RAX := Quotient, RDX := Remainder.
// Move the result to the desired register, if necessary
if (oper == GT_DIV || oper == GT_UDIV)
{
if (targetReg != REG_RAX)
// Signed divide RDX:RAX by r/m64, with result
// stored in RAX := Quotient, RDX := Remainder.
// Move the result to the desired register, if necessary
if (oper == GT_DIV || oper == GT_UDIV)
{
inst_RV_RV(INS_mov, targetReg, REG_RAX, targetType);
if (targetReg != REG_RAX)
{
inst_RV_RV(INS_mov, targetReg, REG_RAX, targetType);
}
}
}
else
{
assert((oper == GT_MOD) || (oper == GT_UMOD));
if (targetReg != REG_RDX)
else
{
inst_RV_RV(INS_mov, targetReg, REG_RDX, targetType);
assert((oper == GT_MOD) || (oper == GT_UMOD));
if (targetReg != REG_RDX)
{
inst_RV_RV(INS_mov, targetReg, REG_RDX, targetType);
}
}
}
}
Expand Down Expand Up @@ -2879,6 +2890,120 @@ CodeGen::genMultiRegCallStoreToLocal(GenTreePtr treeNode)
#endif // !FEATURE_UNIX_AMD64_STRUCT_PASSING
}

// Generate code for division (or mod) by power of two
// or negative powers of two. (meaning -1 * a power of two, not 2^(-1))
// Op2 must be a contained integer constant.
void
CodeGen::genCodeForPow2Div(GenTreeOp* tree)
{
GenTree *dividend = tree->gtOp.gtOp1;
GenTree *divisor = tree->gtOp.gtOp2;
genTreeOps oper = tree->OperGet();
emitAttr size = emitTypeSize(tree);
emitter *emit = getEmitter();
regNumber targetReg = tree->gtRegNum;
var_types targetType = tree->TypeGet();

bool isSigned = oper == GT_MOD || oper == GT_DIV;

// precondition: extended dividend is in RDX:RAX
// which means it is either all zeros or all ones

noway_assert(divisor->isContained());
GenTreeIntConCommon* divImm = divisor->AsIntConCommon();
ssize_t imm = divImm->IconValue();
ssize_t abs_imm = abs(imm);
noway_assert(isPow2(abs_imm));


if (isSigned)
{
if (imm == 1)
{
if (oper == GT_DIV)
{
if (targetReg != REG_RAX)
inst_RV_RV(INS_mov, targetReg, REG_RAX, targetType);
}
else
{
assert(oper == GT_MOD);
instGen_Set_Reg_To_Zero(size, targetReg);
}

return;
}

if (abs_imm == 2)
{
if (oper == GT_MOD)
{
emit->emitIns_R_I(INS_and, size, REG_RAX, 1); // result is 0 or 1
// xor with rdx will flip all bits if negative
emit->emitIns_R_R(INS_xor, size, REG_RAX, REG_RDX); // 111.11110 or 0
}
else
{
assert(oper == GT_DIV);
// add 1 if it's negative
emit->emitIns_R_R(INS_sub, size, REG_RAX, REG_RDX);
}
}
else
{
// add imm-1 if negative
emit->emitIns_R_I(INS_and, size, REG_RDX, abs_imm - 1);
emit->emitIns_R_R(INS_add, size, REG_RAX, REG_RDX);
}

if (oper == GT_DIV)
{
unsigned shiftAmount = genLog2(unsigned(abs_imm));
inst_RV_SH(INS_sar, size, REG_RAX, shiftAmount);

if (imm < 0)
{
emit->emitIns_R(INS_neg, size, REG_RAX);
}
}
else
{
assert(oper == GT_MOD);
if (abs_imm > 2)
{
emit->emitIns_R_I(INS_and, size, REG_RAX, abs_imm - 1);
}
// RDX contains 'imm-1' if negative
emit->emitIns_R_R(INS_sub, size, REG_RAX, REG_RDX);
}

if (targetReg != REG_RAX)
{
inst_RV_RV(INS_mov, targetReg, REG_RAX, targetType);
}
}
else
{
assert (imm > 0);

if (targetReg != dividend->gtRegNum)
{
inst_RV_RV(INS_mov, targetReg, dividend->gtRegNum, targetType);
}

if (oper == GT_UDIV)
{
inst_RV_SH(INS_shr, size, targetReg, genLog2(unsigned(imm)));
}
else
{
assert(oper == GT_UMOD);

emit->emitIns_R_I(INS_and, size, targetReg, imm -1);
}
}
}


/***********************************************************************************************
* Generate code for localloc
Expand Down
Loading

0 comments on commit ce8e7e3

Please sign in to comment.