-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
temporarily disable LLVM assertions on travis #19803
Conversation
According to travis this isn't it then, the reflection test issue with "Cannot define a symbol twice!" caused by #19787 is a different problem and also happens on llvm 3.7. |
aabb19e
to
830d8c9
Compare
The failure happens with just |
Maybe the failure has to do with tuple types, the |
in combination with #19746, maybe there's something now invalid about what broadcast was made to do here? |
You can build with multilib compilers from 64 bit Linux (that's what is done on Travis), or try the new docker image mentioned here: #18927 (comment) Reverting #19746 seems to help locally. It needs some tweaks due to nearby deprecations though. I'll try 19667 instead and see if that makes a difference. edit: by itself, reverting 19667 didn't fix this |
Looking more closely at the issues where there issue has been reported, all points out to #19746 as the cause of the problem, not clear to me why though. |
#19746 made many more operations go through broadcast instead of loops. So probably if the linalg or reflection tests were written to manually go through broadcast the problem would be visible earlier? Suggests either a codegen issue or that the broadcast code is doing something it's not supposed to. edit: |
Oddly enough that PR didn't change any of the underlying |
I may have a less-annoying temporary bandaid: 27166ae Seems to work okay locally without assertions enabled, but presumably that assertion is there for a reason and something is wrong here. |
088813c
to
965e9ba
Compare
965e9ba
to
8838b1e
Compare
@Keno (or anyone else) you should be able to reproduce this with |
@tkelman that docker image has unusually high uids (30000) in a few places, which doesn't play nice with my setup. If it's not too much trouble, could you |
okay, pushing as |
Ah, that didn't quite work, since it's a problem when extracting the layers, and that just added another layer. No worries, I'll adjust my setup. Just something to keep in mind for the future. |
any update? |
Working on it. With the docker container, I did see this, so I have it locally for debugging. |
Reverts #19678 to temporarily work around #19797 and #19792 until they can be properly solved, either in Julia codegen or with an LLVM patch.repurposed the branch