[llvm-commits] [PATCH] Heuristic changes to Hexagon MI scheduler

[Sergei Larin]

Andy,

> NumPreds/SuccsLeft are public members of SUnit. So I don't think you
> need getSingleUnscheduledPred/Succ helpers. In this case you already
> know that the Pred/Succ is unscheduled, so you can just check the
> count.

The problem here seems to be due to aliasing edges in DAG. We could have
nodes that look like this:

SU(54):   ADJCALLSTACKDOWN 0, %R29<imp-def>, %R30<imp-def>, %R31<imp-use>,
%R30<imp-use>, %R29<imp-use>
  # preds left       : 5
  # succs left       : 0
  # rdefs left       : 0
  Latency            : 1
  Depth              : 20
  Height             : 1
  Predecessors:
   out SU(53): Latency=1
   val SU(53): Latency=1 Reg=%R29
   out SU(53): Latency=1
   val SU(53): Latency=1 Reg=%R30
   val SU(53): Latency=1 Reg=%R31
  Successors:
   out SU(57): Latency=1
   val SU(57): Latency=1
   out SU(57): Latency=1
   antiSU(56): Latency=0

SU(53):   ADJCALLSTACKUP 0, 0, %R29<imp-def>, %R30<imp-def>, %R31<imp-def>,
%R29<imp-use>
  # preds left       : 4
  # succs left       : 5
  # rdefs left       : 0
  Latency            : 1
  Depth              : 19
  Height             : 2
  Predecessors:
   out SU(52): Latency=1
   out SU(49): Latency=1
   val SU(49): Latency=1 Reg=%R29
   out SU(49): Latency=1
  Successors:
   out SU(54): Latency=1
   val SU(54): Latency=1
   out SU(54): Latency=1
   val SU(54): Latency=1
   out SU(56): Latency=1
   val SU(54): Latency=1

Going bottom-up, considering SU(54). SU(56) is already scheduled. It has one
predecessor which is 53, but it is listed 5 times... So for 53 #succs left
== 5

The helper function is somewhat expensive, but gets it right. Simplified
check would need to make sure no aliasing is involved, and will require same
number of iterations. It also looks like no other case is affected by this
at the moment, but someone unaware of this possibility can easily get
confused in the future.

 So I'll keep the helper function for now, until we find a better way to
treat aliasing edges.

Thanks.

Sergei

---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation


> -----Original Message-----
> From: Andrew Trick [mailto:atrick at apple.com]
> Sent: Thursday, September 06, 2012 7:43 PM
> To: Sergei Larin
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Heuristic changes to Hexagon MI
> scheduler
> 
> Hi Sergei,
> 
> NumPreds/SuccsLeft are public members of SUnit. So I don't think you
> need getSingleUnscheduledPred/Succ helpers. In this case you already
> know that the Pred/Succ is unscheduled, so you can just check the
> count.
> 
> Other than that, please check it in. As soon as you do, I'll reorganize
> MachineScheduler.h to remove a lot of the redundancy.  After my reorg,
> MachineSchedStrategy and ScheduleDAGMI will become public, but
> ConvergingScheduler will still be local. It will look like you still
> have redundant code in the register pressure heuristics, but that's
> actually what I want. I'll be changing those frequently in the near
> future and don't want to affect your target during the churn. I expect
> you to end up rewriting those heuristics eventually anyway to work
> better for VLIWish targets.
> 
> -Andy
> 
> On Sep 5, 2012, at 7:45 AM, Sergei Larin <slarin at codeaurora.org> wrote:
> 
> > Please review attached patch. It attempts to:
> >
> > - Add "blocked" heuristic to the Hexagon MI scheduler
> > - Tweak the Hexagon local class hierarchy a bit
> > - Clean-up couple minor stylistic violations
> >
> >  It passes make check-all without any additional issues.
> >
> > Thanks.
> >
> > Sergei Larin
> >
> > ---
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > hosted by The Linux Foundation
> >
> >
> <add_blocked_heuristic_to_Hexagon_MI_scheduler.patch>_________________
> > ______________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits