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

[andreadb]

Summary

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

[mattd]

Thanks @andreadb. LGTM.

[craig.topper]

LGTM

utils/TableGen/SubtargetEmitter.cpp

1487 ↗

(On Diff #160126)

Not related to this patch, but is formatted_raw_ostream really needed here? Can we use indent instead of PadToColumn? formatted_raw_ostream has to do internal tracking of column and thus scans every print for a new line character.

[andreadb]

utils/TableGen/SubtargetEmitter.cpp

1487 ↗

(On Diff #160126)

Good point.
We can definitely use PadToColumn in this function.
We can also change the PredicateExpander, so that method expandPredicate() doesn't require a formatted_raw_ostream.
I'll think about it and see if I can send it as a follow-up NFC patch.

Thanks!

llvm/trunk/include/llvm/Target/TargetSchedule.td

 // if-statement's expression. Available variables are MI, SchedModel,
 // and anything defined in a PredicateProlog.
 //
 // SchedModel silences warnings but is ignored.
 class SchedPredicate<code pred> : SchedPredicateBase {
   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
 // operand and assumed to execute sequentially with additive
 // latency. However, if the parent SchedWriteVariant or
 // SchedReadVariant is marked "Variadic", then each Selected
 // SchedReadWrite is mapped in place to the instruction's variadic
 // operands. In this case, latency is not additive. If the current Variant

llvm/trunk/utils/TableGen/SubtargetEmitter.cpp

static void emitPredicateProlog(const RecordKeeper &Records, raw_ostream &OS) {
   for (Record *P : Prologs)
     Stream << P->getValueAsString("Code") << '\n';
 
   Stream.flush();
   OS << Buffer;
 }
 
 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);
+
+  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 (";
-  for (RecIter RI = T.PredTerm.begin(), RE = T.PredTerm.end(); RI != RE; ++RI) {
-    if (RI != T.PredTerm.begin()) {
+        FirstNonTruePredicate = false;
+      } else {
         FOS << "\n";
         FOS.PadToColumn(8);
         FOS << "&& ";
       }
-    const Record *Rec = *RI;
-    if (Rec->isSubClassOf("MCSchedPredicate"))
+
+      if (Rec->isSubClassOf("MCSchedPredicate")) {
         PE.expandPredicate(FOS, Rec->getValueAsDef("Pred"));
-    else
-      FOS << "(" << Rec->getValueAsString("Predicate") << ")";
+        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";
+    FOS << ")\n"; // end of if-stmt
     FOS.PadToColumn(8);
+  }
+
   FOS << "return " << T.ToClassIdx << "; // " << SC.Name << '\n';
   FOS.flush();
   OS << Buffer;
 }
 
 // Used by method `SubtargetEmitter::emitSchedModelHelpersImpl()` to generate
 // epilogue code for the auto-generated helper.
 void emitSchedModelHelperEpilogue(raw_ostream &OS, bool ShouldReturnZero) {