<div dir="ltr">Tim, Jim,<div><br></div><div style>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.</div>
<div style>Take for example the following two NEON instructions: <font face="courier new, monospace">vhadd.i32 d0, d1, d2</font> and <font face="courier new, monospace">vhadd.i32 q0, q1,q2</font>.</div><div style><br></div>
<div style>If all we have is the MCInst, then it gets difficult to check the restriction I mentioned:</div><div style><br></div><div style>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  <font face="courier new, monospace">{ ARM::D0, ... , ARM::D31 }</font> while the operands of the second are part of the set <font face="courier new, monospace">{ ARM::Q0, ... , ARM::Q15 }</font><font face="arial, helvetica, sans-serif">,</font> 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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style>In order to use the same mechanism for assembly, we can define the prototype for such restriction checking methods as:</div><div style><br></div><div style><span style="font-size:13px"><font color="#000000" face="courier new, monospace">static DecodeStatus CheckConstraint(const MCInst &Inst, unsigned Insn = 0);</font></span></div>
<div style><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:13px"><br></span></div><div style><font color="#000000" face="arial, sans-serif">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.</font></div>
<div style><font color="#000000" face="arial, sans-serif"><br></font></div><div style><font color="#000000" face="arial, sans-serif">Regards,</font></div><div style><font color="#000000" face="arial, sans-serif">Mihai</font></div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 1, 2013 at 12:06 PM, Tim Northover <span dir="ltr"><<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Mihail,<br>
<div class="im"><br>
> static DecodeStatus CheckNEONConstraint(const MCInst &Inst, unsigned Insn)<br>
</div>[...]<br>
> ConstraintCheckMethod = "CheckNEONConstraint"<br>
<br>
In general I like the idea of an instruction-validation method. I<br>
think it could also potentially solve the SoftFail/UNPREDICTABLE<br>
issues that are looming (and partially resolved for decoding at<br>
present).<br>
<br>
However, I think that to cope with the other issues the scope of the<br>
method should (at least potentially) be wider than just disassembly.<br>
For example we will eventually want to reject some load instructions<br>
based on complex operand constraints (e.g. if it's write-back indexed<br>
and Rt == Rn). Currently the only way to handle cross-operand<br>
conditions in the AsmParser (I believe) is with a custom<br>
ConvertMethod, which is even more heavyweight than the disassembler<br>
situation.<br>
<br>
I think making this checker work for assembly would be rather simple<br>
if the method acts on an MCInst, but probably not possible if it also<br>
gets to use the bit-pattern as suggested.<br>
<br>
In addition, in my view working on the bit-pattern adds redundancy and<br>
potential bugs where the decoder and checker interpret the bits<br>
differently.<br>
<br>
I've been wrong about MC layer design before though. Jim's been very<br>
helpful at explaining the error of my ways in the past; I've added him<br>
on CC.<br>
<br>
Cheers.<br>
<span class="HOEnZb"><font color="#888888"><br>
Tim.<br>
</font></span></blockquote></div><br></div>