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

Failing test - math.lua #47

Open
Pavel-Durov opened this issue Aug 15, 2023 · 4 comments
Open

Failing test - math.lua #47

Pavel-Durov opened this issue Aug 15, 2023 · 4 comments

Comments

@Pavel-Durov
Copy link
Contributor

Issue

Steps to reproduce

 $ YKD_SERIALISE_COMPILATION=1 ../src/lua ./math.lua
testing numbers and math lib
64-bit integers, 53-bit (mantissa) floats
Floating point instructions are not supported yet:   %3683 = sitofp i64 %3682 to double, !dbg !10380 

$ YKD_SERIALISE_COMPILATION=0 ../src/lua ./math.lua
testing numbers and math lib
64-bit integers, 53-bit (mantissa) floats
testing order (floats cannot represent all integers)
testing -0 and NaN
testing 'math.random'
random seeds: 1692108204, 22478440
float random range in 53000 calls: [0.000087, 0.999992]
Floating point instructions are not supported yet:   %3683 = sitofp i64 %3682 to double, !dbg !10380

Versions

YkLua - lua-tests/fac6e7e244d3a30366bb06b01c00d2462e901fbd
Yk - master/a690e6eaf7253a4fa9c04bb54406c07b7efc0085

@Pavel-Durov
Copy link
Contributor Author

After ykjit/yk#830 change in YK this still fails.
Different error this time

YKD_SERIALISE_COMPILATION=1 try_repeat 100 ../src/lua -e"_U=true" math.lua 
testing numbers and math lib
64-bit integers, 53-bit (mantissa) floats
testing order (floats cannot represent all integers)
instrinsic is missing `yk.intrinsic.inlined` metadata:   %13 = call double @llvm.floor.f64(double %0), !dbg !9453

@ltratt
Copy link
Contributor

ltratt commented Sep 14, 2023

I think that might be semi-expected in the sense that we can't deopt floats at the moment - @ptersilie was looking for a test which caused that to happen. Perhaps this is the test he was looking for!

@Pavel-Durov
Copy link
Contributor Author

From my understanding (and I might be completely wrong) talking to @ptersilie, this behaviour is related to LLVM implementation.
LLVM injects intrinsic functions like @llvm.floor.f64 that are problematic to link back to IR since it's done on the instruction level 😶.

@ptersilie
Copy link
Contributor

Well, we can deopt floats as long as AOT doesn't expect them to be in a float register. I'm all the tests we tried they appear always on the stack.

The above error however has nothing to do with deopt. This is the old intrinsic problem: we need to know whether LLVM has inlined an intrinsic or turned it into a call so we can act accordingly during trace construction (JITModBuilder). A common example for this is memcpy which for small values LLVM just replaces with movs.

I've spent some time looking into llvm.floor and other math intrinsics and unfortunately for us it's very difficult to find all the paths where LLVM inlines them. And in the case of floor the decision is made very late and on the machine IR which means we can't annotate the LLVM IR (which we use during trace building) anymore.

@vext01 and I have discussed this before and the only solution we can think of at the moment is to put wrappers around all the intrinsics to prevent LLVM from inlining them.

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

No branches or pull requests

3 participants