-
Notifications
You must be signed in to change notification settings - Fork 259
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
Use typed null pointers instead of i64 0-values #476
Conversation
@boegel mind giving this a try? |
@sppalkia will do ASAP (hopefully tomorrow) |
I'm not boegel, but with af1e767 on Clang/6.0.1 one test suite has two failing tests:
Backtrace for dict_nested_pointers
Backtrace for dict_pointers
The other test suites seem to pass. |
Aha, all the tests should pass now -- try it out. FYI, I noticed that building Weld with LLVM compiled from source takes much longer for some reason compared to using the standard LLVM 6.0 distribution without various flags enabled (seems like there's some link time optimization that uses a ton of memory and is also pretty expensive) -- I couldn't even build it on my laptop because it used so much memory, so I'd definitely recommend using the |
All tests pass now for me with the aforementioned setup. As a tangent as to why I've got my own LLVM build, on the clusters we prefer having as much software as we can via the module system instead of from OS packaging, to ensure that we have all the versions needed and that it's built the way we need it with architecture optimizations. |
Makes sense, I'll investigate why the builds are so slow when building LLVM from source when I get a chance. Thanks for testing this! |
Fixes #473 when using Weld with an LLVM 6.0 distribution that has
LLVM_ENABLE_ASSERTIONS
enabled.Also fixes some README issues.
The issue was that some places in the code generation used an
i64
0 literal as a substitute fornull
, which was okay with LLVM's module verifier but caused certain debug assertions to complain.