<div dir="ltr"><span style="font-size:12.8px">Hi Jean-Marc, (forgot cc opus@)</span><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Thanks for creating the unit test code.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Attached is the updated optimization patch.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 13, 2017 at 10:17 AM, Jean-Marc Valin <span dir="ltr"><<a href="mailto:jmvalin@jmvalin.ca" target="_blank">jmvalin@jmvalin.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 13/02/17 01:09 PM, Linfeng Zhang wrote:<br>
> For 1), I agree that an explicit unit test would be a good plus to cover<br>
> the cases that "make check" cannot trigger. If you like, we may submit<br>
> an unit test patch for code review.<br>
<br>
</span>Yes, please include a unit test that triggers the overflow detection.<br>
Once that works, I think we can merge this optimization.<br>
<br>
Cheers,<br>
<br>
        Jean-Marc<br>
<span class=""><br>
> Thanks,<br>
> Linfeng<br>
><br>
> On Thu, Feb 9, 2017 at 4:48 PM Jean-Marc Valin <<a href="mailto:jmvalin@jmvalin.ca">jmvalin@jmvalin.ca</a><br>
</span><div><div class="h5">> <mailto:<a href="mailto:jmvalin@jmvalin.ca">jmvalin@jmvalin.ca</a>>> wrote:<br>
><br>
>     Hi Linfeng,<br>
><br>
>     Can you confirm that you the patch went through the same internal review<br>
>     (presumably from James) than the previous ones?<br>
><br>
>     I had a look and did some testing and it looked good to me. There's only<br>
>     two issues I'd like to resolve first -- none of which directly related<br>
>     to your code.<br>
><br>
>     1) The overflow condition is essentially untested because none of the<br>
>     tests in "make check" reliably triggers it. That may be a good case for<br>
>     an explicit unit test. IIRC, the case could be triggered by the<br>
>     following input vector:<br>
>     A_QA[] = { 46596096, -72118272, 78532608, -69447680, 52707328,<br>
>     -22073344, -19890176, 50507776, -54829056, 45518848, -33939456,<br>
>     21086208, -7127040, -4136960, 3993600, -1699840 }<br>
><br>
>     2) I'm not quite sure what to make of silk_LPC_inverse_pred_gain_<wbr>Q24().<br>
>     It seems to never be called anywhere -- except from the MIPS code. Maybe<br>
>     it should just stay as it is (not renamed to _c()) but I need to check.<br>
>     Any thoughts?<br>
><br>
>     Cheers,<br>
><br>
>             Jean-Marc<br>
><br>
>     On 07/02/17 04:06 PM, Linfeng Zhang wrote:<br>
>     > Hi,<br>
>     ><br>
>     > Attached is a patch with arm neon optimizations for<br>
>     > silk_LPC_inverse_pred_gain(). Please review.<br>
>     ><br>
>     > Thanks,<br>
>     > Linfeng<br>
>     ><br>
>     ><br>
>     ><br>
>     > ______________________________<wbr>_________________<br>
>     > opus mailing list<br>
</div></div>>     > <a href="mailto:opus@xiph.org">opus@xiph.org</a> <mailto:<a href="mailto:opus@xiph.org">opus@xiph.org</a>><br>
>     > <a href="http://lists.xiph.org/mailman/listinfo/opus" rel="noreferrer" target="_blank">http://lists.xiph.org/mailman/<wbr>listinfo/opus</a><br>
>     ><br>
><br>
</blockquote></div><br></div>