MachineScheduler: Restore TopCand after comparing top and bottom candidates

[• tstellarAMD]

Summary

In the GenericScheduler, before comparing top and bottom candidates, we set
TopCand.Reason to NoCand, so that we can determine wether or not it was selected
over BotCand. However, in the case where the bottom candidate is selected,
TopCand needs to have Reason restored to its original value, otherwise
the scheduler will continue selecting from the bottom, until a new
instruction is added to the Top queue.

[MatzeB]

Good catch! LGTM.

[MatzeB]

Actually I am not so sure anymore. In cases where the topnode isn't picked the reason shouldn't matter, but we will reset the reason anyway next round when we compare against the top node again. Do you have an example that behaves differently with/without this patch? There may be something else wrong.

[• tstellarAMD]

In D23510#515568, @MatzeB wrote:

Actually I am not so sure anymore. In cases where the topnode isn't picked the reason shouldn't matter, but we will reset the reason anyway next round when we compare against the top node again. Do you have an example that behaves differently with/without this patch? There may be something else wrong.

pickNodeBidirectional() won't re-compare if topnode wasn't scheduled in the previous round.

[MatzeB]

In D23510#516880, @tstellarAMD wrote:

In D23510#515568, @MatzeB wrote:

Actually I am not so sure anymore. In cases where the topnode isn't picked the reason shouldn't matter, but we will reset the reason anyway next round when we compare against the top node again. Do you have an example that behaves differently with/without this patch? There may be something else wrong.

pickNodeBidirectional() won't re-compare if topnode wasn't scheduled in the previous round.

It will always compare the top and bottom pick. Even though one of them is possibly saved from last round they will be subject to a comparison so outdates/old data in the reason field shouldn't affect the decision.

lib/CodeGen/MachineScheduler.cpp

#ifndef NDEBUG
     }
 #endif
   }
 
   // Pick best from BotCand and TopCand.
   assert(BotCand.isValid());
   assert(TopCand.isValid());
   SchedCandidate Cand = BotCand;
+  GenericSchedulerBase::CandReason TopReason = TopCand.Reason;
   TopCand.Reason = NoCand;
   tryCandidate(Cand, TopCand, nullptr);
   if (TopCand.Reason != NoCand) {
     Cand.setBest(TopCand);
     DEBUG(traceCandidate(Cand));
-  }
+  } else
+    // Restore the original Reason for TopCand if it isn't selected.
+    TopCand.Reason = TopReason;
 
   IsTopNode = Cand.AtTop;
   tracePick(Cand);
   return Cand.SU;
 }
 
 /// Pick the best node to balance the schedule. Implements MachineSchedStrategy.
 SUnit *GenericScheduler::pickNode(bool &IsTopNode) {