Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile fails when ARCH_HAVE_SSE #28

Closed
snorkellingcactus opened this issue Jan 26, 2017 · 6 comments
Closed

Compile fails when ARCH_HAVE_SSE #28

snorkellingcactus opened this issue Jan 26, 2017 · 6 comments

Comments

@snorkellingcactus
Copy link

Hi. I am on a sse-only machine. I noticed that in test_unit_rotation.c and test_unit_mathops.c some files that could use celt_inner_prod_sse by macro expansion of celt_inner_prod like pitch.c and vq.c are included before pitch_sse.c resulting in errors.

I only changed the order of the includes and it compiles fine. I'm not sure but also could be needed the inclusion of vq.c after pitch.c.

Maybe unrelated, but i don't know much about c/c++ but i like to learn and i wonder why the compiler could mark a macro function as an implicit declaration of that function. ¿do you know?. It happened to me on pitch_sse.h:115.

I think it's not needed but i will atach a build log and a little patch.

Thanks.

@rillian
Copy link
Contributor

rillian commented Jan 27, 2017

I can reproduce by disabling the OPUS_CHECK_INTRINSICS([SSE2],...) call around configure.ac line 559. The tests just assume SSE2 is available.

I worry that your patch is just disabling the tests, rather than fixing the macros to use the right variant on your system.

Which gcc is this that doesn't support anything past SSE? The other side of our C89 compatibility commitment, I guess. :)

@snorkellingcactus
Copy link
Author

The compiler supports sse and all typical stuff. But i'm using icecc for distributed compilation so i can't use -march=native directly. Instead i set the explicit C[XX]FLAGS that the compiler sends to the preprocessor when i pass -march=native. Because of this you can see some -mno-sseN and a bunch of C[XX]FLAGS.

The compiler is GCC 5.4.0.

I build opus on an Intel Celeron which have sse only. ¿isn't celt_inner_prod_sse the right expansion for it?.

I can try to compile it without icecc or custom C[XX]FLAGS and see if i can reproduce the error.

Thanks.

@jmvalin
Copy link
Member

jmvalin commented May 30, 2017

I think it should work fine in master now, since this fix landed: https://git.xiph.org/?p=opus.git;a=commit;h=4507637c
Can you confirm it fixes the issue you're reporting?

@rillian
Copy link
Contributor

rillian commented Jun 6, 2017

I repeated the reproduction test and the issue is resolved for me. Feel free to reopen if the there are still issues for your setup.

@rillian rillian closed this as completed Jun 6, 2017
@snorkellingcactus
Copy link
Author

Looks like it's solved. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants