[PATCH] D50566: [Tablegen][SubtargetEmitter] Improve expansion of predicates of a variant scheduling class.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb created this revision.
andreadb added reviewers: RKSimon, courbet, gchatelet, craig.topper, mattd, atrick.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.

This patch refactors the logic that expands predicates of a variant scheduling class.

The idea is to improve the readability of the auto-generated code by removing redundant parentheses around predicate expressions, and by removing redundant `if(true)` statements.

This patch replaces the definition of `NoSchedPred` in TargetSchedule.td with an instance of MCSchedPredicate. The new definition is sematically equivalent to the previous one. The main difference is that now SubtargetEmitter knows that it represents predicate "true".

Before this patch, we always generated an `if (true)` for the default transition of a variant scheduling class.

Example (taken from AArch64GenSubtargetInfo.inc) :

  if (SchedModel->getProcessorID() == 3) { // CycloneModel
    if ((TII->isScaledAddr(*MI)))
      return 927; // (WriteIS_WriteLD)_ReadBaseRS
    if ((true))
      return 928; // WriteLD_ReadDefault
  }

Extra parentheses were also generated around the predicate expressions.

With this patch, we get this:

  if (SchedModel->getProcessorID() == 3) { // CycloneModel
    if (TII->isScaledAddr(*MI))
      return 927; // (WriteIS_WriteLD)_ReadBaseRS
    return 928; // WriteLD_ReadDefault
  }

The new auto-generated code behaves exactly the same as before. So, technically this is a NFC(I).

Please let me know if okay to commit.

-Andrea


https://reviews.llvm.org/D50566

Files:
  include/llvm/Target/TargetSchedule.td
  utils/TableGen/SubtargetEmitter.cpp


Index: utils/TableGen/SubtargetEmitter.cpp
===================================================================
--- utils/TableGen/SubtargetEmitter.cpp
+++ utils/TableGen/SubtargetEmitter.cpp
@@ -1480,30 +1480,54 @@
 }
 
 static void emitPredicates(const CodeGenSchedTransition &T,
-                           const CodeGenSchedClass &SC,
-                           PredicateExpander &PE,
+                           const CodeGenSchedClass &SC, PredicateExpander &PE,
                            raw_ostream &OS) {
   std::string Buffer;
   raw_string_ostream StringStream(Buffer);
   formatted_raw_ostream FOS(StringStream);
 
   FOS.PadToColumn(6);
-  FOS << "if (";
-  for (RecIter RI = T.PredTerm.begin(), RE = T.PredTerm.end(); RI != RE; ++RI) {
-    if (RI != T.PredTerm.begin()) {
-      FOS << "\n";
-      FOS.PadToColumn(8);
-      FOS << "&& ";
-    }
-    const Record *Rec = *RI;
-    if (Rec->isSubClassOf("MCSchedPredicate"))
-      PE.expandPredicate(FOS, Rec->getValueAsDef("Pred"));
-    else
-      FOS << "(" << Rec->getValueAsString("Predicate") << ")";
+
+  auto IsTruePredicate = [](const Record *Rec) {
+    return Rec->isSubClassOf("MCSchedPredicate") &&
+           Rec->getValueAsDef("Pred")->isSubClassOf("MCTrue");
+  };
+
+  // If not all predicates are MCTrue, then we need an if-stmt.
+  unsigned NumNonTruePreds =
+      T.PredTerm.size() - count_if(T.PredTerm, IsTruePredicate);
+  if (NumNonTruePreds) {
+    bool FirstNonTruePredicate = true;
+    for (const Record *Rec : T.PredTerm) {
+      // Skip predicates that evaluate to "true".
+      if (IsTruePredicate(Rec))
+        continue;
+
+      if (FirstNonTruePredicate) {
+        FOS << "if (";
+        FirstNonTruePredicate = false;
+      } else {
+        FOS << "\n";
+        FOS.PadToColumn(8);
+        FOS << "&& ";
+      }
+
+      if (Rec->isSubClassOf("MCSchedPredicate")) {
+        PE.expandPredicate(FOS, Rec->getValueAsDef("Pred"));
+        continue;
+      }
+
+      // Expand this legacy predicate and wrap it around braces if there is more
+      // than one predicate to expand.
+      FOS << ((NumNonTruePreds > 1) ? "(" : "")
+          << Rec->getValueAsString("Predicate")
+          << ((NumNonTruePreds > 1) ? ")" : "");
+    }
+
+    FOS << ")\n"; // end of if-stmt
+    FOS.PadToColumn(8);
   }
 
-  FOS << ")\n";
-  FOS.PadToColumn(8);
   FOS << "return " << T.ToClassIdx << "; // " << SC.Name << '\n';
   FOS.flush();
   OS << Buffer;
Index: include/llvm/Target/TargetSchedule.td
===================================================================
--- include/llvm/Target/TargetSchedule.td
+++ include/llvm/Target/TargetSchedule.td
@@ -373,7 +373,7 @@
   SchedMachineModel SchedModel = ?;
   code Predicate = pred;
 }
-def NoSchedPred : SchedPredicate<[{true}]>;
+def NoSchedPred : MCSchedPredicate<TruePred>;
 
 // Associate a predicate with a list of SchedReadWrites. By default,
 // the selected SchedReadWrites are still associated with a single


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50566.160126.patch
Type: text/x-patch
Size: 2977 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180810/040ce441/attachment.bin>