<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">Hi Renato,<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">Thanks for your review!<br>
</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">If you look at the test cases I added, you would be able to understand what the problem is. <br>
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">I added 4 new tests cases, and they are for writeback versions of vst3.v1i64, vst4.v1i64, vld3.v1i64 and vld4.v1i64. Originally they all trigger assertion failures.<br>
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">For example, before my code change, we can only generate "vld1.64    {d16, d17, d18}, [r1:64]", and would fail to generate "vld1.64    {d16, d17, d18}, [r1:64]!"<br>
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">For llvm.arm.neon.vld3.v1i64 plus 3*64 offset writeback, we can use vld1 with writeback to implement the same functionality, because ARM instruction vld1 accept 64-bit vector list.<br>
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">If you look at function ARMDAGToDAGISel::SelectVLD, you may notice there are some comments highlighting some hacking code as below,<br>
<br>// We use a VST1 for v1i64 even if the pseudo says vld2/3/4, so<br>// check for that explicitly too. Horribly hacky, but temporary.<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">
I don't know when this was introduced, and unfortunately this introduced bugs, because ARM::VST1q64wb_fixed is not a writeback version SDNode at all. This is why you can see the SDNode changes in ARMDAGToDAGISel::Select as well. And in the same way I changed ARM::VLD1q64wb_fixed to ARM::VLD1d64QPseudoWB_fixed.<br>
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">In order to make register allocator and scheduler works to correctly allocate vector list, we have to use Pseudo ISD node. This is why we have a mapping from VLD1d64QPseudoWB_fixed to ARM::VLD1d64Qwb_fixed in file ARMExpandPseudoInsts.cpp.<br>
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">Also, there are some other FIXME comments in ARMDAGToDAGISel::SelectVLD as well like below,<br><br>// FIXME: VLD1/VLD2 fixed increment doesn't need Reg0. Remove the reg0<br>
// case entirely when the rest are updated to that form, too.<br><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">This is why we have to introduce a mapping from _fixed version to _register version in function getVLDSTRegisterUpdateOpcode. I think this mapping is really tricky. To avoid the inconsistency between getVLDSTRegisterUpdateOpcode and the code of adding Reg0 operand in ARMDAGToDAGISel::SelectVLD, I introduced two new function isVLDfixed and isVSTfixed to cover all _fixed ISDNode. they are used in two places. One is for assertion in getVLDSTRegisterUpdateOpcode and the other is the condition code of adding Reg0 in ARMDAGToDAGISel::SelectVLD. <br>
</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">Hopefully, you can understand what the problem is now.<br>
<br>Thanks,<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">-Jiangning<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">
<br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014/1/15 Renato Golin <span dir="ltr"><<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Hi Jiangning,<div><br></div><div>Can you elaborate on what the problem was and how you fixed? I have to say I'm a bit lost just by looking at the code.</div><div><br></div><div>Also, you seem to have changed the file mode to executable, you might want to rectify that before commit. ;)</div>

<div><br></div><div>cheers,</div><div>--renato</div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On 13 January 2014 10:08, Jiangning Liu <span dir="ltr"><<a href="mailto:liujiangning1@gmail.com" target="_blank">liujiangning1@gmail.com</a>></span> wrote:<br>

</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">
Hi,</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">
<br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;font-size:small">
Attached patch is to fix the assertion failures exposed by some ld/st instructions requiring writeback. Review, please!</div><span><font color="#888888"><div><br></div>-- <br><div dir="ltr"><font face="courier new, monospace">Thanks,</font><div>

<font face="courier new, monospace">-Jiangning</font></div>
</div>
</font></span></div>
<br></div></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><br>
<br></blockquote></div><br></div>
</blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr"><font face="courier new, monospace">Thanks,</font><div><font face="courier new, monospace">-Jiangning</font></div></div>
</div>