flac dev - Jul 2014 - [PATCH] two patches of doubtful usefulness

1)
lpc.c, FLAC__lpc_quantize_coefficients():

This function declares "const int nshift = -(*shift)" variable
when *shift is less than 0. Then nshift is used in the loop:

     for(i = 0; i < order; i++) {
         error += lp_coeff[i] / (1 << nshift);

This patch adds "const int pshift = *shift" variable.

Pros:
* more symmetry for two branches
* compiler doesn't know that lp_coeff[] and *shift
don't alias, so the code for
     error += lp_coeff[i] * (1 << *shift);
is less efficient than for
     error += lp_coeff[i] * (1 << pshift);

Cons:
* the speed increase is probably negligible



2)
There's the following code in stream_decoder.c:

     if(order <= 8)
         decoder->private_->local_lpc_restore_signal_16bit_order8( ... );
     else
         decoder->private_->local_lpc_restore_signal_16bit( ... );

but local_lpc_restore_signal_16bit is equal to ..._order8 for all
architectures except powerpc/altivec.

The patch hides local_lpc_restore_signal_16bit_order8 under
#ifdef FLAC__CPU_PPC / #endif directives.


Pros:
* one branch less

Cons:
* the same as for the 1st patch: the speed increase is probably negligible
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lpc_shift.patch
Type: application/octet-stream
Size: 506 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20140702/bee98afb/attachment.obj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ppc_order8.patch
Type: application/octet-stream
Size: 4463 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20140702/bee98afb/attachment-0001.obj

lvqcl wrote:
> 1)
> lpc.c, FLAC__lpc_quantize_coefficients():
<snip>
> 2)
> There's the following code in stream_decoder.c:
Like you, I don't see a lot of value in these. I think I'll decline
these.

Cheers,
Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Erik de Castro Lopo wrote:
>> There's the following code in stream_decoder.c:
>
> Like you, I don't see a lot of value in these. I think I'll decline
> these.
FLAC__lpc_restore_signal_asm_ia32_mmx compares 'order' argument with 4
and if it's greater then it jumps to FLAC__lpc_restore_signal_asm_ia32.

I wonder why the same wasn't done for PPC/Altivec: why libFLAC compares
'order' and 8 in C code and not in asm.