Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fixes GT_MOD codegen and three other issues for ARM64 #3390

Merged
merged 1 commit into from
Feb 27, 2016
Merged

Fixes GT_MOD codegen and three other issues for ARM64 #3390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 56 additions & 37 deletions src/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2420,23 +2420,26 @@ CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
// Floating point divide never raises an exception
genCodeForBinary(treeNode);
}
else // integer divide operation
else // an integer divide operation
{
GenTreePtr divisorOp = treeNode->gtGetOp2();
emitAttr size = EA_ATTR(genTypeSize(genActualType(treeNode->TypeGet())));

// TODO-ARM64-CQ: Optimize a divide by power of 2 as we do for AMD64

if (divisorOp->IsZero())
{
emitJumpKind jmpEqual = genJumpKindForOper(GT_EQ, CK_SIGNED);
genJumpToThrowHlpBlk(jmpEqual, SCK_DIV_BY_ZERO);
// We don't need to generate the sdiv/udiv instruction
// We unconditionally throw a divide by zero exception
genJumpToThrowHlpBlk(EJ_jmp, SCK_DIV_BY_ZERO);

// We still need to call genProduceReg
genProduceReg(treeNode);
}
else
else // the divisor is not the constant zero
{
emitAttr cmpSize = EA_ATTR(genTypeSize(genActualType(treeNode->TypeGet())));
regNumber divisorReg = divisorOp->gtRegNum;

// Generate the require runtime checks for GT_DIV or GT_UDIV
if (treeNode->gtOper == GT_DIV)
{
BasicBlock* sdivLabel = genCreateTempLabel();
Expand All @@ -2446,46 +2449,49 @@ CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
// (MinInt / -1) => ArithmeticException
//
bool checkDividend = true;
// Do we have a contained immediate for the 'divisorOp'?
if (divisorOp->isContainedIntOrIImmed())

// Do we have an immediate for the 'divisorOp'?
//
if (divisorOp->IsCnsIntOrI())
{
GenTreeIntConCommon* intConst = divisorOp->AsIntConCommon();
assert(intConst->IconValue() != 0); // already checked above by IsZero()
if (intConst->IconValue() != -1)
GenTreeIntConCommon* intConstTree = divisorOp->AsIntConCommon();
ssize_t intConstValue = intConstTree->IconValue();
assert(intConstValue != 0); // already checked above by IsZero()
if (intConstValue != -1)
{
checkDividend = false; // We statically know that the dividend is not -1
}
}
else
else // insert check for divison by zero
{
// Check if the divisor is zero throw a DivideByZeroException
emit->emitIns_R_I(INS_cmp, cmpSize, divisorReg, 0);
emit->emitIns_R_I(INS_cmp, size, divisorReg, 0);
emitJumpKind jmpEqual = genJumpKindForOper(GT_EQ, CK_SIGNED);
genJumpToThrowHlpBlk(jmpEqual, SCK_DIV_BY_ZERO);

}

if (checkDividend)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my understanding - Arm32 legacy jit doesn't seem to be doing div-by-zero and minint-div-by-minus1 checks. Is this temporarily required for arm64 till VM has support for propagating hardware exceptions as managed exceptions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On arm64 the hardware never generates any faults for the integer divide instruction.
On arm32 there are two possibilities:

  1. the hardware faults and doesn't implement the integer divide instruction
  2. it behaves as arm64 does.

The arm32 hardware that we used back when we developed the arm32 JIT was case #1, so we had to use helper calls to implement integer divide.

// Check if the divisor is not -1 branch to 'sdivLabel'
emit->emitIns_R_I(INS_cmp, cmpSize, divisorReg, -1);
emit->emitIns_R_I(INS_cmp, size, divisorReg, -1);

emitJumpKind jmpNotEqual = genJumpKindForOper(GT_NE, CK_SIGNED);
inst_JMP(jmpNotEqual, sdivLabel);
// If control flow continues past here the 'divisorReg' is known to be -1
}

if (checkDividend)
{

regNumber dividendReg = treeNode->gtGetOp1()->gtRegNum;
// At this point the divisor is known to be -1
//
// Issue 'adds zr, dividendReg, dividendReg' instruction
// this will set the Z and V flags only when dividendReg is MinInt
// Issue the 'adds zr, dividendReg, dividendReg' instruction
// this will set both the Z and V flags only when dividendReg is MinInt
//
emit->emitIns_R_R_R(INS_adds, cmpSize, REG_ZR, dividendReg, dividendReg);
emitJumpKind jmpNotEqual = genJumpKindForOper(GT_NE, CK_SIGNED);
emit->emitIns_R_R_R(INS_adds, size, REG_ZR, dividendReg, dividendReg);

inst_JMP(jmpNotEqual, sdivLabel); // goto sdiv if the Z flag is clear
genJumpToThrowHlpBlk(EJ_vs, SCK_ARITH_EXCPN); // if the V flags is set throw ArithmeticException
}
genJumpToThrowHlpBlk(EJ_vs, SCK_ARITH_EXCPN); // if the V flags is set throw ArithmeticException

genDefineTempLabel(sdivLabel);
genDefineTempLabel(sdivLabel);
}
genCodeForBinary(treeNode); // Generate the sdiv instruction
}
else // (treeNode->gtOper == GT_UDIV)
Expand All @@ -2495,14 +2501,15 @@ CodeGen::genCodeForTreeNode(GenTreePtr treeNode)
//
// Note that division by the constant 0 was already checked for above by the op2->IsZero() check
//
if (!divisorOp->isContainedIntOrIImmed())
{
emit->emitIns_R_I(INS_cmp, cmpSize, divisorReg, 0);
if (!divisorOp->IsCnsIntOrI())
{
// divisorOp is not a constant, so it could be zero
//
emit->emitIns_R_I(INS_cmp, size, divisorReg, 0);
emitJumpKind jmpEqual = genJumpKindForOper(GT_EQ, CK_SIGNED);
genJumpToThrowHlpBlk(jmpEqual, SCK_DIV_BY_ZERO);
}

genCodeForBinary(treeNode); // Generate the udiv instruction
genCodeForBinary(treeNode);
}
}
}
Expand Down Expand Up @@ -3716,11 +3723,13 @@ CodeGen::genLclHeap(GenTreePtr tree)
}
else if (!compiler->info.compInitMem && (amount < CORINFO_PAGE_SIZE)) // must be < not <=
{
// Since the size is a page or less, simply adjust ESP
// ESP might already be in the guard page, must touch it BEFORE
// Since the size is a page or less, simply adjust the SP value
// The SP might already be in the guard page, must touch it BEFORE
// the alloc, not after.
getEmitter()->emitIns_AR_R(INS_TEST, EA_4BYTE, REG_SPBASE, REG_SPBASE, 0);
inst_RV_IV(INS_sub, REG_SPBASE, amount, EA_PTRSIZE);
// ldr wz, [SP, #0]
getEmitter()->emitIns_R_R_I(INS_ldr, EA_4BYTE, REG_ZR, REG_SP, 0);

inst_RV_IV(INS_sub, REG_SP, amount, EA_PTRSIZE);

goto ALLOC_DONE;
}
Expand Down Expand Up @@ -4744,8 +4753,16 @@ void CodeGen::genUnspillRegIfNeeded(GenTree *tree)
GenTreeLclVarCommon* lcl = unspillTree->AsLclVarCommon();
LclVarDsc* varDsc = &compiler->lvaTable[lcl->gtLclNum];

var_types targetType = unspillTree->gtType;
instruction ins = ins_Load(targetType, compiler->isSIMDTypeLocalAligned(lcl->gtLclNum));
emitAttr attr = emitTypeSize(targetType);
emitter * emit = getEmitter();

// Fixes Issue #3326
attr = emit->emitInsAdjustLoadStoreAttr(ins, attr);

// Load local variable from its home location.
inst_RV_TT(ins_Load(unspillTree->gtType, compiler->isSIMDTypeLocalAligned(lcl->gtLclNum)), dstReg, unspillTree);
inst_RV_TT(ins, dstReg, unspillTree, 0, attr);

unspillTree->SetInReg();

Expand Down Expand Up @@ -6070,7 +6087,6 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
movSize = emitTypeSize(extendType);
movRequired = true;
}

else
{
if (genTypeSize(srcType) < genTypeSize(dstType))
Expand Down Expand Up @@ -6101,6 +6117,9 @@ void CodeGen::genIntToIntCast(GenTreePtr treeNode)
}
}

// We should never be generating a load from memory instruction here!
assert(!emit->emitInsIsLoad(ins));

if ((ins != INS_mov) || movRequired || (targetReg != sourceReg))
{
emit->emitIns_R_R(ins, movSize, targetReg, sourceReg);
Expand Down
4 changes: 2 additions & 2 deletions src/jit/gtlist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ GTNODE(MUL , "*" ,1,GTK_BINOP)
GTNODE(DIV , "/" ,0,GTK_BINOP)
GTNODE(MOD , "%" ,0,GTK_BINOP)

GTNODE(UDIV , "/" ,0,GTK_BINOP)
GTNODE(UMOD , "%" ,0,GTK_BINOP)
GTNODE(UDIV , "un-/" ,0,GTK_BINOP)
GTNODE(UMOD , "un-%" ,0,GTK_BINOP)

GTNODE(OR , "|" ,1,GTK_BINOP|GTK_LOGOP)
GTNODE(XOR , "^" ,1,GTK_BINOP|GTK_LOGOP)
Expand Down
22 changes: 19 additions & 3 deletions src/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9107,7 +9107,11 @@ GenTreePtr Compiler::fgMorphSmpOp(GenTreePtr tree, MorphAddrContext* ma

case GT_UMOD:

#ifndef _TARGET_ARM_
#ifdef _TARGET_ARMARCH_
//
// Note for _TARGET_ARMARCH_ we don't have a remainder instruction, so we don't do this optimization
//
#else // _TARGET_XARCH
/* If this is an unsigned long mod with op2 which is a cast to long from a
constant int, then don't morph to a call to the helper. This can be done
faster inline using idiv.
Expand Down Expand Up @@ -9148,7 +9152,7 @@ GenTreePtr Compiler::fgMorphSmpOp(GenTreePtr tree, MorphAddrContext* ma
return tree;
}
}
#endif // !_TARGET_ARM_
#endif // _TARGET_XARCH

ASSIGN_HELPER_FOR_MOD:

Expand Down Expand Up @@ -12283,6 +12287,19 @@ GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree)
assert(!"This should only be called for ARM64");
#endif

if (tree->OperGet() == GT_MOD)
{
tree->SetOper(GT_DIV);
}
else if (tree->OperGet() == GT_UMOD)
{
tree->SetOper(GT_UDIV);
}
else
{
noway_assert(!"Illegal gtOper in fgMorphModToSubMulDiv");
}

var_types type = tree->gtType;
GenTree* denominator = tree->gtOp2;
GenTree* numerator = tree->gtOp1;
Expand All @@ -12297,7 +12314,6 @@ GenTree* Compiler::fgMorphModToSubMulDiv(GenTreeOp* tree)
denominator = fgMakeMultiUse(&tree->gtOp2);
}

tree->SetOper(GT_DIV);
GenTree* mul = gtNewOperNode(GT_MUL, type, tree, gtCloneExpr(denominator));
GenTree* sub = gtNewOperNode(GT_SUB, type, gtCloneExpr(numerator), mul);

Expand Down
Loading