[PATCH] D50977: [TableGen] Prefer user-defined subregister compositions over inferred ones

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz created this revision.
kparzysz added reviewers: MatzeB, craig.topper.

This is to avoid warnings from TableGen when a user-defined subregister index composition conflicts with an inferred one. This is a follow-up to https://reviews.llvm.org/D50725.

The comments in the testcase describe how this situation occurs.


Repository:
  rL LLVM

https://reviews.llvm.org/D50977

Files:
  test/TableGen/ambiguous-composition.td
  utils/TableGen/CodeGenRegisters.cpp


Index: utils/TableGen/CodeGenRegisters.cpp
===================================================================
--- utils/TableGen/CodeGenRegisters.cpp
+++ utils/TableGen/CodeGenRegisters.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/IntEqClasses.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
@@ -1313,6 +1314,13 @@
   // and many registers will share TopoSigs on regular architectures.
   BitVector TopoSigs(getNumTopoSigs());
 
+  using CompositePair = std::pair<const CodeGenSubRegIndex*,
+                                  const CodeGenSubRegIndex*>;
+  SmallSet<CompositePair,4> UserDefined;
+  for (const CodeGenSubRegIndex &Idx : SubRegIndices)
+    for (auto P : Idx.getComposites())
+      UserDefined.insert(std::make_pair(&Idx, P.first));
+
   for (const auto &Reg1 : Registers) {
     // Skip identical subreg structures already processed.
     if (TopoSigs.test(Reg1.getTopoSig()))
@@ -1340,6 +1348,9 @@
         CodeGenSubRegIndex *Idx3 = Reg1.getSubRegIndex(Reg3);
         assert(Idx3 && "Sub-register doesn't have an index");
 
+        // In case of conflicts, give preferences to user-defined compositions.
+        if (UserDefined.count({Idx1, Idx2}))
+          continue;
         // Conflicting composition? Emit a warning but allow it.
         if (CodeGenSubRegIndex *Prev = Idx1->addComposite(Idx2, Idx3))
           PrintWarning(Twine("SubRegIndex ") + Idx1->getQualifiedName() +
Index: test/TableGen/ambiguous-composition.td
===================================================================
--- /dev/null
+++ test/TableGen/ambiguous-composition.td
@@ -0,0 +1,55 @@
+// RUN: llvm-tblgen -gen-register-info -I %p/../../include %s 2>&1 | FileCheck %s
+// Check that user-defined subregister index compositions do not result
+// in warnings.
+// CHECK-NOT: warning: SubRegIndex Test::subreg_h64 and Test::subreg_h32 compose ambiguously as Test::subreg_hh32 or Test::subreg_h32
+
+include "llvm/Target/Target.td"
+
+def TestInstrInfo : InstrInfo {
+}
+
+def Test : Target {
+  let InstructionSet = TestInstrInfo;
+}
+
+let Namespace = "Test" in {
+  def subreg_h32  : SubRegIndex<32, 32>;
+  def subreg_h64  : SubRegIndex<64, 64>;
+  def subreg_hh32 : ComposedSubRegIndex<subreg_h64, subreg_h32>;
+}
+
+class TestReg<string n, list<Register> s> : RegisterWithSubRegs<n, s> {
+  let Namespace = "Test";
+}
+
+class FPR32<string n> : TestReg<n, []> {
+}
+
+class FPR64<string n, FPR32 high> : TestReg<n, [high]> {
+  let SubRegIndices = [subreg_h32];
+}
+
+class FPR128<string n, FPR64 high> : TestReg<n, [high]> {
+  let SubRegIndices = [subreg_h64];
+}
+
+def F0S : FPR32<"f0s">;
+def F0D : FPR64<"f0d",  F0S>;
+def F0Q : FPR128<"f0q", F0D>;
+
+// 1. Because of the explicitly defined composition for subreg_hh32,
+//    subreg_hh32.updateComponents will add to subreg_h64 a composition:
+//    subreg_h64+subreg_h32 -> subreg_hh32.
+// 2. computeSubRegs will add to F0Q a subreg F0D (subreg_h64), and then
+//    transitively F0D's subregs (subreg_h32). It will update F0Q's subreg
+//    map that F0S is a subreg of F0Q reachable via subreg_h32.
+// 3. computeComposites will iterate over subreg maps of each register to
+//    determine subregister index compositions. The composition map already
+//    contains the user-defined composition from (1). Because of (2), i.e.
+//    F0Q.subreg_h32 = F0S, which is also F0Q.subreg_h64.subreg_h32, it is
+//    assumed that subreg_h64+subreg_h32 -> subreg_h32. This creates a
+//    conflict and results in a warning.
+
+def FP32  : RegisterClass<"FP32",  [f32],   32,  (add F0S)>;
+def FP64  : RegisterClass<"FP64",  [f64],   64,  (add F0D)>;
+def FP128 : RegisterClass<"FP128", [v2f64], 128, (add F0Q)>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50977.161502.patch
Type: text/x-patch
Size: 3834 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180820/cf81a28e/attachment.bin>