JIT: Avoid resolving def-use conflicts by changing use registers

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.

[jakobbotsch]

The only benefit is that it saves insertion of some explicit copies, but the trade off is an implicit contract between LSRA and codegen that is not very faithful. When LSRA defines in a different register than the constrained register, it believes that codegen can insert a copy and simultaneously use it from the new location, which is not true. This difference in the model has to be handled by inserting additional kill constraints during LSRA build, for example for shifts on xarch.

This PR additionally fixes a few issues uncovered while making this change:

• GT_CATCH_ARG codegen was straight up wrong, not properly producing its register value
• Multireg definitions cannot have their register assignment changed since that could override another defined register.

Fix#125432
Fix#125685

[Copilot]

Pull request overview

This PR refactors LinearScan::resolveConflictingDefAndUse in the JIT's register allocator (LSRA) to eliminate the practice of changing use-side register assignments when resolving def-use conflicts. The change ensures that only the definition RefPosition's register assignment is ever modified, while explicit copies are inserted for cases where the use constraint cannot be satisfied. This removes a fragile implicit contract between LSRA and codegen where LSRA would assign a def to a different register than constrained and expect codegen to simultaneously insert a copy and use from the new location.

Changes:

• Simplifies resolveConflictingDefAndUse in lsrabuild.cpp by removing old Cases 1 and 4 (which changed the USE register assignment), consolidating the remaining cases into four cleaner ones that only modify the DEF's register assignment.
• Updates the LsraDumpEvent enum in lsra.h to replace numbered case events (CASE1–CASE6) with descriptive names.
• Updates the debug dump handler in lsra.cpp to use the new event names and removes the now-dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE no-op case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/lsrabuild.cpp	Core logic change: removes use-register-changing cases, simplifies conflict resolution to only modify def registers, updates comments
src/coreclr/jit/lsra.h	Replaces numbered DEFUSE case enum values with descriptive names
src/coreclr/jit/lsra.cpp	Updates debug dump switch/case to use new enum names, removes dead LSRA_EVENT_DEFUSE_FIXED_DELAY_USE case

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

Another related issue here is #10196

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

jitstressregs failures looks like the same that are present in main.

Rest looks like infra failures, will rerun CI.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs, runtime-coreclr jitstress2-jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[jakobbotsch]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

Diffs. Some minor regressions mostly. I analyzed the arm64 ones and they happen "by chance": EH write-thru variables that cross calls are not strongly preferred into callee saves (unlike non EH-write thru variables).
Before this change, however, we would often have x0 taken by some interval that went through the def-use conflict resolution, and that would result in some unrelated EH write-thru variable being allocated into a callee save because x0 was taken and the next best register was a callee save.
With this change we end up with x0 being free and LSRA then allocates some EH write-thru variable to that register. That results in now needing additional reloads after some calls.
I tried a few heuristics to solve this but I was not able to come up with something that was a win in the majority of cases.