[llvm] r310922 - Revert r310919 - [globalisel][tablegen] Support zero-instruction emission.

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 15 08:10:31 PDT 2017


Author: dsanders
Date: Tue Aug 15 08:10:31 2017
New Revision: 310922

URL: http://llvm.org/viewvc/llvm-project?rev=310922&view=rev
Log:
Revert r310919 - [globalisel][tablegen] Support zero-instruction emission.

As expected, this failed on the windows bots but the instrumentation showed
something interesting. The ADD8ri and INC8r rules are never directly compared
on the windows machines. That implies that the issue lies in transitivity of
the Compare predicate. I believe I've already verified that but maybe I missed
something.


Modified:
    llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-bitcast.mir
    llvm/trunk/test/TableGen/GlobalISelEmitter.td
    llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp

Modified: llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp?rev=310922&r1=310921&r2=310922&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp Tue Aug 15 08:10:31 2017
@@ -1163,18 +1163,8 @@ bool AArch64InstructionSelector::select(
 
 
   case TargetOpcode::G_INTTOPTR:
-    // The importer is currently unable to import pointer types since they
-    // didn't exist in SelectionDAG.
-    return selectCopy(I, TII, MRI, TRI, RBI);
-
   case TargetOpcode::G_BITCAST:
-    // Imported SelectionDAG rules can handle every bitcast except those that
-    // bitcast from a type to the same type. Ideally, these shouldn't occur
-    // but we might not run an optimizer that deletes them.
-    if (MRI.getType(I.getOperand(0).getReg()) ==
-        MRI.getType(I.getOperand(1).getReg()))
-      return selectCopy(I, TII, MRI, TRI, RBI);
-    return false;
+    return selectCopy(I, TII, MRI, TRI, RBI);
 
   case TargetOpcode::G_FPEXT: {
     if (MRI.getType(I.getOperand(0).getReg()) != LLT::scalar(64)) {

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-bitcast.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-bitcast.mir?rev=310922&r1=310921&r2=310922&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-bitcast.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/select-bitcast.mir Tue Aug 15 08:10:31 2017
@@ -11,8 +11,6 @@
   define void @bitcast_s64_fpr() { ret void }
   define void @bitcast_s64_gpr_fpr() { ret void }
   define void @bitcast_s64_fpr_gpr() { ret void }
-  define void @bitcast_s64_v2f32_fpr() { ret void }
-  define void @bitcast_s64_v8i8_fpr() { ret void }
 ...
 
 ---
@@ -212,53 +210,3 @@ body:             |
     %1(s64) = G_BITCAST %0
     %x0 = COPY %1(s64)
 ...
-
----
-# CHECK-LABEL: name: bitcast_s64_v2f32_fpr
-name:            bitcast_s64_v2f32_fpr
-legalized:       true
-regBankSelected: true
-
-# CHECK:      registers:
-# CHECK-NEXT:  - { id: 0, class: fpr64, preferred-register: '' }
-# CHECK-NEXT:  - { id: 1, class: fpr64, preferred-register: '' }
-registers:
-  - { id: 0, class: fpr }
-  - { id: 1, class: fpr }
-
-# CHECK:  body:
-# CHECK:    %0 = COPY %d0
-# CHECK:    %1 = COPY %0
-body:             |
-  bb.0:
-    liveins: %d0
-
-    %0(s64) = COPY %d0
-    %1(<2 x s32>) = G_BITCAST %0
-    %x0 = COPY %1(<2 x s32>)
-...
-
----
-# CHECK-LABEL: name: bitcast_s64_v8i8_fpr
-name:            bitcast_s64_v8i8_fpr
-legalized:       true
-regBankSelected: true
-
-# CHECK:      registers:
-# CHECK-NEXT:  - { id: 0, class: fpr64, preferred-register: '' }
-# CHECK-NEXT:  - { id: 1, class: fpr64, preferred-register: '' }
-registers:
-  - { id: 0, class: fpr }
-  - { id: 1, class: fpr }
-
-# CHECK:  body:
-# CHECK:    %0 = COPY %d0
-# CHECK:    %1 = COPY %0
-body:             |
-  bb.0:
-    liveins: %d0
-
-    %0(s64) = COPY %d0
-    %1(<8 x s8>) = G_BITCAST %0
-    %x0 = COPY %1(<8 x s8>)
-...

Modified: llvm/trunk/test/TableGen/GlobalISelEmitter.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=310922&r1=310921&r2=310922&view=diff
==============================================================================
--- llvm/trunk/test/TableGen/GlobalISelEmitter.td (original)
+++ llvm/trunk/test/TableGen/GlobalISelEmitter.td Tue Aug 15 08:10:31 2017
@@ -83,13 +83,6 @@ def HasC : Predicate<"Subtarget->hasC()"
 // CHECK-NEXT:    return Features;
 // CHECK-NEXT:  }
 
-// CHECK-LABEL: enum {
-// CHECK-NEXT:    GILLT_s32,
-// CHECK-NEXT:  }
-// CHECK-NEXT:  const static LLT TypeObjects[] = {
-// CHECK-NEXT:    LLT::scalar(32),
-// CHECK-NEXT:  };
-
 // CHECK: bool MyTargetInstructionSelector::selectImpl(MachineInstr &I) const {
 // CHECK-NEXT: MachineFunction &MF = *I.getParent()->getParent();
 // CHECK-NEXT: MachineRegisterInfo &MRI = MF.getRegInfo();

Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=310922&r1=310921&r2=310922&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Tue Aug 15 08:10:31 2017
@@ -110,28 +110,30 @@ public:
   const LLT &get() const { return Ty; }
 
   /// This ordering is used for std::unique() and std::sort(). There's no
-  /// particular logic behind the order but either A < B or B < A must be
-  /// true if A != B.
+  /// particular logic behind the order.
   bool operator<(const LLTCodeGen &Other) const {
-    if (Ty.isValid() != Other.Ty.isValid())
-      return Ty.isValid() < Other.Ty.isValid();
     if (!Ty.isValid())
+      return Other.Ty.isValid();
+    if (Ty.isScalar()) {
+      if (!Other.Ty.isValid())
+        return false;
+      if (Other.Ty.isScalar())
+        return Ty.getSizeInBits() < Other.Ty.getSizeInBits();
       return false;
-
-    if (Ty.isVector() != Other.Ty.isVector())
-      return Ty.isVector() < Other.Ty.isVector();
-    if (Ty.isScalar() != Other.Ty.isScalar())
-      return Ty.isScalar() < Other.Ty.isScalar();
-    if (Ty.isPointer() != Other.Ty.isPointer())
-      return Ty.isPointer() < Other.Ty.isPointer();
-
-    if (Ty.isPointer() && Ty.getAddressSpace() != Other.Ty.getAddressSpace())
-      return Ty.getAddressSpace() < Other.Ty.getAddressSpace();
-
-    if (Ty.isVector() && Ty.getNumElements() != Other.Ty.getNumElements())
-      return Ty.getNumElements() < Other.Ty.getNumElements();
-
-    return Ty.getSizeInBits() < Other.Ty.getSizeInBits();
+    }
+    if (Ty.isVector()) {
+      if (!Other.Ty.isValid() || Other.Ty.isScalar())
+        return false;
+      if (Other.Ty.isVector()) {
+        if (Ty.getNumElements() < Other.Ty.getNumElements())
+          return true;
+        if (Ty.getNumElements() > Other.Ty.getNumElements())
+          return false;
+        return Ty.getSizeInBits() < Other.Ty.getSizeInBits();
+      }
+      return false;
+    }
+    llvm_unreachable("Unhandled LLT");
   }
 };
 
@@ -180,6 +182,14 @@ static Error failedImport(const Twine &R
 static Error isTrivialOperatorNode(const TreePatternNode *N) {
   std::string Explanation = "";
   std::string Separator = "";
+  if (N->isLeaf()) {
+    if (isa<IntInit>(N->getLeafValue()))
+      return Error::success();
+
+    Explanation = "Is a leaf";
+    Separator = ", ";
+  }
+
   if (N->hasAnyPredicate()) {
     Explanation = Separator + "Has a predicate (" + explainPredicates(N) + ")";
     Separator = ", ";
@@ -190,7 +200,7 @@ static Error isTrivialOperatorNode(const
     Separator = ", ";
   }
 
-  if (!N->hasAnyPredicate() && !N->getTransformFn())
+  if (!N->isLeaf() && !N->hasAnyPredicate() && !N->getTransformFn())
     return Error::success();
 
   return failedImport(Explanation);
@@ -448,9 +458,7 @@ class RuleMatcher {
 
   /// A list of actions that need to be taken when all predicates in this rule
   /// have succeeded.
-public:
   std::vector<std::unique_ptr<MatchAction>> Actions;
-protected:
 
   typedef std::map<const InstructionMatcher *, unsigned>
       DefinedInsnVariablesMap;
@@ -626,12 +634,8 @@ protected:
   LLTCodeGen Ty;
 
 public:
-  static std::set<LLTCodeGen> KnownTypes;
-
   LLTOperandMatcher(const LLTCodeGen &Ty)
-      : OperandPredicateMatcher(OPM_LLT), Ty(Ty) {
-    KnownTypes.insert(Ty);
-  }
+      : OperandPredicateMatcher(OPM_LLT), Ty(Ty) {}
 
   static bool classof(const OperandPredicateMatcher *P) {
     return P->getKind() == OPM_LLT;
@@ -647,8 +651,6 @@ public:
   }
 };
 
-std::set<LLTCodeGen> LLTOperandMatcher::KnownTypes;
-
 /// Generates code to check that an operand is a particular target constant.
 class ComplexPatternOperandMatcher : public OperandPredicateMatcher {
 protected:
@@ -1434,9 +1436,7 @@ public:
 class BuildMIAction : public MatchAction {
 private:
   unsigned InsnID;
-public:
   const CodeGenInstruction *I;
-private:
   const InstructionMatcher &Matched;
   std::vector<std::unique_ptr<OperandRenderer>> OperandRenderers;
 
@@ -2287,42 +2287,8 @@ Expected<RuleMatcher> GlobalISelEmitter:
     return failedImport("Src pattern root isn't a trivial operator (" +
                         toString(std::move(Err)) + ")");
 
-  InstructionMatcher &InsnMatcherTemp = M.addInstructionMatcher(Src->getName());
-  unsigned TempOpIdx = 0;
-  auto InsnMatcherOrError =
-      createAndImportSelDAGMatcher(InsnMatcherTemp, Src, TempOpIdx);
-  if (auto Error = InsnMatcherOrError.takeError())
-    return std::move(Error);
-  InstructionMatcher &InsnMatcher = InsnMatcherOrError.get();
-
-  if (Dst->isLeaf()) {
-    Record *RCDef = getInitValueAsRegClass(Dst->getLeafValue());
-
-    const CodeGenRegisterClass &RC = Target.getRegisterClass(RCDef);
-    if (RCDef) {
-      // We need to replace the def and all its uses with the specified
-      // operand. However, we must also insert COPY's wherever needed.
-      // For now, emit a copy and let the register allocator clean up.
-      auto &DstI = Target.getInstruction(RK.getDef("COPY"));
-      const auto &DstIOperand = DstI.Operands[0];
-
-      OperandMatcher &OM0 = InsnMatcher.getOperand(0);
-      OM0.setSymbolicName(DstIOperand.Name);
-      OM0.addPredicate<RegisterBankOperandMatcher>(RC);
-
-      auto &DstMIBuilder = M.addAction<BuildMIAction>(0, &DstI, InsnMatcher);
-      DstMIBuilder.addRenderer<CopyRenderer>(0, InsnMatcher, DstIOperand.Name);
-      DstMIBuilder.addRenderer<CopyRenderer>(0, InsnMatcher, Dst->getName());
-      M.addAction<ConstrainOperandToRegClassAction>(0, 0, RC);
-
-      // We're done with this pattern!  It's eligible for GISel emission; return
-      // it.
-      ++NumPatternImported;
-      return std::move(M);
-    }
-
+  if (Dst->isLeaf())
     return failedImport("Dst pattern root isn't a known leaf");
-  }
 
   // Start with the defined operands (i.e., the results of the root operator).
   Record *DstOp = Dst->getOperator();
@@ -2335,6 +2301,14 @@ Expected<RuleMatcher> GlobalISelEmitter:
                         to_string(Src->getExtTypes().size()) + " def(s) vs " +
                         to_string(DstI.Operands.NumDefs) + " def(s))");
 
+  InstructionMatcher &InsnMatcherTemp = M.addInstructionMatcher(Src->getName());
+  unsigned TempOpIdx = 0;
+  auto InsnMatcherOrError =
+      createAndImportSelDAGMatcher(InsnMatcherTemp, Src, TempOpIdx);
+  if (auto Error = InsnMatcherOrError.takeError())
+    return std::move(Error);
+  InstructionMatcher &InsnMatcher = InsnMatcherOrError.get();
+
   // The root of the match also has constraints on the register bank so that it
   // matches the result instruction.
   unsigned OpIdx = 0;
@@ -2482,37 +2456,16 @@ void GlobalISelEmitter::run(raw_ostream
     Rules.push_back(std::move(MatcherOrErr.get()));
   }
 
-  const auto &IsRuleEmittingOpcode = [](const RuleMatcher &A, StringRef I) -> bool {
-    const BuildMIAction &BMI = (const BuildMIAction &)*A.Actions[1];
-    if (BMI.I->TheDef->getName() == I)
-      return true;
-    return false;
-  };
-
-  std::stable_sort(Rules.begin(), Rules.end(), [&](const RuleMatcher &A,
-                                                   const RuleMatcher &B) {
-    bool EmitResult = false;
-    bool AIsADD8ri = false;
-    if (IsRuleEmittingOpcode(A, "ADD8ri") && IsRuleEmittingOpcode(B, "INC8r")) {
-      EmitResult = true;
-      AIsADD8ri = true;
-    }
-    if (IsRuleEmittingOpcode(B, "ADD8ri") && IsRuleEmittingOpcode(A, "INC8r"))
-      EmitResult = true;
-
-    if (EmitResult)
-      errs() << "A=" << (AIsADD8ri ? "ADD8ri" : "INC8r") << ", B=" << (!AIsADD8ri ? "ADD8ri" : "INC8r") << "\n";
-    if (A.isHigherPriorityThan(B)) {
-      if (EmitResult)
-        errs() << "A higher priority than B\n";
-      assert(!B.isHigherPriorityThan(A) && "Cannot be more important "
-                                           "and less important at "
-                                           "the same time");
-      return true;
-    } else if (EmitResult)
-      errs() << "A lower priority than B\n";
-    return false;
-  });
+  std::stable_sort(Rules.begin(), Rules.end(),
+            [&](const RuleMatcher &A, const RuleMatcher &B) {
+              if (A.isHigherPriorityThan(B)) {
+                assert(!B.isHigherPriorityThan(A) && "Cannot be more important "
+                                                     "and less important at "
+                                                     "the same time");
+                return true;
+              }
+              return false;
+            });
 
   std::vector<Record *> ComplexPredicates =
       RK.getAllDerivedDefinitions("GIComplexOperandMatcher");
@@ -2582,9 +2535,16 @@ void GlobalISelEmitter::run(raw_ostream
 
   // Emit a table containing the LLT objects needed by the matcher and an enum
   // for the matcher to reference them with.
-  std::vector<LLTCodeGen> TypeObjects;
-  for (const auto &Ty : LLTOperandMatcher::KnownTypes)
-    TypeObjects.push_back(Ty);
+  std::vector<LLTCodeGen> TypeObjects = {
+      LLT::scalar(8),      LLT::scalar(16),     LLT::scalar(32),
+      LLT::scalar(64),     LLT::scalar(80),     LLT::vector(8, 1),
+      LLT::vector(16, 1),  LLT::vector(32, 1),  LLT::vector(64, 1),
+      LLT::vector(8, 8),   LLT::vector(16, 8),  LLT::vector(32, 8),
+      LLT::vector(64, 8),  LLT::vector(4, 16),  LLT::vector(8, 16),
+      LLT::vector(16, 16), LLT::vector(32, 16), LLT::vector(2, 32),
+      LLT::vector(4, 32),  LLT::vector(8, 32),  LLT::vector(16, 32),
+      LLT::vector(2, 64),  LLT::vector(4, 64),  LLT::vector(8, 64),
+  };
   std::sort(TypeObjects.begin(), TypeObjects.end());
   OS << "enum {\n";
   for (const auto &TypeObject : TypeObjects) {




More information about the llvm-commits mailing list