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.

Sign up for GitHub

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

Jump to bottom

Build fails on i386 systems: error: no matching function for call to 'make_ul' #136

Closed
yurivict opened this issue Jun 9, 2019 · 10 comments
Closed

Build fails on i386 systems: error: no matching function for call to 'make_ul' #136

yurivict opened this issue Jun 9, 2019 · 10 comments

Comments

@yurivict
Copy link

yurivict commented Jun 9, 2019

See the log here: http://beefy5.nyi.freebsd.org/data/120i386-default/503690/logs/rumur-2019.06.05.log
Smattr added a commit that referenced this issue Jun 9, 2019
@Smattr
fix compilation on platforms where size_t != unsigned long
21b1847 
The make_ul function and its usage was mistakenly assuming that size_t was a
typedef for unsigned long. This is not true on, e.g., i386 FreeBSD. This caused
a compilation failure when building Rumur. To repair this, we remove make_ul
entirely and open code the string-to-number conversion.

Github: closes #136 "Build fails on i386 systems: error: no matching function
  for call to 'make_ul'"
Smattr added a commit that referenced this issue Jun 9, 2019
@Smattr
fix compilation on platforms where size_t != unsigned long
b28fca4 
The make_ul function and its usage was mistakenly assuming that size_t was a
typedef for unsigned long. This is not true on, e.g., i386 FreeBSD. This caused
a compilation failure when building Rumur. Instead of trying to fix this
locally, we turn most of these option variables into GMP mpz_classes. Given that
the platform the verifier is generated on may not be the same as the platform it
is compiled on, it is easier to think of most of these values as unbounded
numbers.

Github: closes #136 "Build fails on i386 systems: error: no matching function
  for call to 'make_ul'"
@Smattr
Copy link
Owner

Smattr commented Jun 9, 2019

Thanks for reporting this. Looks like the code incorrectly assumes size_t is the same as unsigned long. I don't have an i386 machine on which to reproduce this, but I've pushed some changes to the branch bugfix/github-136 that I think should resolve the problem. It hasn't passed Travis CI yet, but could you have a look at the changes in that branch and let me know if they solve your problem?

@yurivict
Copy link
Author

yurivict commented Jun 9, 2019

I don't have an i386 system either, and wouldn't be able to test it.

If you found some obvious problem, please just merge the fix and it will be auto-tested with the next port update.

@Smattr
Copy link
Owner

Smattr commented Jun 9, 2019

Hm a little awkward. I'll see if I have a platform that can bring up an i386 FreeBSD VM to test. Stay tuned...

Smattr added a commit that referenced this issue Jun 10, 2019
@Smattr
fix compilation on platforms where size_t != unsigned long
b04ecbc 
The make_ul function and its usage was mistakenly assuming that size_t was a
typedef for unsigned long. This is not true on, e.g., i386 FreeBSD. This caused
a compilation failure when building Rumur. Instead of trying to fix this
locally, we turn most of these option variables into GMP mpz_classes. Given that
the platform the verifier is generated on may not be the same as the platform it
is compiled on, it is easier to think of most of these values as unbounded
numbers.

Github: closes #136 "Build fails on i386 systems: error: no matching function
  for call to 'make_ul'"
@Smattr
Copy link
Owner

Smattr commented Jun 11, 2019

I brought up an i386 FreeBSD 12 VM and b04ecbc does indeed fix the compilation error. Unfortunately this unmasks the next problem.

The test suite fails because compilation of the generated verifier includes calls to undefined functions __atomic_store_8 and __atomic_load_8. This is a situation I'm familiar with, where the compiler is expecting you to link against libatomic. However, the verifier is intended to be standalone (e.g. we pass -mcx16 on x86_64 to force 128-bit operations to be emitted as cmpxchg16b, avoiding the libatomic call).

I tried to get Clang to emit a cmpxchg8b instead of the libatomic calls, using -march=i586 and upgrading to Clang 8.0.0, but with no success. So I went digging for some details. It seems this problem was independently discovered during RC testing of LLVM on FreeBSD 11 (https://lists.llvm.org/pipermail/llvm-dev/2018-August/125090.html). The general consensus seems to be that the upstream in change in LLVM is actually a correction, but this doesn't help FreeBSD much whose tale seems to lead to this: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230888.

I'm not sure what the best solution here is, as it seems libatomic is not available on FreeBSD and Rumur's generated verifier relies on double-word (64-bit on i386) atomics. We could work around this by taking a mutex, but it would be sort of self-defeating as the purpose of the atomics is to support a lock-free algorithm. As I can't imagine anyone using an i386 machine for the kind of memory-/runtime-intensive verification that Rumur is designed for, I'm inclined to just note we don't support i386 and leave it at that. What do you think?

@yurivict
Copy link
Author

libatomic might be unavailable by mistake.
There are some ports tat are disabled specifically on i386 for a similar/same reason:

astro/siril/Makefile:BROKEN_i386=       undefined reference to `__atomic_load' and `__atomic_compare_exchange'
science/cctbx/bootstrap.pyscience/gromacs/Makefile:BROKEN_i386=   undefined reference to `__atomic_load' and `__atomic_compare_exchange'
science/qmcpack/Makefile:BROKEN_i386=     undefined reference to `__atomic_load'
science/gabedit/Makefile:BROKEN_i386=   undefined reference to `__atomic_load'
science/openmx/Makefile:BROKEN_i386=    undefined reference to `__atomic_load', see bug#229605 and https://reviews.llvm.org/D42154
science/erkale/Makefile:BROKEN_i386=    liberkale_omp.so.0.1.0: undefined reference to `__atomic_compare_exchange'

And there's this bug report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230888

So I suggest to label rumur likewise broken on i386 until this problem is resolved in the system.

@yurivict
Copy link
Author

yurivict commented Jun 11, 2019 •
edited

Please commit your fixes up to the point of this libatomic-related failure.

The port is already labeled BROKEN on i386.

@Smattr
Copy link
Owner

Smattr commented Jun 11, 2019

I toyed with this some more and discovered it's reproducible on non-FreeBSD platforms as well (at least macOS). If you do a 64-bit __atomic_* you end up with calls to ___atomic_*_8 in the emitted code when building with -m32. This all works out on macOS because Clang/LLVM itself provides these instead of relying on libatomic. Even so, this is undesirable as it makes this lock-free code no longer lock-free.

It seems we can force a cmpxchg8 by using the older __sync_*_compare_and_swap. I think we should do this as we do for x86-64. I'll work on implementing this and see if it repairs FreeBSD i386 builds.

Smattr added a commit that referenced this issue Jun 12, 2019
@Smattr
fix compilation on platforms where size_t != unsigned long
38489a8 
The make_ul function and its usage was mistakenly assuming that size_t was a
typedef for unsigned long. This is not true on, e.g., i386 FreeBSD. This caused
a compilation failure when building Rumur. Instead of trying to fix this
locally, we turn most of these option variables into GMP mpz_classes. Given that
the platform the verifier is generated on may not be the same as the platform it
is compiled on, it is easier to think of most of these values as unbounded
numbers.

Github: related to #136 "Build fails on i386 systems: error: no matching
  function for call to 'make_ul'"
Smattr added a commit that referenced this issue Jun 12, 2019
@Smattr
fix: use __sync built-ins for atomic 64-bit ops on i386
0da9825 
On i386, some compilers emit a call to libatomic for __atomic operations on a
64-bit variable. E.g. __atomic_load_n produces a call to ___atomic_load_8. This
causes problems for a couple of reasons:

  1. These macros are intended for use in a lock-free algorithm and the
     libatomic functions take a mutex; and
  2. Some platforms like FreeBSD don't have a libatomic implementation.

Github: related to #136 "Build fails on i386 systems: error: no matching
  function for call to 'make_ul'"
@Smattr
Copy link
Owner

Smattr commented Jun 12, 2019 •
edited

I've just pushed a new release, v2019.06.12, that passes the test suite for me on FreeBSD i386. I'll wait for the Travis tests to pass and then tag the release commit.

By the way: if you're in touch with other FreeBSD devs and/or this is a blocker for other ports, you might want to suggest this __sync workaround if they're open to changing source code to cope with the toolchain.

@Smattr
Copy link
Owner

Smattr commented Jun 13, 2019

The tests passed and I've pushed the release tag. This should be everything to get the FreeBSD i386 port passing I think. Let me know if you have further problems, and thanks again for the packaging work!

@Smattr Smattr closed this as completed Jun 13, 2019
@yurivict
Copy link
Author

Thank you for fixing this issue!
I've just updated the port.

Smattr added a commit that referenced this issue Jun 14, 2019
@Smattr
add some test cases for calls to libatomic
659cab9 
Github: related to #136 "Build fails on i386 systems: error: no matching
  function for call to 'make_ul'"
Smattr added a commit that referenced this issue Jun 15, 2019
@Smattr
add some test cases for calls to libatomic
c26efcd 
Github: related to #136 "Build fails on i386 systems: error: no matching
  function for call to 'make_ul'"
Smattr added a commit that referenced this issue Jun 19, 2019
@Smattr
enable testing 32-bit compilation in Travis
060cf68 
This is useful for running the test_lock_freedom_i386 test.

Github: related to #136 "Build fails on i386 systems: error: no matching
  function for call to 'make_ul'"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Smattr @yurivict