Skip to content

Commit 23bc7e3

Browse files
EgorBoegorbot
EgorBo
and
egorbot
authored
More AP-related optimizations (#124711)
This PR creates more assertions that can be used by various optimizations. Minimal examples: ```cs object Case1(object[] arr, int i, object b) { // covariant stores used to be ignored by range check arr[3] = b; // arr is >= 4 elements long return arr[2]; // no bounds check anymore } bool Case2(int a, int b) { int c = a / b; // b is never 0 here return b == 0; } byte[] Case3(int numBytes, IntPtr pbData) { byte[] data = new byte[numBytes]; if (numBytes < 0) // never true throw new InvalidOperationException(); return data; } ``` ```diff ; Method Examples:Case1(System.Object[],int,System.Object):System.Object:this (FullOpts) push rbx sub rsp, 32 mov rbx, rdx mov rcx, rbx mov r8, r9 mov edx, 3 call CORINFO_HELP_ARRADDR_ST - cmp dword ptr [rbx+0x08], 2 - jbe SHORT G_M25522_IG04 mov rax, gword ptr [rbx+0x20] add rsp, 32 pop rbx ret -G_M25522_IG04: - call CORINFO_HELP_RNGCHKFAIL - int3 -; Total bytes of code: 46 +; Total bytes of code: 34 ; Method Examples:Case2(int,int):bool:this (FullOpts) mov eax, edx cdq idiv edx:eax, r8d - test r8d, r8d - sete al - movzx rax, al + xor eax, eax ret -; Total bytes of code: 16 +; Total bytes of code: 9 ; Method Examples:Case3(int,nint):byte[]:this (FullOpts) - push rbx - sub rsp, 32 - mov ebx, edx - movsxd rdx, ebx + sub rsp, 40 + movsxd rdx, edx mov rcx, 0x7FFF08D305B8 ; byte[] call CORINFO_HELP_NEWARR_1_VC - test ebx, ebx - jl SHORT G_M3253_IG04 - add rsp, 32 - pop rbx - ret -G_M3253_IG04: - mov rcx, 0x7FFF08D8CB60 ; System.InvalidOperationException - call CORINFO_HELP_NEWSFAST - mov rbx, rax - mov rcx, rbx - call [System.InvalidOperationException:.ctor():this] - mov rcx, rbx - call CORINFO_HELP_THROW - int3 -; Total bytes of code: 71 + nop + add rsp, 40 + ret +; Total bytes of code: 28 ``` [diffs](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1304188&view=ms.vss-build-web.run-extensions-tab) --------- Co-authored-by: egorbot <egorbot@egorbo.com>
1 parent 6e15912 commit 23bc7e3

File tree

6 files changed

+117
-28
lines changed

6 files changed

+117
-28
lines changed

‎src/coreclr/jit/assertionprop.cpp‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2086,9 +2086,79 @@ void Compiler::optAssertionGen(GenTree* tree)
20862086
assert(thisArg != nullptr);
20872087
assertionInfo = optCreateAssertion(thisArg, nullptr, /*equals*/ false);
20882088
}
2089+
else if (!optLocalAssertionProp)
2090+
{
2091+
// Array allocation creates an assertion that the length argument is non-negative.
2092+
//
2093+
// var arr = new int[n]; - creates n >= 0 assertion
2094+
//
2095+
GenTree* lenArg = getArrayLengthFromAllocation(call);
2096+
if (lenArg != nullptr)
2097+
{
2098+
ValueNum lenVN = vnStore->VNIgnoreIntToLongCast(optConservativeNormalVN(lenArg));
2099+
if ((lenVN != ValueNumStore::NoVN) && !vnStore->IsVNConstant(lenVN) &&
2100+
(vnStore->TypeOfVN(lenVN) == TYP_INT))
2101+
{
2102+
ValueNum zeroVN = vnStore->VNZeroForType(TYP_INT);
2103+
assertionInfo = optAddAssertion(AssertionDsc::CreateConstantBound(this, VNF_GE, lenVN, zeroVN));
2104+
break;
2105+
}
2106+
}
2107+
2108+
// CORINFO_HELP_ARRADDR_ST(arrRef, idx, value) creates an assertion that "idx" is within the bounds of
2109+
// "arrRef" array
2110+
//
2111+
// arr[idx] = value; - creates idx is within bounds of arr assertion
2112+
//
2113+
CorInfoHelpFunc helperId = eeGetHelperNum(call->gtCallMethHnd);
2114+
if ((helperId == CORINFO_HELP_ARRADDR_ST) || (helperId == CORINFO_HELP_LDELEMA_REF))
2115+
{
2116+
assert(call->gtArgs.CountUserArgs() == 3);
2117+
GenTree* arrRef = call->gtArgs.GetUserArgByIndex(0)->GetNode();
2118+
GenTree* idx = call->gtArgs.GetUserArgByIndex(1)->GetNode();
2119+
2120+
ValueNum idxVN = vnStore->VNIgnoreIntToLongCast(optConservativeNormalVN(idx));
2121+
if ((idxVN != ValueNumStore::NoVN) && (vnStore->TypeOfVN(idxVN) == TYP_INT))
2122+
{
2123+
ValueNum arrRefVN = optConservativeNormalVN(arrRef);
2124+
if (arrRefVN != ValueNumStore::NoVN)
2125+
{
2126+
// Compose a VN_ARR_LENGTH VN for the array reference and use it
2127+
// to create a bounds check assertion on the index.
2128+
ValueNum lenVN = vnStore->VNForFunc(TYP_INT, VNF_ARR_LENGTH, arrRefVN);
2129+
assertionInfo = optAddAssertion(AssertionDsc::CreateNoThrowArrBnd(this, idxVN, lenVN));
2130+
break;
2131+
}
2132+
}
2133+
}
2134+
}
20892135
}
20902136
break;
20912137

2138+
case GT_DIV:
2139+
case GT_UDIV:
2140+
case GT_MOD:
2141+
case GT_UMOD:
2142+
if (!optLocalAssertionProp)
2143+
{
2144+
// For division/modulo, we can create an assertion that the divisor is not zero
2145+
//
2146+
// c = a / b; - creates b != 0 assertion
2147+
//
2148+
ValueNum divisorVN = optConservativeNormalVN(tree->AsOp()->gtGetOp2());
2149+
if ((divisorVN != ValueNumStore::NoVN) && !vnStore->IsVNConstant(divisorVN))
2150+
{
2151+
var_types divisorType = vnStore->TypeOfVN(divisorVN);
2152+
if (varTypeIsIntegral(divisorType))
2153+
{
2154+
ValueNum zeroVN = vnStore->VNZeroForType(divisorType);
2155+
assertionInfo =
2156+
optAddAssertion(AssertionDsc::CreateConstantBound(this, VNF_NE, divisorVN, zeroVN));
2157+
}
2158+
}
2159+
}
2160+
break;
2161+
20922162
case GT_JTRUE:
20932163
assertionInfo = optAssertionGenJtrue(tree);
20942164
break;

‎src/coreclr/jit/compiler.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7592,7 +7592,7 @@ class Compiler
75927592

75937593
typedef JitHashTable<unsigned, JitSmallPrimitiveKeyFuncs<unsigned>, GenTree*> LocalNumberToNullCheckTreeMap;
75947594

7595-
GenTree* getArrayLengthFromAllocation(GenTree* tree DEBUGARG(BasicBlock* block));
7595+
GenTree* getArrayLengthFromAllocation(GenTree* tree);
75967596
GenTree* optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropKind valueKind, int walkDepth);
75977597
GenTree* optPropGetValue(unsigned lclNum, unsigned ssaNum, optPropKind valueKind);
75987598
GenTree* optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap);

‎src/coreclr/jit/earlyprop.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK
281281
{
282282
if (valueKind == optPropKind::OPK_ARRAYLEN)
283283
{
284-
value = getArrayLengthFromAllocation(defValue DEBUGARG(ssaVarDsc->GetBlock()));
284+
value = getArrayLengthFromAllocation(defValue);
285285
if (value != nullptr)
286286
{
287287
if (!value->IsCnsIntOrI())

‎src/coreclr/jit/gentree.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2050,13 +2050,12 @@ bool GenTreeCall::IsPure(Compiler* compiler) const
20502050
// helper call.
20512051
//
20522052
// Arguments:
2053-
// tree - The array allocation helper call.
2054-
// block - tree's basic block.
2053+
// tree - The array allocation helper call.
20552054
//
20562055
// Return Value:
20572056
// Return the array length node.
20582057

2059-
GenTree* Compiler::getArrayLengthFromAllocation(GenTree* tree DEBUGARG(BasicBlock* block))
2058+
GenTree* Compiler::getArrayLengthFromAllocation(GenTree* tree)
20602059
{
20612060
assert(tree != nullptr);
20622061

@@ -2252,7 +2251,7 @@ bool GenTreeCall::HasSideEffects(Compiler* compiler, bool ignoreExceptions, bool
22522251
// Consider array allocators side-effect free for constant length (if it's not negative and fits into i32)
22532252
if (helperProperties.IsAllocator(helper))
22542253
{
2255-
GenTree* arrLen = compiler->getArrayLengthFromAllocation((GenTree*)this DEBUGARG(nullptr));
2254+
GenTree* arrLen = compiler->getArrayLengthFromAllocation((GenTree*)this);
22562255
// if arrLen is nullptr it means it wasn't an array allocator
22572256
if ((arrLen != nullptr) && arrLen->IsIntCnsFitsInI32())
22582257
{

‎src/coreclr/jit/valuenum.cpp‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,6 +2045,44 @@ ValueNum ValueNumStore::VNForCastOper(var_types castToType, bool srcIsUnsigned)
20452045
return result;
20462046
}
20472047

2048+
//------------------------------------------------------------------------
2049+
// VNIgnoreIntToLongCast: Looks through a sign-extending int-to-long cast
2050+
// or convert long-typed integral constants to int.
2051+
//
2052+
// Arguments:
2053+
// vn - The value number to inspect.
2054+
//
2055+
// Return Value:
2056+
// The value number of the original TYP_INT operand if 'vn' is a VNF_Cast
2057+
// that sign-extends a TYP_INT to TYP_LONG; or the value number of a TYP_INT
2058+
// constant if 'vn' is a TYP_LONG constant that fits in an int; otherwise, 'vn' itself.
2059+
//
2060+
ValueNum ValueNumStore::VNIgnoreIntToLongCast(ValueNum vn)
2061+
{
2062+
if (TypeOfVN(vn) == TYP_LONG)
2063+
{
2064+
VNFuncApp funcApp;
2065+
if (GetVNFunc(vn, &funcApp) && funcApp.FuncIs(VNF_Cast))
2066+
{
2067+
var_types castToType;
2068+
bool srcIsUnsigned;
2069+
GetCastOperFromVN(funcApp.m_args[1], &castToType, &srcIsUnsigned);
2070+
if ((castToType == TYP_LONG) && !srcIsUnsigned && TypeOfVN(funcApp.m_args[0]) == TYP_INT)
2071+
{
2072+
return funcApp.m_args[0];
2073+
}
2074+
}
2075+
2076+
// Also look through any long-typed integral constant that fits in an int.
2077+
int intCns;
2078+
if (IsVNIntegralConstant(vn, &intCns))
2079+
{
2080+
return VNForIntCon(intCns);
2081+
}
2082+
}
2083+
return vn;
2084+
}
2085+
20482086
void ValueNumStore::GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* pSrcIsUnsigned)
20492087
{
20502088
assert(pCastToType != nullptr);
@@ -2625,28 +2663,8 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN)
26252663
VNFuncApp newArrFuncApp;
26262664
if (GetVNFunc(arg0VN, &newArrFuncApp) && (newArrFuncApp.m_func == VNF_JitNewArr))
26272665
{
2628-
ValueNum actualSizeVN = newArrFuncApp.m_args[1];
2629-
var_types actualSizeVNType = TypeOfVN(actualSizeVN);
2630-
2631-
// JitNewArr's size argument (args[1]) is typically upcasted to TYP_LONG via VNF_Cast.
2632-
if (actualSizeVNType == TYP_LONG)
2633-
{
2634-
VNFuncApp castFuncApp;
2635-
if (GetVNFunc(actualSizeVN, &castFuncApp) && (castFuncApp.m_func == VNF_Cast))
2636-
{
2637-
var_types castToType;
2638-
bool srcIsUnsigned;
2639-
GetCastOperFromVN(castFuncApp.m_args[1], &castToType, &srcIsUnsigned);
2640-
2641-
// Make sure we have exactly (TYP_LONG)myInt32 cast:
2642-
if (!srcIsUnsigned && (castToType == TYP_LONG) && TypeOfVN(castFuncApp.m_args[0]) == TYP_INT)
2643-
{
2644-
// If that is the case, return the original size argument
2645-
*resultVN = castFuncApp.m_args[0];
2646-
}
2647-
}
2648-
}
2649-
else if (actualSizeVNType == TYP_INT)
2666+
ValueNum actualSizeVN = VNIgnoreIntToLongCast(newArrFuncApp.m_args[1]);
2667+
if (TypeOfVN(actualSizeVN) == TYP_INT)
26502668
{
26512669
*resultVN = actualSizeVN;
26522670
}

‎src/coreclr/jit/valuenum.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,8 @@ class ValueNumStore
508508
// Unpacks the information stored by VNForCastOper in the constant represented by the value number.
509509
void GetCastOperFromVN(ValueNum vn, var_types* pCastToType, bool* pSrcIsUnsigned);
510510

511+
ValueNum VNIgnoreIntToLongCast(ValueNum vn);
512+
511513
// We keep handle values in a separate pool, so we don't confuse a handle with an int constant
512514
// that happens to be the same...
513515
ValueNum VNForHandle(ssize_t cnsVal, GenTreeFlags iconFlags);

0 commit comments

Comments
 (0)