[PATCH] D17747: TableGen: Check scheduling models for completeness

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 7 09:37:08 PST 2016


dsanders added a comment.

I've looked into it and it seems we did have a few holes in our model. In particular, MSA has been overlooked and a few of the rarer instructions are also missing. However, the majority are unsupported on the only processor which currently has a SchedModel.

> // Exclude micromips instructions

>  def : InstRW<[], (instregex ".*_MM$", ".*_MMR[0-9]$")> { let Unsupported = 1; }

> 

> Is that okay?


I believe this works by providing an empty list of scheduling information. Won't this disable the dynamic check for these instructions? As far as I can tell, the Unsupported field only serves as documentation.

Another issue is that, some parts of our instruction are difficult to create regexes for. The examples I'm thinking of are:

- Most of the R6 instructions lack the '_R6' suffix. It was only used where it would otherwise conflict with pre-R6 instructions.
- DSP/MSA/etc. don't have 'DSP'/'MSA'/etc. in their name. They just use the mnemonic.
- When we add R6-only processors like the I6400 we'll want to mark the pre-R6 versions of the '*_R6' instructions unsupported.

At the moment, I'm thinking we could add a class that marks all instructions with a specified Predicate as being unsupported. For example:

  def : UnsupportedFeatures<"HasMips3">;    // All 64-bit ISA's are unsupported (HasMips3 and any superset are unsupported).
  def : UnsupportedFeatures<"HasMips32R6">; // All R6 ISA's are unsupported (HasMips32R6 and any superset are unsupported).
  def : UnsupportedFeatures<"HasCnMips">;   // Octeon ASE's are unsupported.
  .. etc.

Then CodeGenSchedModels::checkCompleteness() could check for unsupported instructions just after the `Inst->hasNoSchedulingInfo` check. This suggestion deals with the first two examples, but not the third. I don't have a good solution for that one yet. We could solve it by listing each instruction individually but it would be a shame to have to do that for each and every R6 processor we add in the future.


Repository:
  rL LLVM

http://reviews.llvm.org/D17747





More information about the llvm-commits mailing list