ignore unreachable preds in optVisitReachingAssertions

[EgorBo]

Diffs

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR updates optVisitReachingAssertions to tolerate SSA PHI arguments that originate from blocks that are no longer reachable, avoiding conservative early-aborts in those cases.

Changes:

• Adds a local helper to classify predecessor blocks as unreachable based on missing normal predecessors (with EH handler/filter entry exceptions).
• Skips visiting PHI args coming from such unreachable blocks and logs via JITDUMP.
• Marks skipped blocks as visited so the later “phi-preds cover block preds” check doesn’t fail spuriously.

[AndyAyersMS]

We could / should do likewise in the assertion prop data flow computations in AssertionPropFlowCallback::Merge

[jakobbotsch]

We could / should do likewise in the assertion prop data flow computations in AssertionPropFlowCallback::Merge

I wonder if we can remove unreachable blocks after RBO instead, though who knows how downstream phases will cope with that given the SSA state. Maybe repairing SSA if blocks are removed wouldn't be that hard, though.

[EgorBo]

@jakobbotsch @AndyAyersMS
So I've applied both of your suggestions and it had almost no impact on overall diffs: -38,755 total diffs on win-x64 with this PR, your suggesions (to also check DFS and ignore unreachable preds during forward analysis): -38,799.

I checked a few diffs and it turned out that such blocks become unreachable during global assertion prop itself (when we fold something and then call fgMorphStatements), so it is expected why both of the suggestions do not help.

Example dump https://gist.githubusercontent.com/EgorBo/40a1238e362fbc0e47a3381e0ab32c7c/raw/d028badf544677688cafc964b9c05ecab1eb86d3/dump.txt (see is unreachable, ignoring). in this case it's BB28 block.

So I suggest to leave as is for simplicity?

[BoyBaykiller]

With this change does the increment get constant folded into STORE(4) after AP?

int Unoptimized(int a, int b)
{
    int num = 0;

    if (a == b)
    {
        num += 3;
    }
    if (a == b)
    {
        num++;
    }
    
    return num;
}

[EgorBo]

With this change does the increment get constant folded into STORE(4) after AP?

int Unoptimized(int a, int b)
{
    int num = 0;

    if (a == b)
    {
        num += 3;
    }
    if (a == b)
    {
        num++;
    }
    
    return num;
}

doesn't look so

[EgorBo]

/ba-g timeouts