JIT: Use successor edges instead of block targets for remaining block kinds

[amanasifkhalid]

Part of #93020. Replaces BasicBlock::bbTarget/bbFalseTarget/bbTrueTarget with FlowEdge* members.

[ghost]

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

Issue Details

---

Part of #93020. Replaces BasicBlock::bbTarget/bbFalseTarget/bbTrueTarget with FlowEdge* members.

Author:	amanasifkhalid
Assignees:	amanasifkhalid
Labels:	area-CodeGen-coreclr
Milestone:	-

[amanasifkhalid]

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).

[amanasifkhalid]

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).

[amanasifkhalid]

cc @dotnet/jit-contrib, @AndyAyersMS PTAL. Sorry for the large change; here are a few notes that will hopefully expedite reviews:

• This is a no-diff change. There is some pretty big MinOpts TP impact on x86, though that's only in coreclr_tests, so I imagine it's not the norm. I can dig into that, if you'd like.
• I had to move the definition of FlowEdge to above the definition of BasicBlock in block.h, hence the large diffs in that file.
• Most of the changes are quite repetitive (i.e. replacing SetTarget with SetTargetEdge, passing new FlowEdge pointers to SetKindAndTargetEdge, etc.). I'll mention a few interesting cases below.
• In impImportLeave, we previously would set up block targets as soon as we could, and then set up the successor edge after each if-elseif-else statement. Now, we leave bbTargetEdge uninitialized until we create the edge after each if-elseif-else statement. This method is hard to read, so I thought I'd point out the edge creation behavior should be the same here.
• I touched on this offline, but our edge likelihood logic in fgAddRefPred when initializingPreds=true is probably wrong now: We need to call NumSucc to calculate the edge likelihood, and NumSucc compares bbTrueEdge and bbFalseEdge for BBJ_COND blocks to determine the true number of successors. Because those edge pointers aren't set yet when initializing preds, we're technically reading uninitialized memory. As far as I know, the worst this can do is cause us to calculate incorrect likelihoods for BBJ_COND blocks, and we aren't asserting that likelihoods are sensible yet. I think the simplest fix is to move these edge likelihood calculations to fgLinkBasicBlocks (and wherever else we call fgAddRefPred<initializingPreds = true>. I can do that in a follow-up PR.
• There are now two versions of BasicBlock::SetCond. The "normal" version initializes the true and false edges, and is currently used in all cases except one. The other version only sets the true edge, and allows us to set the false edge later. I added this latter version to facilitate lowering switch blocks into jump sequences; our current implementation of that defers setting the false target of each conditional block until a later iteration of a loop. I'm open to suggestions for refactoring this logic, but I found the approach with the least amount of churn was to simply allow deferring the false edge's initialization.

JitStress failure is #98971. SPMI failure is a timeout.

[AndyAyersMS]

This was pleasantly mechanical to review. Nice work!

My recent on likelihood checking PR will clash. Yours should go first.

[AndyAyersMS]

We should consider renaming the field too, the logical name would be m_isFalseEdgeAssertion but that might mix up whether it's an assertion about the false edge, or a false assertion about an edge. So maybe m_assertionHoldsOnFalseEdge?

But I suggest deferring this until later.

[amanasifkhalid]

I have some cleanup tasks planned as a follow-up to this PR. I'll make sure to include the rename; m_assertionHoldsOnFalseEdge sounds good to me.

[AndyAyersMS]

I can already see that this removeRef/addRef/setTarget pattern is super common. Looking forward to us encapsulating this sort of thing in a future PR.

[amanasifkhalid]

My recent on likelihood checking PR will clash. Yours should go first.

Thanks for letting me know -- I'll merge this now.