[opus] [PATCH] Optimize silk_LPC_inverse_pred_gain() for ARM NEON

[Jean-Marc Valin]

Hi Linfeng,

Thanks for the updated patch. Just pushed it to master. One thing that
still bothers me a bit is that the
if( ( max > 0 ) || ( min < -1 ) )
line is still pretty much untested. By which I mean that if I remove the
condition, then the tests (including the new unit tests) still pass. I
wasn't able to figure out a case that triggers it -- and I'm not even
100% sure it's actually possible. It would be nice if you could look
into that, but I didn't think it was worth holding up your patch over that.

Cheers,

	Jean-Marc

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