Skip to content

Conversation

@arichardson
Copy link
Member

Previously we would assert when a ValueTypeByHwMode was missing a case
for the current mode, now we report an error instead. Interestingly this
error only ocurrs when the DAG patterns use RegClassByHwMode, but not
normal RegisterClass instances. Found while I added RegClassByHwMode
to RISC-V and was getting an assertion due to XLenFVT/XLenVecI32VT
not having an entry for the default mode.

Created using spr 1.3.8-beta.1

[skip ci]
Created using spr 1.3.8-beta.1
@llvmbot
Copy link
Member

llvmbot commented Dec 9, 2025

@llvm/pr-subscribers-tablegen

Author: Alexander Richardson (arichardson)

Changes

Previously we would assert when a ValueTypeByHwMode was missing a case
for the current mode, now we report an error instead. Interestingly this
error only ocurrs when the DAG patterns use RegClassByHwMode, but not
normal RegisterClass instances. Found while I added RegClassByHwMode
to RISC-V and was getting an assertion due to XLenFVT/XLenVecI32VT
not having an entry for the default mode.


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

4 Files Affected:

  • (modified) llvm/test/TableGen/RegClassByHwModeErrors.td (+33)
  • (modified) llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp (+19-5)
  • (modified) llvm/utils/TableGen/Common/InfoByHwMode.cpp (+10-6)
  • (modified) llvm/utils/TableGen/Common/InfoByHwMode.h (+4-1)
diff --git a/llvm/test/TableGen/RegClassByHwModeErrors.td b/llvm/test/TableGen/RegClassByHwModeErrors.td
index 5d91b6e70410b..0ee6370ccd0ce 100644
--- a/llvm/test/TableGen/RegClassByHwModeErrors.td
+++ b/llvm/test/TableGen/RegClassByHwModeErrors.td
@@ -7,6 +7,8 @@
 // RUN:   %t/compress-regclass-by-hwmode.td -o /dev/null 2>&1 | FileCheck %t/compress-regclass-by-hwmode.td --implicit-check-not="error:"
 // RUN: not llvm-tblgen --gen-compress-inst-emitter -I %p/../../include -I %t -I %S \
 // RUN:   %t/compress-regclass-by-hwmode-2.td -o /dev/null 2>&1 | FileCheck %t/compress-regclass-by-hwmode-2.td --implicit-check-not="error:"
+// RUN: not llvm-tblgen --gen-dag-isel  -I %p/../../include -I %t -I %S \
+// RUN:   %t/vt-by-hwmode-missing.td -o /dev/null 2>&1 | FileCheck %t/vt-by-hwmode-missing.td --implicit-check-not="error:"
 
 //--- Common.td
 include "Common/RegClassByHwModeCommon.td"
@@ -86,3 +88,34 @@ def : CompressPat<(X_MOV_BIG XRegs:$dst, XRegs:$src),
 // CHECK: [[#@LINE-2]]:1: error: Type mismatch between Input and Output Dag operand 'dst'
 def MyTargetISA : InstrInfo;
 def MyTarget : Target { let InstructionSet = MyTargetISA; }
+
+
+//--- vt-by-hwmode-missing.td
+include "Common.td"
+/// This should fail since we are missing a DefaultMode entry for the
+/// ValueTypeByHwMode and can't resolve it for the Ptr32 (default) case.
+def BadVT : ValueTypeByHwMode<[Ptr64], [i64]>;
+def BadVTRegClass : RegisterClass<"MyTarget", [i64, BadVT], 64, (add Y0, Y1)>;
+/// NOTE: this error only occurs for RegClassByHwMode, normal RegisterClass
+/// logic for VT resolution takes a different code path and does not error.
+def TEST_OK : TestInstruction {
+  let OutOperandList = (outs BadVTRegClass:$dst);
+  let InOperandList = (ins BadVTRegClass:$src1, BadVTRegClass:$src2);
+  let AsmString = "test $dst, $src1, $src2";
+  let Pattern = [(set BadVTRegClass:$dst, (add BadVTRegClass:$src1, BadVTRegClass:$src2))];
+}
+/// Once we use RegClassByHwMode, we get an error about missing modes:
+def BadPtrRC : RegClassByHwMode<[Ptr32, Ptr64], [BadVTRegClass, YRegs]>;
+// CHECK: vt-by-hwmode-missing.td:[[#@LINE-1]]:5: error: Could not resolve VT for Mode DefaultMode
+// CHECK: vt-by-hwmode-missing.td:4:5: note: ValueTypeByHwMode BadVT defined here
+// CHECK: vt-by-hwmode-missing.td:[[#@LINE+1]]:5: note: pattern instantiated here
+def TEST : TestInstruction {
+  let OutOperandList = (outs BadPtrRC:$dst);
+  let InOperandList = (ins BadPtrRC:$src1, BadPtrRC:$src2);
+  let AsmString = "test $dst, $src1, $src2";
+  let opcode = 0;
+  let Pattern = [(set BadPtrRC:$dst, (add BadPtrRC:$src1, BadPtrRC:$src2))];
+}
+
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index 364817fa6d030..61c5a9f138bfa 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -1798,15 +1798,27 @@ bool llvm::operator<(const SDTypeConstraint &LHS, const SDTypeConstraint &RHS) {
 /// RegClassByHwMode acts like ValueTypeByHwMode, taking the type of the
 /// register class from the active mode.
 static TypeSetByHwMode getTypeForRegClassByHwMode(const CodeGenTarget &T,
-                                                  const Record *R) {
+                                                  const Record *R,
+                                                  ArrayRef<SMLoc> Loc) {
   TypeSetByHwMode TypeSet;
   RegClassByHwMode Helper(R, T.getHwModes(), T.getRegBank());
 
   for (auto [ModeID, RegClass] : Helper) {
     ArrayRef<ValueTypeByHwMode> RegClassVTs = RegClass->getValueTypes();
     MachineValueTypeSet &ModeTypeSet = TypeSet.getOrCreate(ModeID);
-    for (const ValueTypeByHwMode &VT : RegClassVTs)
+    for (const ValueTypeByHwMode &VT : RegClassVTs) {
+      if (!VT.hasMode(ModeID) && !VT.hasDefault()) {
+        PrintError(R->getLoc(), "Could not resolve VT for Mode " +
+                                    T.getHwModes().getModeName(ModeID, true));
+        if (VT.getRecord())
+          PrintNote(VT.getRecord()->getLoc(), "ValueTypeByHwMode " +
+                                                  VT.getRecord()->getName() +
+                                                  " defined here");
+        PrintFatalNote(Loc, "pattern instantiated here");
+        continue;
+      }
       ModeTypeSet.insert(VT.getType(ModeID));
+    }
   }
 
   return TypeSet;
@@ -1841,7 +1853,9 @@ bool TreePatternNode::UpdateNodeTypeFromInst(unsigned ResNo,
   assert(RC && "Unknown operand type");
   CodeGenTarget &Tgt = TP.getDAGPatterns().getTargetInfo();
   if (RC->isSubClassOf("RegClassByHwMode"))
-    return UpdateNodeType(ResNo, getTypeForRegClassByHwMode(Tgt, RC), TP);
+    return UpdateNodeType(
+        ResNo, getTypeForRegClassByHwMode(Tgt, RC, TP.getRecord()->getLoc()),
+        TP);
 
   return UpdateNodeType(ResNo, Tgt.getRegisterClass(RC).getValueTypes(), TP);
 }
@@ -2331,7 +2345,7 @@ static TypeSetByHwMode getImplicitType(const Record *R, unsigned ResNo,
     const CodeGenTarget &T = TP.getDAGPatterns().getTargetInfo();
 
     if (RegClass->isSubClassOf("RegClassByHwMode"))
-      return getTypeForRegClassByHwMode(T, RegClass);
+      return getTypeForRegClassByHwMode(T, RegClass, TP.getRecord()->getLoc());
 
     return TypeSetByHwMode(T.getRegisterClass(RegClass).getValueTypes());
   }
@@ -2354,7 +2368,7 @@ static TypeSetByHwMode getImplicitType(const Record *R, unsigned ResNo,
 
   if (R->isSubClassOf("RegClassByHwMode")) {
     const CodeGenTarget &T = CDP.getTargetInfo();
-    return getTypeForRegClassByHwMode(T, R);
+    return getTypeForRegClassByHwMode(T, R, TP.getRecord()->getLoc());
   }
 
   if (R->isSubClassOf("PatFrags")) {
diff --git a/llvm/utils/TableGen/Common/InfoByHwMode.cpp b/llvm/utils/TableGen/Common/InfoByHwMode.cpp
index a16fdbb58e788..a3f8909c36090 100644
--- a/llvm/utils/TableGen/Common/InfoByHwMode.cpp
+++ b/llvm/utils/TableGen/Common/InfoByHwMode.cpp
@@ -29,8 +29,8 @@ std::string llvm::getModeName(unsigned Mode) {
   return (Twine('m') + Twine(Mode)).str();
 }
 
-ValueTypeByHwMode::ValueTypeByHwMode(const Record *R,
-                                     const CodeGenHwModes &CGH) {
+ValueTypeByHwMode::ValueTypeByHwMode(const Record *R, const CodeGenHwModes &CGH)
+    : InfoByHwMode<llvm::MVT>(R) {
   const HwModeSelect &MS = CGH.getHwModeSelect(R);
   for (const HwModeSelect::PairType &P : MS.Items) {
     auto I = Map.try_emplace(P.first, MVT(llvm::getValueType(P.second)));
@@ -140,7 +140,8 @@ void RegSizeInfo::writeToStream(raw_ostream &OS) const {
 }
 
 RegSizeInfoByHwMode::RegSizeInfoByHwMode(const Record *R,
-                                         const CodeGenHwModes &CGH) {
+                                         const CodeGenHwModes &CGH)
+    : InfoByHwMode<llvm::RegSizeInfo>(R) {
   const HwModeSelect &MS = CGH.getHwModeSelect(R);
   for (const HwModeSelect::PairType &P : MS.Items) {
     auto I = Map.try_emplace(P.first, RegSizeInfo(P.second));
@@ -188,7 +189,8 @@ void RegSizeInfoByHwMode::writeToStream(raw_ostream &OS) const {
 }
 
 RegClassByHwMode::RegClassByHwMode(const Record *R, const CodeGenHwModes &CGH,
-                                   const CodeGenRegBank &RegBank) {
+                                   const CodeGenRegBank &RegBank)
+    : InfoByHwMode<const llvm::CodeGenRegisterClass *>(R) {
   const HwModeSelect &MS = CGH.getHwModeSelect(R);
 
   for (auto [ModeID, RegClassRec] : MS.Items) {
@@ -206,7 +208,8 @@ SubRegRange::SubRegRange(const Record *R) {
 }
 
 SubRegRangeByHwMode::SubRegRangeByHwMode(const Record *R,
-                                         const CodeGenHwModes &CGH) {
+                                         const CodeGenHwModes &CGH)
+    : InfoByHwMode<llvm::SubRegRange>(R) {
   const HwModeSelect &MS = CGH.getHwModeSelect(R);
   for (const HwModeSelect::PairType &P : MS.Items) {
     auto I = Map.try_emplace(P.first, SubRegRange(P.second));
@@ -216,7 +219,8 @@ SubRegRangeByHwMode::SubRegRangeByHwMode(const Record *R,
 }
 
 EncodingInfoByHwMode::EncodingInfoByHwMode(const Record *R,
-                                           const CodeGenHwModes &CGH) {
+                                           const CodeGenHwModes &CGH)
+    : InfoByHwMode<const llvm::Record *>(R) {
   const HwModeSelect &MS = CGH.getHwModeSelect(R);
   for (const HwModeSelect::PairType &P : MS.Items) {
     assert(P.second && P.second->isSubClassOf("InstructionEncoding") &&
diff --git a/llvm/utils/TableGen/Common/InfoByHwMode.h b/llvm/utils/TableGen/Common/InfoByHwMode.h
index bd24fb84b085a..d53a218a40512 100644
--- a/llvm/utils/TableGen/Common/InfoByHwMode.h
+++ b/llvm/utils/TableGen/Common/InfoByHwMode.h
@@ -91,7 +91,7 @@ template <typename InfoT> struct InfoByHwMode {
   using iterator = typename MapType::iterator;
   using const_iterator = typename MapType::const_iterator;
 
-  InfoByHwMode() = default;
+  explicit InfoByHwMode(const Record *Def = nullptr) : Def(Def) {};
   InfoByHwMode(const MapType &M) : Map(M) {}
 
   LLVM_ATTRIBUTE_ALWAYS_INLINE
@@ -150,8 +150,11 @@ template <typename InfoT> struct InfoByHwMode {
     Map.try_emplace(DefaultMode, I);
   }
 
+  const Record *getRecord() const { return Def; }
+
 protected:
   MapType Map;
+  const Record *Def;
 };
 
 struct ValueTypeByHwMode : public InfoByHwMode<MVT> {

Created using spr 1.3.8-beta.1

[skip ci]
Created using spr 1.3.8-beta.1
@arichardson arichardson changed the base branch from users/arichardson/spr/main.tablegen-improve-error-message-for-bad-vtbyhwmode-in-registerbyhwmode to main December 10, 2025 21:59
@arichardson arichardson merged commit f672f32 into main Dec 10, 2025
16 of 17 checks passed
@arichardson arichardson deleted the users/arichardson/spr/tablegen-improve-error-message-for-bad-vtbyhwmode-in-registerbyhwmode branch December 10, 2025 21:59
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Dec 10, 2025
…isterByHwMode

Previously we would assert when a ValueTypeByHwMode was missing a case
for the current mode, now we report an error instead. Interestingly this
error only ocurrs when the DAG patterns use RegClassByHwMode, but not
normal RegisterClass instances. Found while I added RegClassByHwMode
to RISC-V and was getting an assertion due to `XLenFVT`/`XLenVecI32VT`
not having an entry for the default mode.

Reviewed By: arsenm

Pull Request: llvm/llvm-project#171254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants