Cristian RodrÃguez
2012-May-04 15:13 UTC
[flac-dev] [Flac-dev] Git branch with compiling fixes for win32
El 03/05/12 12:19, Miroslav Lichvar escribi?:> Hi Josh, > > nice to see you here again. > > On Wed, Apr 25, 2012 at 04:26:05PM -0700, Josh Coalson wrote: >> (Jumping in again, maybe at the wrong point since this doesn't seem >> to involve encoding, but here goes.) >> >> Miroslav's patches have always been high-quality for sure. But >> regardless of submitter, any patch that affects encoding must be >> reviewed very carefully, preferably by several other people and >> definitely me. If there were ever a libFLAC release that had a bug >> and was not always lossless, that would be very damaging to the >> format. > > The bitreader patch touches only the rice decoding code which I > believe is very well covered by the test suite and any bugs would be > quickly seen. Also, it has also been included in the Fedora packages > for several years, no bug reports about MD5 mismatch were received > yet :). > > It makes the C function faster than the corresponding asm routine, so > if it's included I'd suggest to just drop the asm function to not keep > around more asm code than is necessary. > > I'm not sure if anyone is planning to port the asm code to x86_64, I > think that it will be quite a lot of work, perhaps it would be a good > time to reconsider using inline assembly instead of nasm to minimize > the amount of asm code? It would be useful to know how much are the > individual asm routines actually faster, it has been a long time > since I played with it. >Hi: Both Erick and I did already submitted patches to the tree that do just exactly what your flac-1.2.1-bitreader.patch intended.. please checkout current GIT tree. Cheers!
Miroslav Lichvar
2012-May-04 15:53 UTC
[flac-dev] [Flac-dev] Git branch with compiling fixes for win32
On Fri, May 04, 2012 at 11:13:05AM -0400, Cristian Rodr?guez wrote:> Both Erick and I did already submitted patches to the tree that do just > exactly what your flac-1.2.1-bitreader.patch intended.. please checkout > current GIT tree.The most interesting part of the patch is the rewrite of the FLAC__bitreader_read_rice_signed_block function, which in the git repo seems to have only couple lines changed since 1.2.1. BTW, how much faster is the code with the clz builtin? If the architecture doesn't have the instruction, will be the gcc code as fast as the original code? -- Miroslav Lichvar
Miroslav Lichvar
2012-May-04 16:09 UTC
[flac-dev] [Flac-dev] Git branch with compiling fixes for win32
On Fri, May 04, 2012 at 05:53:23PM +0200, Miroslav Lichvar wrote:> The most interesting part of the patch is the rewrite of the > FLAC__bitreader_read_rice_signed_block function, which in the git repo > seems to have only couple lines changed since 1.2.1.Here is that part of the patch rebased against current git. In a quick test it gives a 10% speedup in decoding. -- Miroslav Lichvar -------------- next part --------------
Cristian RodrÃguez
2012-May-04 19:06 UTC
[flac-dev] [Flac-dev] Git branch with compiling fixes for win32
El 04/05/12 11:53, Miroslav Lichvar escribi?:> On Fri, May 04, 2012 at 11:13:05AM -0400, Cristian Rodr?guez wrote: >> Both Erick and I did already submitted patches to the tree that do just >> exactly what your flac-1.2.1-bitreader.patch intended.. please checkout >> current GIT tree. > > The most interesting part of the patch is the rewrite of the > FLAC__bitreader_read_rice_signed_block function, which in the git repo > seems to have only couple lines changed since 1.2.1.Ah ok, I will check it out !> > BTW, how much faster is the code with the clz builtin? If the > architecture doesn't have the instruction, will be the gcc code as > fast as the original code?I do not have access to a host that does not have either bsr, clz, lzcnt, cntlz or ctlz. if you do, plz share your results ;) I have no idea what code will be generated in exotic archs, in x86 it translates to bsr ^ 31U and in the others supported to a single instruction.