[LLVMdev] A simpler method to reject undefined encodings

[Mihail Popa]

Tim, Jim,

I don't think it would be very helpful if such a method will act on the
MCInst alone. The reason is that certain conditions are far easier and less
error prone to check for directly on bitpatterns.
Take for example the following two NEON instructions: vhadd.i32 d0,
d1, d2and vhadd.i32
q0, q1,q2.

If all we have is the MCInst, then it gets difficult to check the
restriction I mentioned:

1. it will be hard to tell the operands apart. While it is true that the
operands of the first instruction will be part of the set  { ARM::D0, ... ,
ARM::D31 } while the operands of the second are part of the set { ARM::Q0,
... , ARM::Q15 }, they are only enums and not type safe. They only way to
check that a register operand belongs to one set or another is to test for
boundaries of the enum sets. However this now becomes dependent on
tablegen's behaviour - to my knowledge there is no guarantee that enums
representing registers from the same register file will be dumped in
sequence. It is natural behaviour, but unless it's clearly documented this
happens we should not rely on this.

2. Remember the nature of the restriction: every time bit Q == 1, the
encoded register fields need to contain even numbers. I do not have access
to the Q bit, so I would need to take into account the value of the opcode
and remember which are the opcodes represent encodings with Q == 1.

3. Because of 2, I would potentially have to apply this restriction checker
in multiple places, only for the instruction definitions which should have
Q == 1. If I have access to the Q bit I can set this restriction checker in
a upper class and leave the behaviour to be inherited by all definitions.

The truth of the matter is that certain conditions are more easily checked
on bitpatterns while others are more easily checked on MCInsts - this is
why I believe it's useful to have both.

In order to use the same mechanism for assembly, we can define the
prototype for such restriction checking methods as:

static DecodeStatus CheckConstraint(const MCInst &Inst, unsigned Insn = 0);

Tablegen will be able to pass a single parameter when assembling and
the implementation of the checker will simply have to ignore the zero'ed
out bit pattern.

Regards,
Mihai


On Wed, May 1, 2013 at 12:06 PM, Tim Northover <t.p.northover at gmail.com>wrote:

> Hi Mihail,
>
> > static DecodeStatus CheckNEONConstraint(const MCInst &Inst, unsigned
> Insn)
> [...]
> > ConstraintCheckMethod = "CheckNEONConstraint"
>
> In general I like the idea of an instruction-validation method. I
> think it could also potentially solve the SoftFail/UNPREDICTABLE
> issues that are looming (and partially resolved for decoding at
> present).
>
> However, I think that to cope with the other issues the scope of the
> method should (at least potentially) be wider than just disassembly.
> For example we will eventually want to reject some load instructions
> based on complex operand constraints (e.g. if it's write-back indexed
> and Rt == Rn). Currently the only way to handle cross-operand
> conditions in the AsmParser (I believe) is with a custom
> ConvertMethod, which is even more heavyweight than the disassembler
> situation.
>
> I think making this checker work for assembly would be rather simple
> if the method acts on an MCInst, but probably not possible if it also
> gets to use the bit-pattern as suggested.
>
> In addition, in my view working on the bit-pattern adds redundancy and
> potential bugs where the decoder and checker interpret the bits
> differently.
>
> I've been wrong about MC layer design before though. Jim's been very
> helpful at explaining the error of my ways in the past; I've added him
> on CC.
>
> Cheers.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130501/18ecafd9/attachment.html>