Skip to content

x86-64 SIMD build fixes#20

Merged
WayneD merged 2 commits into
RsyncProject:masterfrom
Chainfire:CSUM1-BUILD
Jun 18, 2020
Merged

x86-64 SIMD build fixes#20
WayneD merged 2 commits into
RsyncProject:masterfrom
Chainfire:CSUM1-BUILD

Conversation

@Chainfire
Copy link
Copy Markdown
Contributor

configure.ac was modified to detect g++ 5+ and clang++ 7+. Additionally
some script malfunctions on FreeBSD were corrected.

The get_checksum1() code has been modified to fix clang and g++ 10
compilation.

This version of the code and configure.ac has been tested on:

Ubuntu 16 - gcc 7.3.0, clang 6.0.0
Debian 10 - gcc 5.4.0, 6.4.0, 7.2.0, 8.4.0, 9.2.1, 10.0.1, clang 5.0.2,
6.0.1, 7.0.1, 8.0.0, 9.0.0, 10.0.0
ArchLinux 20200605 - gcc 10.1.0, clang 10.0.0
FreeBSD 12.1 - gcc 9.3.0, clang 8.0.1

It is unknown if it will work on gcc 5.0-5.3, but the script currently
allows it.

configure.ac was modified to detect g++ 5+ and clang++ 7+. Additionally
some script malfunctions on FreeBSD were corrected.

The get_checksum1() code has been modified to fix clang and g++ 10
compilation.

This version of the code and configure.ac has been tested on:

Ubuntu 16 - gcc 7.3.0, clang 6.0.0
Debian 10 - gcc 5.4.0, 6.4.0, 7.2.0, 8.4.0, 9.2.1, 10.0.1, clang 5.0.2,
6.0.1, 7.0.1, 8.0.0, 9.0.0, 10.0.0
ArchLinux 20200605 - gcc 10.1.0, clang 10.0.0
FreeBSD 12.1 - gcc 9.3.0, clang 8.0.1

It is unknown if it will work on gcc 5.0-5.3, but the script currently
allows it.
@Chainfire
Copy link
Copy Markdown
Contributor Author

See #13

@hhoffstaette
Copy link
Copy Markdown
Contributor

hhoffstaette commented Jun 18, 2020

Can confirm that SIMD now builds with gcc-10 / clang-10 using various optimizations on Gentoo amd64 and without having to futz with -march flags, despite building on an older non-AVX2 CPU. Great work, thank you!

@hhoffstaette
Copy link
Copy Markdown
Contributor

Also thanks for the built-in benchmark/sanity check, it clearly shows that everything works correctly and with multiversion selection. 😃

@eworm-de
Copy link
Copy Markdown
Contributor

I can confirm this builds in a clean Arch Linux build environment with default flags. Thanks a lot!

@Chainfire
Copy link
Copy Markdown
Contributor Author

Can confirm that SIMD now builds with gcc-10 / clang-10 using various optimizations on Gentoo amd64 and without having to futz with -march flags, despite building on an older non-AVX2 CPU. Great work, thank you!

Thanks for pointing out the problems. Especially GCC 10 is an interesting case, as it has problems not present on GCC 5-9. There's a regression in GCC 10's multiversion function handling, I should probably submit that to the GCC maintainers.

Also thanks for the built-in benchmark/sanity check, it clearly shows that everything works correctly and with multiversion selection. 😃

No worries. I had the code externally, thought I might as well put it in. Depending on your configure flags it might not work but I guess anyone who really wants to test can figure it out.

@WayneD
Copy link
Copy Markdown
Member

WayneD commented Jun 18, 2020

Keep in mind that the shell that is running configure cannot be assumed to be bash, so things like $(...) instead of `...` are not allowed. I'm editing those out.

@hhoffstaette
Copy link
Copy Markdown
Contributor

hhoffstaette commented Jun 18, 2020

Keep in mind that the shell that is running configure cannot be assumed to be bash

This made me curious, so I checked and $(..) is POSIX, see the bash FAQ on this.

@Chainfire
Copy link
Copy Markdown
Contributor Author

Keep in mind that the shell that is running configure cannot be assumed to be bash, so things like $(...) instead of ... are not allowed. I'm editing those out.

I did not know this; but the FreeBSD I tested on doesn't even have bash and it worked fine.

Either way, seems reasonable :)

Use backticks for a subprocess & case for simple wildcard match & tabs for indentation.
@WayneD WayneD merged commit 4f539cc into RsyncProject:master Jun 18, 2020
@Chainfire
Copy link
Copy Markdown
Contributor Author

Nitpicking maybe, but just in case clang ever changes their description to clang++ in the output, shouldn't the clang case come first? This is why my submit checked for ^g++ rather than just g++.

@WayneD
Copy link
Copy Markdown
Member

WayneD commented Jun 18, 2020

The match is anchored at the start of the string. A case match of g++*) not *g++*)

@WayneD
Copy link
Copy Markdown
Member

WayneD commented Jun 18, 2020

Thanks for the fixes! Seems like it's a good deal more portable now.

@Chainfire
Copy link
Copy Markdown
Contributor Author

Chainfire commented Jun 18, 2020

I see. In that case it breaks FreeBSD clang compilation, which uses this version string:

FreeBSD clang version 8.0.1 (tags/RELEASE_801/final 366581) (based on LLVM 8.0.1)

Hence my grep for clang not having ^. I did just test this (your version), and it does break.

Built successfully (your version) on all the other mentioned distros/gcc/clang combinations mentioned in the commit message (if they satisfy version requirements).

@WayneD
Copy link
Copy Markdown
Member

WayneD commented Jun 18, 2020

Oops, I missed that it was unanchored. Fixed.

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

Successfully merging this pull request may close these issues.

4 participants