[llvm] r320420 - [CodeGen] Improve the consistency of instruction fusion*

[Philip Reames via llvm-commits]

Tests?


On 12/11/2017 01:09 PM, Evandro Menezes via llvm-commits wrote:
> Author: evandro
> Date: Mon Dec 11 13:09:27 2017
> New Revision: 320420
>
> URL: http://llvm.org/viewvc/llvm-project?rev=320420&view=rev
> Log:
> [CodeGen] Improve the consistency of instruction fusion*
>
> When either instruction in a fused pair has no other dependency, besides on
> the other instruction, make sure that other instructions do not get
> scheduled between them.  Additionally, avoid fusing an instruction more than
> once along the same dependency chain.
>
> Differential revision: https://reviews.llvm.org/D36704
>
> Modified:
>      llvm/trunk/lib/CodeGen/MacroFusion.cpp
>
> Modified: llvm/trunk/lib/CodeGen/MacroFusion.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MacroFusion.cpp?rev=320420&r1=320419&r2=320420&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MacroFusion.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MacroFusion.cpp Mon Dec 11 13:09:27 2017
> @@ -33,42 +33,74 @@ using namespace llvm;
>   static cl::opt<bool> EnableMacroFusion("misched-fusion", cl::Hidden,
>     cl::desc("Enable scheduling for macro fusion."), cl::init(true));
>   
> -static void fuseInstructionPair(ScheduleDAGMI &DAG, SUnit &FirstSU,
> +static bool isHazard(const SDep &Dep) {
> +  return Dep.getKind() == SDep::Anti || Dep.getKind() == SDep::Output;
> +}
> +
> +static bool fuseInstructionPair(ScheduleDAGMI &DAG, SUnit &FirstSU,
>                                   SUnit &SecondSU) {
> +  // Check that neither instr is already paired with another along the edge
> +  // between them.
> +  for (SDep &SI : FirstSU.Succs)
> +    if (SI.isCluster())
> +      return false;
> +
> +  for (SDep &SI : SecondSU.Preds)
> +    if (SI.isCluster())
> +      return false;
> +  // Though the reachability checks above could be made more generic,
> +  // perhaps as part of ScheduleDAGMI::addEdge(), since such edges are valid,
> +  // the extra computation cost makes it less interesting in general cases.
> +
>     // Create a single weak edge between the adjacent instrs. The only effect is
>     // to cause bottom-up scheduling to heavily prioritize the clustered instrs.
> -  DAG.addEdge(&SecondSU, SDep(&FirstSU, SDep::Cluster));
> +  if (!DAG.addEdge(&SecondSU, SDep(&FirstSU, SDep::Cluster)))
> +    return false;
>   
> -  // Adjust the latency between the anchor instr and its
> -  // predecessors.
> -  for (SDep &IDep : SecondSU.Preds)
> -    if (IDep.getSUnit() == &FirstSU)
> -      IDep.setLatency(0);
> -
> -  // Adjust the latency between the dependent instr and its
> -  // predecessors.
> -  for (SDep &IDep : FirstSU.Succs)
> -    if (IDep.getSUnit() == &SecondSU)
> -      IDep.setLatency(0);
> +  // Adjust the latency between both instrs.
> +  for (SDep &SI : FirstSU.Succs)
> +    if (SI.getSUnit() == &SecondSU)
> +      SI.setLatency(0);
> +
> +  for (SDep &SI : SecondSU.Preds)
> +    if (SI.getSUnit() == &FirstSU)
> +      SI.setLatency(0);
>   
> -  DEBUG(dbgs() << DAG.MF.getName() << "(): Macro fuse ";
> +  DEBUG(dbgs() << "Macro fuse: ";
>           FirstSU.print(dbgs(), &DAG); dbgs() << " - ";
>           SecondSU.print(dbgs(), &DAG); dbgs() << " /  ";
>           dbgs() << DAG.TII->getName(FirstSU.getInstr()->getOpcode()) << " - " <<
>                     DAG.TII->getName(SecondSU.getInstr()->getOpcode()) << '\n'; );
>   
> +  // Make data dependencies from the FirstSU also dependent on the SecondSU to
> +  // prevent them from being scheduled between the FirstSU and the SecondSU.
>     if (&SecondSU != &DAG.ExitSU)
> -    // Make instructions dependent on FirstSU also dependent on SecondSU to
> -    // prevent them from being scheduled between FirstSU and and SecondSU.
>       for (const SDep &SI : FirstSU.Succs) {
> -      if (SI.getSUnit() == &SecondSU)
> +      SUnit *SU = SI.getSUnit();
> +      if (SI.isWeak() || isHazard(SI) ||
> +          SU == &DAG.ExitSU || SU == &SecondSU || SU->isPred(&SecondSU))
> +        continue;
> +      DEBUG(dbgs() << "  Bind ";
> +            SecondSU.print(dbgs(), &DAG); dbgs() << " - ";
> +            SU->print(dbgs(), &DAG); dbgs() << '\n';);
> +      DAG.addEdge(SU, SDep(&SecondSU, SDep::Artificial));
> +    }
> +
> +  // Make the FirstSU also dependent on the dependencies of the SecondSU to
> +  // prevent them from being scheduled between the FirstSU and the SecondSU.
> +  if (&FirstSU != &DAG.EntrySU)
> +    for (const SDep &SI : SecondSU.Preds) {
> +      SUnit *SU = SI.getSUnit();
> +      if (SI.isWeak() || isHazard(SI) || &FirstSU == SU || FirstSU.isSucc(SU))
>           continue;
> -      DEBUG(dbgs() << "  Copy Succ ";
> -            SI.getSUnit()->print(dbgs(), &DAG); dbgs() << '\n';);
> -      DAG.addEdge(SI.getSUnit(), SDep(&SecondSU, SDep::Artificial));
> +      DEBUG(dbgs() << "  Bind ";
> +            SU->print(dbgs(), &DAG); dbgs() << " - ";
> +            FirstSU.print(dbgs(), &DAG); dbgs() << '\n';);
> +      DAG.addEdge(&FirstSU, SDep(SU, SDep::Artificial));
>       }
>   
>     ++NumFused;
> +  return true;
>   }
>   
>   namespace {
> @@ -116,9 +148,8 @@ bool MacroFusion::scheduleAdjacentImpl(S
>   
>     // Explorer for fusion candidates among the dependencies of the anchor instr.
>     for (SDep &Dep : AnchorSU.Preds) {
> -    // Ignore dependencies that don't enforce ordering.
> -    if (Dep.getKind() == SDep::Anti || Dep.getKind() == SDep::Output ||
> -        Dep.isWeak())
> +    // Ignore dependencies other than data or strong ordering.
> +    if (Dep.isWeak() || isHazard(Dep))
>         continue;
>   
>       SUnit &DepSU = *Dep.getSUnit();
> @@ -129,8 +160,8 @@ bool MacroFusion::scheduleAdjacentImpl(S
>       if (!shouldScheduleAdjacent(TII, ST, DepMI, AnchorMI))
>         continue;
>   
> -    fuseInstructionPair(DAG, DepSU, AnchorSU);
> -    return true;
> +    if (fuseInstructionPair(DAG, DepSU, AnchorSU))
> +      return true;
>     }
>   
>     return false;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits