[MachinePipeliner] Consideration of `SchedStart` is omitted in some cases?

[kasuga-fj]

We found that the SchedStart may not be considered when scheduling instructions.
Please refer to the following link for details.
https://godbolt.org/z/G16Kd7Y6P

The following is part of the debugging output.

Inst (11)   %77:fpr128 = LDRQui %10:gpr64sp, 0, debug-location !41 :: (load (s128) from %ir.lsr.iv63, align 4, !tbaa !36); example.c:10:24

	es: ffffffe8 ls: ffffffe9 me: 7fffffff ms: ffffffe9
Trying to insert node between -24 and -23 II: 47
	insert at cycle -24   %77:fpr128 = LDRQui %10:gpr64sp, 0, debug-location !41 :: (load (s128) from %ir.lsr.iv63, align 4, !tbaa !36); example.c:10:24

In this case, SchedStart is -23, so we must insert this instruction at cycle -23, however it's inserted at -24.
Perhaps following part causes this problem.

llvm-project/llvm/lib/CodeGen/MachinePipeliner.cpp

Lines 2441 to 2449 in e1cc9e4

	SchedEnd =
	std::min(SchedEnd, std::min(LateStart, EarlyStart + (int)II - 1));
	// When scheduling a Phi it is better to start at the late cycle and go
	// backwards. The default order may insert the Phi too far away from
	// its first dependence.
	if (SU->getInstr()->isPHI())
	scheduleFound = Schedule.insert(SU, SchedEnd, EarlyStart, II);
	else
	scheduleFound = Schedule.insert(SU, EarlyStart, SchedEnd, II);

By the way, is there any reason why SchedStart and EarlyStart are treated separately (same for SchedEnd and LateStart) ?
If not, I think merging them makes code simpler.

[kasuga-fj]

@bcahoon Could you please check?

[bcahoon]

Hi @kasuga-fj, I agree that SchedStart should be used in the case that you've identified. It will be interesting to see if that change will have any impact on existing scheduler. I also agree that is makes sense to make the code simpler by attempting to use a single, merged value rather than the two current values.

[kasuga-fj]

Thanks for checking. I'll check the impact on the schedule results and post it with the patch.