<div dir="ltr">New patch attached.<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 2, 2013 at 4:12 PM, Mihail Popa <span dir="ltr"><<a href="mailto:mihail.popa@gmail.com" target="_blank">mihail.popa@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Tim, <div><br></div><div>even if it only applies to the branches, there will be plenty of occurrences:</div>
<div><br></div><div>1. branch conditional T1 encoding - signed, 8 bit, shift 1</div><div>2. branch unconditional T2 encoding - signed, 11 bit, shift 1</div>
<div>3. branch conditional T3 encoding - signed 24 bit, shift 1</div><div>4. branch linked conditional T2 encoding - signed 24 bit, shift 2</div><div><br></div><div>For load and store literal, with sign-magnitude format:</div>

<div><br></div><div>1. T1 encoding: unsigned, 8 bit, shift 2</div><div>2. T2 encoding: signed 12 bit, no shift</div><div><br></div><div>These cover only functional issues, where an encoding that is too short is chosen</div>

<div>over the one of correct length. </div><div><br></div><div>There are probably more examples if we add constraints for the purpose of diagnostics</div><div>and error checking.</div><div><br></div><div>I do agree it'd be probably better to split the template in unsigned, 2's complement signed </div>

<div>and magnitude signed.</div><div><br></div><div>Mihai</div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 2, 2013 at 12:56 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>
<br>
Thanks for working on this, it's looking better now.<br>
<div><br>
> Please see attached the revised patch. I have made the template function and<br>
> the new operand classes clearly refer to PC-relative addresses<br>
<br>
</div>So how widely are you planning to apply this method? I don't think it<br>
will account for the other forms of ADR for example, which have the<br>
same quirk as LDR that Min = -Max. It seems it could work for:<br>
  + Branches<br>
  + T1 encoding of ADR<br>
  + ADD, SUB (even if not PC-rel, assuming we're not going to support<br>
"ADD r0, r0, #-4" as an alias).<br>
  + MOVW/MOVT<br>
<br>
And it would fail for:<br>
  + Other encodings of ADR (as LLVM implements them, with +/- in same MCInst)<br>
  + Loads & stores.<br>
<br>
I don't think the "PCOffset" is a particularly better description than<br>
"MemoryOffset" for those instructions, given that of the three kinds<br>
of PC-offsetting instructions (ADR, LDR (lit), B) it works for 1.5 of<br>
them.<br>
<br>
Perhaps it's too general, and should split into an unsigned (added<br>
now) and two signed variants (added as and when they're needed), not<br>
controlled by templates?<br>
<div><br>
> and changed the shifting policy.<br>
<br>
</div>Thanks.<br>
<br>
Some more boundary tests would probably be good (e.g. that "ADR r0,<br>
#1020" and "ADR r0, #-4" really are the edges of what's representable<br>
by the T1 encoding)?<br>
<br>
Regards.<br>
<span><font color="#888888"><br>
Tim.<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>