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

Bug: Undefined symbol _roundeven #10475

Closed
straight-shoota opened this issue Mar 6, 2021 · 11 comments · Fixed by #10479
Closed

Bug: Undefined symbol _roundeven #10475

straight-shoota opened this issue Mar 6, 2021 · 11 comments · Fixed by #10479

Comments

@straight-shoota
Copy link
Member

straight-shoota commented Mar 6, 2021

#10413 introduced Number#round_even which binds to the LLVM intrinsic roundeven which was only introduced in LLVM 11. For older LLVM versions it falls back to rint.
It seems that the intrinsic is not (yet) available on darwin, for some reason.


I can't compile the compiler anymore :-(

Undefined symbols for architecture x86_64:
  "_roundeven", referenced from:
      _*Float64#round_even:Float64 in F-loat64.o

This is on Mac OSX, LLVM 11.1.0

Originally posted by @asterite in #10474 (comment)

@asterite
Copy link
Member

asterite commented Mar 6, 2021

Thank you!

@straight-shoota
Copy link
Member Author

The roundeven intrinsic should be available since LLVM 11.0. For older LLVM versions it is not even used. So I really can't figure why you get the same error with LLVM 10 which uses rint instead.

This might have gone unnoticed because of #10110. But it seems the nightly jobs succeeded to build the compiler (artifacts). So it might be an individual issue on your end.
Can others with a mac try to reproduce?

I'm currently running a re-activated test_darwin on circle-ci.

A quick would obviously be to use rint unconditionally (at least on darwin). It's not strictly necessary to use roundeven.

--- i/src/float.cr
+++ w/src/float.cr
@@ -156,7 +156,7 @@ struct Float32
   def round_even : self
     # LLVM 11 introduced llvm.roundeven.* intrinsics which may replace rint in
     # the future.
-    {% if compare_versions(Crystal::LLVM_VERSION, "11.0.0") >= 0 %}
+    {% if compare_versions(Crystal::LLVM_VERSION, "11.0.0") >= 0 && !flag?(:darwin) %}
       LibM.roundeven_f32(self)
     {% else %}
       LibM.rint_f32(self)
@@ -257,7 +257,7 @@ struct Float64
   def round_even : self
     # LLVM 11 introduced llvm.roundeven.* intrinsics which may replace rint in
     # the future.
-    {% if compare_versions(Crystal::LLVM_VERSION, "11.0.0") >= 0 %}
+    {% if compare_versions(Crystal::LLVM_VERSION, "11.0.0") >= 0 && !flag?(:darwin) %}
       LibM.roundeven_f64(self)
     {% else %}
       LibM.rint_f64(self)

@straight-shoota
Copy link
Member Author

Indeed, the error reproduces on circleci: https://app.circleci.com/pipelines/github/crystal-lang/crystal/5401/workflows/4e3883a7-5f92-4b6c-aa4b-3bf02eb2f086/jobs/58514

I'll try to poke around with SSH on the CI runner. Don't have a mac around.

@straight-shoota
Copy link
Member Author

straight-shoota commented Mar 6, 2021

I couldn't install LLVM 10 with brew, so I tested with LLVM 9 and that sure works. So the LLVM 10 failure needs more inspection. Could you verify that it is indeed broken? It just seems very unlikely.

The patch from #10475 (comment) fixes for LLVM 11 as expected.

Oh and the nightly builds still link against LLVM 6 (I think), that's why the nightly job didn't error.

@maxfierke
Copy link
Contributor

Can reproduce on darwin (macOS Big Sur), and also ran into it on one of the BSDs when I was testing #10463. (I believe it was OpenBSD. NetBSD might also be affected.)

@asterite
Copy link
Member

asterite commented Mar 8, 2021

Given that using roundeven may not be supported in some platforms while rint seems to be well supported (otherwise we would have known by now), and that the advantages of roundeven over rint doesn't seem to be clear ("slightly improved semantics" doesn't seem to be a good reason if we can't understand exactly how it's better).

Put another way: let's write a benchmark for roundeven and rint, and also compare their semantics. If they both behave exactly the same way, I see no reason to choose roundeven in any case. If rint is removed in the future, then by all means yes, but that's not the case right now.

@straight-shoota
Copy link
Member Author

For reference, this is the feature review at LLVM: https://reviews.llvm.org/D75670
There's also a current PR pending to add bindings to Rust: rust-lang/rust#82273

@asterite
Copy link
Member

asterite commented Mar 8, 2021

Somewhere in the first link it says:

In terms of LLVM IR, the unconstrained intrinsics llvm.rint, llvm.nearbyint, and llvm.roundeven are semantially identical. Backends should be able to lower them all the same way. If not, that's a bug. For this reason, I'm not sure we need all three.

I think it would be great to use roundeven just after we know it's available in platforms where rint is already working.

@straight-shoota
Copy link
Member Author

Just to fill in: I think what's happening is that if LLVM doesn't have an impl for the intrinsic, it lowers to a call to roundeven. That's why we get an "undefined symbol" for an intrinsic.
It seems that libm on darwin currently doesn't provide roundeven. And similar for other BSDs per #10475 (comment).

So "Backends should be able to lower them all the same way" doesn't seem to be true, unfortunately.

@asterite
Copy link
Member

asterite commented Mar 8, 2021

Yes, it's the usual unfulfilled promise of "if you use LLVM then you don't need to worry about specific platforms" 😞

@SlayerShadow
Copy link

SlayerShadow commented Mar 10, 2021

Hello everyone! May it helps: building Crystal under Alpine, also I built LLVM from sources, release version 11.0.0.

(Update: right now I compiled it with LLVM 11.1.0).

Using recursive docker build that targets a master branch I got this error (note: with LLVM 11).

Now for compiling compiler:

  • With make clean crystal release=1 or make clean crystal release=1 static=1 the error looks very similar to current CI error on Alpine:
Invalid memory access (signal 11) at address 0x56117d1db1d2
[0x56107c3dbac6] ???
[0x56107c3326f2] __crystal_sigfault_handler +514
[0x56107d180fbd] sigfault_handler +40
[0x7f3564506c4b] ???
  • With make clean crystal I got this:
CRYSTAL_CONFIG_LIBRARY_PATH="" CRYSTAL_CONFIG_BUILD_COMMIT="9a3d0c4" SOURCE_DATE_EPOCH="1615364351" ./bin/crystal build  -o .build/crystal src/compiler/crystal.cr -D without_openssl -D without_zlib
/usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: F-loat64.o: in function `round_even':
/sources/crystal/src/float.cr:260: undefined reference to `roundeven'
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc "${@}" -o /sources/crystal/.build/crystal  -rdynamic /sources/crystal/src/llvm/ext/llvm_ext.o `"/usr/local/llvm/bin/llvm-config" --libs --system-libs --ldflags 2> /dev/null` -lstdc++ -lpcre -lm -lgc -lpthread /sources/crystal/src/ext/libcrystal.a -levent  -lrt`
make: *** [Makefile:122: .build/crystal] Error 1

LLVM 11.1 does not supported for now but please note that I got this error also with LLVM 11.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants