[llvm] 01e75e2 - Revert "[SelectionDAG] Use SLEB128 for signed integers in isel table instead of 'signed rotated'. NFC (#173928)"

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 30 22:21:10 PST 2025


Author: Craig Topper
Date: 2025-12-30T22:13:58-08:00
New Revision: 01e75e2eafcea00c448e289154d8adeecc1c6c3a

URL: https://github.com/llvm/llvm-project/commit/01e75e2eafcea00c448e289154d8adeecc1c6c3a
DIFF: https://github.com/llvm/llvm-project/commit/01e75e2eafcea00c448e289154d8adeecc1c6c3a.diff

LOG: Revert "[SelectionDAG] Use SLEB128 for signed integers in isel table instead of 'signed rotated'. NFC (#173928)"

This reverts commit 3ff2637d867a6cc23ea5d5127b065efb8299d196.

I accidentally merged another PR into this during a rebase. Reverting
to commit it correctly.

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/SelectionDAGISel.h
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
    llvm/test/TableGen/DAGDefaultOps.td
    llvm/test/TableGen/dag-isel-regclass-emit-enum.td
    llvm/test/TableGen/dag-isel-subregs.td
    llvm/utils/TableGen/Common/DAGISelMatcher.h
    llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
    llvm/utils/TableGen/DAGISelMatcherGen.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index 569353670c532..61c5fdc9ef2ba 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -261,6 +261,9 @@ class SelectionDAGISel {
     OPC_EmitIntegerI16,
     OPC_EmitIntegerI32,
     OPC_EmitIntegerI64,
+    OPC_EmitStringInteger,
+    // Space-optimized forms that implicitly encode integer VT.
+    OPC_EmitStringIntegerI32,
     OPC_EmitRegister,
     OPC_EmitRegisterI32,
     OPC_EmitRegisterI64,

diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 5bee87bb52322..1c9e6bd5672be 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -96,7 +96,6 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/KnownBits.h"
-#include "llvm/Support/LEB128.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
@@ -2720,14 +2719,6 @@ GetVBR(uint64_t Val, const uint8_t *MatcherTable, unsigned &Idx) {
   return Val;
 }
 
-LLVM_ATTRIBUTE_ALWAYS_INLINE static int64_t
-GetSignedVBR(const unsigned char *MatcherTable, unsigned &Idx) {
-  unsigned Len;
-  int64_t Val = decodeSLEB128(&MatcherTable[Idx], &Len);
-  Idx += Len;
-  return Val;
-}
-
 /// getSimpleVT - Decode a value in MatcherTable, if it's a VBR encoded value,
 /// use GetVBR to decode it.
 LLVM_ATTRIBUTE_ALWAYS_INLINE static MVT::SimpleValueType
@@ -3031,9 +3022,24 @@ CheckValueType(const uint8_t *MatcherTable, unsigned &MatcherIndex, SDValue N,
   return VT == MVT::iPTR && cast<VTSDNode>(N)->getVT() == TLI->getPointerTy(DL);
 }
 
+// Bit 0 stores the sign of the immediate. The upper bits contain the magnitude
+// shifted left by 1.
+static uint64_t decodeSignRotatedValue(uint64_t V) {
+  if ((V & 1) == 0)
+    return V >> 1;
+  if (V != 1)
+    return -(V >> 1);
+  // There is no such thing as -0 with integers.  "-0" really means MININT.
+  return 1ULL << 63;
+}
+
 LLVM_ATTRIBUTE_ALWAYS_INLINE static bool
 CheckInteger(const uint8_t *MatcherTable, unsigned &MatcherIndex, SDValue N) {
-  int64_t Val = GetSignedVBR(MatcherTable, MatcherIndex);
+  int64_t Val = MatcherTable[MatcherIndex++];
+  if (Val & 128)
+    Val = GetVBR(Val, MatcherTable, MatcherIndex);
+
+  Val = decodeSignRotatedValue(Val);
 
   ConstantSDNode *C = dyn_cast<ConstantSDNode>(N);
   return C && C->getAPIntValue().trySExtValue() == Val;
@@ -3693,7 +3699,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
 
     case OPC_CheckType:
     case OPC_CheckTypeI32:
-    case OPC_CheckTypeI64: {
+    case OPC_CheckTypeI64:
       MVT::SimpleValueType VT;
       switch (Opcode) {
       case OPC_CheckTypeI32:
@@ -3709,7 +3715,6 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
       if (!::CheckType(VT, N, TLI, CurDAG->getDataLayout()))
         break;
       continue;
-    }
 
     case OPC_CheckTypeRes: {
       unsigned Res = MatcherTable[MatcherIndex++];
@@ -3892,7 +3897,9 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
     case OPC_EmitIntegerI8:
     case OPC_EmitIntegerI16:
     case OPC_EmitIntegerI32:
-    case OPC_EmitIntegerI64: {
+    case OPC_EmitIntegerI64:
+    case OPC_EmitStringInteger:
+    case OPC_EmitStringIntegerI32: {
       MVT::SimpleValueType VT;
       switch (Opcode) {
       case OPC_EmitIntegerI8:
@@ -3902,6 +3909,7 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
         VT = MVT::i16;
         break;
       case OPC_EmitIntegerI32:
+      case OPC_EmitStringIntegerI32:
         VT = MVT::i32;
         break;
       case OPC_EmitIntegerI64:
@@ -3911,7 +3919,11 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
         VT = getSimpleVT(MatcherTable, MatcherIndex);
         break;
       }
-      int64_t Val = GetSignedVBR(MatcherTable, MatcherIndex);
+      int64_t Val = MatcherTable[MatcherIndex++];
+      if (Val & 128)
+        Val = GetVBR(Val, MatcherTable, MatcherIndex);
+      if (Opcode >= OPC_EmitInteger && Opcode <= OPC_EmitIntegerI64)
+        Val = decodeSignRotatedValue(Val);
       RecordedNodes.emplace_back(
           CurDAG->getSignedConstant(Val, SDLoc(NodeToMatch), VT,
                                     /*isTarget=*/true),

diff  --git a/llvm/test/TableGen/DAGDefaultOps.td b/llvm/test/TableGen/DAGDefaultOps.td
index 78cc58fcf9790..83c95b471373f 100644
--- a/llvm/test/TableGen/DAGDefaultOps.td
+++ b/llvm/test/TableGen/DAGDefaultOps.td
@@ -83,7 +83,7 @@ def MulIRRPat : Pat<(mul i32:$x, i32:$y), (MulIRR Reg:$x, Reg:$y)>;
 // ADDINT-NEXT: OPC_CheckChild0Integer
 // ADDINT-NEXT: OPC_RecordChild1
 // ADDINT-NEXT: OPC_RecordChild2
-// ADDINT-NEXT: OPC_EmitIntegerI32, 1
+// ADDINT-NEXT: OPC_EmitIntegerI32, 2
 // ADDINT-NEXT: OPC_MorphNodeTo1None, TARGET_VAL(::AddRRI)
 
 // SUB: SwitchOpcode{{.*}}TARGET_VAL(ISD::SUB)

diff  --git a/llvm/test/TableGen/dag-isel-regclass-emit-enum.td b/llvm/test/TableGen/dag-isel-regclass-emit-enum.td
index f2858fc7504a4..c15835ee4fce3 100644
--- a/llvm/test/TableGen/dag-isel-regclass-emit-enum.td
+++ b/llvm/test/TableGen/dag-isel-regclass-emit-enum.td
@@ -25,14 +25,14 @@ def GPRAbove127 : RegisterClass<"TestTarget", [i32], 32,
 // CHECK-NEXT: OPC_RecordChild0, // #0 = $src
 // CHECK-NEXT: OPC_Scope, 12, /*->18*/ // 2 children in Scope
 // CHECK-NEXT: OPC_CheckChild1Integer, 0,
-// CHECK-NEXT: OPC_EmitIntegerI32, 0|128,1/*128*/, // #1 = TestNamespace::GPRAbove127RegClassID
+// CHECK-NEXT: OPC_EmitIntegerI32, 0|128,2/*256*/,
 // CHECK-NEXT: OPC_MorphNodeTo1None, TARGET_VAL(TargetOpcode::COPY_TO_REGCLASS),
 // CHECK-NEXT:     /*MVT::i32*/7, 2/*#Ops*/, 1, 0,
 def : Pat<(i32 (add i32:$src, (i32 0))),
           (COPY_TO_REGCLASS GPRAbove127, GPR0:$src)>;
 
-// CHECK:      OPC_CheckChild1Integer, 1,
-// CHECK-NEXT: OPC_EmitIntegerI32, TestNamespace::GPR127RegClassID,
+// CHECK:      OPC_CheckChild1Integer, 2,
+// CHECK-NEXT: OPC_EmitStringIntegerI32, TestNamespace::GPR127RegClassID,
 // CHECK-NEXT: OPC_MorphNodeTo1None, TARGET_VAL(TargetOpcode::COPY_TO_REGCLASS),
 // CHECK-NEXT:     /*MVT::i32*/7, 2/*#Ops*/, 1, 0,
 def : Pat<(i32 (add i32:$src, (i32 1))),

diff  --git a/llvm/test/TableGen/dag-isel-subregs.td b/llvm/test/TableGen/dag-isel-subregs.td
index e0e94e033a9b2..c66cb9e31ac0a 100644
--- a/llvm/test/TableGen/dag-isel-subregs.td
+++ b/llvm/test/TableGen/dag-isel-subregs.td
@@ -4,11 +4,11 @@ include "reg-with-subregs-common.td"
 
 // CHECK-LABEL: OPC_CheckOpcode, TARGET_VAL(ISD::EXTRACT_SUBVECTOR),
 // CHECK: OPC_CheckChild1Integer, 0,
-// CHECK: OPC_EmitIntegerI32, sub0_sub1,
+// CHECK: OPC_EmitStringIntegerI32, sub0_sub1,
 def : Pat<(v2i32 (extract_subvector v32i32:$src, (i32 0))),
           (EXTRACT_SUBREG GPR_1024:$src, sub0_sub1)>;
 
-// CHECK: OPC_CheckChild1Integer, 15,
-// CHECK: OPC_EmitIntegerI32, 5|128,1/*133*/,
+// CHECK: OPC_CheckChild1Integer, 30,
+// CHECK: OPC_EmitIntegerI32, 10|128,2/*266*/,
 def : Pat<(v2i32 (extract_subvector v32i32:$src, (i32 15))),
           (EXTRACT_SUBREG GPR_1024:$src, sub30_sub31)>;

diff  --git a/llvm/utils/TableGen/Common/DAGISelMatcher.h b/llvm/utils/TableGen/Common/DAGISelMatcher.h
index 093da829b8966..e947dac3e6482 100644
--- a/llvm/utils/TableGen/Common/DAGISelMatcher.h
+++ b/llvm/utils/TableGen/Common/DAGISelMatcher.h
@@ -857,20 +857,16 @@ class EmitIntegerMatcher : public Matcher {
 /// EmitStringIntegerMatcher - A target constant whose value is represented
 /// by a string.
 class EmitStringIntegerMatcher : public Matcher {
-  std::string Str;
-  int64_t Val;
+  std::string Val;
   MVT VT;
 
   unsigned ResultNo;
 
 public:
-  EmitStringIntegerMatcher(const std::string &str, int64_t val, MVT vt,
-                           unsigned resultNo)
-      : Matcher(EmitStringInteger), Str(str), Val(val), VT(vt),
-        ResultNo(resultNo) {}
+  EmitStringIntegerMatcher(const std::string &val, MVT vt, unsigned resultNo)
+      : Matcher(EmitStringInteger), Val(val), VT(vt), ResultNo(resultNo) {}
 
-  const std::string &getString() const { return Str; }
-  int64_t getValue() const { return Val; }
+  const std::string &getValue() const { return Val; }
   MVT getVT() const { return VT; }
   unsigned getResultNo() const { return ResultNo; }
 

diff  --git a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
index a224dca5e5b91..1c809db2986a6 100644
--- a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
@@ -22,7 +22,6 @@
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Format.h"
-#include "llvm/Support/LEB128.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
@@ -259,18 +258,13 @@ static unsigned EmitVBRValue(uint64_t Val, raw_ostream &OS) {
 /// Emit the specified signed value as a VBR. To improve compression we encode
 /// positive numbers shifted left by 1 and negative numbers negated and shifted
 /// left by 1 with bit 0 set.
-static unsigned EmitSignedVBRValue(int64_t Val, raw_ostream &OS) {
-  uint8_t Buffer[10];
-  unsigned Len = encodeSLEB128(Val, Buffer);
-
-  for (unsigned i = 0; i != Len - 1; ++i)
-    OS << static_cast<unsigned>(Buffer[i] & 127) << "|128,";
-
-  OS << static_cast<unsigned>(Buffer[Len - 1]);
-  if ((Len > 1 || Val < 0) && !OmitComments)
-    OS << "/*" << Val << "*/";
-  OS << ", ";
-  return Len;
+static unsigned EmitSignedVBRValue(uint64_t Val, raw_ostream &OS) {
+  if ((int64_t)Val >= 0)
+    Val = Val << 1;
+  else
+    Val = (-Val << 1) | 1;
+
+  return EmitVBRValue(Val, OS);
 }
 
 // This is expensive and slow.
@@ -818,33 +812,27 @@ unsigned MatcherTableEmitter::EmitMatcher(const Matcher *N,
     return Bytes;
   }
   case Matcher::EmitStringInteger: {
-    const auto *SIM = cast<EmitStringIntegerMatcher>(N);
-    int64_t Val = SIM->getValue();
-    const std::string &Str = SIM->getString();
-    MVT VT = SIM->getVT();
-    unsigned TypeBytes = 0;
+    const std::string &Val = cast<EmitStringIntegerMatcher>(N)->getValue();
+    MVT VT = cast<EmitStringIntegerMatcher>(N)->getVT();
+    // These should always fit into 7 bits.
+    unsigned OpBytes;
     switch (VT.SimpleTy) {
     case MVT::i32:
-      OS << "OPC_EmitIntegerI" << VT.getSizeInBits() << ", ";
+      OpBytes = 1;
+      OS << "OPC_EmitStringIntegerI" << VT.getSizeInBits() << ", ";
       break;
     default:
-      OS << "OPC_EmitInteger, ";
+      OS << "OPC_EmitStringInteger, ";
       if (!OmitComments)
         OS << "/*" << getEnumName(VT) << "*/";
-      TypeBytes = EmitVBRValue(VT.SimpleTy, OS) + 1;
+      OpBytes = EmitVBRValue(VT.SimpleTy, OS) + 1;
       break;
     }
-    // If the value is 63 or smaller, use the string directly. Otherwise, use
-    // a VBR.
-    unsigned ValBytes = 1;
-    if (Val <= 63)
-      OS << Str << ',';
-    else
-      ValBytes = EmitSignedVBRValue(Val, OS);
+    OS << Val << ',';
     if (!OmitComments)
-      OS << " // #" << SIM->getResultNo() << " = " << Str;
+      OS << " // #" << cast<EmitStringIntegerMatcher>(N)->getResultNo();
     OS << '\n';
-    return 1 + TypeBytes + ValBytes;
+    return OpBytes + 1;
   }
 
   case Matcher::EmitRegister: {

diff  --git a/llvm/utils/TableGen/DAGISelMatcherGen.cpp b/llvm/utils/TableGen/DAGISelMatcherGen.cpp
index 264fd063d2e29..d1c63d787de71 100644
--- a/llvm/utils/TableGen/DAGISelMatcherGen.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherGen.cpp
@@ -705,9 +705,14 @@ void MatcherGen::EmitResultLeafAsOperand(const TreePatternNode &N,
       // StringIntegerMatcher. In this case, fallback to using IntegerMatcher.
       const CodeGenRegisterClass &RC =
           CGP.getTargetInfo().getRegisterClass(Def);
-      std::string Name = RC.getQualifiedIdName();
-      AddMatcher(new EmitStringIntegerMatcher(Name, RC.EnumValue, MVT::i32,
-                                              NextRecordedOperandNo));
+      if (RC.EnumValue <= 127) {
+        std::string Value = RC.getQualifiedIdName();
+        AddMatcher(new EmitStringIntegerMatcher(Value, MVT::i32,
+                                                NextRecordedOperandNo));
+      } else {
+        AddMatcher(new EmitIntegerMatcher(RC.EnumValue, MVT::i32,
+                                          NextRecordedOperandNo));
+      }
       ResultOps.push_back(NextRecordedOperandNo++);
       return;
     }
@@ -715,10 +720,20 @@ void MatcherGen::EmitResultLeafAsOperand(const TreePatternNode &N,
     // Handle a subregister index. This is used for INSERT_SUBREG etc.
     if (Def->isSubClassOf("SubRegIndex")) {
       const CodeGenRegBank &RB = CGP.getTargetInfo().getRegBank();
-      const CodeGenSubRegIndex *I = RB.findSubRegIdx(Def);
-      std::string Name = getQualifiedName(Def);
-      AddMatcher(new EmitStringIntegerMatcher(Name, I->EnumValue, MVT::i32,
-                                              NextRecordedOperandNo));
+      // If we have more than 127 subreg indices the encoding can overflow
+      // 7 bit and we cannot use StringInteger.
+      if (RB.getSubRegIndices().size() > 127) {
+        const CodeGenSubRegIndex *I = RB.findSubRegIdx(Def);
+        if (I->EnumValue > 127) {
+          AddMatcher(new EmitIntegerMatcher(I->EnumValue, MVT::i32,
+                                            NextRecordedOperandNo));
+          ResultOps.push_back(NextRecordedOperandNo++);
+          return;
+        }
+      }
+      std::string Value = getQualifiedName(Def);
+      AddMatcher(
+          new EmitStringIntegerMatcher(Value, MVT::i32, NextRecordedOperandNo));
       ResultOps.push_back(NextRecordedOperandNo++);
       return;
     }


        


More information about the llvm-commits mailing list