<div dir="ltr">Sure - sorry for delaying - it'll be in by tomorrow.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 23, 2013 at 8:34 PM, Jim Grosbach <span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Ping. Please fix this sooner rather than later. As-is, the test case is fundamentally wrong and the diagnostic is bogus. The end result is that the behavior after this patch is overall worse than before.<div>
<span class="HOEnZb"><font color="#888888"><div><br></div><div>-Jim</div></font></span><div><div class="h5"><div><br></div><div><div><div>On Apr 18, 2013, at 5:19 AM, Mihail Popa <<a href="mailto:mihail.popa@gmail.com" target="_blank">mihail.popa@gmail.com</a>> wrote:</div>
<br><blockquote type="cite"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div dir="ltr">Jim,<div><br></div><div>I will address your concerns in a subsequent patch. </div>
<div>You are correct - the same issue is present in the Thumb encoding space as well. </div><div>I have left that alone simply because ARM encodings are of higher priority at the moment, but will fix in the near future.</div>
<div><br></div><div>Mihai</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Apr 17, 2013 at 11:52 PM, Jim Grosbach<span> </span><span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>></span><span> </span>wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div>Thanks, much clearer now. This is a nice tightening up of things.<div>
<br></div><div>Also, thanks for reviewing this Quentin. I just a have a few additional comments on some of the details.</div><div><br></div><div>+def Imm0_4AsmOperand : ImmAsmOperand { let Name = "Imm0_4"; }</div>
<div><br></div><div>This should have a custom DiagnosticType as well to help get better error messages for out of range values. We don’t use this facility as much as we should in the ARM backend, but it’s worth doing. Imm0_15AsmOperand has an example of how this works.</div>
<div><br></div><div><br></div><div><div>-def HINT : AI<(outs), (ins imm0_255:$imm), MiscFrm, NoItinerary,</div><div>+def HINT : AI<(outs), (ins imm0_4:$imm), MiscFrm, NoItinerary,</div><div> "hint", "\t$imm", []>, Requires<[IsARM, HasV6]> {</div>
<div>- bits<8> imm;</div><div>- let Inst{27-8} = 0b00110010000011110000;</div><div>- let Inst{7-0} = imm;</div><div>+ bits<3> imm;</div><div>+ let Inst{27-3} = 0b0011001000001111000000000;</div><div>+ let Inst{2-0} = imm;</div>
<div> }</div><div><br></div><div>This is only for the ARM mode of the instruction. Don’t restrictions like this also apply to the Thumb2 encodings?</div><div><br></div><div><div> @ CHECK: yield @ encoding: [0x01,0xf0,0x20,0xe3]</div>
<div> @ CHECK: yieldne @ encoding: [0x01,0xf0,0x20,0x13]</div><div>-@ CHECK: hint<span style="white-space:pre-wrap"> </span>#5 @ encoding: [0x05,0xf0,0x20,0xe3]</div><div>+@ CHECK-NOT: hint<span style="white-space:pre-wrap"> </span>#5</div>
<div> @ CHECK: sev </div></div><div><br></div><div>A better way to handle this is that “hint #5” should be removed entirely from this test file since it’s not a valid encoding. It should be replaced with test cases in diagnostics.s and thumb-diagnostics.s which check that the expected diagnostic is produced. As is, for example, we get "error: instruction requires: thumb2” which isn’t correct, since this instruction shouldn’t be valid for thumb2 either. We should explicitly check for the new diagnostic we add via DiagnosticType.</div>
<div><br></div><div>Jim</div><div><br></div><div><br></div><div><div><div>On Apr 17, 2013, at 3:10 AM, Mihail Popa <<a href="mailto:mihail.popa@gmail.com" target="_blank">mihail.popa@gmail.com</a>> wrote:</div><br></div>
<blockquote type="cite"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div><div><div dir="ltr">Hi.<div><br></div><div>I have amended the patch by removing white space trimming and replacing grep with FileCheck.</div>
<div><br></div><div>Regards,</div><div>Mihai</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 16, 2013 at 7:47 PM, Quentin Colombet<span> </span><span dir="ltr"><<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>></span><span> </span>wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Oh yes, that’s true. I’ve forgotten about that!<div>
<span><font color="#888888"><br><div><div style="text-indent:0px;letter-spacing:normal;font-variant:normal;text-align:-webkit-auto;font-style:normal;font-weight:normal;line-height:normal;text-transform:none;font-size:medium;white-space:normal;font-family:Helvetica;word-wrap:break-word;word-spacing:0px">
-Quentin</div></div></font></span><div><br><div><div>On Apr 16, 2013, at 11:36 AM, Tim Northover <<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>> wrote:</div><br><blockquote type="cite">
<div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote type="cite">Maybe I’m missing something but you’ve defined a new isImm0_4 method but<br>
you’re never using it.<br></blockquote><br>That's automatically used by TableGen based on the Name member in the<br>AsmOperandClass, isn't it?<br><br>Tim.</div></blockquote></div><br></div></div></div><br>_______________________________________________<br>
llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div></div><span><LLVM-713.hints.patch></span>_______________________________________________<div><br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></div></blockquote></div></div></div></blockquote></div></div></div></blockquote>
</div><br></div></div></div></div></div></blockquote></div><br></div>