[PATCH] D147102: [M68k] Add `TRAP`, `TRAPV`, `BKPT`, `ILLEGAL` instructions

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added a comment.

In D147102#4238899 <https://reviews.llvm.org/D147102#4238899>, @ids1024 wrote:

>>> This does not validate that the 4 and 3 bit immediates taken by TRAP and BKPT are in range. But it seems the backend isn't validating the range of immediates in general.
>>
>> This can be done by adding a new immediate Operand with a different `AsmOperandClass`, instead of using MxImm <https://github.com/llvm/llvm-project/blob/241ad16eb012ed75d1ff2d8eed8db9464b5c7787/llvm/lib/Target/M68k/M68kInstrInfo.td#L374>. In this new `AsmOperandClass` you can use different `PredicateMethod` / `RenderMethod` / `ParserMethod` to validate the range.
>
> Ah, so this would be best done with another `AsmOperandClass`? And not somehow using `MxImm` and `MxOp`. I'm still trying to understand how these sorts of things are handled in tablegen.

I don't think it's necessary to have a `MxOp` alternative because you can do something like:

  // New AsmOperandClass for 3-bit vector
  def MxVec3Imm : AsmOperandClass { ... }
  ...
  let ParserMatchClass = MxVec3Imm in
  def MxVec3imm  : MxOp<i8,  MxSize8,  "i">;



> With the current version here, it accepts ` __asm__ ("bkpt #6666666666666666");` without error. So `Mxi8imm` doesn't seem to be correctly validating that the immediate fits in 8 bits.

Right, this is something we need to fix in another patch, thanks for pointing it out :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147102