Remove AreVNsEquivalent

[EgorBo]

Seems to be useless and decreases JIT TP.

Zero size diffs with TP improvements.

The function seems to try to compare two PHI defs and make sure their Phi args are the same. Judging by the diffs that never happens and we just pay a big TP price for it.

Maybe someday we'll have some generic tooling to decide whether two different VNs are in fact the same, but not today.

[dotnet-policy-service]

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

[Copilot]

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

[EgorBo]

@AndyAyersMS @jakobbotsch @dotnet/jit-contrib PTAL, simple PR, zero-diff, TP improvements - see desc.

[AndyAyersMS]

The PR that added this had some diffs and improvements: #104752. You might look at the case mentioned there. Maybe we're now able to get these cases by other means?

I had more ambitious plans for this but reasoning about memory proved challenging.

[EgorBo]

The PR that added this had some diffs and improvements: #104752. You might look at the case mentioned there. Maybe we're now able to get these cases by other means?

I had more ambitious plans for this but reasoning about memory proved challenging.

From top of my head, since then we did:

• PhiDefs no longer have their own VN functions (@jakobbotsch removed them recently). I wonder if Phis where all args have the same VN now have that VN at the def effectively.
• ValueNum/Assertprop now iterate Phi Args for optimizations via VNVisitReachingVNs and optVisitReachingAssertions and catch (both were introduced recently too)

[jakobbotsch]

I recall we had to revert parts of that change because of soundness issues due to the fact that VNs from different loop iterations can be equal without actually having the same value. Looks like the revert happened in #106229, so perhaps that took all of the diffs with it as well.
There might be a bit more code that can be removed if that is the case.

[EgorBo]

I recall we had to revert parts of that change because of soundness issues due to the fact that VNs from different loop iterations can be equal without actually having the same value. Looks like the revert happened in #106229, so perhaps that took all of the diffs with it as well. There might be a bit more code that can be removed if that is the case.

Ah that explains it. Looks like number of benchmarks improved and then regressed match for these PRs

[AndyAyersMS]

I recall we had to revert parts of that change because of soundness issues due to the fact that VNs from different loop iterations can be equal without actually having the same value. Looks like the revert happened in #106229, so perhaps that took all of the diffs with it as well. There might be a bit more code that can be removed if that is the case.

Yeah reading the notes on the revert it looks like we had regressions, so perhaps the net was zero diffs.