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

Int128: fix how LLVM::Type.const_int emit 128 literals #7135

Merged
merged 1 commit into from Dec 2, 2018

Conversation

Projects
None yet
3 participants
@bcardiff
Copy link
Member

bcardiff commented Dec 1, 2018

The llvm wrapper is not able to emit 128bits ints.

Instead of going the way #7093 did of using String. This will allow to use LLVMConstIntOfArbitraryPrecision method that accept the number as an array of UInt64.

@bcardiff bcardiff added this to the 0.27.1 milestone Dec 1, 2018

@RX14

RX14 approved these changes Dec 2, 2018

@RX14 RX14 merged commit 4754d53 into crystal-lang:master Dec 2, 2018

4 checks passed

ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
if !value.is_a?(Int128) && !value.is_a?(UInt128) && int_width != 128
Value.new LibLLVM.const_int(self, value, 0)
else
encoded_value = UInt64[value & UInt64::MAX, (value >> 64) & UInt64::MAX]

This comment has been minimized.

@asterite

asterite Dec 2, 2018

Contributor

But this allocates memory. Why not use static_array?

This comment has been minimized.

@bcardiff

bcardiff Dec 2, 2018

Author Member

AFAIK the reference is kept in llvm side. I couldn't find where the pointer was used to duplicate the data so I assume a reference was used. I could be wrong the documentation didn't said much.

This comment has been minimized.

@asterite

asterite Dec 3, 2018

Contributor

If it's kept in LLVM side then we must keep a reference on our side, otherwise the GC will collect it.

This comment has been minimized.

@RX14

@bcardiff bcardiff deleted the bcardiff:feature/llvm-emit-128 branch Dec 2, 2018

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