Provide operand indices to adjustSchedDependency

[frasercrmck]

Summary

This allows targets to know exactly which operands are contributing to
the dependency, which is required for targets with per-operand
scheduling models.

Required, at least, for our downstream target. I added some seemingly-relevant people from AMDGPU and Hexagon to see if they agree with the idea.

Also, I was tempted to change AMDGPU's and Hexagon's use of Src & Dst to Def & Use, as are used in the base method. The discrepancy could be a little confusing.

[foad]

Seems reasonable to me.

Also, I was tempted to change AMDGPU's and Hexagon's use of Src & Dst to Def & Use, as are used in the base method. The discrepancy could be a little confusing.

That sounds fine too.

[foad]

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

229–234

Why int? unsigned would be more usual.

[frasercrmck]

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

229–234

Good question. I went with int because there's already the idom that -1 means operand not found. I suspect there could be cases where an operand isn't known, possibly for a non-data dependency. In that case we could still allow the passing of -1.

If we go with this idea, I'd suggest we update the documentation to make that clearer.

[frasercrmck]

Updating Hexagon to use Def and Use terminology now seems too an intrusive change to me: there are also private functions updateLatency, restoreLatency`, changeLatency, isBestZeroLatency which all use Src and Dst. So I think I'll leave it be.

AMDGPU is simpler in that regard.

[frasercrmck]

Add documentation for adjustSchedDependency

Rename variables in AMDGPU's adjustSchedDependency, to better match the naming convention used by the base method.

[frasercrmck]

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

229–234

I've updated the documentation to describe how DefOpIdx and UseOpIdx may be used.

[frasercrmck]

Ping.

[foad]

LGTM.

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h

public:
   /// Override generic scheduling policy within a region.
   ///
   /// This is a convenient way for targets that don't provide any custom
   /// scheduling heuristics (no custom MachineSchedStrategy) to make
   /// changes to the generic scheduling policy.
   virtual void overrideSchedPolicy(MachineSchedPolicy &Policy,
                                    unsigned NumRegionInstrs) const {}
 
-  // Perform target specific adjustments to the latency of a schedule
+  // Perform target-specific adjustments to the latency of a schedule
   // dependency.
-  virtual void adjustSchedDependency(SUnit *def, SUnit *use, SDep &dep) const {}
+  // If a pair of operands is associated with the schedule dependency, DefOpIdx
+  // and UseOpIdx are the indices of the operands in Def and Use, respectively.
+  // Otherwise, either may be -1.
+  virtual void adjustSchedDependency(SUnit *Def, int DefOpIdx, SUnit *Use,
+                                     int UseOpIdx, SDep &Dep) const {}
 
   // For use with PostRAScheduling: get the anti-dependence breaking that should
   // be performed before post-RA scheduling.
   virtual AntiDepBreakMode getAntiDepBreakMode() const { return ANTIDEP_NONE; }
 
   // For use with PostRAScheduling: in CriticalPathRCs, return any register
   // classes that should only be considered for anti-dependence breaking if they
   // are on the critical path.
   virtual void getCriticalPathRCs(RegClassVector &CriticalPathRCs) const {

llvm/lib/CodeGen/MachinePipeliner.cpp

for (MachineInstr::mop_iterator MOI = MI->operands_begin(),
         MachineInstr *DefMI = MRI.getUniqueVRegDef(Reg);
         if (DefMI == nullptr)
           continue;
         SUnit *SU = getSUnit(DefMI);
         if (SU != nullptr && DefMI->isPHI()) {
           if (!MI->isPHI()) {
             SDep Dep(SU, SDep::Data, Reg);
             Dep.setLatency(0);
-            ST.adjustSchedDependency(SU, &I, Dep);
+            ST.adjustSchedDependency(SU, 0, &I, MI->getOperandNo(MOI), Dep);
             I.addPred(Dep);
           } else {
             HasPhiUse = Reg;
             // Add a chain edge to a dependent Phi that isn't an existing
             // predecessor.
             if (SU->NodeNum < I.NodeNum && !I.isPred(SU))
               I.addPred(SDep(SU, SDep::Barrier));
           }

llvm/lib/CodeGen/ScheduleDAGInstrs.cpp

for (Reg2SUnitsMap::iterator I = Uses.find(*Alias); I != Uses.end(); ++I) {
       const MCInstrDesc *UseMIDesc =
           (RegUse ? &UseSU->getInstr()->getDesc() : nullptr);
       bool ImplicitPseudoUse =
           (UseMIDesc && UseOp >= ((int)UseMIDesc->getNumOperands()) &&
            !UseMIDesc->hasImplicitUseOfPhysReg(*Alias));
       if (!ImplicitPseudoDef && !ImplicitPseudoUse) {
         Dep.setLatency(SchedModel.computeOperandLatency(SU->getInstr(), OperIdx,
                                                         RegUse, UseOp));
-        ST.adjustSchedDependency(SU, UseSU, Dep);
+        ST.adjustSchedDependency(SU, OperIdx, UseSU, UseOp, Dep);
       } else {
         Dep.setLatency(0);
         // FIXME: We could always let target to adjustSchedDependency(), and
         // remove this condition, but that currently asserts in Hexagon BE.
         if (SU->getInstr()->isBundle() || (RegUse && RegUse->isBundle()))
-          ST.adjustSchedDependency(SU, UseSU, Dep);
+          ST.adjustSchedDependency(SU, OperIdx, UseSU, UseOp, Dep);
       }
 
       UseSU->addPred(Dep);
     }
   }
 }
 
 /// Adds register dependencies (data, anti, and output) from this SUnit
for (VReg2SUnitOperIdxMultiMap::iterator I = CurrentVRegUses.find(Reg),
       }
 
       if ((LaneMask & DefLaneMask).any()) {
         SUnit *UseSU = I->SU;
         MachineInstr *Use = UseSU->getInstr();
         SDep Dep(SU, SDep::Data, Reg);
         Dep.setLatency(SchedModel.computeOperandLatency(MI, OperIdx, Use,
                                                         I->OperandIndex));
-        ST.adjustSchedDependency(SU, UseSU, Dep);
+        ST.adjustSchedDependency(SU, OperIdx, UseSU, I->OperandIndex, Dep);
         UseSU->addPred(Dep);
       }
 
       LaneMask &= ~KillLaneMask;
       // If we found a Def for all lanes of this use, remove it from the list.
       if (LaneMask.any()) {
         I->LaneMask = LaneMask;
         ++I;

llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp

for (SDNode *N = SU->getNode(); N; N = N->getGluedNode()) {
         while (NumUsed != 0 && !N->hasAnyUseOfValue(NumUsed - 1))
           --NumUsed;    // Skip over unused values at the end.
         if (NumUsed > TII->get(N->getMachineOpcode()).getNumDefs())
           SU->hasPhysRegDefs = true;
       }
 
       for (unsigned i = 0, e = N->getNumOperands(); i != e; ++i) {
         SDNode *OpN = N->getOperand(i).getNode();
+        unsigned DefIdx = N->getOperand(i).getResNo();
         if (isPassiveNode(OpN)) continue;   // Not scheduled.
         SUnit *OpSU = &SUnits[OpN->getNodeId()];
         assert(OpSU && "Node has no SUnit!");
         if (OpSU == SU) continue;           // In the same group.
 
         EVT OpVT = N->getOperand(i).getValueType();
         assert(OpVT != MVT::Glue && "Glued nodes should be in same sunit!");
         bool isChain = OpVT == MVT::Other;
for (SDNode *N = SU->getNode(); N; N = N->getGluedNode()) {
         if(isChain && OpN->getOpcode() == ISD::TokenFactor)
           OpLatency = 0;
 
         SDep Dep = isChain ? SDep(OpSU, SDep::Barrier)
           : SDep(OpSU, SDep::Data, PhysReg);
         Dep.setLatency(OpLatency);
         if (!isChain && !UnitLatencies) {
           computeOperandLatency(OpN, N, i, Dep);
-          ST.adjustSchedDependency(OpSU, SU, Dep);
+          ST.adjustSchedDependency(OpSU, DefIdx, SU, i, Dep);
         }
 
         if (!SU->addPred(Dep) && !Dep.isCtrl() && OpSU->NumRegDefsLeft > 1) {
           // Multiple register uses are combined in the same SUnit. For example,
           // we could have a set of glued nodes with all their defs consumed by
           // another set of glued nodes. Register pressure tracking sees this as
           // a single use, so to keep pressure balanced we reduce the defs.
           //

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.h

public:
   }
 
   /// \returns Minimum number of waves per execution unit supported by the
   /// subtarget.
   unsigned getMinWavesPerEU() const override {
     return AMDGPU::IsaInfo::getMinWavesPerEU(this);
   }
 
-  void adjustSchedDependency(SUnit *Src, SUnit *Dst, SDep &Dep) const override;
+  void adjustSchedDependency(SUnit *Def, int DefOpIdx, SUnit *Use, int UseOpIdx,
+                             SDep &Dep) const override;
 };
 
 class R600Subtarget final : public R600GenSubtargetInfo,
                             public AMDGPUSubtarget {
 private:
   R600InstrInfo InstrInfo;
   R600FrameLowering FrameLowering;
   bool FMA;

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

if (F.hasFnAttribute("amdgpu-num-vgpr")) {
 
     if (Requested)
       MaxNumVGPRs = Requested;
   }
 
   return MaxNumVGPRs;
 }
 
-void GCNSubtarget::adjustSchedDependency(SUnit *Src, SUnit *Dst,
-                                         SDep &Dep) const {
+void GCNSubtarget::adjustSchedDependency(SUnit *Def, int DefOpIdx, SUnit *Use,
+                                         int UseOpIdx, SDep &Dep) const {
   if (Dep.getKind() != SDep::Kind::Data || !Dep.getReg() ||
-      !Src->isInstr() || !Dst->isInstr())
+      !Def->isInstr() || !Use->isInstr())
     return;
 
-  MachineInstr *SrcI = Src->getInstr();
-  MachineInstr *DstI = Dst->getInstr();
+  MachineInstr *DefI = Def->getInstr();
+  MachineInstr *UseI = Use->getInstr();
 
-  if (SrcI->isBundle()) {
+  if (DefI->isBundle()) {
     const SIRegisterInfo *TRI = getRegisterInfo();
     auto Reg = Dep.getReg();
-    MachineBasicBlock::const_instr_iterator I(SrcI->getIterator());
-    MachineBasicBlock::const_instr_iterator E(SrcI->getParent()->instr_end());
+    MachineBasicBlock::const_instr_iterator I(DefI->getIterator());
+    MachineBasicBlock::const_instr_iterator E(DefI->getParent()->instr_end());
     unsigned Lat = 0;
     for (++I; I != E && I->isBundledWithPred(); ++I) {
       if (I->modifiesRegister(Reg, TRI))
         Lat = InstrInfo.getInstrLatency(getInstrItineraryData(), *I);
       else if (Lat)
         --Lat;
     }
     Dep.setLatency(Lat);
-  } else if (DstI->isBundle()) {
+  } else if (UseI->isBundle()) {
     const SIRegisterInfo *TRI = getRegisterInfo();
     auto Reg = Dep.getReg();
-    MachineBasicBlock::const_instr_iterator I(DstI->getIterator());
-    MachineBasicBlock::const_instr_iterator E(DstI->getParent()->instr_end());
-    unsigned Lat = InstrInfo.getInstrLatency(getInstrItineraryData(), *SrcI);
+    MachineBasicBlock::const_instr_iterator I(UseI->getIterator());
+    MachineBasicBlock::const_instr_iterator E(UseI->getParent()->instr_end());
+    unsigned Lat = InstrInfo.getInstrLatency(getInstrItineraryData(), *DefI);
     for (++I; I != E && I->isBundledWithPred() && Lat; ++I) {
       if (I->readsRegister(Reg, TRI))
         break;
       --Lat;
     }
     Dep.setLatency(Lat);
   }
 }

llvm/lib/Target/Hexagon/HexagonSubtarget.h

void getSMSMutations(
       const override;
 
   /// Enable use of alias analysis during code generation (during MI
   /// scheduling, DAGCombine, etc.).
   bool useAA() const override;
 
   /// Perform target specific adjustments to the latency of a schedule
   /// dependency.
-  void adjustSchedDependency(SUnit *def, SUnit *use, SDep& dep) const override;
+  void adjustSchedDependency(SUnit *Def, int DefOpIdx, SUnit *Use, int UseOpIdx,
+                             SDep &Dep) const override;
 
   unsigned getVectorLength() const {
     assert(useHVXOps());
     if (useHVX64BOps())
       return 64;
     if (useHVX128BOps())
       return 128;
     llvm_unreachable("Invalid HVX vector length settings");

llvm/lib/Target/Hexagon/HexagonSubtarget.cpp

 bool HexagonSubtarget::useAA() const {
   if (OptLevel != CodeGenOpt::None)
     return true;
   return false;
 }
 
 /// Perform target specific adjustments to the latency of a schedule
 /// dependency.
-void HexagonSubtarget::adjustSchedDependency(SUnit *Src, SUnit *Dst,
+void HexagonSubtarget::adjustSchedDependency(SUnit *Src, int SrcOpIdx,
+                                             SUnit *Dst, int DstOpIdx,
                                              SDep &Dep) const {
   MachineInstr *SrcInst = Src->getInstr();
   MachineInstr *DstInst = Dst->getInstr();
   if (!Src->isInstr() || !Dst->isInstr())
     return;
 
   const HexagonInstrInfo *QII = getInstrInfo();