[PATCH] D132205: [llvm-tblgen] CodeGenSchedModels::hasReadOfWrite gets wrong predication result

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

The chage looks good. However, the test must be improved.

Your test only checks the content of the write latency table. However, that is not enough to prove that this bug is fixed; you also need to show that the read advance entry for Read_D is associated with Write_B.

I suggest to modify the InstRW for Inst_C as follows:

  def : InstRW<[Write_C, Read_D], (instrs Inst_C)>;

That way, Read_D will be associated to the first read operand of Inst_C.

Since Read_D is now explicitly used, a new `MCReadAvanceEntry` is added to table `MyTargetReadAdvanceTable` (see entry #1 below).

  // {UseIdx, WriteResourceID, Cycles}
  extern const llvm::MCReadAdvanceEntry MyTargetReadAdvanceTable[] = {
    {0,  0,  0}, // Invalid
    {0,  2,  1} // #1
  }; // MyTargetReadAdvanceTable

In your test, you need to CHECK the presence of that {0, 2, 1} entry. That's the only way to know that there is a (at least one) SchedRead associated to Write_B (WriteResourceID = 2).
You should also check that Inst_C now uses that ReadAdvance. That's how we know that Read_D is effectively associated with Write_B.

In the .inc output, you should be able to check the last line of the table below:

  // {Name, NumMicroOps, BeginGroup, EndGroup, RetireOOO, WriteProcResIdx,#, WriteLatencyIdx,#, ReadAdvanceIdx,#}
  static const llvm::MCSchedClassDesc SchedModel_ASchedClasses[] = {
    {DBGFIELD("InvalidSchedClass")  8191, false, false, false, 0, 0,  0, 0,  0, 0},
    {DBGFIELD("Inst_A")             1, false, false, false,  0, 0,  1, 1,  0, 0}, // #1
    {DBGFIELD("Inst_B")             1, false, false, false,  0, 0,  1, 1,  0, 0}, // #2
    {DBGFIELD("Inst_C")             1, false, false, false,  0, 0,  1, 1,  1, 1}, // #3
  }; // SchedModel_ASchedClasses

Based on that last descriptor, Inst_C declares a single read-advance entry (i.e. entry #1 in `MyTargetReadAdvanceTable`).
Entry #1 in `MyTargetReadAdvanceTable` declares a 1cy of read advance when associated with Write_B.

In conclusion, for completeness, you need to check also tables `MyTargetReadAdvanceTable` and `SchedModel_ASchedClasses`.

I hope it helps.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132205/new/

https://reviews.llvm.org/D132205