JIT: Copy loop memory dependence recursively on hoisting

[jakobbotsch]

When hoisting we were cloning the full tree but only copying memory dependence of the root node. Ensure we copy it for the full tree.

Fix #115109

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs.

[Copilot]

Pull Request Overview

This PR fixes issue #115109 by ensuring loop memory dependencies are copied recursively when hoisting expression trees. It also streamlines statement insertion and adds a regression test to validate the fix.

• Simplify hoisted statement insertion in optPerformHoistExpr using fgInsertStmtAtEnd
• Enhance optCopyLoopMemoryDependence to recurse through all tree operands
• Add a JIT regression test project and C# test (Runtime_115109) to cover the full-tree dependence copy

Reviewed Changes

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

File	Description
src/tests/JIT/Regression/JitBlue/Runtime_115109/Runtime_115109.csproj	New test project (missing framework target)
src/tests/JIT/Regression/JitBlue/Runtime_115109/Runtime_115109.cs	Generated fuzz test to verify memory-dependence cloning
src/coreclr/jit/optimizer.cpp	Refactored hoist insertion and recursive memory-dependence copy

[Copilot]

The project file is missing a <TargetFramework> entry, so it may fail to build. Please add something like <TargetFramework>net7.0</TargetFramework> under the PropertyGroup.

Suggested change

	<Optimize>True</Optimize>
	<Optimize>True</Optimize>
	<TargetFramework>net7.0</TargetFramework>

[Copilot]

[nitpick] The variable vr5 is assigned but never used; consider removing it or using the result directly in the return or an assertion to make its purpose clear.

Suggested change

	bool vr5 = 0 <= M10();
	Assert.True(0 <= M10(), "Expected M10() to return a value greater than or equal to 0.");

[jakobbotsch]

Fair point, but I don't think it's worth rerunning CI.

[Copilot]

[nitpick] The remark about 'toTree' needing to be in its own statement is unclear. It would help to clarify the precondition (e.g., both trees must be part of the same statement sequence) or remove if not needed.

Suggested change

	// This requires 'toTree' to be in its own statement
	//
	// 'toTree' must be part of a separate statement sequence to ensure proper
	// traversal and mapping in the loop memory dependence analysis.

[AndyAyersMS]

LGTM.

Alternative idea: make this be an optional action for gtCloneExpr, since it has old and new nodes in hand ... (and do likewise for optRecordSsaUses).

[jakobbotsch]

Alternative idea: make this be an optional action for gtCloneExpr, since it has old and new nodes in hand ... (and do likewise for optRecordSsaUses).

It means everyone gets to pay for it. I removed optional actions from gtCloneExpr in #97411 because I didn't think that tradeoff is good. Perhaps if we templatized gtCloneExpr in some way.