build: fix missing sse42 in depends builds #10971

1. 
   theuni commented at 9:27 PM on August 1, 2017: member

   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.

2. MarcoFalke added the label Build system on Aug 1, 2017
3. laanwj added this to the milestone 0.15.0 on Aug 2, 2017
4. 
   laanwj commented at 11:48 AM on August 2, 2017: member

   But what if CXXFLAGS is overridden manually with a depends build? (or is that not supported/possible)

5. 
   ryanofsky commented at 2:35 PM on August 3, 2017: member

   But what if CXXFLAGS is overridden manually with a depends build? (or is that not supported/possible)

   It does seem to be possible:

   https://github.com/bitcoin/bitcoin/blob/e222618a32a16cf0efae392e6349c10c38e57db6/depends/config.site.in#L93

   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.

6. 
   theuni commented at 5:57 PM on August 3, 2017: member

   Yep, what @ryanofsky said. I was trying to keep this simple.

   I think that's a good suggestion though, will do.

7. build: always attempt to enable targeted sse42 cxxflags

   This avoids a counter-intuitive drop in performance when manually adjusting the
   flags.

   9baca41985
8. theuni force-pushed on Aug 4, 2017
9. 
   theuni commented at 7:45 PM on August 4, 2017: member

   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.

10. theuni renamed this:
    build: fix missing warnings and sse42 in depends builds
    build: fix missing sse42 in depends builds
    on Aug 4, 2017
11. 
    laanwj commented at 11:18 AM on August 5, 2017: member

    Not coupling sse42 to CXXFLAGS override seems like a better solution to me, too. Thanks. utACK 9baca41

12. laanwj merged this on Aug 5, 2017
13. laanwj closed this on Aug 5, 2017

    ---

14. laanwj referenced this in commit 88b1e4bc0e on Aug 5, 2017
15. PastaPastaPasta referenced this in commit 84dd420fdc on Aug 6, 2019
16. PastaPastaPasta referenced this in commit 58666c6e48 on Aug 6, 2019
17. PastaPastaPasta referenced this in commit b52c12ee96 on Aug 6, 2019
18. PastaPastaPasta referenced this in commit b48f8d2e00 on Aug 7, 2019
19. PastaPastaPasta referenced this in commit 189edeeae1 on Aug 8, 2019
20. PastaPastaPasta referenced this in commit a0fb110e98 on Aug 12, 2019
21. barrystyle referenced this in commit 000d5230cc on Jan 22, 2020
22. DrahtBot locked this on Sep 8, 2021

Contributors

theunilaanwjryanofsky

Milestone
0.15.0