[llvm] [CodeGen][MachineVerifier] Use TypeSize instead of unsigned for getRe… (PR #70881)

Michael Maitland via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 31 21:12:44 PDT 2023


https://github.com/michaelmaitland updated https://github.com/llvm/llvm-project/pull/70881

>From 2ab957c6d5e74912baf97848ec67fc4d76dd8d73 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 31 Oct 2023 20:39:46 -0700
Subject: [PATCH 1/5] [CodeGen][MIR] Support parsing of scalable vectors in MIR

This patch builds on the support for vectors by adding ability to parse
scalable vectors in MIR.
---
 llvm/lib/CodeGen/MIRParser/MIParser.cpp       | 37 +++++++++++++++----
 .../MIR/Generic/scalable-vector-type.mir      | 23 ++++++++++++
 2 files changed, 52 insertions(+), 8 deletions(-)
 create mode 100644 llvm/test/CodeGen/MIR/Generic/scalable-vector-type.mir

diff --git a/llvm/lib/CodeGen/MIRParser/MIParser.cpp b/llvm/lib/CodeGen/MIRParser/MIParser.cpp
index c01b34d6f490b0e..0da664e935f5442 100644
--- a/llvm/lib/CodeGen/MIRParser/MIParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIParser.cpp
@@ -1946,12 +1946,28 @@ bool MIParser::parseLowLevelType(StringRef::iterator Loc, LLT &Ty) {
 
   // Now we're looking for a vector.
   if (Token.isNot(MIToken::less))
-    return error(Loc,
-                 "expected sN, pA, <M x sN>, or <M x pA> for GlobalISel type");
+    return error(Loc, "expected sN, pA, <M x sN>, <M x pA>, <vscale x M x sN>, "
+                      "or <vscale x M x pA> for GlobalISel type");
   lex();
 
+  bool HasVScale = Token.stringValue() == "vscale";
+  if (HasVScale) {
+    lex();
+    if (Token.stringValue() != "x")
+      return error("expected <vscale x M x sN> or <vscale x M x pA>");
+    lex();
+  }
+
+  auto GetError = [&](bool HasVScale, StringRef::iterator Loc) {
+    if (HasVScale)
+      return error(
+          Loc, "expected <vscale x M x sN> or <vscale M x pA> for vector type");
+    else
+      return error(Loc, "expected <M x sN> or <M x pA> for vector type");
+  };
+
   if (Token.isNot(MIToken::IntegerLiteral))
-    return error(Loc, "expected <M x sN> or <M x pA> for vector type");
+    return GetError(HasVScale, Loc);
   uint64_t NumElements = Token.integerValue().getZExtValue();
   if (!verifyVectorElementCount(NumElements))
     return error("invalid number of vector elements");
@@ -1959,11 +1975,12 @@ bool MIParser::parseLowLevelType(StringRef::iterator Loc, LLT &Ty) {
   lex();
 
   if (Token.isNot(MIToken::Identifier) || Token.stringValue() != "x")
-    return error(Loc, "expected <M x sN> or <M x pA> for vector type");
+    return GetError(HasVScale, Loc);
   lex();
 
   if (Token.range().front() != 's' && Token.range().front() != 'p')
-    return error(Loc, "expected <M x sN> or <M x pA> for vector type");
+    return GetError(HasVScale, Loc);
+
   StringRef SizeStr = Token.range().drop_front();
   if (SizeStr.size() == 0 || !llvm::all_of(SizeStr, isdigit))
     return error("expected integers after 's'/'p' type character");
@@ -1981,14 +1998,18 @@ bool MIParser::parseLowLevelType(StringRef::iterator Loc, LLT &Ty) {
 
     Ty = LLT::pointer(AS, DL.getPointerSizeInBits(AS));
   } else
-    return error(Loc, "expected <M x sN> or <M x pA> for vector type");
+    return GetError(HasVScale, Loc);
   lex();
 
   if (Token.isNot(MIToken::greater))
-    return error(Loc, "expected <M x sN> or <M x pA> for vector type");
+    return GetError(HasVScale, Loc);
+
   lex();
 
-  Ty = LLT::fixed_vector(NumElements, Ty);
+  if (HasVScale)
+    Ty = LLT::scalable_vector(NumElements, Ty);
+  else
+    Ty = LLT::fixed_vector(NumElements, Ty);
   return false;
 }
 
diff --git a/llvm/test/CodeGen/MIR/Generic/scalable-vector-type.mir b/llvm/test/CodeGen/MIR/Generic/scalable-vector-type.mir
new file mode 100644
index 000000000000000..61ac8411b64505b
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/Generic/scalable-vector-type.mir
@@ -0,0 +1,23 @@
+# RUN: llc -run-pass=none -o - %s | FileCheck %s
+
+---
+name:            scalable_vector_type_s
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: scalable_vector_type_s
+    ; CHECK: [[DEF:%[0-9]+]]:_(<vscale x 1 x s8>) = IMPLICIT_DEF
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<vscale x 1 x s8>) = COPY [[DEF]](<vscale x 1 x s8>)
+    %0:_(<vscale x 1 x s8>) = IMPLICIT_DEF
+    %1:_(<vscale x 1 x s8>) = COPY %0
+...
+
+---
+name:            scalable_vector_type_p
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: scalable_vector_type_p
+    ; CHECK: [[DEF:%[0-9]+]]:_(<vscale x 1 x p0>) = IMPLICIT_DEF
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<vscale x 1 x p0>) = COPY [[DEF]](<vscale x 1 x p0>)
+    %0:_(<vscale x 1 x p0>) = IMPLICIT_DEF
+    %1:_(<vscale x 1 x p0>) = COPY %0
+...

>From 24f292e9ed4cac19cccf6fc648d8e1925391c750 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 31 Oct 2023 18:33:47 -0700
Subject: [PATCH 2/5] [CodeGen][MachineVerifier] Use TypeSize instead of
 unsigned for getRegSizeInBits

This patch changes getRegSizeInBits to return a TypeSize instead of an
unsigned in the case that a virtual register has a scalable LLT. In the
case that register is physical, a Fixed TypeSize is returned.

The MachineVerifier pass is updated to allow copies between fixed and
scalable operands as long as the Src size will fit into the Dest size.

This is a precommit which will be stacked on by a change to GISel to
generate COPYs with a scalable destination but a fixed size source.
---
 .../include/llvm/CodeGen/TargetRegisterInfo.h |  6 ++---
 llvm/lib/CodeGen/MachineVerifier.cpp          | 22 ++++++++++++++-----
 llvm/lib/CodeGen/TargetRegisterInfo.cpp       | 19 ++++++++--------
 3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 5bf27e40eee8909..3f64bf972daf21e 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -278,8 +278,8 @@ class TargetRegisterInfo : public MCRegisterInfo {
   // DenseMapInfo<unsigned> uses -1u and -2u.
 
   /// Return the size in bits of a register from class RC.
-  unsigned getRegSizeInBits(const TargetRegisterClass &RC) const {
-    return getRegClassInfo(RC).RegSize;
+  TypeSize getRegSizeInBits(const TargetRegisterClass &RC) const {
+    return TypeSize::Fixed(getRegClassInfo(RC).RegSize);
   }
 
   /// Return the size in bytes of the stack slot allocated to hold a spilled
@@ -853,7 +853,7 @@ class TargetRegisterInfo : public MCRegisterInfo {
     const TargetRegisterClass *RC) const = 0;
 
   /// Returns size in bits of a phys/virtual/generic register.
-  unsigned getRegSizeInBits(Register Reg, const MachineRegisterInfo &MRI) const;
+  TypeSize getRegSizeInBits(Register Reg, const MachineRegisterInfo &MRI) const;
 
   /// Get the weight in units of pressure for this register unit.
   virtual unsigned getRegUnitWeight(unsigned RegUnit) const = 0;
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index dadaf60fa09da04..9837a93d8339974 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1937,8 +1937,9 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
 
     // If we have only one valid type, this is likely a copy between a virtual
     // and physical register.
-    unsigned SrcSize = 0;
-    unsigned DstSize = 0;
+    TypeSize SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
+    TypeSize DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
+
     if (SrcReg.isPhysical() && DstTy.isValid()) {
       const TargetRegisterClass *SrcRC =
           TRI->getMinimalPhysRegClassLLT(SrcReg, DstTy);
@@ -1946,7 +1947,7 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         SrcSize = TRI->getRegSizeInBits(*SrcRC);
     }
 
-    if (SrcSize == 0)
+    if (SrcSize.isZero())
       SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
 
     if (DstReg.isPhysical() && SrcTy.isValid()) {
@@ -1956,10 +1957,21 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         DstSize = TRI->getRegSizeInBits(*DstRC);
     }
 
-    if (DstSize == 0)
+    if (DstSize.isZero())
       DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
 
-    if (SrcSize != 0 && DstSize != 0 && SrcSize != DstSize) {
+    // If the Dst is scalable and the Src is fixed, then the Dst can only hold
+    // the Src if the minimum size Dst can hold is at least as big as Src.
+    if (DstSize.isScalable() && !SrcSize.isScalable() &&
+        DstSize.getKnownMinValue() <= SrcSize.getFixedValue())
+      break;
+    // If the Src is scalable and the Dst is fixed, then Dest can only hold
+    // the Src is known to fit in Dest
+    if (SrcSize.isScalable() && !DstSize.isScalable() &&
+        TypeSize::isKnownLE(DstSize, SrcSize))
+      break;
+
+    if (SrcSize.isNonZero() && DstSize.isNonZero() && SrcSize != DstSize) {
       if (!DstOp.getSubReg() && !SrcOp.getSubReg()) {
         report("Copy Instruction is illegal with mismatching sizes", MI);
         errs() << "Def Size = " << DstSize << ", Src Size = " << SrcSize
diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
index 1bb35f40facfd0f..c50b1cf9422717a 100644
--- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp
+++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp
@@ -499,7 +499,7 @@ bool TargetRegisterInfo::regmaskSubsetEqual(const uint32_t *mask0,
   return true;
 }
 
-unsigned
+TypeSize
 TargetRegisterInfo::getRegSizeInBits(Register Reg,
                                      const MachineRegisterInfo &MRI) const {
   const TargetRegisterClass *RC{};
@@ -508,16 +508,15 @@ TargetRegisterInfo::getRegSizeInBits(Register Reg,
     // Instead, we need to access a register class that contains Reg and
     // get the size of that register class.
     RC = getMinimalPhysRegClass(Reg);
-  } else {
-    LLT Ty = MRI.getType(Reg);
-    unsigned RegSize = Ty.isValid() ? Ty.getSizeInBits() : 0;
-    // If Reg is not a generic register, query the register class to
-    // get its size.
-    if (RegSize)
-      return RegSize;
-    // Since Reg is not a generic register, it must have a register class.
-    RC = MRI.getRegClass(Reg);
+    assert(RC && "Unable to deduce the register class");
+    return getRegSizeInBits(*RC);
   }
+  LLT Ty = MRI.getType(Reg);
+  if (Ty.isValid())
+    return Ty.getSizeInBits();
+
+  // Since Reg is not a generic register, it may have a register class.
+  RC = MRI.getRegClass(Reg);
   assert(RC && "Unable to deduce the register class");
   return getRegSizeInBits(*RC);
 }

>From ec7261d596dab9280c126db46c4b2d558232cfea Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 31 Oct 2023 18:57:14 -0700
Subject: [PATCH 3/5] Avoid duplicate computation

---
 llvm/lib/CodeGen/MachineVerifier.cpp | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 9837a93d8339974..ca0c963a7aa7158 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1939,7 +1939,6 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
     // and physical register.
     TypeSize SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
     TypeSize DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
-
     if (SrcReg.isPhysical() && DstTy.isValid()) {
       const TargetRegisterClass *SrcRC =
           TRI->getMinimalPhysRegClassLLT(SrcReg, DstTy);
@@ -1947,9 +1946,6 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         SrcSize = TRI->getRegSizeInBits(*SrcRC);
     }
 
-    if (SrcSize.isZero())
-      SrcSize = TRI->getRegSizeInBits(SrcReg, *MRI);
-
     if (DstReg.isPhysical() && SrcTy.isValid()) {
       const TargetRegisterClass *DstRC =
           TRI->getMinimalPhysRegClassLLT(DstReg, SrcTy);
@@ -1957,9 +1953,6 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
         DstSize = TRI->getRegSizeInBits(*DstRC);
     }
 
-    if (DstSize.isZero())
-      DstSize = TRI->getRegSizeInBits(DstReg, *MRI);
-
     // If the Dst is scalable and the Src is fixed, then the Dst can only hold
     // the Src if the minimum size Dst can hold is at least as big as Src.
     if (DstSize.isScalable() && !SrcSize.isScalable() &&

>From 80a6a035a472f1223e45a204650f276a33944cde Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 31 Oct 2023 20:39:46 -0700
Subject: [PATCH 4/5] Add MachineVerifier test case

---
 llvm/test/MachineVerifier/copy-scalable.mir | 23 +++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 llvm/test/MachineVerifier/copy-scalable.mir

diff --git a/llvm/test/MachineVerifier/copy-scalable.mir b/llvm/test/MachineVerifier/copy-scalable.mir
new file mode 100644
index 000000000000000..f4088f7aed34dde
--- /dev/null
+++ b/llvm/test/MachineVerifier/copy-scalable.mir
@@ -0,0 +1,23 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
+# RUN: llc -mtriple=riscv64 -o - -global-isel -run-pass=none -verify-machineinstrs %s | FileCheck %s
+# REQUIRES: riscv64-registered-target
+
+---
+name:            test_copy_fixed_to_scalable
+legalized:       true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _, preferred-register: '' }
+liveins:
+body:             |
+  bb.0:
+    liveins: $v8
+
+    ; CHECK-LABEL: name: test_copy_fixed_to_scalable
+    ; CHECK: liveins: $v8
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<vscale x 1 x s8>) = COPY $v8
+    %0:_(<vscale x 1 x s8>) = COPY $v8
+...

>From 8f068948426195b07e884cfb2d4f34c56301517c Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 31 Oct 2023 20:50:07 -0700
Subject: [PATCH 5/5] fix failing test

---
 llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll
index 5dd62de8a6bc415..a3a913d8ce02d83 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/fallback.ll
@@ -22,7 +22,7 @@ entry:
   ret <vscale x 1 x i8> %a
 }
 
-; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction{{.*}}scalable_inst
+; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction: call:
 ; FALLBACK-WITH-REPORT-OUT-LABEL: scalable_inst
 define <vscale x 1 x i8> @scalable_inst(i64 %0) nounwind {
 entry:
@@ -35,7 +35,7 @@ entry:
   ret <vscale x 1 x i8> %a
 }
 
-; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction{{.*}}scalable_alloca
+; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction: alloca:
 ; FALLBACK-WITH-REPORT-OUT-LABEL: scalable_alloca
 define void @scalable_alloca() #1 {
   %local0 = alloca <vscale x 16 x i8>



More information about the llvm-commits mailing list