JIT: Reset compGenTreeID on failed Materialize to avoid leaking gen tree IDs

[Copilot]

A Fuzzlyn-generated case tripped the m_compGenTreeID == m_compiler->compGenTreeID assertion (phase.cpp:47) during 'Optimize Induction Variables'. The phase reported MODIFIED_NOTHING, yet compGenTreeID had advanced.

Root cause

The GenTree* Materialize(Scev* scev) overload calls the internal Materialize(scev, /*createIR*/ true, ...), which allocates IR nodes (bumping compGenTreeID) as it recurses but can still return false partway through — e.g. on an AddRec, a non-null TYP_REF/TYP_BYREF constant, or a 32-bit MUL of TYP_LONG. The orphaned nodes leak their ID increments, so a phase that ultimately makes no change observes a changed compGenTreeID.

Changes

• scev.cpp — GenTree* Materialize(Scev* scev): snapshot compGenTreeID before materializing IR and restore it on a false return. Since compGenTreeID is a DEBUG-only field, the snapshot and reset are guarded with INDEBUG.

INDEBUG(unsigned prevGenTreeID = m_compiler->compGenTreeID);
if (Materialize(scev, true, &result, &vnp))
{
    return result;
}

INDEBUG(m_compiler->compGenTreeID = prevGenTreeID);
return nullptr;

MaterializeVN uses createIR=false and never allocates IR, so it needs no change.

[jakobbotsch]

@copilot These fields are DEBUG only, so add #ifdef DEBUG too

[Copilot]

@copilot These fields are DEBUG only, so add #ifdef DEBUG too

Wrapped the snapshot/reset in INDEBUG since compGenTreeID is DEBUG-only. Verified both checked and release builds in 61314ae.

[jakobbotsch]

Fixes #129527

[jakobbotsch]

@copilot Add the regression test from #129527. Remember not to add a .csproj for it, but to just add the .cs to the merged Regression_2.csproj instead.

[Copilot]

Pull request overview

This PR updates JIT scalar-evolution IR materialization so that a failed attempt to materialize a Scev into GenTree IR does not advance the debug-only Compiler::compGenTreeID counter. This keeps Phase::Observations invariants intact when a phase reports MODIFIED_NOTHING but speculative IR construction partially occurred and then aborted.

Changes:

• Snapshot m_compiler->compGenTreeID (DEBUG-only) before attempting IR materialization and restore it on failure.
• Expand the wrapper ScalarEvolutionContext::Materialize(Scev* scev) from a single conditional expression into an explicit success/failure path with a rollback.

[Copilot]

@copilot Add the regression test from #129527. Remember not to add a .csproj for it, but to just add the .cs to the merged Regression_2.csproj instead.

Added the regression test in src/tests/JIT/Regression/JitBlue/Runtime_129527/Runtime_129527.cs and wired it into the merged regression project (Regression_ro_2.csproj) in commit 8975decf.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

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

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @EgorBo

Debated with myself the best ways to fix this but eventually ended up with this one. There are a few:

• Always report IR as modified after calling Materialize from IV opts, regardless of the return. Not satisfying to me because the caller should not need to be aware of such oddities.
• Condition phase return status from IV opts also on compiler->compGenTreeID == startCompGenTreeID. Not satisfying to me because that makes the phase return status different between debug and release.
• Remove this particular post-phase assert. This seems like the best solution if we could instead check that the actual IR hadn't been modified, but that seems hard.
• This fix. Not satisfying because if you are breaking on "tree with some ID being created" then you may see a tree with the same ID being created twice. But to me this is the least unsatisfying fix.

[BoyBaykiller]

Btw I believe we have the same thing in if-conversion where we always create a SELECT, but then may bail out without actually changing the IR. Currently it just always returns true from that point on.