Unconditionally disable LLVM's FastISel on AArch64 #21671

akosthekiss · 2015-01-26T21:58:12Z

Before ca07e256f62f772d14c42f41af46f2aeacc54983, LLVM's AArch64FastISel
had a sign (and zero?) extension bug. Until rustc gets its LLVM past
that commit, the way of workaround is to pass an option to LLVM that
forces the disabling of FastISel (which would normally kick in for -O0).

Fixes #21627

Before ca07e256f62f772d14c42f41af46f2aeacc54983, LLVM's AArch64FastISel had a sign (and zero?) extension bug. Until rustc gets its LLVM past that commit, the way of workaround is to pass an option to LLVM that forces the disabling of FastISel (which would normally kick in for -O0). Fixes rust-lang#21627

rust-highfive · 2015-01-26T21:58:27Z

r? @Aatch

(rust_highfive has picked a reviewer for you, use r? to override)

huonw · 2015-01-26T22:03:09Z

src/librustc_trans/back/write.rs

@@ -1012,6 +1012,9 @@ unsafe fn configure_llvm(sess: &Session) {
        if sess.time_llvm_passes() { add("-time-passes"); }
        if sess.print_llvm_passes() { add("-debug-pass=Structure"); }

+        // FIXME #21627 disable faulty FastISel on AArch64 (even for -O0)


I think this number should refer to an open bug (i.e. you can preemptively open one "reenable FastISel on AArch64" before this lands) so that it gets picked up in future triage/general bug handling.

huonw · 2015-01-26T22:04:12Z

An alternative would be to just update the LLVM Rust uses, @alexcrichton would know the right process for this.

alexcrichton · 2015-01-26T22:08:40Z

We currently have a fork of LLVM which basically only serves the purpose of allowing us to pin to specific revisions as well as pile on some custom optimizations (just the top 6 patches).

Updating LLVM is basically just doing something like the following:

git clone git://github.com/rust-lang/llvm
cd llvm
git remote add llvm git://github.com/llvm-mirror/llvm
git fetch llvm
git checkout -b rust-llvm-$date rust-llvm-$old_date
git rebase llvm/master
# fix conflicts and make a PR to rust-lang/llvm

I'll merge it to a new branch of rust-lang/llvm and then we can update the in-tree copy. This is also fine to do now, either way works!

richo · 2015-01-26T22:20:57Z

I had a similar tree kicking around anyway. I just redid the rebase quickly
(The last patch is now upstream).

I just made a fork and pushed it to richo/llvm
@ 070a227e1848ec2cd23cc8d0ad6e72460f2e7438 (rust-llvm-2014-01-26)

On Mon, Jan 26, 2015 at 2:08 PM, Alex Crichton notifications@github.com
wrote:

We currently have a fork of LLVM https://github.com/rust-lang/llvm
which basically only serves the purpose of allowing us to pin to specific
revisions as well as pile on some custom optimizations
https://github.com/rust-lang/llvm/commits/rust-llvm-2014-12-30 (just
the top 6 patches).

Updating LLVM is basically just doing something like the following:

git clone git://github.com/rust-lang/llvm
cd llvm
git remote add llvm git://github.com/llvm-mirror/llvm
git fetch llvm
git checkout -b rust-llvm-$date rust-llvm-$old_date
git rebase llvm/master

fix conflicts and make a PR to rust-lang/llvm

I'll merge it to a new branch of rust-lang/llvm and then we can update
the in-tree copy. This is also fine to do now, either way works!

—
Reply to this email directly or view it on GitHub
#21671 (comment).

akosthekiss · 2015-01-27T00:44:32Z

I'd vote/ask for accepting this PR (no surprise, since it's mine :) until the LLVM update happens. (Except if that happens soon. I have no experience with that, don't know how much work it takes.) It'll be easy to remove these two lines after that.

richo · 2015-01-27T00:50:27Z

Relevant PR for llvm: rust-lang/llvm#26

alexcrichton · 2015-01-27T01:37:55Z

I've now merged rust-lang/llvm#26, @akiss77 would you be ok trying out updating LLVM in this patch instead?

If it's too complicated due to random build failures or such we can also hold off for now and merge this instead.

richo · 2015-01-27T02:15:15Z

Looks like something went pearshaped in my rebase. I am poking a little
now, but I can definitely amend tonight.

Sorry!

On Mon, Jan 26, 2015 at 5:38 PM, Alex Crichton notifications@github.com
wrote:

I've now merged rust-lang/llvm#26
rust-lang/llvm#26, @akiss77
https://github.com/akiss77 would you be ok trying out updating LLVM in
this patch instead?

If it's too complicated due to random build failures or such we can also
hold off for now and merge this instead.

—
Reply to this email directly or view it on GitHub
#21671 (comment).

richo · 2015-01-27T03:35:50Z

I've updated my branch. I'm running make check now, I'll report back later

akosthekiss · 2015-01-27T07:23:11Z

I'll be OK trying out the new LLVM, but let's see what @richo has to report back first. (I have quite a long build cycle, so I rather wouldn't start it up if there are some known issues around.)

richo · 2015-01-27T07:25:02Z

I fixed the original issues, but I'm currently fighting with some internal conversions that have been broken in the update.

richo · 2015-01-27T07:25:18Z

If you feel like writing some C++ I can push the trees for you to have a look at though!

akosthekiss · 2015-01-27T08:07:17Z

I don't dare to undertake this right now since I'll be off travelling in some hours for the coming 3-4 days. (Eventhough I do love C++.)

richo · 2015-01-27T10:14:30Z

As an update, I poked a little tonight and then Alex poked quite a bit more. This will probably make it out in the not too distant future, but this workaround is rpobably a good idea if it's breaking things on aarch64.

alexcrichton · 2015-01-27T19:06:46Z

Yeah it looks like the LLVM update will take awhile, so we can go ahead and put this in the queue in the meantime.

alexcrichton · 2015-01-27T19:07:03Z

@bors: r+ ea50bf8 rollup

akosthekiss · 2015-01-29T08:08:48Z

what happened to @bors ?

rust-highfive assigned Aatch Jan 26, 2015

huonw reviewed Jan 26, 2015
View reviewed changes

richo mentioned this pull request Jan 27, 2015

Rust llvm 2015 01 26 rust-lang/llvm#26

Closed

alexcrichton assigned alexcrichton and unassigned Aatch Jan 27, 2015

bors merged commit ea50bf8 into rust-lang:master Jan 29, 2015

akosthekiss deleted the pr-aarch64-fastisel0 branch January 29, 2015 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unconditionally disable LLVM's FastISel on AArch64 #21671

Unconditionally disable LLVM's FastISel on AArch64 #21671

akosthekiss commented Jan 26, 2015

rust-highfive commented Jan 26, 2015

huonw Jan 26, 2015

huonw commented Jan 26, 2015

alexcrichton commented Jan 26, 2015

richo commented Jan 26, 2015

fix conflicts and make a PR to rust-lang/llvm

akosthekiss commented Jan 27, 2015

richo commented Jan 27, 2015

alexcrichton commented Jan 27, 2015

richo commented Jan 27, 2015

richo commented Jan 27, 2015

akosthekiss commented Jan 27, 2015

richo commented Jan 27, 2015

richo commented Jan 27, 2015

akosthekiss commented Jan 27, 2015

richo commented Jan 27, 2015

alexcrichton commented Jan 27, 2015

alexcrichton commented Jan 27, 2015

akosthekiss commented Jan 29, 2015

Unconditionally disable LLVM's FastISel on AArch64 #21671

Unconditionally disable LLVM's FastISel on AArch64 #21671

Conversation

akosthekiss commented Jan 26, 2015

rust-highfive commented Jan 26, 2015

huonw Jan 26, 2015

Choose a reason for hiding this comment

huonw commented Jan 26, 2015

alexcrichton commented Jan 26, 2015

richo commented Jan 26, 2015

fix conflicts and make a PR to rust-lang/llvm

akosthekiss commented Jan 27, 2015

richo commented Jan 27, 2015

alexcrichton commented Jan 27, 2015

richo commented Jan 27, 2015

richo commented Jan 27, 2015

akosthekiss commented Jan 27, 2015

richo commented Jan 27, 2015

richo commented Jan 27, 2015

akosthekiss commented Jan 27, 2015

richo commented Jan 27, 2015

alexcrichton commented Jan 27, 2015

alexcrichton commented Jan 27, 2015

akosthekiss commented Jan 29, 2015