[llvm-dev] Verifying Backend Schedule (Over)Coverage

[Andrew Trick via llvm-dev]

+cc Matthias, who has worked on the machine model generator more recently and currently owns it.

> On Jun 21, 2017, at 2:08 PM, Joel Jones via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> I ran into an interesting problem when helping to land a scheduler .td file
> that my colleague had written. The problem that came up was that a 
> multiply/add pair was not combined into an madd, but just for our CPU. Upon
> digging into it, the problem turned out to be that '(instregex "^SUB" ...'
> was matching "SUBREG_TO_REG" and incorrectly increasing the schedule length.
> I removed the overly aggressive match, but I noticed that there were lots
> of instructions that matched multiple regex patterns in InstegexOp::apply in
> utils/TableGen/CodeGenSchedule. I modified the apply method to output 
> <idx, pat> pairs and <idx, name> pairs and then joined them togather
> using a script. However, I couldn't easily determine from within that method
> what specific subtarget the patterns came from. Is there a better place to do
> this check? It seems that CodeGenSchedModels::checkCompleteness would be the
> logical place.
> 
> Joel Jones


Hi Joel,

As a general reminder, it's crucial to check the output from
-debug-only=subtarget-emitter after changing the machine model to make
sure it matches expectations. I'm afraid many people don't do that
because it's awkward and not at all discoverable.

If there's anyway you can improve on the presentation, diagnostics, and debugging
of the generated model would be very helpful.

We should be catching overcoverage somewhere--I know that was the intention.

There are at least a few issues that makes all this confusing:

1. All subtargets of an architecture are "compressed" into a single
   table of MCWriteProcResEntries. That compression is done on-the-fly
   during tablegen subtarget emission.

2. An architecture can list default read/write resources for
   instruction classes, which are only overriden by the subtarget for
   selected opcodes.

3. Some scheduling classes are inferred based on custom "predicates"
   so they only apply to certain subtargets.

4. We still support itinerary classes defined on instruction
   definitions. That was done to continue supporting Cortex A9
   itineraries while simultaneously testing the new machine model on
   that target. If we could drop support for itinerary classes, it
   seems that a lot of complexity could be ripped out.

It's hard to say without running some experiments myself, but it looks
to me like the code in SubtargetEmitter::GenSchedClassTables just
picks the first subtarget-specific match for a given scheduling class:

        if (&ProcModel == &SchedModels.getProcModel(RWModelDef)) {
          RWDef = RW;
          break;
        }

It's strange to me that we don't assert here that there's only one match.

Even if we add this assert, I agree that this should be diagnosed
first in checkCompleteness, before we ever start emitting tables. But
it should not be limited to "complete" machine models.

-Andy