-
Notifications
You must be signed in to change notification settings - Fork 14
Final continuation jump missing from MBB #49
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
Comments
This is actually similar to #46. The selection DAG for
but the final jump is present with
Could it be that this is all fine & dandy, the fall-through is intentional (and so the fix from #46 is not needed), and the real problem is that |
I think the problem is indeed that I'll experiment with replacing
with
|
The change described in #49 (comment) fixes the original issue, but breaks other code spectacularly; in particular, given the following LLVM IR:
compiling it to AVR crashes with
|
|
|
That's because
is off by one compared to the original code. So it should be
|
… same spot as the original MBB (avr-rust/rust-legacy-fork#49)
How do I add some kind of 'has-patch' tag? |
Once you've got a commit that's been merged into LLVM, notify us here and we can cherry-pick it into our LLVM fork. I'm not 100% sure what the LLVM submission process is, but I'm sure @dylanmckay would be willing to point the way if you can't find anything online. |
This guide is really good. Prefer submitting the patch via Phabricator instead of email because it's much easier to review. Feel free to add me as a reviewer (my phab username is Nice debugging! |
… same spot as the original MBB (avr-rust/rust-legacy-fork#49)
I've added the testcase from this issue into the LLVM patch and I've also upstreamed it in |
… same spot Contributed by Dr. Gergő Érdi. Fixes a bug. Raised from (avr-rust/rust-legacy-fork#49). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@302973 91177308-0d34-0410-b5e6-96231b3b80d8
… same spot Contributed by Dr. Gergő Érdi. Fixes a bug. Raised from (avr-rust/rust-legacy-fork#49). llvm-svn=302973
Cherry picked into avr-rust in 6df5771. Thanks for the work @gergoerdi! |
@gergoerdi Were you able to compile libcore with these changes? |
… same spot Contributed by Dr. Gergő Érdi. Fixes a bug. Raised from (avr-rust/rust-legacy-fork#49). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@302973 91177308-0d34-0410-b5e6-96231b3b80d8
@jackpot51 you can see @gergoerdi's blog post and comments on Reddit. TL;DR, no. Like we've needed to do before, a subset of libcore was created. |
@shepmaster @dylanmckay Thanks for covering the last mile on this. I'd like to add similar test cases to my other commits, so I tried following http://llvm.org/docs/TestingGuide.html. How do I run LLVM tests from inside the rustc build tree? I've tried
|
That looks to be related to the fact that LLVM is compiled inside the Rust build dir. Normally when you compile and run LLVM, it will run a configure script which will fill a file named I'm not sure how to work around it, but I know for certain that if you build LLVM on its own, it will work fine. I think in order to compile it on its own, all you need to do it |
The fix has been upstreamed in |
Fix has been cherry picked into |
… same spot Contributed by Dr. Gergő Érdi. Fixes a bug. Raised from (avr-rust/rust-legacy-fork#49). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@302973 91177308-0d34-0410-b5e6-96231b3b80d8
Here's the minimized LLVM IR:
And here's the generated assembly:
The problem is that
BB#0
doesn't jump to its continuation, and so falls through toLBB0_1
which should only happen at the end of the loop.The text was updated successfully, but these errors were encountered: