Support deleting "entire" stores in the VN-based dead store removal phase

[SingleAccretion]

In the initial change, I reverted the bits needed to make this work, since it did not seem worthwhile. Since then, I came across a scenario which this helps: removing zero-initis of tracked structs that codegen is guaranteed to make "must-init", hence this improvement.

Minor positive diffs, with minor TP impact as well (payed for with the recent ADDR-related changes).

[ghost]

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

Issue Details

---

In the initial change, I reverted the bits needed to make this work, since it did not seem worthwhile. Since then, I came across a scenario which this helps: removing zero-initis of tracked structs that codegen is guaranteed to make "must-init", hence this improvement.

Minor positive diffs are expected, with minor TP impact as well.

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

[SingleAccretion]

@dotnet/jit-contrib

[jakobbotsch]

/azp run runtime-coreclr superpmi-diffs, runtime-coreclr superpmi-replay, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

[azure-pipelines]

Azure Pipelines successfully started running 5 pipeline(s).

[SingleAccretion]

System.Private.Xml x86 failure is also present in an earlier (few days old) run:

  Starting:    System.Private.Xml.Tests (parallel test collections = on, max threads = 4)
   System.Private.Xml.Tests: [Long Running Test] 'System.Xml.XmlConvertTests.XmlConvertTests.RunTests', Elapsed: 00:02:12

Assert failure(PID 4892 [0x0000131c], Thread: 4344 [0x10f8]): !PreemptiveGCDisabled()

CORECLR! GetCLRRuntimeHost + 0x1406D6 (0x727d78e6)
CORECLR! GetCLRRuntimeHost + 0x5A7011 (0x72c3e221)
CORECLR! coreclr_shutdown_2 + 0xAE56 (0x72c507f6)
NTDLL! RtlDecompressBuffer + 0xDE (0x778fea4e)
NTDLL! LdrShutdownThread + 0x386 (0x778ceeb6)
NTDLL! LdrSetAppCompatDllRedirectionCallback + 0x1052A (0x77923c6a)
NTDLL! LdrShutdownProcess + 0x15F (0x778e2d7f)
NTDLL! RtlExitUserProcess + 0x96 (0x778e3886)
KERNEL32! ExitProcess + 0x13 (0x7592b3e3)
CORECLR! coreclr_shutdown_2 + 0x452D8 (0x72c8ac78)
    File: D:\a\_work\1\s\src\coreclr\vm\threads.cpp Line: 979
    Image: C:\h\w\A7CC0910\p\dotnet.exe

[jakobbotsch]

Is my understanding correct that we model the implicit zero init as an SSA def (that we end up getting via defIndex - 1), so this is removing subsequent explicit zero inits based on those?
Presumably liveness DCE would otherwise (often) remove the first def in this particular case?

[SingleAccretion]

Is my understanding correct that we model the implicit zero init as an SSA def (that we end up getting via defIndex - 1), so this is removing subsequent explicit zero inits based on those?

Yes, that is correct.

Presumably liveness DCE would otherwise (often) remove the first def in this particular case?

Liveness does not model this "implicit zero-init", so it would not remove explicit inits based on it. It is why we have a whole separate phase dedicated just to this (optRemoveRedundantZeroInits).

Edit: but optRemoveRedundantZeroInits doesn't work as well for tracked locals because it needs to keep liveness up-to-date, hence this change. I actually tried improving optRemoveRedundantZeroInits first, but it was "hard" to keep all the live sets in sync, and this change is more general anyway.

[jakobbotsch]

Liveness does not model this "implicit zero-init", so it would not remove explicit inits based on it. It is why we have a whole separate phase dedicated just to this (optRemoveRedundantZeroInits).

Right, that's what I meant with "otherwise" -- I assume the "whole" case is otherwise often handled by liveness (when the explicit IR is there).

[jakobbotsch]

This looks good to me, but would like to get @AndyAyersMS's review too.

[AndyAyersMS]

At some point it would be good to think about how to update SSA, but since this opt runs last in the opt phases I suppose it doesn't matter today.