Skip to content

Commit 213a41d

Browse files
authored
[Wasm RyuJit] Null checks for calls, overflow checks for int to int casts, resolve stack ordering, more (#124534)
Various fixes from running crossgen * fix return type classification for structs returned as primitives * null checks for calls * overflow checks for int to int casts * stack ordering and null check for delegate invoke * stack ordering for NOT
1 parent 0601ec7 commit 213a41d

File tree

11 files changed

+266
-23
lines changed

11 files changed

+266
-23
lines changed

src/coreclr/jit/codegen.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ class CodeGen final : public CodeGenInterface
216216
unsigned findTargetDepth(BasicBlock* target);
217217
void WasmProduceReg(GenTree* node);
218218
regNumber GetMultiUseOperandReg(GenTree* operand);
219+
void genEmitNullCheck(regNumber reg);
219220
#endif
220221

221222
void genEmitStartBlock(BasicBlock* block);

src/coreclr/jit/codegenwasm.cpp

Lines changed: 131 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -747,16 +747,19 @@ static constexpr uint32_t PackTypes(var_types toType, var_types fromType)
747747
//
748748
void CodeGen::genIntToIntCast(GenTreeCast* cast)
749749
{
750-
if (cast->gtOverflow())
750+
GenIntCastDesc desc(cast);
751+
752+
if (desc.CheckKind() != GenIntCastDesc::CHECK_NONE)
751753
{
752-
NYI_WASM("Overflow checks");
754+
GenTree* castValue = cast->gtGetOp1();
755+
regNumber castReg = GetMultiUseOperandReg(castValue);
756+
genIntCastOverflowCheck(cast, desc, castReg);
753757
}
754758

755-
GenIntCastDesc desc(cast);
756-
var_types toType = genActualType(cast->CastToType());
757-
var_types fromType = genActualType(cast->CastOp());
758-
int extendSize = desc.ExtendSrcSize();
759-
instruction ins = INS_none;
759+
var_types toType = genActualType(cast->CastToType());
760+
var_types fromType = genActualType(cast->CastOp());
761+
int extendSize = desc.ExtendSrcSize();
762+
instruction ins = INS_none;
760763
assert(fromType == TYP_INT || fromType == TYP_LONG);
761764

762765
genConsumeOperands(cast);
@@ -819,6 +822,98 @@ void CodeGen::genIntToIntCast(GenTreeCast* cast)
819822
WasmProduceReg(cast);
820823
}
821824

825+
//------------------------------------------------------------------------
826+
// genIntCastOverflowCheck: Generate overflow checking code for an integer cast.
827+
//
828+
// Arguments:
829+
// cast - The GT_CAST node
830+
// desc - The cast description
831+
// reg - The register containing the value to check
832+
//
833+
void CodeGen::genIntCastOverflowCheck(GenTreeCast* cast, const GenIntCastDesc& desc, regNumber reg)
834+
{
835+
bool const is64BitSrc = (desc.CheckSrcSize() == 8);
836+
emitAttr const srcSize = is64BitSrc ? EA_8BYTE : EA_4BYTE;
837+
838+
GetEmitter()->emitIns_I(INS_local_get, srcSize, WasmRegToIndex(reg));
839+
840+
switch (desc.CheckKind())
841+
{
842+
case GenIntCastDesc::CHECK_POSITIVE:
843+
{
844+
// INT or LONG to ULONG
845+
GetEmitter()->emitIns_I(is64BitSrc ? INS_i64_const : INS_i32_const, srcSize, 0);
846+
GetEmitter()->emitIns(is64BitSrc ? INS_i64_lt_s : INS_i32_lt_s);
847+
genJumpToThrowHlpBlk(SCK_OVERFLOW);
848+
break;
849+
}
850+
851+
case GenIntCastDesc::CHECK_UINT_RANGE:
852+
{
853+
// (U)LONG to UINT
854+
assert(is64BitSrc);
855+
GetEmitter()->emitIns_I(INS_i64_const, srcSize, UINT32_MAX);
856+
// We can re-interpret LONG as ULONG
857+
// Then negative values will be larger than UINT32_MAX
858+
GetEmitter()->emitIns(INS_i64_gt_u);
859+
genJumpToThrowHlpBlk(SCK_OVERFLOW);
860+
break;
861+
}
862+
863+
case GenIntCastDesc::CHECK_POSITIVE_INT_RANGE:
864+
{
865+
// ULONG to INT
866+
GetEmitter()->emitIns_I(INS_i64_const, srcSize, INT32_MAX);
867+
GetEmitter()->emitIns(INS_i64_gt_u);
868+
genJumpToThrowHlpBlk(SCK_OVERFLOW);
869+
break;
870+
}
871+
872+
case GenIntCastDesc::CHECK_INT_RANGE:
873+
{
874+
// LONG to INT
875+
GetEmitter()->emitIns(INS_i64_extend32_s);
876+
GetEmitter()->emitIns_I(INS_local_get, srcSize, WasmRegToIndex(reg));
877+
GetEmitter()->emitIns(INS_i64_ne);
878+
genJumpToThrowHlpBlk(SCK_OVERFLOW);
879+
break;
880+
}
881+
882+
case GenIntCastDesc::CHECK_SMALL_INT_RANGE:
883+
{
884+
// (U)(INT|LONG) to Small INT
885+
const int castMaxValue = desc.CheckSmallIntMax();
886+
const int castMinValue = desc.CheckSmallIntMin();
887+
888+
if (castMinValue == 0)
889+
{
890+
// When the minimum is 0, a single unsigned upper-bound check is sufficient.
891+
// For signed sources, negative values become large unsigned values and
892+
// thus also trigger the overflow via the same comparison.
893+
GetEmitter()->emitIns_I(is64BitSrc ? INS_i64_const : INS_i32_const, srcSize, castMaxValue);
894+
GetEmitter()->emitIns(is64BitSrc ? INS_i64_gt_u : INS_i32_gt_u);
895+
}
896+
else
897+
{
898+
// We need to check a range around zero, eg [-128, 127] for 8-bit signed.
899+
// Do two compares and combine the results: (src > max) | (src < min).
900+
assert(!cast->IsUnsigned());
901+
GetEmitter()->emitIns_I(is64BitSrc ? INS_i64_const : INS_i32_const, srcSize, castMaxValue);
902+
GetEmitter()->emitIns(is64BitSrc ? INS_i64_gt_s : INS_i32_gt_s);
903+
GetEmitter()->emitIns_I(INS_local_get, srcSize, WasmRegToIndex(reg));
904+
GetEmitter()->emitIns_I(is64BitSrc ? INS_i64_const : INS_i32_const, srcSize, castMinValue);
905+
GetEmitter()->emitIns(is64BitSrc ? INS_i64_lt_s : INS_i32_lt_s);
906+
GetEmitter()->emitIns(INS_i32_or);
907+
}
908+
genJumpToThrowHlpBlk(SCK_OVERFLOW);
909+
break;
910+
}
911+
912+
default:
913+
unreached();
914+
}
915+
}
916+
822917
//------------------------------------------------------------------------
823918
// genFloatToIntCast: Generate code for a floating point to integer cast
824919
//
@@ -1368,6 +1463,22 @@ void CodeGen::genJumpToThrowHlpBlk(SpecialCodeKind codeKind)
13681463
void CodeGen::genCodeForNullCheck(GenTreeIndir* tree)
13691464
{
13701465
genConsumeAddress(tree->Addr());
1466+
genEmitNullCheck(REG_NA);
1467+
}
1468+
1469+
//---------------------------------------------------------------------
1470+
// genEmitNullCheck - generate code for a null check
1471+
//
1472+
// Arguments:
1473+
// regNum - register to check, or REG_NA if value to check is on the stack
1474+
//
1475+
void CodeGen::genEmitNullCheck(regNumber reg)
1476+
{
1477+
if (reg != REG_NA)
1478+
{
1479+
GetEmitter()->emitIns_I(INS_local_get, EA_PTRSIZE, WasmRegToIndex(reg));
1480+
}
1481+
13711482
GetEmitter()->emitIns_I(INS_I_const, EA_PTRSIZE, m_compiler->compMaxUncheckedOffsetForNullObject);
13721483
GetEmitter()->emitIns(INS_I_le_u);
13731484
genJumpToThrowHlpBlk(SCK_NULL_CHECK);
@@ -1629,15 +1740,20 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree)
16291740
//------------------------------------------------------------------------
16301741
// genCall: Produce code for a GT_CALL node
16311742
//
1743+
// Arguments:
1744+
// call - the GT_CALL node
1745+
//
16321746
void CodeGen::genCall(GenTreeCall* call)
16331747
{
1748+
regNumber thisReg = REG_NA;
1749+
16341750
if (call->NeedsNullCheck())
16351751
{
1636-
NYI_WASM("Insert nullchecks for calls that need it in lowering");
1752+
CallArg* thisArg = call->gtArgs.GetThisArg();
1753+
GenTree* thisNode = thisArg->GetNode();
1754+
thisReg = GetMultiUseOperandReg(thisNode);
16371755
}
16381756

1639-
assert(!call->IsTailCall());
1640-
16411757
for (CallArg& arg : call->gtArgs.EarlyArgs())
16421758
{
16431759
genConsumeReg(arg.GetEarlyNode());
@@ -1648,6 +1764,11 @@ void CodeGen::genCall(GenTreeCall* call)
16481764
genConsumeReg(arg.GetLateNode());
16491765
}
16501766

1767+
if (call->NeedsNullCheck())
1768+
{
1769+
genEmitNullCheck(thisReg);
1770+
}
1771+
16511772
genCallInstruction(call);
16521773
WasmProduceReg(call);
16531774
}

src/coreclr/jit/compiler.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd,
766766
}
767767
else
768768
{
769-
howToReturnStruct = SPK_ByValue;
769+
howToReturnStruct = SPK_PrimitiveType;
770770
useType = WasmClassifier::ToJitType(abiType);
771771
}
772772

src/coreclr/jit/gentree.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31035,6 +31035,11 @@ void ReturnTypeDesc::InitializeStructReturnType(Compiler* comp,
3103531035
m_regType[i] = comp->getJitGCType(gcPtrs[i]);
3103631036
}
3103731037

31038+
#elif defined(TARGET_WASM)
31039+
31040+
// For Wasm, structs are either returned by-ref or as primitives.
31041+
unreached();
31042+
3103831043
#else // TARGET_XXX
3103931044

3104031045
// This target needs support here!
@@ -34175,3 +34180,26 @@ ValueSize ValueSize::FromJitType(var_types type)
3417534180
return ValueSize(genTypeSize(type));
3417634181
}
3417734182
}
34183+
34184+
//------------------------------------------------------------------------
34185+
// gtFirstNodeInOperandOrder : return the first node of this tree
34186+
// in operand order
34187+
//
34188+
// Returns:
34189+
// If tree is a leaf, return the tree.
34190+
// If the tree has operands, recurse on the first operand.
34191+
//
34192+
GenTree* GenTree::gtFirstNodeInOperandOrder()
34193+
{
34194+
GenTree* op = this;
34195+
GenTree::VisitResult visitResult;
34196+
do
34197+
{
34198+
visitResult = op->VisitOperands([&op](GenTree* operand) {
34199+
op = operand;
34200+
return GenTree::VisitResult::Abort;
34201+
});
34202+
} while (visitResult == GenTree::VisitResult::Abort);
34203+
34204+
return op;
34205+
}

src/coreclr/jit/gentree.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,6 +1984,8 @@ struct GenTree
19841984

19851985
inline GenTree* gtCommaStoreVal();
19861986

1987+
GenTree* gtFirstNodeInOperandOrder();
1988+
19871989
// Return the child of this node if it is a GT_RELOAD or GT_COPY; otherwise simply return the node itself
19881990
inline GenTree* gtSkipReloadOrCopy();
19891991

src/coreclr/jit/lower.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,8 +2012,24 @@ void Lowering::LowerArgsForCall(GenTreeCall* call)
20122012
#endif // defined(TARGET_X86) && defined(FEATURE_IJW)
20132013

20142014
LegalizeArgPlacement(call);
2015+
AfterLowerArgsForCall(call);
20152016
}
20162017

2018+
#if !defined(TARGET_WASM)
2019+
2020+
//------------------------------------------------------------------------
2021+
// AfterLowerArgsForCall: post processing after call args are lowered
2022+
//
2023+
// Arguments:
2024+
// call - Call node
2025+
//
2026+
void Lowering::AfterLowerArgsForCall(GenTreeCall* call)
2027+
{
2028+
// no-op for non-Wasm targets
2029+
}
2030+
2031+
#endif // !defined(TARGET_WASM)
2032+
20172033
#if defined(TARGET_X86) && defined(FEATURE_IJW)
20182034
//------------------------------------------------------------------------
20192035
// LowerSpecialCopyArgs: Lower special copy arguments for P/Invoke IL stubs

src/coreclr/jit/lower.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ class Lowering final : public Phase
211211
GenTree* LowerVirtualVtableCall(GenTreeCall* call);
212212
GenTree* LowerVirtualStubCall(GenTreeCall* call);
213213
void LowerArgsForCall(GenTreeCall* call);
214+
void AfterLowerArgsForCall(GenTreeCall* call);
214215
#if defined(TARGET_X86) && defined(FEATURE_IJW)
215216
void LowerSpecialCopyArgs(GenTreeCall* call);
216217
void InsertSpecialCopyArg(GenTreePutArgStk* putArgStk, CORINFO_CLASS_HANDLE argType, unsigned lclNum);

src/coreclr/jit/lowerwasm.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,12 @@ GenTree* Lowering::LowerNeg(GenTreeOp* node)
121121
//
122122
GenTree* x = node->gtGetOp1();
123123
GenTree* zero = m_compiler->gtNewZeroConNode(node->TypeGet());
124-
BlockRange().InsertBefore(x, zero);
124+
125+
// To preserve stack order we must insert the zero before the entire
126+
// tree rooted at x.
127+
//
128+
GenTree* insertBefore = x->gtFirstNodeInOperandOrder();
129+
BlockRange().InsertBefore(insertBefore, zero);
125130
LowerNode(zero);
126131
node->ChangeOper(GT_SUB);
127132
node->gtOp1 = zero;
@@ -221,6 +226,11 @@ void Lowering::LowerPutArgStk(GenTreePutArgStk* putArgNode)
221226
void Lowering::LowerCast(GenTree* tree)
222227
{
223228
assert(tree->OperIs(GT_CAST));
229+
230+
if (tree->gtOverflow())
231+
{
232+
tree->gtGetOp1()->gtLIRFlags |= LIR::Flags::MultiplyUsed;
233+
}
224234
ContainCheckCast(tree->AsCast());
225235
}
226236

@@ -463,3 +473,19 @@ void Lowering::AfterLowerBlock()
463473
Stackifier stackifier(this);
464474
stackifier.StackifyCurrentBlock();
465475
}
476+
477+
//------------------------------------------------------------------------
478+
// AfterLowerArgsForCall: post processing after call args are lowered
479+
//
480+
// Arguments:
481+
// call - Call node
482+
//
483+
void Lowering::AfterLowerArgsForCall(GenTreeCall* call)
484+
{
485+
if (call->NeedsNullCheck())
486+
{
487+
// Prepare for explicit null check
488+
CallArg* thisArg = call->gtArgs.GetThisArg();
489+
thisArg->GetNode()->gtLIRFlags |= LIR::Flags::MultiplyUsed;
490+
}
491+
}

0 commit comments

Comments
 (0)