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

[jrtc27]

Summary

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, provided my existing
patches to fix the use of ComplexPatterns in the AArch64 and AMDGPU
backends are applied.

[kparzysz]

Edit: this comment is about AArch64. I didn't try to see if there are any other compilation errors.

This fails to build because SVDup0 is a complex pattern defined with type i64, but it's used in places that require vectors:

def SVEDup0 : ComplexPattern<i64, 0, "SelectDupZero", []>;
def SVEDup0Undef : ComplexPattern<i64, 0, "SelectDupZeroOrUndef", []>;

and

multiclass SVE_SETCC_Pat_With_Zero<CondCode cc, CondCode invcc, ValueType predvt,
                                   ValueType intvt, Instruction cmp> {
  def : Pat<(predvt (AArch64setcc_z predvt:$Op1, intvt:$Op2, (SVEDup0), cc)),
            (cmp $Op1, $Op2)>;
  def : Pat<(predvt (AArch64setcc_z predvt:$Op1, (SVEDup0), intvt:$Op2, invcc)),
            (cmp $Op1, $Op2)>;
}

Looks good to me otherwise.

[jrtc27]

In D109035#2977420, @kparzysz wrote:

This fails to build because SVDup0 is a complex pattern defined with type i64, but it's used in places that require vectors:

def SVEDup0 : ComplexPattern<i64, 0, "SelectDupZero", []>;
def SVEDup0Undef : ComplexPattern<i64, 0, "SelectDupZeroOrUndef", []>;

and

multiclass SVE_SETCC_Pat_With_Zero<CondCode cc, CondCode invcc, ValueType predvt,
                                   ValueType intvt, Instruction cmp> {
  def : Pat<(predvt (AArch64setcc_z predvt:$Op1, intvt:$Op2, (SVEDup0), cc)),
            (cmp $Op1, $Op2)>;
  def : Pat<(predvt (AArch64setcc_z predvt:$Op1, (SVEDup0), intvt:$Op2, invcc)),
            (cmp $Op1, $Op2)>;
}

Looks good to me otherwise.

Yes, this turned up a bunch of cases where the types were wrong (as you would expect, though fewer than I feared); you'll need all 3 parent revisions listed under Stack for it to both build (the first AArch64 revision) and not crash in various CodeGen tests (the AMDGPU revision and other AArch64 one).

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

+// 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)];
+}

llvm/utils/TableGen/CodeGenDAGPatterns.cpp

if (R->isSubClassOf("CondCode")) {
     // Using a CondCodeSDNode.
     return TypeSetByHwMode(MVT::Other);
   }
 
   if (R->isSubClassOf("ComplexPattern")) {
     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!");
     TypeSetByHwMode VTS(MVT::iPTR);
     TP.getInfer().expandOverloads(VTS);
     return VTS;
   }
 
if (getOperator()->isSubClassOf("Instruction")) {
     for (unsigned i = 0, e = getNumChildren(); i != e; ++i)
       MadeChange |= getChild(i)->ApplyTypeConstraints(TP, NotRegisters);
     return MadeChange;
   }
 
   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);
 
     return MadeChange;
   }
 
   assert(getOperator()->isSubClassOf("SDNodeXForm") && "Unknown node type!");
 

llvm/utils/TableGen/CodeGenTarget.h

 
 private:
   void ComputeInstrsByEnum() const;
 };
 
 /// 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;
   unsigned Properties; // Node properties
   unsigned Complexity;
 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 {
     return RootNodes;
   }
   bool hasProperty(enum SDNP Prop) const { return Properties & (1 << Prop); }
   unsigned getComplexity() const { return Complexity; }
 };
 
 } // End llvm namespace
 
 #endif

llvm/utils/TableGen/CodeGenTarget.cpp

 bool CodeGenTarget::guessInstructionProperties() const {
   return getInstructionSet()->getValueAsBit("guessInstructionProperties");
 }
 
 //===----------------------------------------------------------------------===//
 // 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");
 
   // FIXME: This is a hack to statically increase the priority of patterns which
   // maps a sub-dag to a complex pattern. e.g. favors LEA over ADD. To get best
   // possible pattern match we'll need to dynamically calculate the complexity
   // of all patterns a dag can potentially map to.