[TableGen] Fix a bug that MCSchedClassDesc is interfered between different SchedModel

[steven.zhang]

Summary

Assume that, ModelA has scheduling resource for InstA and ModelB has scheduling resource for InstB. This is what the llvm::MCSchedClassDesc looks like:

llvm::MCSchedClassDesc ModelASchedClasses[] = {
...
InstA, 0, ...
InstB, -1,...
};

llvm::MCSchedClassDesc ModelBSchedClasses[] = {
...
InstA, -1,...
InstB, 0,...
};

The -1 means invalid num of macro ops, while it is valid if it is >=0. This is what we look like now:

llvm::MCSchedClassDesc ModelASchedClasses[] = {
...
InstA, 0, ...
InstB, 0,...
};

llvm::MCSchedClassDesc ModelBSchedClasses[] = {
...
InstA, 0,...
InstB, 0,...
};

And compiler hit the assertion here because the SCDesc is valid now for both InstA and InstB.

if (SCDesc->isValid() && !DefMI->getOperand(DefOperIdx).isImplicit()
      && !DefMI->getDesc().OpInfo[DefOperIdx].isOptionalDef()
      && SchedModel.isComplete()) {
    errs() << "DefIdx " << DefIdx << " exceeds machine model writes for "
           << *DefMI << " (Try with MCSchedModel.CompleteModel set to false)";
    llvm_unreachable("incomplete machine model");
  }

[jsji]

llvm/test/CodeGen/ARM/ParallelDSP/multi-use-loads.ll

23 ↗

(On Diff #221485)

Do these changes in scheduling means ParallelDSP SchedModel has some interference with others?
If so, maybe we need another patch to fix that.

[samparker]

llvm/test/CodeGen/ARM/ParallelDSP/multi-use-loads.ll

23 ↗

(On Diff #221485)

I think if this patch was rebased, this change would go away. This brought to our attention that some instructions weren't modelled, but I hope this is fixed now.

[jsji]

llvm/test/CodeGen/ARM/ParallelDSP/multi-use-loads.ll

23 ↗

(On Diff #221485)

Good to know that! Thanks @samparker !

[steven.zhang]

rebase the patch.

[steven.zhang]

I have rebased the patch.

llvm/test/CodeGen/ARM/ParallelDSP/multi-use-loads.ll

23 ↗

(On Diff #221485)

Good catch, and yes, it has been fixed by https://reviews.llvm.org/rL373163

[jsji]

LGTM. It would be great if you can figure out why we still have difference in ARM tests, but it shouldn't block this.

llvm/test/CodeGen/ARM/ParallelDSP/unroll-n-jam-smlad.ll

48

Why do we still have this difference? Shouldn't it be fixed as well?

[andreadb]

I am not convinced that this patch is correct. Isn’t the problem that your model was wrongly marked as complete?

[andreadb]

In D67950#1705613, @andreadb wrote:

I am not convinced that this patch is correct. Isn’t the problem that your model was wrongly marked as complete?

Nevermind. If the issue was related to the model being marked as complete, then the tablegen backend would have complained earlier on when building the scheduling classes.

[steven.zhang]

In D67950#1705613, @andreadb wrote:

I am not convinced that this patch is correct. Isn’t the problem that your model was wrongly marked as complete?

Even we marked the model complete wrongly, the table-gen should generate the data or diagnose correctly. And yes, the table-gen will emit diagnose messages, but still emit the SCDesc as valid, which messy the model.

llvm/test/CodeGen/ARM/ParallelDSP/unroll-n-jam-smlad.ll

 ; CHECK-REG-PRESSURE: .LBB0_1:
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
-; CHECK-REG-PRESSURE: ldr{{.*}}, [sp
 ; CHECK-REG-PRESSURE: bne .LBB0_1
 
 for.body:
   %A3 = phi i32 [ %add9.us.i.3361.i, %for.body ], [ 0, %entry ]
   %j.026.us.i.i = phi i32 [ %inc.us.i.3362.i, %for.body ], [ 0, %entry ]
   %A4 = phi i32 [ %add9.us.i.1.3.i, %for.body ], [ 0, %entry ]
   %A5 = phi i32 [ %add9.us.i.2.3.i, %for.body ], [ 0, %entry ]
   %A6 = phi i32 [ %add9.us.i.3.3.i, %for.body ], [ 0, %entry ]

llvm/test/TableGen/InvalidMCSchedClassDesc.td

+// RUN: llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
+// Check if it is valid MCSchedClassDesc if didn't have the resources. 
+
+include "llvm/Target/Target.td"
+
+def MyTarget : Target;
+
+let OutOperandList = (outs), InOperandList = (ins) in {
+  def Inst_A : Instruction; 
+  def Inst_B : Instruction; 
+}
+
+let CompleteModel = 0 in {
+  def SchedModel_A: SchedMachineModel;
+  def SchedModel_B: SchedMachineModel;
+  def SchedModel_C: SchedMachineModel;
+}
+
+// Inst_B didn't have the resoures, and it is invalid.
+// CHECK: SchedModel_ASchedClasses[] = {
+// CHECK: {DBGFIELD("Inst_A")             1
+// CHECK-NEXT: {DBGFIELD("Inst_B")             16383 
+let SchedModel = SchedModel_A in {
+  def Write_A : SchedWriteRes<[]>;
+  def : InstRW<[Write_A], (instrs Inst_A)>;
+}
+
+// Inst_A didn't have the resoures, and it is invalid.
+// CHECK: SchedModel_BSchedClasses[] = {
+// CHECK: {DBGFIELD("Inst_A")             16383 
+// CHECK-NEXT: {DBGFIELD("Inst_B")             1 
+let SchedModel = SchedModel_B in {
+  def Write_B: SchedWriteRes<[]>; 
+  def : InstRW<[Write_B], (instrs Inst_B)>;
+}
+
+// CHECK: SchedModel_CSchedClasses[] = {
+// CHECK: {DBGFIELD("Inst_A")             1
+// CHECK-NEXT: {DBGFIELD("Inst_B")             1
+let SchedModel = SchedModel_C in {
+  def Write_C: SchedWriteRes<[]>; 
+  def : InstRW<[Write_C], (instrs Inst_A, Inst_B)>;
+}
+
+def ProcessorA: ProcessorModel<"ProcessorA", SchedModel_A, []>;
+def ProcessorB: ProcessorModel<"ProcessorB", SchedModel_B, []>;
+def ProcessorC: ProcessorModel<"ProcessorC", SchedModel_C, []>;

llvm/utils/TableGen/SubtargetEmitter.cpp

if (Writes.empty()) {
                               Writes, Reads);
           break;
         }
       }
       if (Writes.empty()) {
         LLVM_DEBUG(dbgs() << ProcModel.ModelName
                           << " does not have resources for class " << SC.Name
                           << '\n');
+        SCDesc.NumMicroOps = MCSchedClassDesc::InvalidNumMicroOps;
       }
     }
     // Sum resources across all operand writes.
     std::vector<MCWriteProcResEntry> WriteProcResources;
     std::vector<MCWriteLatencyEntry> WriteLatencies;
     std::vector<std::string> WriterNames;
     std::vector<MCReadAdvanceEntry> ReadAdvanceEntries;
     for (unsigned W : Writes) {