opus - Aug 2017 - [PATCH] fix alignment exceptions

Hi,

Please find attached a patch to fix alignment exceptions. Without this
change, we were seeing occasional alignment faults when using this with
clang.

Thanks,
Felicia
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20170818/e0d6bb06/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: e5c277c.diff
Type: text/x-patch
Size: 916 bytes
Desc: not available
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20170818/e0d6bb06/attachment.bin>

This would revert a patch I submitted two years ago
(http://git.xiph.org/?p=opus.git;a=commitdiff;h=1d60b49e9d95672a17ebe5578319c59fa3963224).

At the time, clang produced an unnecessary MOVQ instruction when it compiled
with the version of the code with the explicit _mm_loadl_epi64 intrinsic.  Do
you no longer see that?

How does the code compile for you, and what is the issue you’re seeing?  (One
issue might be that clang’s address sanitizer isn’t smart enough to know that
PMOVSXWD only loads 8 bytes, despite _mm_cvtepi16_epi32’s argument being an
__mm128i; I’ve seen it trigger incorrect out-of-bounds read errors.)

On Aug 18, 2017, at 12:34 PM, Felicia Lim <flim at google.com<mailto:flim
at google.com>> wrote:

Hi,

Please find attached a patch to fix alignment exceptions. Without this change,
we were seeing occasional alignment faults when using this with clang.

Thanks,
Felicia
<e5c277c.diff>_______________________________________________
opus mailing list
opus at xiph.org<mailto:opus at xiph.org>
http://lists.xiph.org/mailman/listinfo/opus

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20170818/fa80aab9/attachment.html>

We see the MOVQ instruction but this patch deliberately uses it rather than
MOVQDA (load 128-bits aligned). We were seeing that with the trace below,
the final invocation is not 128-bit aligned but MOVQDA insists on it (the
calling function was pitch_sse4_1.c:90, in the 4-way N - i >= 4 loop).

07-31 11:00:13.469   210  2540 <(469)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3deb0 y 0xeff3deb0 N 32
07-31 11:00:13.469   210  2540 <(469)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3d7b0 y 0xeff3d7b0 N 32
07-31 11:00:13.469   210  2540 <(469)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3df30 y 0xeff3df30 N 32
07-31 11:00:13.470   210  2540 <(470)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3d850 y 0xeff3d850 N 48
07-31 11:00:13.470   210  2540 <(470)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3dfd0 y 0xeff3dfd0 N 48
07-31 11:00:13.470   210  2540 <(470)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3d8b0 y 0xeff3d8b0 N 64
07-31 11:00:13.470   210  2540 <(470)%20210-2540> D opus_sse1: RBE
celt_inner_prod_sse4_1: x 0xeff3e030 y 0xeff3e030 N 64
07-31 11:00:13.476   210  2540 D opus_sse1: RBE celt_inner_prod_sse4_1: x
0xeff3da38 y 0xeff3da38 N 36


On Fri, Aug 18, 2017 at 11:44 AM Jonathan Lennox <jonathan at vidyo.com>
wrote:
> This would revert a patch I submitted two years ago (
>
http://git.xiph.org/?p=opus.git;a=commitdiff;h=1d60b49e9d95672a17ebe5578319c59fa3963224
> ).
>
> At the time, clang produced an unnecessary MOVQ instruction when it
> compiled with the version of the code with the explicit _mm_loadl_epi64
> intrinsic.  Do you no longer see that?
>
> How does the code compile for you, and what is the issue you’re seeing?
>  (One issue might be that clang’s address sanitizer isn’t smart enough to
> know that PMOVSXWD only loads 8 bytes, despite _mm_cvtepi16_epi32’s
> argument being an __mm128i; I’ve seen it trigger incorrect out-of-bounds
> read errors.)
>
> On Aug 18, 2017, at 12:34 PM, Felicia Lim <flim at google.com> wrote:
>
> Hi,
>
> Please find attached a patch to fix alignment exceptions. Without this
> change, we were seeing occasional alignment faults when using this with
> clang.
>
> Thanks,
> Felicia
>
> <e5c277c.diff>_______________________________________________
> opus mailing list
> opus at xiph.org
> http://lists.xiph.org/mailman/listinfo/opus
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xiph.org/pipermail/opus/attachments/20170818/55dd7ab9/attachment.html>