<div>Can probably just remove the radar comments from x86-16.s since there's no comments on them and it's likely just a note in the test case of which radar corresponded to these instructions.</div><div><br></div><div>
They're also useless comments so I'll probably just remove them from x86-32.s anyhow.</div><div><br></div><div>-eric<br><br><div>On Thu Dec 19 2013 at 4:11:45 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Couple of meta comments on the patches as I've been going through, these will seem like nit picking and they are, however... <div>
<br></div><div>1) Comments should be full sentences including punctuation when possible.</div><div><br></div><div>2) When referencing outside sources it's often best to explain in more detail what's going on in the particular section as if the external reference had evaporated.</div>
<div><br></div><div>3) I appreciate the splitting up of the patch into multiple and easily reviewable chunks.</div><div><br></div><div>4) Testcases should be part of the diff that adds the functionality (in particular the ones adding instructions for 16-bit mode).</div>
<div><br></div><div>5) In general the patches look pretty good. I've been working through the various llvm specific things that you've called out - it's been a while since I've worked on any of the assemblers as well :)</div>
<div><br></div><div></div><div>-eric</div><div><br><br><div>On Thu Dec 19 2013 at 3:48:05 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:</div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><br><div>On Thu Dec 19 2013 at 3:47:00 PM, David Woodhouse <<a href="mailto:dwmw2@infradead.org" target="_blank">dwmw2@infradead.org</a>> wrote:</div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Thu, 2013-12-19 at 23:02 +0000, David Woodhouse wrote:<br>
> I'm just doing a test build before I push the rebased version to my<br>
> git tree....<br>
<br>
Tested and pushed. I elect not to care about seven test failures due to<br>
the lack of a ppc64 disassembler...<br></blockquote><div><br></div><div>Yeah, that requires a reconfigure unfortunately.</div><div><br></div><div>-eric </div></blockquote></div></blockquote></div>