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

Implement round, floor, ceil on the LLVM backend #606

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

jiribenes
Copy link
Contributor

@jiribenes jiribenes commented Sep 25, 2024

Cherry-picked from #542.

I don't really know why the tests fail on CI: the intrinsics are there from at least LLVM 12, and it does work locally...
The CI fails failed with:

/usr/bin/ld: ./out/tests/effekt.llvmtests/doubles.o: in function `round_1113':
doubles.ll:(.text+0xff2): undefined reference to `round'
/usr/bin/ld: ./out/tests/effekt.llvmtests/doubles.o: in function `floor_1118':
doubles.ll:(.text+0x1002): undefined reference to `floor'
/usr/bin/ld: ./out/tests/effekt.llvmtests/doubles.o: in function `ceil_1120':
doubles.ll:(.text+0x1012): undefined reference to `ceil'
collect2: error: ld returned 1 exit status

See below for the resolution.

@jiribenes jiribenes added feature New feature or request quality-of-life labels Sep 25, 2024
@jiribenes
Copy link
Contributor Author

Oops, I thought we were linking -lm already!

Comment on lines +324 to +326
val linkedLibraries = Seq(
"-lm", // Math library
) ++ libuvArgs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @phischu, do you foresee any problems with linking in -lm in the LLVM backend?
This is only relevant when using gcc, it works fine without it on clang.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an actual problem, just that this is the beginning of a never ending stream of C libraries we are linking against.

Copy link
Collaborator

@phischu phischu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be subtle differences between the backends, but I don't care.

Comment on lines +324 to +326
val linkedLibraries = Seq(
"-lm", // Math library
) ++ libuvArgs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an actual problem, just that this is the beginning of a never ending stream of C libraries we are linking against.

@jiribenes jiribenes merged commit 8f0d1e3 into master Sep 25, 2024
2 checks passed
@jiribenes jiribenes deleted the feature/llvm-rounding branch September 25, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants