build: fix missing sse42 in depends builds #10971
Conversation
|
But what if CXXFLAGS is overridden manually with a depends build? |
There was a problem hiding this comment.
But what if CXXFLAGS is overridden manually with a depends build?
(or is that not supported/possible)
It does seem to be possible:
bitcoin/depends/config.site.in
Line 93 in e222618
Could handle this case by adding a line to config.site like USER_CXXFLAGS="$CXXFLAGS" and changing the condition added here to && test "x${USER_CXXFLAGS+set}" = x -o -n "$USER_CXXFLAGS"
Or if this is too complicated, it would be good to add a comment explaining the ac_site_file check, since it is slightly broken.
|
Yep, what @ryanofsky said. I was trying to keep this simple. I think that's a good suggestion though, will do. |
This avoids a counter-intuitive drop in performance when manually adjusting the flags.
theuni
force-pushed
the
fix-config-override
branch
from
4397a5b to
9baca41
Compare
|
After experimenting with this for far too long, I've decided that it just doesn't make sense to tie sse42 to CXXFLAGS. Gitian, for example, sets the CXXFLAGS manually in order to enable debugging. Linux distros will probably set them to "-O2" by default to reduce the object size. Gentoo will have some value set by the user's flags. Etc. All of those would end up with quietly disabled crc optimizations. Instead, I think we should just enable this flags if they work, possibly behind configure switches. For 0.15, I propose that we just keep it simple and always check for sse42, and punt worrying about the warnings until 0.16. |
theuni
changed the title
|
Not coupling sse42 to CXXFLAGS override seems like a better solution to me, too. Thanks. |
9baca41 build: always attempt to enable targeted sse42 cxxflags (Cory Fields) Pull request description: For depends builds without this, configure thinks that the user has overridden CXXFLAGS manually, when really they've just been set by the config.site. The effect is that warnings and extra cxxflags (sse4.2 for example) were not being added. Tree-SHA512: 9fd615ad0e926bd9d6b541ffcf7fc555e2147e8761f57ff3b5fb5d196c9cef0f26aa99681ff72db8c83c0f9a7ed91f4253f46bab09f2c835044b68047358fa47
9baca41 build: always attempt to enable targeted sse42 cxxflags (Cory Fields) Pull request description: For depends builds without this, configure thinks that the user has overridden CXXFLAGS manually, when really they've just been set by the config.site. The effect is that warnings and extra cxxflags (sse4.2 for example) were not being added. Tree-SHA512: 9fd615ad0e926bd9d6b541ffcf7fc555e2147e8761f57ff3b5fb5d196c9cef0f26aa99681ff72db8c83c0f9a7ed91f4253f46bab09f2c835044b68047358fa47
4 participants
For depends builds without this, configure thinks that the user has overridden CXXFLAGS manually, when really they've just been set by the config.site.
The effect is that warnings and extra cxxflags (sse4.2 for example) were not being added.