llvm dev - Jan 2013 - [LLVMdev] Arguments to setLatencyPolicy calls swapped by accident in ConvergingScheduler::checkResourceLimits?

Hi,

In ConvergingScheduler::checkResourceLimits on line 1535 of 
MachineScheduler.cpp setLatencyPolicy is called as follows:

   // Set ReduceLatency to true if needed.
   Bot.setLatencyPolicy(TopCand.Policy);
   Top.setLatencyPolicy(BotCand.Policy);

So the Bot scheduling boundary is used to set the latency policy of the 
Top candidate and the other way around.

I think this should be:

   // Set ReduceLatency to true if needed.
   Bot.setLatencyPolicy(BotCand.Policy);
   Top.setLatencyPolicy(TopCand.Policy);

or not?

Jordy

On Jan 11, 2013, at 8:55 AM, Jordy Potman <jordy.potman.recore at
gmail.com> wrote:
> Hi,
> 
> In ConvergingScheduler::checkResourceLimits on line 1535 of
MachineScheduler.cpp setLatencyPolicy is called as follows:
> 
>  // Set ReduceLatency to true if needed.
>  Bot.setLatencyPolicy(TopCand.Policy);
>  Top.setLatencyPolicy(BotCand.Policy);
> 
> So the Bot scheduling boundary is used to set the latency policy of the Top
candidate and the other way around.
> 
> I think this should be:
> 
>  // Set ReduceLatency to true if needed.
>  Bot.setLatencyPolicy(BotCand.Policy);
>  Top.setLatencyPolicy(TopCand.Policy);
> 
> or not?
Yes, it's as simple as it looks. That was a typo in my last round of
cleanup: r170452. Fixed in r172224.

Given the interaction between so many heuristics, I haven't figured out how
to create unit tests. But I was planning to look at the performance data from
that pre-vacation checkin today. So you just saved me some time. Thanks.

-Andy