[llvm] f8d044b - [TBLGEN] Fix subreg value overflow in DAGISelMatcher
Stanislav Mekhanoshin via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 12 13:30:10 PST 2020
Author: Stanislav Mekhanoshin
Date: 2020-02-12T13:29:57-08:00
New Revision: f8d044bbcfdc9e1ddc02247ffb86fe39e1f277f0
URL: https://github.com/llvm/llvm-project/commit/f8d044bbcfdc9e1ddc02247ffb86fe39e1f277f0
DIFF: https://github.com/llvm/llvm-project/commit/f8d044bbcfdc9e1ddc02247ffb86fe39e1f277f0.diff
LOG: [TBLGEN] Fix subreg value overflow in DAGISelMatcher
Tablegen's DAGISelMatcher emits integers in a VBR format,
so if an integer is below 128 it can fit into a single
byte, otherwise high bit is set, next byte is used etc.
MatcherTable is essentially an unsigned char table. When
SelectionDAGISel parses the table it does a reverse translation.
In a situation when numeric value of an integer to emit is
unknown it can be emitted not as OPC_EmitInteger but as
OPC_EmitStringInteger using a symbolic name of the value.
In this situation the value should not exceed 127.
One of the situations when OPC_EmitStringInteger is used is
if we need to emit a subreg into a matcher table. However,
number of subregs can exceed 127. Currently last defined subreg
for AMDGPU is 192. That results in a silent bug in the ISel
with matcher reading from an invalid offset.
Fixed this bug to emit actual VBR encoded value for a subregs
which value exceeds 127.
Differential Revision: https://reviews.llvm.org/D74368
Added:
llvm/test/TableGen/Common/reg-with-subregs-common.td
llvm/test/TableGen/dag-isel-subregs.td
Modified:
llvm/utils/TableGen/CodeGenRegisters.cpp
llvm/utils/TableGen/CodeGenRegisters.h
llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
llvm/utils/TableGen/DAGISelMatcherGen.cpp
llvm/utils/TableGen/RegisterInfoEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/test/TableGen/Common/reg-with-subregs-common.td b/llvm/test/TableGen/Common/reg-with-subregs-common.td
new file mode 100644
index 000000000000..eb73518e9972
--- /dev/null
+++ b/llvm/test/TableGen/Common/reg-with-subregs-common.td
@@ -0,0 +1,128 @@
+include "llvm/Target/Target.td"
+
+def TestTargetInstrInfo : InstrInfo;
+
+def TestTarget : Target {
+ let InstructionSet = TestTargetInstrInfo;
+}
+
+class Indexes<int N> {
+ list<int> all = [0, 1, 2, 3, 4, 5, 6 , 7,
+ 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23,
+ 24, 25, 26, 27, 28, 29, 30, 31];
+ list<int> slice =
+ !foldl([]<int>, all, acc, cur,
+ !listconcat(acc, !if(!lt(cur, N), [cur], [])));
+}
+
+foreach Index = 0-31 in {
+ def sub#Index : SubRegIndex<32, !shl(Index, 5)>;
+}
+
+foreach Size = {2,4,8,16} in {
+ foreach Index = Indexes<!add(33, !mul(Size, -1))>.slice in {
+ def !foldl("", Indexes<Size>.slice, acc, cur,
+ !strconcat(acc#!if(!eq(acc,""),"","_"), "sub"#!add(cur, Index))) :
+ SubRegIndex<!mul(Size, 32), !shl(Index, 5)> {
+ let CoveringSubRegIndices =
+ !foldl([]<SubRegIndex>, Indexes<Size>.slice, acc, cur,
+ !listconcat(acc, [!cast<SubRegIndex>(sub#!add(cur, Index))]));
+ }
+ }
+}
+
+foreach Index = 0-255 in {
+ def R#Index : Register <"r"#Index>;
+}
+
+def GPR32 : RegisterClass<"TestTarget", [i32], 32,
+ (add (sequence "R%u", 0, 255))>;
+
+def GPR64 : RegisterTuples<[sub0, sub1],
+ [(decimate (shl GPR32, 0), 1),
+ (decimate (shl GPR32, 1), 1)
+ ]>;
+
+def GPR128 : RegisterTuples<[sub0, sub1, sub2, sub3],
+ [
+ (decimate (shl GPR32, 0), 1),
+ (decimate (shl GPR32, 1), 1),
+ (decimate (shl GPR32, 2), 1),
+ (decimate (shl GPR32, 3), 1)
+ ]>;
+
+def GPR256 : RegisterTuples<[sub0, sub1, sub2, sub3, sub4, sub5, sub6, sub7],
+ [
+ (decimate (shl GPR32, 0), 1),
+ (decimate (shl GPR32, 1), 1),
+ (decimate (shl GPR32, 2), 1),
+ (decimate (shl GPR32, 3), 1),
+ (decimate (shl GPR32, 4), 1),
+ (decimate (shl GPR32, 5), 1),
+ (decimate (shl GPR32, 6), 1),
+ (decimate (shl GPR32, 7), 1)
+ ]>;
+
+def GPR512 : RegisterTuples<[sub0, sub1, sub2, sub3, sub4, sub5, sub6, sub7,
+ sub8, sub9, sub10, sub11, sub12, sub13, sub14, sub15],
+ [
+ (decimate (shl GPR32, 0), 1),
+ (decimate (shl GPR32, 1), 1),
+ (decimate (shl GPR32, 2), 1),
+ (decimate (shl GPR32, 3), 1),
+ (decimate (shl GPR32, 4), 1),
+ (decimate (shl GPR32, 5), 1),
+ (decimate (shl GPR32, 6), 1),
+ (decimate (shl GPR32, 7), 1),
+ (decimate (shl GPR32, 8), 1),
+ (decimate (shl GPR32, 9), 1),
+ (decimate (shl GPR32, 10), 1),
+ (decimate (shl GPR32, 11), 1),
+ (decimate (shl GPR32, 12), 1),
+ (decimate (shl GPR32, 13), 1),
+ (decimate (shl GPR32, 14), 1),
+ (decimate (shl GPR32, 15), 1)
+ ]>;
+
+def GPR1024 : RegisterTuples<[sub0, sub1, sub2, sub3, sub4, sub5, sub6, sub7,
+ sub8, sub9, sub10, sub11, sub12, sub13, sub14, sub15,
+ sub16, sub17, sub18, sub19, sub20, sub21, sub22, sub23,
+ sub24, sub25, sub26, sub27, sub28, sub29, sub30, sub31],
+ [
+ (decimate (shl GPR32, 0), 1),
+ (decimate (shl GPR32, 1), 1),
+ (decimate (shl GPR32, 2), 1),
+ (decimate (shl GPR32, 3), 1),
+ (decimate (shl GPR32, 4), 1),
+ (decimate (shl GPR32, 5), 1),
+ (decimate (shl GPR32, 6), 1),
+ (decimate (shl GPR32, 7), 1),
+ (decimate (shl GPR32, 8), 1),
+ (decimate (shl GPR32, 9), 1),
+ (decimate (shl GPR32, 10), 1),
+ (decimate (shl GPR32, 11), 1),
+ (decimate (shl GPR32, 12), 1),
+ (decimate (shl GPR32, 13), 1),
+ (decimate (shl GPR32, 14), 1),
+ (decimate (shl GPR32, 15), 1),
+ (decimate (shl GPR32, 16), 1),
+ (decimate (shl GPR32, 17), 1),
+ (decimate (shl GPR32, 18), 1),
+ (decimate (shl GPR32, 19), 1),
+ (decimate (shl GPR32, 20), 1),
+ (decimate (shl GPR32, 21), 1),
+ (decimate (shl GPR32, 22), 1),
+ (decimate (shl GPR32, 23), 1),
+ (decimate (shl GPR32, 24), 1),
+ (decimate (shl GPR32, 25), 1),
+ (decimate (shl GPR32, 26), 1),
+ (decimate (shl GPR32, 27), 1),
+ (decimate (shl GPR32, 28), 1),
+ (decimate (shl GPR32, 29), 1),
+ (decimate (shl GPR32, 30), 1),
+ (decimate (shl GPR32, 31), 1)
+ ]>;
+
+def GPR_64 : RegisterClass<"", [v2i32], 64, (add GPR64)>;
+def GPR_1024 : RegisterClass<"", [v32i32], 1024, (add GPR1024)>;
diff --git a/llvm/test/TableGen/dag-isel-subregs.td b/llvm/test/TableGen/dag-isel-subregs.td
new file mode 100644
index 000000000000..963e5fc7a95c
--- /dev/null
+++ b/llvm/test/TableGen/dag-isel-subregs.td
@@ -0,0 +1,14 @@
+// RUN: llvm-tblgen -gen-dag-isel -I %p/../../include -I %p/Common %s | FileCheck %s
+
+include "reg-with-subregs-common.td"
+
+// CHECK-LABEL: OPC_CheckOpcode, TARGET_VAL(ISD::EXTRACT_SUBVECTOR),
+// CHECK: OPC_CheckChild1Integer, 0,
+// CHECK: OPC_EmitInteger, MVT::i32, sub0_sub1,
+def : Pat<(v2i32 (extract_subvector v32i32:$src, (i32 0))),
+ (EXTRACT_SUBREG GPR_1024:$src, sub0_sub1)>;
+
+// CHECK: OPC_CheckChild1Integer, 15,
+// CHECK: OPC_EmitInteger, MVT::i32, 5|128,1/*133*/,
+def : Pat<(v2i32 (extract_subvector v32i32:$src, (i32 15))),
+ (EXTRACT_SUBREG GPR_1024:$src, sub30_sub31)>;
diff --git a/llvm/utils/TableGen/CodeGenRegisters.cpp b/llvm/utils/TableGen/CodeGenRegisters.cpp
index 66a37aae00eb..dd70e2ff6896 100644
--- a/llvm/utils/TableGen/CodeGenRegisters.cpp
+++ b/llvm/utils/TableGen/CodeGenRegisters.cpp
@@ -1220,6 +1220,12 @@ CodeGenSubRegIndex *CodeGenRegBank::getSubRegIdx(Record *Def) {
return Idx;
}
+const CodeGenSubRegIndex *
+CodeGenRegBank::findSubRegIdx(const Record* Def) const {
+ auto I = Def2SubRegIdx.find(Def);
+ return (I == Def2SubRegIdx.end()) ? nullptr : I->second;
+}
+
CodeGenRegister *CodeGenRegBank::getReg(Record *Def) {
CodeGenRegister *&Reg = Def2Reg[Def];
if (Reg)
diff --git a/llvm/utils/TableGen/CodeGenRegisters.h b/llvm/utils/TableGen/CodeGenRegisters.h
index b423da0f3a92..8d790321a002 100644
--- a/llvm/utils/TableGen/CodeGenRegisters.h
+++ b/llvm/utils/TableGen/CodeGenRegisters.h
@@ -626,9 +626,13 @@ namespace llvm {
return SubRegIndices;
}
- // Find a SubRegIndex form its Record def.
+ // Find a SubRegIndex from its Record def or add to the list if it does
+ // not exist there yet.
CodeGenSubRegIndex *getSubRegIdx(Record*);
+ // Find a SubRegIndex from its Record def.
+ const CodeGenSubRegIndex *findSubRegIdx(const Record* Def) const;
+
// Find or create a sub-register index representing the A+B composition.
CodeGenSubRegIndex *getCompositeSubRegIndex(CodeGenSubRegIndex *A,
CodeGenSubRegIndex *B);
diff --git a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
index 60f501cbfebe..d741953c4cad 100644
--- a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
@@ -619,7 +619,7 @@ EmitMatcher(const Matcher *N, unsigned Indent, unsigned CurrentIdx,
}
case Matcher::EmitStringInteger: {
const std::string &Val = cast<EmitStringIntegerMatcher>(N)->getValue();
- // These should always fit into one byte.
+ // These should always fit into 7 bits.
OS << "OPC_EmitInteger, "
<< getEnumName(cast<EmitStringIntegerMatcher>(N)->getVT()) << ", "
<< Val << ",\n";
diff --git a/llvm/utils/TableGen/DAGISelMatcherGen.cpp b/llvm/utils/TableGen/DAGISelMatcherGen.cpp
index cce9383049f2..8db9aa6bf8a3 100644
--- a/llvm/utils/TableGen/DAGISelMatcherGen.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherGen.cpp
@@ -715,6 +715,18 @@ 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();
+ // 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);
+ assert(I && "Cannot find subreg index by name!");
+ if (I->EnumValue > 127) {
+ AddMatcher(new EmitIntegerMatcher(I->EnumValue, MVT::i32));
+ ResultOps.push_back(NextRecordedOperandNo++);
+ return;
+ }
+ }
std::string Value = getQualifiedName(Def);
AddMatcher(new EmitStringIntegerMatcher(Value, MVT::i32));
ResultOps.push_back(NextRecordedOperandNo++);
diff --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index 944c97de386a..472a3de47816 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -173,7 +173,7 @@ void RegisterInfoEmitter::runEnums(raw_ostream &OS,
std::string Namespace = SubRegIndices.front().getNamespace();
if (!Namespace.empty())
OS << "namespace " << Namespace << " {\n";
- OS << "enum {\n NoSubRegister,\n";
+ OS << "enum : uint16_t {\n NoSubRegister,\n";
unsigned i = 0;
for (const auto &Idx : SubRegIndices)
OS << " " << Idx.getName() << ",\t// " << ++i << "\n";
More information about the llvm-commits
mailing list