[llvm] [Codegen] Change getSpillSize/getReloadSize to LocationSize. NFC (PR #82636)

David Green via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 22 07:45:12 PST 2024


https://github.com/davemgreen created https://github.com/llvm/llvm-project/pull/82636

This is a small part of #70452, attempting to take a small simpler part of it in isolation to simplify what remains.  It changes the getSpillSize, getFoldedSpillSize, getRestoreSize and getFoldedRestoreSize methods to return optional<LocationSize> instead of unsigned. The code is intended to be the same, keeping the optional to specify when there was no size found, with some minor adjustments to make sure that unknown (~UINT64_C(0)) sizes are handled sensibly. Hopefully as more unsigned's are converted to LocationSize's the use of ~UINT64_C(0) can be cleaned up too.

>From 0fe0ab1fbcde864fbecc2b0e81702ce26b552313 Mon Sep 17 00:00:00 2001
From: David Green <david.green at arm.com>
Date: Thu, 22 Feb 2024 15:16:07 +0000
Subject: [PATCH] [Codegen] Change getSpillSize/getReloadSize to LocationSize.
 NFC

This is a small part of #70452, attempting to take a small simpler part of it
in isolation to simplify what remains.  It changes the getSpillSize,
getFoldedSpillSize, getRestoreSize and getFoldedRestoreSize methods to return
optional<LocationSize> instead of unsigned. The code is intended to be the
same, with some minor adjustments to make sure that unknown (~UINT64_C(0))
sizes are handled correctly. Hopefully as more unsigned's are converted to
LocationSize's the use of ~UINT64_C(0) can be cleaned up too.
---
 llvm/include/llvm/CodeGen/MachineInstr.h   | 10 ++++---
 llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 26 +++++++----------
 llvm/lib/CodeGen/MachineInstr.cpp          | 34 +++++++++++++---------
 3 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index bd72ac23fc9c08..cc3d6fd9528ee9 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ilist.h"
 #include "llvm/ADT/ilist_node.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/Analysis/MemoryLocation.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/TargetOpcodes.h"
@@ -1744,16 +1745,17 @@ class MachineInstr
   bool allImplicitDefsAreDead() const;
 
   /// Return a valid size if the instruction is a spill instruction.
-  std::optional<unsigned> getSpillSize(const TargetInstrInfo *TII) const;
+  std::optional<LocationSize> getSpillSize(const TargetInstrInfo *TII) const;
 
   /// Return a valid size if the instruction is a folded spill instruction.
-  std::optional<unsigned> getFoldedSpillSize(const TargetInstrInfo *TII) const;
+  std::optional<LocationSize>
+  getFoldedSpillSize(const TargetInstrInfo *TII) const;
 
   /// Return a valid size if the instruction is a restore instruction.
-  std::optional<unsigned> getRestoreSize(const TargetInstrInfo *TII) const;
+  std::optional<LocationSize> getRestoreSize(const TargetInstrInfo *TII) const;
 
   /// Return a valid size if the instruction is a folded restore instruction.
-  std::optional<unsigned>
+  std::optional<LocationSize>
   getFoldedRestoreSize(const TargetInstrInfo *TII) const;
 
   /// Copy implicit register operands from specified
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index e89a1c26c23e6b..0efe7a0e733672 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1106,25 +1106,21 @@ static void emitComments(const MachineInstr &MI, raw_ostream &CommentOS) {
 
   // We assume a single instruction only has a spill or reload, not
   // both.
-  std::optional<unsigned> Size;
+  std::optional<LocationSize> Size;
   if ((Size = MI.getRestoreSize(TII))) {
-    CommentOS << *Size << "-byte Reload\n";
+    CommentOS << Size->getValue() << "-byte Reload\n";
   } else if ((Size = MI.getFoldedRestoreSize(TII))) {
-    if (*Size) {
-      if (*Size == unsigned(MemoryLocation::UnknownSize))
-        CommentOS << "Unknown-size Folded Reload\n";
-      else
-        CommentOS << *Size << "-byte Folded Reload\n";
-    }
+    if (!Size->hasValue())
+      CommentOS << "Unknown-size Folded Reload\n";
+    else if (Size->getValue())
+      CommentOS << Size->getValue() << "-byte Folded Reload\n";
   } else if ((Size = MI.getSpillSize(TII))) {
-    CommentOS << *Size << "-byte Spill\n";
+    CommentOS << Size->getValue() << "-byte Spill\n";
   } else if ((Size = MI.getFoldedSpillSize(TII))) {
-    if (*Size) {
-      if (*Size == unsigned(MemoryLocation::UnknownSize))
-        CommentOS << "Unknown-size Folded Spill\n";
-      else
-        CommentOS << *Size << "-byte Folded Spill\n";
-    }
+    if (!Size->hasValue())
+      CommentOS << "Unknown-size Folded Spill\n";
+    else if (Size->getValue())
+      CommentOS << Size->getValue() << "-byte Folded Spill\n";
   }
 
   // Check for spill-induced copies
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index efbcc839f22d0c..6654e1d6ceceea 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -2354,29 +2354,35 @@ void MachineInstr::changeDebugValuesDefReg(Register Reg) {
 
 using MMOList = SmallVector<const MachineMemOperand *, 2>;
 
-static unsigned getSpillSlotSize(const MMOList &Accesses,
-                                 const MachineFrameInfo &MFI) {
-  unsigned Size = 0;
+static LocationSize getSpillSlotSize(const MMOList &Accesses,
+                                     const MachineFrameInfo &MFI) {
+  uint64_t Size = 0;
   for (const auto *A : Accesses)
     if (MFI.isSpillSlotObjectIndex(
             cast<FixedStackPseudoSourceValue>(A->getPseudoValue())
-                ->getFrameIndex()))
-      Size += A->getSize();
+                ->getFrameIndex())) {
+      uint64_t S = A->getSize();
+      if (S == ~UINT64_C(0))
+        return LocationSize::beforeOrAfterPointer();
+      Size += S;
+    }
   return Size;
 }
 
-std::optional<unsigned>
+std::optional<LocationSize>
 MachineInstr::getSpillSize(const TargetInstrInfo *TII) const {
   int FI;
   if (TII->isStoreToStackSlotPostFE(*this, FI)) {
     const MachineFrameInfo &MFI = getMF()->getFrameInfo();
-    if (MFI.isSpillSlotObjectIndex(FI))
-      return (*memoperands_begin())->getSize();
+    if (MFI.isSpillSlotObjectIndex(FI)) {
+      uint64_t Size = (*memoperands_begin())->getSize();
+      return Size == ~UINT64_C(0) ? LocationSize::beforeOrAfterPointer() : Size;
+    }
   }
   return std::nullopt;
 }
 
-std::optional<unsigned>
+std::optional<LocationSize>
 MachineInstr::getFoldedSpillSize(const TargetInstrInfo *TII) const {
   MMOList Accesses;
   if (TII->hasStoreToStackSlot(*this, Accesses))
@@ -2384,18 +2390,20 @@ MachineInstr::getFoldedSpillSize(const TargetInstrInfo *TII) const {
   return std::nullopt;
 }
 
-std::optional<unsigned>
+std::optional<LocationSize>
 MachineInstr::getRestoreSize(const TargetInstrInfo *TII) const {
   int FI;
   if (TII->isLoadFromStackSlotPostFE(*this, FI)) {
     const MachineFrameInfo &MFI = getMF()->getFrameInfo();
-    if (MFI.isSpillSlotObjectIndex(FI))
-      return (*memoperands_begin())->getSize();
+    if (MFI.isSpillSlotObjectIndex(FI)) {
+      uint64_t Size = (*memoperands_begin())->getSize();
+      return Size == ~UINT64_C(0) ? LocationSize::beforeOrAfterPointer() : Size;
+    }
   }
   return std::nullopt;
 }
 
-std::optional<unsigned>
+std::optional<LocationSize>
 MachineInstr::getFoldedRestoreSize(const TargetInstrInfo *TII) const {
   MMOList Accesses;
   if (TII->hasLoadFromStackSlot(*this, Accesses))



More information about the llvm-commits mailing list