[PATCH] D106176: [Scheduler] Treat weak edges uniformly at entry

[Joe Nash via Phabricator via llvm-commits]

Joe_Nash added a subscriber: courbet.
Joe_Nash added a comment.

In D106176#2889686 <https://reviews.llvm.org/D106176#2889686>, @critson wrote:

> LGTM
>
> I agree manipulation of the available queue here needs to be made consistent with the scheduling loop below it.
>
> I wonder whether we want to be selecting these instructions (leaf with remaining weak preds) to the pending queue at all?
> For example, if the condition on line 536 was extended to exclude SUnits with weak predecessors as well?

The only time nodes are added to pending is right at the entry here, and in ReleaseSucc, which is called when strong predecessors are scheduled. So if leaves are not put onto the pending queue here they will never be scheduled

> That said, in the absence of other leaf nodes then we do want to break weak conditions and schedule these SUnits.

Unfortunately, that is not what will happen. Without this change, we would get an assertion failure. With this change, we will stall.

> Otherwise we risk not being able to schedule anything, so this seems to be the correct solution.
>
> I guess with this change, in the presence of a single leaf with weak preds then `CurCycle` will be progressively raised until the assertion in `ScheduleNodeTopDown` passes?

Yes

> And such a case is relatively atypical?

I would guess the incidence is zero, since it would cause an assertion failure without this patch.

Something like this patch is necessary to create consistency and avoid breakage, but I do not think it is possible to achieve good schedule results by feeding weak edges to the ListScheduler. For instance, in X86/testb-je-fusion.ll has the same results in many cases whether or not the PostRA scheduler is enabled. 
@courbet


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106176/new/

https://reviews.llvm.org/D106176