[llvm] c1048e3 - [TableGen][SelectionDAG] Use ComplexPattern type for non-leaf nodes

[Jessica Clarke via llvm-commits]

Author: Jessica Clarke
Date: 2021-12-03T07:04:59Z
New Revision: c1048e3eb920958bb48723c5b996f1877554575f

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

LOG: [TableGen][SelectionDAG] Use ComplexPattern type for non-leaf nodes

When used as a non-leaf node, TableGen does not currently use the type
of a ComplexPattern for type inference, which also means it does not
check it doesn't conflict with the use. This differs from when used as a
leaf value, where the type is used for inference. This addresses that
discrepancy. The test case is not representative of most real-world uses
but is sufficient to demonstrate inference is working.

Some of these uses also make use of ValueTypeByHwMode rather than
SimpleValueType and so the existing type inference is extended to
support that alongside the new type inference.

There are also currently various cases of using ComplexPatterns with an
untyped type, but only for non-leaf nodes. For compatibility this is
permitted, and uses the old behaviour of not inferring for non-leaf
nodes, but the existing logic is still used for leaf values. This
remaining discrepancy should eventually be eliminated, either by
removing all such uses of untyped so the special case goes away (I
imagine Any, or a more specific type in certain cases, would be
perfectly sufficient), or by copying it to the leaf value case so
they're consistent with one another if this is something that does need
to keep being supported.

All non-experimental targets have been verified to produce bit-for-bit
identical TableGen output with this change applied.

Reviewed By: kparzysz

Differential Revision: https://reviews.llvm.org/D109035

Added: 
    llvm/test/TableGen/dag-isel-complexpattern.td

Modified: 
    llvm/utils/TableGen/CodeGenDAGPatterns.cpp
    llvm/utils/TableGen/CodeGenTarget.cpp
    llvm/utils/TableGen/CodeGenTarget.h

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/dag-isel-complexpattern.td b/llvm/test/TableGen/dag-isel-complexpattern.td
new file mode 100644
index 000000000000..40fd03cc8839
--- /dev/null
+++ b/llvm/test/TableGen/dag-isel-complexpattern.td
@@ -0,0 +1,30 @@
+// RUN: llvm-tblgen -gen-dag-isel -I %p/../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def TestTargetInstrInfo : InstrInfo;
+
+def TestTarget : Target {
+  let InstructionSet = TestTargetInstrInfo;
+}
+
+def REG32 : Register<"REG32">;
+def REG64 : Register<"REG64">;
+def GPR32 : RegisterClass<"TestTarget", [i32], 32, (add REG32)>;
+def GPR64 : RegisterClass<"TestTarget", [i64], 64, (add REG64)>;
+
+def CP32 : ComplexPattern<i32, 0, "SelectCP32">;
+
+// Without using ComplexPattern's type, this pattern would be ambiguous as to
+// what integer type is being used, since both i32 and i64 are legal, and used
+// to erroneously happen, whilst using a leaf value like CP32:$a/b instead of
+// (CP32) still worked.
+def INSTR : Instruction {
+// CHECK-LABEL: OPC_CheckOpcode, TARGET_VAL(ISD::STORE)
+// CHECK: OPC_CheckType, MVT::i32
+// CHECK: OPC_CheckComplexPat, /*CP*/0, /*#*/1, // SelectCP32:$
+// CHECK: Src: (st (add:{ *:[i32] } (CP32:{ *:[i32] }), (CP32:{ *:[i32] })), i64:{ *:[i64] }:$addr)
+  let OutOperandList = (outs);
+  let InOperandList = (ins GPR64:$addr);
+  let Pattern = [(store (add (CP32), (CP32)), i64:$addr)];
+}

diff  --git a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
index 4a247050ceeb..5710b76bf69a 100644
--- a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
@@ -2268,7 +2268,9 @@ static TypeSetByHwMode getImplicitType(Record *R, unsigned ResNo,
     assert(ResNo == 0 && "FIXME: ComplexPattern with multiple results?");
     if (NotRegisters)
       return TypeSetByHwMode(); // Unknown.
-    return TypeSetByHwMode(CDP.getComplexPattern(R).getValueType());
+    Record *T = CDP.getComplexPattern(R).getValueType();
+    const CodeGenHwModes &CGH = CDP.getTargetInfo().getHwModes();
+    return TypeSetByHwMode(getValueTypeByHwMode(T, CGH));
   }
   if (R->isSubClassOf("PointerLikeRegClass")) {
     assert(ResNo == 0 && "Regclass can only have one result!");
@@ -2673,6 +2675,22 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
   if (getOperator()->isSubClassOf("ComplexPattern")) {
     bool MadeChange = false;
 
+    if (!NotRegisters) {
+      assert(Types.size() == 1 && "ComplexPatterns only produce one result!");
+      Record *T = CDP.getComplexPattern(getOperator()).getValueType();
+      const CodeGenHwModes &CGH = CDP.getTargetInfo().getHwModes();
+      const ValueTypeByHwMode VVT = getValueTypeByHwMode(T, CGH);
+      // TODO: AArch64 and AMDGPU use ComplexPattern<untyped, ...> and then
+      // exclusively use those as non-leaf nodes with explicit type casts, so
+      // for backwards compatibility we do no inference in that case. This is
+      // not supported when the ComplexPattern is used as a leaf value,
+      // however; this inconsistency should be resolved, either by adding this
+      // case there or by altering the backends to not do this (e.g. using Any
+      // instead may work).
+      if (!VVT.isSimple() || VVT.getSimple() != MVT::Untyped)
+        MadeChange |= UpdateNodeType(0, VVT, TP);
+    }
+
     for (unsigned i = 0; i < getNumChildren(); ++i)
       MadeChange |= getChild(i)->ApplyTypeConstraints(TP, NotRegisters);
 

diff  --git a/llvm/utils/TableGen/CodeGenTarget.cpp b/llvm/utils/TableGen/CodeGenTarget.cpp
index d3beaf61989e..2c1583f7979d 100644
--- a/llvm/utils/TableGen/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/CodeGenTarget.cpp
@@ -576,7 +576,7 @@ bool CodeGenTarget::guessInstructionProperties() const {
 // ComplexPattern implementation
 //
 ComplexPattern::ComplexPattern(Record *R) {
-  Ty          = ::getValueType(R->getValueAsDef("Ty"));
+  Ty          = R->getValueAsDef("Ty");
   NumOperands = R->getValueAsInt("NumOperands");
   SelectFunc = std::string(R->getValueAsString("SelectFunc"));
   RootNodes   = R->getValueAsListOfDefs("RootNodes");

diff  --git a/llvm/utils/TableGen/CodeGenTarget.h b/llvm/utils/TableGen/CodeGenTarget.h
index 9de9b512f74f..5bd84c873f2f 100644
--- a/llvm/utils/TableGen/CodeGenTarget.h
+++ b/llvm/utils/TableGen/CodeGenTarget.h
@@ -202,7 +202,7 @@ class CodeGenTarget {
 /// ComplexPattern - ComplexPattern info, corresponding to the ComplexPattern
 /// tablegen class in TargetSelectionDAG.td
 class ComplexPattern {
-  MVT::SimpleValueType Ty;
+  Record *Ty;
   unsigned NumOperands;
   std::string SelectFunc;
   std::vector<Record*> RootNodes;
@@ -211,7 +211,7 @@ class ComplexPattern {
 public:
   ComplexPattern(Record *R);
 
-  MVT::SimpleValueType getValueType() const { return Ty; }
+  Record *getValueType() const { return Ty; }
   unsigned getNumOperands() const { return NumOperands; }
   const std::string &getSelectFunc() const { return SelectFunc; }
   const std::vector<Record*> &getRootNodes() const {