Closed sppalkia closed 4 years ago
@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:
zao@freja:/scratch/zao/buildtests/weld$ target/debug/deps/serialize_tests-e9aa7bd8833ed150 dict_nested_pointers
running 1 test
serialize_tests-e9aa7bd8833ed150: /hp/eb/build/Clang/6.0.1/GCC-8.3.0-2.32/llvm-6.0.1.src/lib/IR/Constants.cpp:2055: static llvm::Constant *llvm::ConstantExpr::getInsertValue(llvm::Constant *, llvm::Constant *, ArrayRef<unsigned int>, llvm::Type *): Assertion `ExtractValueInst::getIndexedType(Agg->getType(), Idxs) == Val->getType() && "insertvalue indices invalid!"' failed.
Aborted (core dumped)
```
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff6e31801 in __GI_abort () at abort.c:79
#2 0x00007ffff6e2139a in __assert_fail_base (fmt=0x7ffff6fa87d8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x555557f83877 "ExtractValueInst::getIndexedType(Agg->getType(), Idxs) == Val->getType() && \"insertvalue indices invalid!\"",
file=file@entry=0x555557f80844 "/hp/eb/build/Clang/6.0.1/GCC-8.3.0-2.32/llvm-6.0.1.src/lib/IR/Constants.cpp", line=line@entry=2055,
function=function@entry=0x555557f837f3 "static llvm::Constant *llvm::ConstantExpr::getInsertValue(llvm::Constant *, llvm::Constant *, ArrayRef
zao@freja:/scratch/zao/buildtests/weld$ target/debug/deps/serialize_tests-e9aa7bd8833ed150 dict_pointers
running 1 test
serialize_tests-e9aa7bd8833ed150: /hp/eb/build/Clang/6.0.1/GCC-8.3.0-2.32/llvm-6.0.1.src/lib/IR/Constants.cpp:2055: static llvm::Constant *llvm::ConstantExpr::getInsertValue(llvm::Constant *, llvm::Constant *, ArrayRef<unsigned int>, llvm::Type *): Assertion `ExtractValueInst::getIndexedType(Agg->getType(), Idxs) == Val->getType() && "insertvalue indices invalid!"' failed.
Aborted (core dumped)
```
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff6e31801 in __GI_abort () at abort.c:79
#2 0x00007ffff6e2139a in __assert_fail_base (fmt=0x7ffff6fa87d8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x555557f83877 "ExtractValueInst::getIndexedType(Agg->getType(), Idxs) == Val->getType() && \"insertvalue indices invalid!\"",
file=file@entry=0x555557f80844 "/hp/eb/build/Clang/6.0.1/GCC-8.3.0-2.32/llvm-6.0.1.src/lib/IR/Constants.cpp", line=line@entry=2055,
function=function@entry=0x555557f837f3 "static llvm::Constant *llvm::ConstantExpr::getInsertValue(llvm::Constant *, llvm::Constant *, ArrayRef
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 apt-get
or brew
distribution if possible.
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!
FWIW: I can confirm that these changes fix the problem reported in #473, thanks for the quick fix @sppalkia!
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.