flac dev - May 2016 - [PATCH] MSVC2015U2 workaround, version 2

Here's a new version of a patch that fixes a problem with MSVC2105 update2,
but it doesn't disable any optimization, so the resulting encoding
performance should be almost unaffected by this workaround.


MSVC compiles

     abs_residual_partition_sums[partition] =
(FLAC__uint32)_mm_cvtsi128_si32(mm_sum);

into this:

     movq    QWORD PTR [rsi], xmm2

while it should be

     movd    eax, xmm2
     mov     QWORD PTR [rsi], rax

With this patch, MSVC emits

     movq    QWORD PTR [rsi], xmm2
     mov     DWORD PTR [rsi+4], r9d

so the price of this workaround is 1 extra write instruction per partition.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: msvc_bug_v2.patch
Type: application/octet-stream
Size: 2700 bytes
Desc: not available
URL:
<http://lists.xiph.org/pipermail/flac-dev/attachments/20160502/ea751800/attachment.obj>

On 5/2/16, lvqcl <lvqcl.mail at gmail.com> wrote:
> Here's a new version of a patch that fixes a problem with MSVC2105
update2,
> but it doesn't disable any optimization, so the resulting encoding
> performance should be almost unaffected by this workaround.
>
>
> MSVC compiles
>
>      abs_residual_partition_sums[partition] >
(FLAC__uint32)_mm_cvtsi128_si32(mm_sum);
>
> into this:
>
>      movq    QWORD PTR [rsi], xmm2
>
> while it should be
>
>      movd    eax, xmm2
>      mov     QWORD PTR [rsi], rax
>
> With this patch, MSVC emits
>
>      movq    QWORD PTR [rsi], xmm2
>      mov     DWORD PTR [rsi+4], r9d
>
> so the price of this workaround is 1 extra write instruction per partition.
Why not use a 64bit suffix to that 0xFFFFFFFF, e.g. 0xFFFFFFFFi64
to make the intention clear?  (since you are specifically targeting
msvc, the non-portability of i64 suffix shouldn't be a problem.)

--
O.S.

Ozkan Sezer wrote:
> Why not use a 64bit suffix to that 0xFFFFFFFF, e.g. 0xFFFFFFFFi64
> to make the intention clear?  (since you are specifically targeting
> msvc, the non-portability of i64 suffix shouldn't be a problem.)
Maybe it's better to use ui64 suffix then? Also, ULL suffix works too.

But I don't know is it better to use it or not...

lvqcl wrote:
> Here's a new version of a patch that fixes a problem with MSVC2105
update2,
> but it doesn't disable any optimization, so the resulting encoding
> performance should be almost unaffected by this workaround.
Applied. Thanks.

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