Skip to content

Commit bc7081e

Browse files
Sergey Andreenkodavidwrighton
andauthored
Fix VN incorrect optimizations with a new JitEEInterface function. (#57282)
* Add/return tests. * improve the test naming. * Add a new JitEE method. with a naive implementation so far. * Handle generically dissimilar type concerns as well as handle derived type case in the managed compiler * Fix the field and Unsafe issues. * fix some failures. It appeared that we have many trees where we generate unique VN for rhs and don't inform `fgValueNumberBlockAssignment` about it. For example, for ``` N005 ( 10, 8) [000033] -A--G---R--- * ASG struct (copy) N004 ( 3, 2) [000031] D------N---- +--* LCL_VAR struct<eightByteStruct, 8> V01 loc1 d:2 N003 ( 6, 5) [000030] n---G------- \--* IND struct N002 ( 3, 3) [000043] ------------ \--* ADDR byref N001 ( 3, 2) [000044] -------N---- \--* LCL_VAR struct<largeStruct, 32> V00 loc0 u:6 (last use) ``` we will generate unique rhs but then `fgValueNumberBlockAssignment` will still try to work it as with a non-unique one. * Fix an oooold bug in VN. For a tree like: ``` N005 ( 11, 10) [001400] -A------R---- * ASG long N004 ( 6, 5) [001401] *------N----- +--* IND long N003 ( 3, 3) [001402] ------------- | \--* ADDR byref Zero Fseq[_00] N002 ( 3, 2) [001403] U------N----- | \--* LCL_VAR struct<System.Runtime.Intrinsics.Vector128`1[Byte], 16> V45 tmp38 ud:3->4 N001 ( 1, 1) [001404] ------------- \--* LCL_VAR long V69 tmp62 u:2 (last use) ``` SSA was working correctly and printing that `001403` is using SSA-3 and defining SSA-4. However, this code, for some reason, was writing result as a define for SSA-3. * Add new test cases and a fix for them. * correct a test copy paste * response review. Co-authored-by: David Wrighton <davidwr@microsoft.com>
1 parent 1ae2f79 commit bc7081e

File tree

33 files changed

+718
-112
lines changed

33 files changed

+718
-112
lines changed

src/coreclr/ToolBox/superpmi/superpmi-shared/lwmlist.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ LWM(TryResolveToken, Agnostic_CORINFO_RESOLVED_TOKENin, TryResolveTokenValue)
157157
LWM(SatisfiesClassConstraints, DWORDLONG, DWORD)
158158
LWM(SatisfiesMethodConstraints, DLDL, DWORD)
159159
LWM(GetUnmanagedCallConv, MethodOrSigInfoValue, DD)
160+
LWM(DoesFieldBelongToClass, DLDL, DWORD)
160161
DENSELWM(SigInstHandleMap, DWORDLONG)
161162

162163
#undef LWM

src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6265,6 +6265,40 @@ DWORD MethodContext::repGetExpectedTargetArchitecture()
62656265
return value;
62666266
}
62676267

6268+
void MethodContext::recDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls, bool result)
6269+
{
6270+
if (DoesFieldBelongToClass == nullptr)
6271+
DoesFieldBelongToClass = new LightWeightMap<DLDL, DWORD>();
6272+
6273+
DLDL key;
6274+
ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding
6275+
key.A = CastHandle(fld);
6276+
key.B = CastHandle(cls);
6277+
6278+
DWORD value = (DWORD)result;
6279+
DoesFieldBelongToClass->Add(key, value);
6280+
DEBUG_REC(dmpDoesFieldBelongToClass(key, result));
6281+
}
6282+
6283+
void MethodContext::dmpDoesFieldBelongToClass(DLDL key, bool value)
6284+
{
6285+
printf("DoesFieldBelongToClass key fld=%016llX, cls=%016llx, result=%d", key.A, key.B, value);
6286+
}
6287+
6288+
bool MethodContext::repDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls)
6289+
{
6290+
DLDL key;
6291+
ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding
6292+
key.A = CastHandle(fld);
6293+
key.B = CastHandle(cls);
6294+
6295+
AssertMapAndKeyExist(DoesFieldBelongToClass, key, ": key %016llX %016llX", key.A, key.B);
6296+
6297+
bool value = (bool)DoesFieldBelongToClass->Get(key);
6298+
DEBUG_REP(dmpDoesFieldBelongToClass(key, value));
6299+
return value;
6300+
}
6301+
62686302
void MethodContext::recIsValidToken(CORINFO_MODULE_HANDLE module, unsigned metaTOK, bool result)
62696303
{
62706304
if (IsValidToken == nullptr)

src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,10 @@ class MethodContext
769769
void dmpGetExpectedTargetArchitecture(DWORD key, DWORD result);
770770
DWORD repGetExpectedTargetArchitecture();
771771

772+
void recDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls, bool result);
773+
void dmpDoesFieldBelongToClass(DLDL key, bool value);
774+
bool repDoesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls);
775+
772776
void recIsValidToken(CORINFO_MODULE_HANDLE module, unsigned metaTOK, bool result);
773777
void dmpIsValidToken(DLD key, DWORD value);
774778
bool repIsValidToken(CORINFO_MODULE_HANDLE module, unsigned metaTOK);
@@ -1063,7 +1067,7 @@ enum mcPackets
10631067
Packet_TryResolveToken = 158, // Added 4/26/2016
10641068
Packet_SatisfiesClassConstraints = 110,
10651069
Packet_SatisfiesMethodConstraints = 111,
1066-
Packet_ShouldEnforceCallvirtRestriction = 112, // Retired 2/18/2020
1070+
Packet_DoesFieldBelongToClass = 112, // Added 8/12/2021
10671071
Packet_SigInstHandleMap = 184,
10681072
Packet_AllocPgoInstrumentationBySchema = 186, // Added 1/4/2021
10691073
Packet_GetPgoInstrumentationResults = 187, // Added 1/4/2021

src/coreclr/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2010,3 +2010,13 @@ bool interceptor_ICJI::notifyInstructionSetUsage(CORINFO_InstructionSet instruct
20102010
{
20112011
return original_ICorJitInfo->notifyInstructionSetUsage(instructionSet, supported);
20122012
}
2013+
2014+
bool interceptor_ICJI::doesFieldBelongToClass(
2015+
CORINFO_FIELD_HANDLE fldHnd,
2016+
CORINFO_CLASS_HANDLE cls)
2017+
{
2018+
mc->cr->AddCall("doesFieldBelongToClass");
2019+
bool result = original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
2020+
mc->recDoesFieldBelongToClass(fldHnd, cls, result);
2021+
return result;
2022+
}

src/coreclr/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,3 +1389,11 @@ uint32_t interceptor_ICJI::getJitFlags(
13891389
return original_ICorJitInfo->getJitFlags(flags, sizeInBytes);
13901390
}
13911391

1392+
bool interceptor_ICJI::doesFieldBelongToClass(
1393+
CORINFO_FIELD_HANDLE fldHnd,
1394+
CORINFO_CLASS_HANDLE cls)
1395+
{
1396+
mcs->AddCall("doesFieldBelongToClass");
1397+
return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
1398+
}
1399+

src/coreclr/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,3 +1217,10 @@ uint32_t interceptor_ICJI::getJitFlags(
12171217
return original_ICorJitInfo->getJitFlags(flags, sizeInBytes);
12181218
}
12191219

1220+
bool interceptor_ICJI::doesFieldBelongToClass(
1221+
CORINFO_FIELD_HANDLE fldHnd,
1222+
CORINFO_CLASS_HANDLE cls)
1223+
{
1224+
return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
1225+
}
1226+

src/coreclr/ToolBox/superpmi/superpmi/icorjitinfo.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,6 +1574,13 @@ uint32_t MyICJI::getJitFlags(CORJIT_FLAGS* jitFlags, uint32_t sizeInBytes)
15741574
return ret;
15751575
}
15761576

1577+
bool MyICJI::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
1578+
{
1579+
jitInstance->mc->cr->AddCall("doesFieldBelongToClass");
1580+
bool result = jitInstance->mc->repDoesFieldBelongToClass(fldHnd, cls);
1581+
return result;
1582+
}
1583+
15771584
// Runs the given function with the given parameter under an error trap
15781585
// and returns true if the function completes successfully. We fake this
15791586
// up a bit for SuperPMI and simply catch all exceptions.

src/coreclr/inc/corjit.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,12 @@ class ICorJitInfo : public ICorDynamicInfo
484484
uint32_t sizeInBytes /* IN: The size of the buffer. Note that this is effectively a
485485
version number for the CORJIT_FLAGS value. */
486486
) = 0;
487+
488+
// Checks if a field belongs to a given class.
489+
virtual bool doesFieldBelongToClass(
490+
CORINFO_FIELD_HANDLE fldHnd, /* IN: the field that we are checking */
491+
CORINFO_CLASS_HANDLE cls /* IN: the class that we are checking */
492+
) = 0;
487493
};
488494

489495
/**********************************************************************************/

src/coreclr/inc/icorjitinfoimpl_generated.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,10 @@ uint32_t getJitFlags(
710710
CORJIT_FLAGS* flags,
711711
uint32_t sizeInBytes) override;
712712

713+
bool doesFieldBelongToClass(
714+
CORINFO_FIELD_HANDLE fldHnd,
715+
CORINFO_CLASS_HANDLE cls) override;
716+
713717
/**********************************************************************************/
714718
// clang-format on
715719
/**********************************************************************************/

src/coreclr/inc/jiteeversionguid.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,12 @@ typedef const GUID *LPCGUID;
4343
#define GUID_DEFINED
4444
#endif // !GUID_DEFINED
4545

46-
constexpr GUID JITEEVersionIdentifier = { /* c2e4a466-795d-4e64-a776-0ae7b2eed65b */
47-
0xc2e4a466,
48-
0x795d,
49-
0x4e64,
50-
{0xa7, 0x76, 0x0a, 0xe7, 0xb2, 0xee, 0xd6, 0x5b}
51-
};
46+
constexpr GUID JITEEVersionIdentifier = { /* 5ed35c58-857b-48dd-a818-7c0136dc9f73 */
47+
0x5ed35c58,
48+
0x857b,
49+
0x48dd,
50+
{0xa8, 0x18, 0x7c, 0x01, 0x36, 0xdc, 0x9f, 0x73}
51+
};
5252

5353
//////////////////////////////////////////////////////////////////////////////////////////////////////////
5454
//

0 commit comments

Comments
 (0)