Skip to content

Move CHECK_IR before morph global #127352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AndyAyersMS wants to merge 1 commit into from
+112 −2
Closed

Move CHECK_IR before morph global #127352

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
37 changes: 35 additions & 2 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,36 @@ void Compiler::compSetProcessor()

opts.preferredVectorByteLength = preferredVectorBitWidth / BITS_PER_BYTE;

if (info.compClassHnd != NO_CLASS_HANDLE)
{
const char* namespaceName;
const char* className = getClassNameFromMetadata(info.compClassHnd, &namespaceName);

if ((strcmp(namespaceName, "System.Numerics") == 0) && (strcmp(className, "Vector`1") == 0))
{
uint32_t const classSize = info.compCompHnd->getClassSize(info.compClassHnd);

instructionSetFlags.RemoveInstructionSet(InstructionSet_VectorT128);
instructionSetFlags.RemoveInstructionSet(InstructionSet_VectorT256);
instructionSetFlags.RemoveInstructionSet(InstructionSet_VectorT512);

switch (classSize)
{
case 16:
instructionSetFlags.AddInstructionSet(InstructionSet_VectorT128);
break;

case 32:
instructionSetFlags.AddInstructionSet(InstructionSet_VectorT256);
break;

case 64:
instructionSetFlags.AddInstructionSet(InstructionSet_VectorT512);
break;
}
}
}

// Only one marker ISA should have been passed in, and it should now be cleared.
assert(!instructionSetFlags.HasInstructionSet(InstructionSet_Vector128) &&
!instructionSetFlags.HasInstructionSet(InstructionSet_Vector256) &&
Expand Down Expand Up @@ -2022,6 +2052,8 @@ void Compiler::compSetProcessor()
instructionSetFlags.AddInstructionSet(InstructionSet_Vector128);
#endif // TARGET_ARM64

instructionSetFlags.Set64BitInstructionSetVariants();
instructionSetFlags = EnsureInstructionSetFlagsAreValid(instructionSetFlags);
assert(instructionSetFlags.Equals(EnsureInstructionSetFlagsAreValid(instructionSetFlags)));
opts.setSupportedISAs(instructionSetFlags);

Expand Down Expand Up @@ -4571,6 +4603,9 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
fgStress64RsltMul();
#endif // DEBUG

// Enable IR checks before global morph so the post-phase check runs on morphed IR.
activePhaseChecks |= PhaseChecks::CHECK_IR;

// Morph the trees in all the blocks of the method
//
unsigned const preMorphBBCount = fgBBcount;
Expand All @@ -4591,8 +4626,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
compCurBB = nullptr;
#endif // DEBUG

// Enable IR checks
activePhaseChecks |= PhaseChecks::CHECK_IR;
};
DoPhase(this, PHASE_POST_MORPH, postMorphPhase);

Expand Down
37 changes: 37 additions & 0 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,43 @@ var_types Compiler::impNormStructType(CORINFO_CLASS_HANDLE structHnd, var_types*

if (structSizeMightRepresentSIMDType(originalSize))
{
const char* namespaceName = nullptr;
const char* className = info.compCompHnd->getClassNameFromMetadata(structHnd, &namespaceName);

if (isNumericsNamespace(namespaceName) && (strcmp(className, "Vector`1") == 0))
{
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT512);

switch (originalSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT128);
break;

case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT256);
break;

case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT512);
Comment on lines +1202 to +1226
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

Avoid mutating opts.compSupportsISAReported / opts.compSupportsISAExactly in impNormStructType. These sets are used to track which ISAs were actually reported to the VM (see compExactlyDependsOn); manually adding VectorT* here can prevent notifyInstructionSetUsage from running later and can corrupt dependency tracking. If you need Vector sizing to follow the struct size, prefer deriving a local size value (or adjusting only compSupportsISA during option setup) and leave *Reported/*Exactly to compExactlyDependsOn.

Suggested change
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT512);
switch (originalSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT128);
break;
case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT256);
break;
case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT512);
switch (originalSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
break;
case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
break;
case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);

Copilot uses AI. Check for mistakes.
break;
Comment on lines +1199 to +1227
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

This switch clears all VectorT* flags first, but does nothing for unexpected originalSize values. If originalSize is not 16/32/64, the method will silently leave VectorT sizing unset for the rest of the compilation. Consider validating originalSize and either preserving the existing VectorT* flags or asserting to avoid silently changing global ISA state.

Suggested change
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT512);
switch (originalSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT128);
break;
case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT256);
break;
case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT512);
break;
const bool isValidVectorTSize = (originalSize == 16) || (originalSize == 32) || (originalSize == 64);
assert(isValidVectorTSize);
if (isValidVectorTSize)
{
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT512);
switch (originalSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT128);
break;
case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT256);
break;
case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT512);
break;
}

Copilot uses AI. Check for mistakes.
}
}

unsigned int sizeBytes;
var_types simdBaseType = getBaseTypeAndSizeOfSIMDType(structHnd, &sizeBytes);
if (simdBaseType != TYP_UNDEF)
Expand Down
40 changes: 40 additions & 0 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10835,6 +10835,46 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
}

uint32_t size = getVectorTByteLength();

if (isVectorT)
{
CORINFO_CLASS_HANDLE const vectorClass = info.compCompHnd->getMethodClass(method);
uint32_t const classSize = info.compCompHnd->getClassSize(vectorClass);

opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT512);

switch (classSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT128);
size = 16;
Comment on lines +10854 to +10860
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

This switch has no default/assert, but the code has already cleared all VectorT* flags above. If classSize is ever not 16/32/64 (e.g., unexpected layout), the compiler will silently proceed with no VectorT ISA set, leaving compilation state inconsistent. Consider guarding the flag update so it only happens for the expected sizes, and otherwise keep the existing VectorT* flags (or assert/unreached).

Copilot uses AI. Check for mistakes.
break;

case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT256);
size = 32;
break;

case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT512);
Comment on lines +10844 to +10873
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

Avoid mutating opts.compSupportsISAReported / opts.compSupportsISAExactly here. Those sets are intended to be managed by compExactlyDependsOn (which calls notifyInstructionSetUsage and then updates *Reported/Exactly); pre-populating them here can suppress VM notification for VectorT dependencies and make R2R/ISA tracking incorrect. Consider only adjusting opts.compSupportsISA (if absolutely necessary) and let compExactlyDependsOn report usage, or compute size from classSize without changing global ISA tracking state.

Suggested change
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISA.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.RemoveInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.RemoveInstructionSet(InstructionSet_VectorT512);
switch (classSize)
{
case 16:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT128);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT128);
size = 16;
break;
case 32:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT256);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT256);
size = 32;
break;
case 64:
opts.compSupportsISA.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAReported.AddInstructionSet(InstructionSet_VectorT512);
opts.compSupportsISAExactly.AddInstructionSet(InstructionSet_VectorT512);
switch (classSize)
{
case 16:
size = 16;
break;
case 32:
size = 32;
break;
case 64:

Copilot uses AI. Check for mistakes.
size = 64;
break;
}
}
#ifdef TARGET_ARM64
assert((size == 16) || (size == SIZE_UNKNOWN));
#else
Expand Down
Loading