Fix copy propagation

[SingleAccretion]

Now that we don't push any defs for shadowed parameters, we don't want to pop them as well.

[ghost]

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

Issue Details

---

Now that we don't push any defs for shadowed parameters, we don't want to pop them as well.

Author:	SingleAccretion
Assignees:	-
Labels:	area-CodeGen-coreclr, community-contribution
Milestone:	-

[kunalspathak]

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

[kunalspathak]

@BruceForstall - if jitstress legs are clean with this PR, we won't need #66060

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).

[kunalspathak]

LGTM

[BruceForstall]

I'm willing to take this instead of the revert; unfortunately, the test build is broken :-( #66099

[BruceForstall]

Actually, the libraries jitstress jobs here have a lot more failures than the "revert" job had, so possibly there's still a bug in the code.

E.g., the "revert" jobs:
https://dev.azure.com/dnceng/public/_build/results?buildId=1639743&view=ms.vss-test-web.build-test-results-tab
https://dev.azure.com/dnceng/public/_build/results?buildId=1639742&view=ms.vss-test-web.build-test-results-tab

And these jobs:
https://dev.azure.com/dnceng/public/_build/results?buildId=1640729&view=ms.vss-test-web.build-test-results-tab
https://dev.azure.com/dnceng/public/_build/results?buildId=1640726&view=ms.vss-test-web.build-test-results-tab

In particular, it looks like with this change, System.Reflection.Tests.NullabilityInfoContextTests.NullablePublicOnlyOtherTypesTest is failing everywhere with "System.ArgumentNullException : Value cannot be null. (Parameter 'fieldInfo')"

So, I'm inclined to take the revert change.

[SingleAccretion]

In particular, it looks like with this change, System.Reflection.Tests.NullabilityInfoContextTests.NullablePublicOnlyOtherTypesTest is failing everywhere with "System.ArgumentNullException : Value cannot be null. (Parameter 'fieldInfo')"

FWIW, looks like it's failing elsewhere too: #66100 (without stress).

(Also, being behind main somewhat, I could not reproduce it locally)

[BruceForstall]

FWIW, looks like it's failing elsewhere too: #66100 (without stress).

Thanks. Looks like I found the problem before an issue was opened, so I assumed (incorrectly) that it might be related to this. It's hard to keep up with all the issues causing test failures :-(

[BruceForstall]

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

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).

[BruceForstall]

I'm not sure what to do about this. I want to validate it with the extra CI jobs, but they keep massively failing due to, apparently, infrastructure related issues, but re-running and restarting the tests don't fix them. And I don't see (many?) cases of these failures elsewhere.

[SingleAccretion]

:(

I did see a similar CI-out on one of my other changes about 12 hours ago: https://dev.azure.com/dnceng/public/_build/results?buildId=1642520&view=results.

Now it seems stress is failing on #66143, that has been fixed, but not picked up (?).

FWIW, I do think this is a very safe change from the perspective of not "making things worse": if we didn't have a def to pop, we'd always crash, and if we did the current code is identical to what was there before.

[BruceForstall]

I'm going to take a risk and merge this and hope for the best in the CI...