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

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 8 11:52:58 PST 2016


> On Mar 7, 2016, at 9:37 AM, Daniel Sanders <daniel.sanders at imgtec.com> wrote:
> 
> 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.
Yes the unsupported bit for the whole instruction is not used at the moment (the WriteRes Unsupported bit is used, but that can't really be used to mark instructions as unsupported). We should indeed have runtime asserts for those, I'll look into that.

> 
> 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.
Maybe we can provide an operator for matching those similar to the existing (instrs ...) and (instregex ...).

- Matthias



More information about the llvm-commits mailing list