@MichalStrehovsky

I remember we discussed this a lot when we added the existing limited support for methods with parameters - and there was a relatively good reason not to do better.

What is the use case for this - where do we need it?
I would not be too worried if we scoped this only to explicitly substituted methods, but allowing full constant prop across methods with arbitrary parameters feels wrong. For example, if the method has a ref parameter and mutates it - I know we're going to still call it, but still - it feels risky.

I think this needs a good reason to add and probably lot more tests.

I can't come up with examples of when ref parameters specifically would be risky, but the Rewrite any arguments pushed to stack to nop to allow possible body reduction into constant result sounds concerning. Do we know the pushes are not side-effecting?

What is the use case for this - where do we need it?

We cannot evaluate as much as we could even for dotnet/runtime code. For example with this change, this condition can be now removed because it calls into method which couldn't be evaluated as const returning before.

I remember we discussed this a lot when we added the existing limited support for methods with parameters - and there was a relatively good reason not to do better.

Where do you see the risk? The logic for computing the constant value abort when argument value is used I'd like to understand what could be risky here.

allowing full constant prop across methods with arbitrary parameters feels wrong. For example, if the method has a ref parameter and mutates it - I know we're going to still call it, but still - it feels risky.

That's not what this change does. It only expands support of return constant value propagation over calls with parameters.

Rewrite any arguments pushed to stack to nop to allow possible body reduction into constant result sounds concerning. Do we know the pushes are not side-effecting?

Why is it concerning? This rewrite is done only for the purpose of the return value evaluation. It does not have an effect on the final IL.

I think this needs a good reason to add and probably lot more tests.

I originally wanted but as the change is very narrow we already had test coverage for it. Could you please suggest what other tests would you like to see added?

What is the impact of this change - how much does it help? (just curious)

We should add tests for more complex parameters.

Added a few more, let me know if there is some other interesting il patter to cover

all use simple constants and fields only

There are already some tests, see e.g. https://github.com/marek-safar/linker/blob/constprop/test/Mono.Linker.Tests.Cases/UnreachableBlock/MethodWithParametersSubstitutions.cs#L145

What is the impact of this change - how much does it help? (just curious)

Low single digit Kb for console app but we are experimenting with more stuff for Xamarin which could benefit from this too