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

Embree is broken if compiled with -march=native #115

Closed
gdonval opened this issue Mar 7, 2017 · 8 comments
Closed

Embree is broken if compiled with -march=native #115

gdonval opened this issue Mar 7, 2017 · 8 comments

Comments

@gdonval
Copy link

gdonval commented Mar 7, 2017

Problem

If compiled with -march=native, embree does compile but does not work (i.e. missing symbols making linking impossible).

Expected behaviour

  1. If compilation fails, it should fail and not report a success so that it gets installed on a system without no one noticing.
  2. embree shouldn't break with -march=native set: no other program I am aware of does, not even OpenBLAS which has been there, done that.

Context and proposed solutions

The problem with things like -march=native is that it is supposed to be safe (yet not portable) so it can be set by default and it is a huge pain to disable it only for that one and only special case.

So I suggest that if the code is too brittle anyway to allow for that flag to be activated that the configuration scripts are changed to sanitize the compilation flags for these specific files.

About failure: there must be something somewhere that is catching the failure, it should be identified and modified so that make reports the failure should it occur.

Versions

  • embree: 2.14.0
  • gcc: 6.3.1
@ingowald
Copy link
Contributor

ingowald commented Mar 7, 2017

Gael,

Embree automatically sets the right build flags during compilation; it has to do so because it internally builds different functions with different vector ISAs, which in turn allows it to eventually select the best (working) ISA during runtime. The same behavior cannot easily be achieved with a single hard-coded ISA target such as "-march=native", which necessarily builds all functions with the same ISA.

Since the proper selection of command-line flags can be a tricky business (and even -march=native isn't supported on all compilers and platforms) the embree project invests significant effort in properly setting all flags for all kind of configurations in its cmake scripts, and these should be ready-to-go without changing them. If you find any compiler/OS/ISA combinations where those included cmake scripts fail please let us know, but something like "-march=native" is not supported.

Finally, let me point out that there is no reason whatsoever to even try and use -march-mative: embree will automatically select the proper ISA for the given CPU type, during runtime. It will, as such, always be a superset of what you could possibly have gotten with -march-native.

Yours,

Ingo

PS:
As to the specific issue you ran into when changing the command line flags: The missing symbols are caused because embree builds certain functions (only) for SSE, some only for AVX, etc; if you explicitly change its build system to only build a single ISA these symbols will be missing.

@ingowald ingowald closed this as completed Mar 7, 2017
@gdonval
Copy link
Author

gdonval commented Mar 7, 2017

Hi,

It seems I was not clear enough... Could you please tell me whether or not you agree with the following:

  1. If something compiles but segfaults or does not link, then that something is broken.
  2. If something sets the "right build flags", yet uses random system-provided flags, then it is not really setting the "right build flags".

What I am asking for is simple:

  1. Throw an error at compile time if symbols are missing.
  2. Use your compile flags and only your compile flags for anything that does not support any change from the user (which is fine: but embrace it completely and forbid anything else :)).

Is that too much to ask? OpenBLAS and FFTW for instance do that very well...

And let me put that in bold, even though I am not trying to be offensive nor rude, I want to be heard:

I am NOT setting -march=native to get more performance out of embree, it is set in my system-wide BUILD SYSTEM because it makes a difference in most of the other things I build BUT disabling that option for embree alone is a pain which is why I ask for this.

@gdonval
Copy link
Author

gdonval commented Mar 7, 2017

By the way, I'll open a new issue every day as long as I am not heard :D

If you have reasons not to accept 1. and 2., then it is fine, but I won't abandon on a misunderstanding. :)

@cbenthin
Copy link
Contributor

cbenthin commented Mar 7, 2017

Is the -march=native visible in the cmake compiler options? If yes, as Ingo said, it will break our compile-for-different-ISA build system so you will end of with something that is broken. Do you constantly need to completely re-configure (and rebuild) Embree or why is disabling the system-wide once setting such a big deal?

@gdonval
Copy link
Author

gdonval commented Mar 8, 2017

Is the -march=native visible in the cmake compiler options?

Yes it does.

If yes, as Ingo said, it will break our compile-for-different-ISA build system so you will end of with something that is broken.

I understand that and the only problem I have with that specific point is that one doesn't know when it is broken.

If it is broken, it must be reported.

Do you constantly need to completely re-configure (and rebuild) Embree or why is disabling the system-wide once setting such a big deal?

I am building/updating embree from Archlinux AUR among many other programs that may or may not benefit from -march=native. Some of them do benefit so much from it that they are more than twice as fast compared to the default version. But embree is the ONLY SINGLE PROGRAM that breaks.

If parts of your source code does not handle user-provided flags, please ignore them completely instead of dictating what the user DEFAULTS should be.

I repeat that I understand why -march=native is failing. I absolutely don't want nor request that the code should be changed to support that flag: what you are doing is fine. All I am asking is that if you need that amount of control anyway, please ignore user-provided flags in cmake at least for these specific files.

@ingowald
Copy link
Contributor

ingowald commented Mar 8, 2017 via email

@svenwoop
Copy link
Contributor

svenwoop commented Mar 9, 2017

Both issues are now fixed in the devel branch and will come into the next release.

  1. ld seems to allow undefined symbols in so files by default. We set the --no-undefined option now, which will avoid embree.so to be generated when there are unresolved symbols.
  2. We by default ignore now user provided CMAKE_CXX_FLAGS, thus you can pass -march=native without issues. Users that need to pass some options can still disable the new EMBREE_IGNORE_CMAKE_CXX_FLAGS cmake option and set further flags (e.g. to disable compile warnings)

@gdonval
Copy link
Author

gdonval commented Mar 15, 2017

Sorry for the delay and for not being that helpful in describing the problem but I am very happy with the result!

Thanks a lot for your time and the awesome fixes!

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

4 participants