[TableGen] Emit more variant transitions

[evandro]

Summary

llvm-mca relies on the predicates to be based on MCSchedPredicate in order to resolve the scheduling for variant instructions. Otherwise, it aborts the building of the instruction model early.

However, the scheduling model emitter in TableGen gives up too soon, unless all processors use only such predicates.

In order to allow more processors to be used with llvm-mca, this patch emits scheduling transitions if any processor uses these predicates. The transition emitted for the processors using legacy predicates is the one specified with NoSchedPred, which is based on MCSchedPredicate.

[evandro]

Exclude test that does not apply anymore.

[andreadb]

Hi Evandro,

Sorry for the very late reply.
I am back from holidays. I plan to review this patch and your other patch (D54777).

More in general: I am very happy to see that there is interest in llvm-mca from people working on aarch64. So, I will do my best to help you with reviews and suggesting alternative approaches.

llvm-mca relies on the predicates to be based on MCSchedPredicate in order to resolve the scheduling for variant instructions. Otherwise, it aborts the building of the instruction model early.
However, the scheduling model emitter in TableGen gives up too soon, unless all processors use only such predicates.

This is done intentionally.

Transitions are expanded into an if-then-else sequence, and scheduling predicates are predicate expressions used to check if a branch (of the if-then-else construct) is taken or not.
Predicates have to be processed strictly in sequence, and the order matters.

In order to allow more processors to be used with llvm-mca, this patch emits scheduling transitions if any processor uses these predicates. The transition emitted for the processors using legacy predicates is the one specified with NoSchedPred, which is based on MCSchedPredicate.

I don't think this is the right approach. As I mentioned before, predicates must be processed in sequence.

We cannot just emit "some" transitions and pretend that everything works fine; the resolved scheduling class would be potentially incorrect. We cannot assume that NoSchedPred is a good guess (i.e. default) for most scheduling write variants... NoSchedPred is just the last predicate of the sequence; we cannot make assumptions on how often it is used to resolve write variants. Using it as a default might be okay for your target processor in some cases. But it would lead to inaccurate results on other target processors.

In all honesty. If you really care about supporting llvm-mca for aarch64, then the right approach is to port existing scheduling predicates to the new MCSchedPredicate framework.
This patch sound more like a hack to get llvm-mca working for some Aarch64 instructions. I personally don't like it.

On the other hand, your D54777 seems to go the right direction. I plan to review it next.

-Andrea

[evandro]

The issue that I'm trying to avoid is that it's not enough for me to add predicates based on MCSchedPredicate for Exynos processors is other processors don't. Then, if an instruction that I model by using a variant schedule is also modeled by another processor, TableGen will emit no solution at all for the instruction. This patch, which I recognize is just an attempt, aims at allowing the proper solution for a processor using such predicates, while indeed resulting in a clumsy solution the scheduling of the same instruction for other processors.

The issue is that it's virtually impossible at the moment to model AArch64 without running on llvm-mca giving right up. I was thinking that instead of giving up, llvm-mca should resort to a reasobale default and highlight it in its result. I proposed NoSchedPred as this default, but, though we can discuss what the default should be, I think that no default does not make sense as is.

[andreadb]

In D54648#1305567, @evandro wrote:

The issue that I'm trying to avoid is that it's not enough for me to add predicates based on MCSchedPredicate for Exynos processors is other processors don't. Then, if an instruction that I model by using a variant schedule is also modeled by another processor, TableGen will emit no solution at all for the instruction. This patch, which I recognize is just an attempt, aims at allowing the proper solution for a processor using such predicates, while indeed resulting in a clumsy solution the scheduling of the same instruction for other processors.

The issue is that it's virtually impossible at the moment to model AArch64 without running on llvm-mca giving right up. I was thinking that instead of giving up, llvm-mca should resort to a reasobale default and highlight it in its result. I proposed NoSchedPred as this default, but, though we can discuss what the default should be, I think that no default does not make sense as is.

Right. I see what you mean.

I was under the impression that transitions would have been correctly expanded at least for your Exynos models.
However, it seems like the lack of MCSchedPredicates in other processor models is somehow causing problems.. If that's the case, then I think your patch makes sense.
Although it is not idea, at least, it unblocks your work.

Could you add a brief comment (a FIXME ) in the code, and the XFAIL test to explain what is happening (and why we are less strict with the check)?
We may want to revisit this in future. But for now, your patch is a good starting point.

Thanks!
-Andrea

llvm/test/tools/llvm-mca/ARM/unsupported-write-variant.s

 # RUN: not llvm-mca -march=arm -mcpu=swift -all-views=false 2>&1 < %s | FileCheck %s
+# XFAIL: *
 
 add r3, r1, r12, lsl #2
 
 # CHECK:      error: unable to resolve scheduling class for write variant.
 # CHECK-NEXT: note: instruction:    add r3, r1, r12, lsl #2

llvm/utils/TableGen/SubtargetEmitter.cpp

void collectVariantClasses(const CodeGenSchedModels &SchedModels,
                            IdxVec &VariantClasses,
                            bool OnlyExpandMCInstPredicates) {
   for (const CodeGenSchedClass &SC : SchedModels.schedClasses()) {
     // Ignore non-variant scheduling classes.
     if (SC.Transitions.empty())
       continue;
 
     if (OnlyExpandMCInstPredicates) {
-      // Ignore this variant scheduling class if transitions don't uses any
+      // Ignore this variant scheduling class if no transitions use any
       // MCSchedPredicate definitions.
-      if (!all_of(SC.Transitions, [](const CodeGenSchedTransition &T) {
+      if (!any_of(SC.Transitions, [](const CodeGenSchedTransition &T) {
             return hasMCSchedPredicates(T);
           }))
         continue;
     }
 
     VariantClasses.push_back(SC.Index);
   }
 }
for (unsigned VC : VariantClasses) {
 
     OS << "  case " << VC << ": // " << SC.Name << '\n';
 
     PredicateExpander PE(Target);
     PE.setByRef(false);
     PE.setExpandForMC(OnlyExpandMCInstPredicates);
     for (unsigned PI : ProcIndices) {
       OS << "    ";
+
       // Emit a guard on the processor ID.
       if (PI != 0) {
         OS << (OnlyExpandMCInstPredicates
                    ? "if (CPUID == "
                    : "if (SchedModel->getProcessorID() == ");
         OS << PI << ") ";
         OS << "{ // " << (SchedModels.procModelBegin() + PI)->ModelName << '\n';
       }
 
       // Now emit transitions associated with processor PI.
       for (const CodeGenSchedTransition &T : SC.Transitions) {
         if (PI != 0 && !count(T.ProcIndices, PI))
           continue;
+
+        // Emit only transitions based on MCSchedPredicate, if it's the case.
+        // At least the transition specified by NoSchedPred is emitted,
+        // which becomes the default transition for those variants otherwise
+        // not based on MCSchedPredicate .
+        if (OnlyExpandMCInstPredicates && !hasMCSchedPredicates(T))
+          continue;
+
         PE.setIndentLevel(3);
         emitPredicates(T, SchedModels.getSchedClass(T.ToClassIdx), PE, OS);
       }
 
       OS << "    }\n";
+
       if (PI == 0)
         break;
     }
 
     if (SC.isInferred())
       OS << "    return " << SC.Index << ";\n";
     OS << "    break;\n";
   }