dotnet / linker

388 stars 127 forks source link

Fix branch removal in compiler generated code #3088

Closed vitek-karas closed 1 year ago

vitek-karas commented 1 year ago

Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking

This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method.

To make the code simple I added a hashset to prevent multiple optimization runs on the same method. Technically this can be guaranteed by the caller, but it's really complex and very fragile.

Added tests.

Note that there's still a whole in analysis of compiler generated code around state machines, see https://github.com/dotnet/linker/issues/3087

vitek-karas commented 1 year ago

I reworked this change to guard against accesses to Body and make sure that const. prop/branch removal happens always.

This does have one negative effect: https://github.com/dotnet/linker/issues/2937 is not consistent and happens in all cases - meaning if the callsite to a local function is removed, we can't figure out to which method the local function belongs and may incorrectly report some warnings.

Personally I prefer the consistent behavior (which is not order dependent anymore) - even if it's incorrect. We can use https://github.com/dotnet/linker/issues/2937 to track ideas on how to fix the problem.

vitek-karas commented 1 year ago

I measured perf of this change (using the sfx project from runtime) and there's no visible regression (although locally the noise is rather high).