[llvm] [MCP] Move dependencies if they block copy propagation (PR #105562)

via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 11:16:31 PDT 2024


github-actions[bot] wrote:

<!--LLVM CODE FORMAT COMMENT: {clang-format}-->


:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

``````````bash
git-clang-format --diff 22d3fb182c9199ac3d51e5577c6647508a7a37f0 ee39a4154ab84763d141e02a31b4d085ee1babd2 --extensions cpp -- llvm/lib/CodeGen/MachineCopyPropagation.cpp
``````````

</details>

<details>
<summary>
View the diff from clang-format here.
</summary>

``````````diff
diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index 493d7cd7d8..8ded3fa7a8 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -121,8 +121,7 @@ public:
   }
 };
 
-static bool moveInstructionsOutOfTheWayIfWeCan(SUnit *Dst,
-                                               SUnit *Src,
+static bool moveInstructionsOutOfTheWayIfWeCan(SUnit *Dst, SUnit *Src,
                                                ScheduleDAGMCP &DG) {
   MachineInstr *DstInstr = Dst->getInstr();
   MachineInstr *SrcInstr = Src->getInstr();
@@ -185,15 +184,15 @@ static bool moveInstructionsOutOfTheWayIfWeCan(SUnit *Dst,
   // processing stage. In some context it does matter what the parent of the
   // instruction was: Namely when we are starting the traversal with the source
   // of the copy propagation. This instruction must have the destination as a
-  // dependency. In case of other instruction than has the destination as a dependency, this
-  // dependency would mean the end of the traversal, but in this scenario this
-  // must be ignored. Let's say that we can not control what nodes to process
-  // and we come across the copy source. How do I know what node has that copy
-  // source as their dependency? We can check of which node is the copy source
-  // the dependency of. This list will alway contain the source. To decide if we
-  // have it as dependency of another instruction, we must check in the already
-  // traversed list if any of the instructions that is depended on the source is
-  // contained. This would introduce extra costs.
+  // dependency. In case of other instruction than has the destination as a
+  // dependency, this dependency would mean the end of the traversal, but in
+  // this scenario this must be ignored. Let's say that we can not control what
+  // nodes to process and we come across the copy source. How do I know what
+  // node has that copy source as their dependency? We can check of which node
+  // is the copy source the dependency of. This list will alway contain the
+  // source. To decide if we have it as dependency of another instruction, we
+  // must check in the already traversed list if any of the instructions that is
+  // depended on the source is contained. This would introduce extra costs.
   ProcessSNodeChildren(Edges, Dst, true);
   while (!Edges.empty()) {
     const auto *Current = Edges.front();
@@ -252,9 +251,7 @@ public:
     }
   }
 
-  int getInvalidCopiesSize() {
-    return InvalidCopies.size();
-  }
+  int getInvalidCopiesSize() { return InvalidCopies.size(); }
 
   /// Remove register from copy maps.
   void invalidateRegister(MCRegister Reg, const TargetRegisterInfo &TRI,
@@ -382,9 +379,7 @@ public:
     return !Copies.empty();
   }
 
-  bool hasAnyInvalidCopies() {
-    return !InvalidCopies.empty();
-  }
+  bool hasAnyInvalidCopies() { return !InvalidCopies.empty(); }
 
   MachineInstr *findCopyForUnit(MCRegUnit RegUnit,
                                 const TargetRegisterInfo &TRI,
@@ -398,8 +393,8 @@ public:
   }
 
   MachineInstr *findInvalidCopyForUnit(MCRegUnit RegUnit,
-                                const TargetRegisterInfo &TRI,
-                                bool MustBeAvailable = false) {
+                                       const TargetRegisterInfo &TRI,
+                                       bool MustBeAvailable = false) {
     auto CI = InvalidCopies.find(RegUnit);
     if (CI == InvalidCopies.end())
       return nullptr;
@@ -420,7 +415,7 @@ public:
   }
 
   MachineInstr *findInvalidCopyDefViaUnit(MCRegUnit RegUnit,
-                                   const TargetRegisterInfo &TRI) {
+                                          const TargetRegisterInfo &TRI) {
     auto CI = InvalidCopies.find(RegUnit);
     if (CI == InvalidCopies.end())
       return nullptr;
@@ -431,8 +426,8 @@ public:
   }
 
   // TODO: This is ugly there shall be a more elegant solution to invalid
-  //       copy searching. Create a variant that either returns a valid an invalid
-  //       copy or no copy at all (std::monotype).
+  //       copy searching. Create a variant that either returns a valid an
+  //       invalid copy or no copy at all (std::monotype).
   MachineInstr *findAvailBackwardCopy(MachineInstr &I, MCRegister Reg,
                                       const TargetRegisterInfo &TRI,
                                       const TargetInstrInfo &TII,
@@ -542,7 +537,7 @@ public:
   }
 };
 
-using Copy = MachineInstr*;
+using Copy = MachineInstr *;
 using InvalidCopy = std::pair<Copy, MachineInstr *>;
 using CopyLookupResult = std::variant<std::monostate, Copy, InvalidCopy>;
 
@@ -583,11 +578,13 @@ private:
   void ReadRegister(MCRegister Reg, MachineInstr &Reader, DebugType DT);
   void readSuccessorLiveIns(const MachineBasicBlock &MBB);
   void ForwardCopyPropagateBlock(MachineBasicBlock &MBB);
-  void BackwardCopyPropagateBlock(MachineBasicBlock &MBB, bool ResolveAntiDeps = false);
+  void BackwardCopyPropagateBlock(MachineBasicBlock &MBB,
+                                  bool ResolveAntiDeps = false);
   void EliminateSpillageCopies(MachineBasicBlock &MBB);
   bool eraseIfRedundant(MachineInstr &Copy, MCRegister Src, MCRegister Def);
   void forwardUses(MachineInstr &MI);
-  void propagateDefs(MachineInstr &MI, ScheduleDAGMCP &DG, bool ResolveAntiDeps = false);
+  void propagateDefs(MachineInstr &MI, ScheduleDAGMCP &DG,
+                     bool ResolveAntiDeps = false);
   bool isForwardableRegClassCopy(const MachineInstr &Copy,
                                  const MachineInstr &UseI, unsigned UseIdx);
   bool isBackwardPropagatableRegClassCopy(const MachineInstr &Copy,
@@ -596,7 +593,7 @@ private:
   bool hasImplicitOverlap(const MachineInstr &MI, const MachineOperand &Use);
   bool hasOverlappingMultipleDef(const MachineInstr &MI,
                                  const MachineOperand &MODef, Register Def);
-  
+
   /// Candidates for deletion.
   SmallSetVector<MachineInstr *, 8> MaybeDeadCopies;
 
@@ -1155,9 +1152,9 @@ static bool isBackwardPropagatableCopy(const DestSourcePair &CopyOperands,
   return CopyOperands.Source->isRenamable() && CopyOperands.Source->isKill();
 }
 
-void MachineCopyPropagation::propagateDefs(MachineInstr &MI,
-                                           ScheduleDAGMCP &DG,
-                                           bool MoveDependenciesForBetterCopyPropagation) {
+void MachineCopyPropagation::propagateDefs(
+    MachineInstr &MI, ScheduleDAGMCP &DG,
+    bool MoveDependenciesForBetterCopyPropagation) {
   if (!Tracker.hasAnyCopies() && !Tracker.hasAnyInvalidCopies())
     return;
 
@@ -1247,15 +1244,14 @@ void MachineCopyPropagation::BackwardCopyPropagateBlock(
     DG.buildSchedGraph(nullptr);
     // DG.viewGraph();
   }
- 
 
   LLVM_DEBUG(dbgs() << "MCP: BackwardCopyPropagateBlock " << MBB.getName()
                     << "\n");
 
   for (MachineInstr &MI : llvm::make_early_inc_range(llvm::reverse(MBB))) {
-    //llvm::errs() << "Next MI: ";
-    //MI.dump();
-    // Ignore non-trivial COPYs.
+    // llvm::errs() << "Next MI: ";
+    // MI.dump();
+    //  Ignore non-trivial COPYs.
     std::optional<DestSourcePair> CopyOperands =
         isCopyInstr(MI, *TII, UseCopyInstr);
     if (CopyOperands && MI.getNumOperands() == 2) {
@@ -1266,8 +1262,8 @@ void MachineCopyPropagation::BackwardCopyPropagateBlock(
         // Unlike forward cp, we don't invoke propagateDefs here,
         // just let forward cp do COPY-to-COPY propagation.
         if (isBackwardPropagatableCopy(*CopyOperands, *MRI)) {
-          Tracker.invalidateRegister(SrcReg.asMCReg(), *TRI, *TII,
-                                     UseCopyInstr, MoveDependenciesForBetterCopyPropagation);
+          Tracker.invalidateRegister(SrcReg.asMCReg(), *TRI, *TII, UseCopyInstr,
+                                     MoveDependenciesForBetterCopyPropagation);
           Tracker.invalidateRegister(DefReg.asMCReg(), *TRI, *TII,
                                      UseCopyInstr);
           Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
@@ -1309,7 +1305,8 @@ void MachineCopyPropagation::BackwardCopyPropagateBlock(
           }
         } else {
           Tracker.invalidateRegister(MO.getReg().asMCReg(), *TRI, *TII,
-                                     UseCopyInstr, MoveDependenciesForBetterCopyPropagation);
+                                     UseCopyInstr,
+                                     MoveDependenciesForBetterCopyPropagation);
         }
       }
     }
@@ -1336,7 +1333,6 @@ void MachineCopyPropagation::BackwardCopyPropagateBlock(
     DG.fixupKills(MBB);
   }
 
-
   MaybeDeadCopies.clear();
   CopyDbgUsers.clear();
   Tracker.clear();
@@ -1699,8 +1695,8 @@ bool MachineCopyPropagation::runOnMachineFunction(MachineFunction &MF) {
     //
     // The reason for this two stage approach is that the ScheduleDAG can not
     // handle register renaming.
-    // QUESTION: I think these two stages could be merged together, if I were to change
-    // the renaming mechanism.
+    // QUESTION: I think these two stages could be merged together, if I were to
+    // change the renaming mechanism.
     //
     // The renaming wouldn't happen instantly. There would be a data structure
     // that contained what register should be renamed to what. Then after the

``````````

</details>


https://github.com/llvm/llvm-project/pull/105562


More information about the llvm-commits mailing list