PeepholeOpt: Simplify tracking of current op for copy and reg_sequence

[arsenm]

Set the starting index in the constructor instead of treating
0 as a special case. There should also be no need for bounds
checking in the rewrite.

[arsenm]

• PeepholeOpt: Avoid double map lookup #124531
• PeepholeOpt: Remove check for reg_sequence def of subregister #124512
• PeepholeOpt: Simplify tracking of current op for copy and reg_sequence #124224 👈 (View in Graphite)
• PeepholeOpt: Do not add subregister indexes to reg_sequence operands #124111
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

Set the starting index in the constructor instead of treating
0 as a special case. There should also be no need for bounds
checking in the rewrite.

---

Full diff: https://github.com/llvm/llvm-project/pull/124224.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/PeepholeOptimizer.cpp (+8-23)

diff --git a/llvm/lib/CodeGen/PeepholeOptimizer.cpp b/llvm/lib/CodeGen/PeepholeOptimizer.cpp
index af4f2dc49b690b..2fc48209126acd 100644
--- a/llvm/lib/CodeGen/PeepholeOptimizer.cpp
+++ b/llvm/lib/CodeGen/PeepholeOptimizer.cpp
@@ -153,7 +153,7 @@ class RecurrenceInstr;
 class Rewriter {
 protected:
   MachineInstr &CopyLike;
-  unsigned CurrentSrcIdx = 0; ///< The index of the source being rewritten.
+  int CurrentSrcIdx = 0; ///< The index of the source being rewritten.
 public:
   Rewriter(MachineInstr &CopyLike) : CopyLike(CopyLike) {}
   virtual ~Rewriter() = default;
@@ -201,12 +201,9 @@ class CopyRewriter : public Rewriter {
 
   bool getNextRewritableSource(RegSubRegPair &Src,
                                RegSubRegPair &Dst) override {
-    // CurrentSrcIdx > 0 means this function has already been called.
-    if (CurrentSrcIdx > 0)
+    if (CurrentSrcIdx++ > 1)
       return false;
-    // This is the first call to getNextRewritableSource.
-    // Move the CurrentSrcIdx to remember that we made that call.
-    CurrentSrcIdx = 1;
+
     // The rewritable source is the argument.
     const MachineOperand &MOSrc = CopyLike.getOperand(1);
     Src = RegSubRegPair(MOSrc.getReg(), MOSrc.getSubReg());
@@ -217,8 +214,6 @@ class CopyRewriter : public Rewriter {
   }
 
   bool RewriteCurrentSource(Register NewReg, unsigned NewSubReg) override {
-    if (CurrentSrcIdx != 1)
-      return false;
     MachineOperand &MOSrc = CopyLike.getOperand(CurrentSrcIdx);
     MOSrc.setReg(NewReg);
     MOSrc.setSubReg(NewSubReg);
@@ -229,7 +224,7 @@ class CopyRewriter : public Rewriter {
 /// Helper class to rewrite uncoalescable copy like instructions
 /// into new COPY (coalescable friendly) instructions.
 class UncoalescableRewriter : public Rewriter {
-  unsigned NumDefs; ///< Number of defs in the bitcast.
+  int NumDefs; ///< Number of defs in the bitcast.
 
 public:
   UncoalescableRewriter(MachineInstr &MI) : Rewriter(MI) {
@@ -383,6 +378,7 @@ class RegSequenceRewriter : public Rewriter {
 public:
   RegSequenceRewriter(MachineInstr &MI) : Rewriter(MI) {
     assert(MI.isRegSequence() && "Invalid instruction");
+    CurrentSrcIdx = -1;
   }
 
   /// \see Rewriter::getNextRewritableSource()
@@ -404,16 +400,10 @@ class RegSequenceRewriter : public Rewriter {
   bool getNextRewritableSource(RegSubRegPair &Src,
                                RegSubRegPair &Dst) override {
     // We are looking at v0 = REG_SEQUENCE v1, sub1, v2, sub2, etc.
+    CurrentSrcIdx += 2;
+    if (static_cast<unsigned>(CurrentSrcIdx) >= CopyLike.getNumOperands())
+      return false;
 
-    // If this is the first call, move to the first argument.
-    if (CurrentSrcIdx == 0) {
-      CurrentSrcIdx = 1;
-    } else {
-      // Otherwise, move to the next argument and check that it is valid.
-      CurrentSrcIdx += 2;
-      if (CurrentSrcIdx >= CopyLike.getNumOperands())
-        return false;
-    }
     const MachineOperand &MOInsertedReg = CopyLike.getOperand(CurrentSrcIdx);
     Src.Reg = MOInsertedReg.getReg();
     // If we have to compose sub-register indices, bail out.
@@ -431,11 +421,6 @@ class RegSequenceRewriter : public Rewriter {
   }
 
   bool RewriteCurrentSource(Register NewReg, unsigned NewSubReg) override {
-    // We cannot rewrite out of bound operands.
-    // Moreover, rewritable sources are at odd positions.
-    if ((CurrentSrcIdx & 1) != 1 || CurrentSrcIdx > CopyLike.getNumOperands())
-      return false;
-
     // Do not introduce new subregister uses in a reg_sequence. Until composing
     // subregister indices is supported while folding, we're just blocking
     // folding of subregister copies later in the function.

[arsenm]

Merge activity

• Jan 30, 8:37 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Jan 30, 8:44 AM EST: Graphite rebased this pull request as part of a merge.
• Jan 30, 8:49 AM EST: A user merged this pull request with Graphite.