Regression in 3.4's register allocator?

Hi,

The RoboVM project recently upgraded from LLVM 3.3 to 3.4 and with the new version we are sometimes seeing “ran out of registers during register allocation” errors when targeting x86 32-bit that we haven’t seen before. Nothing has changed in our bitcode generator. I have attached an IR file which can be used to reproduce this problem. The problem occurs with -O2 and -disable-fp-elim specified.

Running llc from 3.3 works without problems:
$ ~/Downloads/clang+llvm-3.3-x86_64-apple-darwin12/bin/llc -mtriple i386-unknown-macosx -O2 -disable-fp-elim reg-alloc-test.ll

While llc from 3.4 fails:

$ ~/Downloads/clang+llvm-3.4-x86_64-apple-darwin10.9/bin/llc -mtriple i386-unknown-macosx -O2 -disable-fp-elim reg-alloc-test.ll
LLVM ERROR: ran out of registers during register allocation

I haven’t been able to test this with the latest trunk but I got the same error when trying with the 3.4.1 branch.

Has anyone else seen this? Is it a known bug? Or should I report it?

Regards,
Niklas

reg-alloc-test.ll (19.9 KB)

Hi Niklas,

This works for me with trunk r207129.

We did fix some register allocation issues recently, don’t know which change fixes the issue though.

Thanks,

-Quentin

I can confirm that this regression has been fixed in trunk. Should I report this as a bug in 3.4?

The 3.4.2 release is coming, you might want to consider finding the
patches responsible for the fix and proposing it for back-porting.

Please email Tom (cc'd) if you end up with a proposal.

cheers,
--renato

I think we have located the revision which fixes this regression: r206094 (or commit 6bb00df in llvm-mirror on GitHub). I have attached a patch which can be applied to the current release_34 branch (tested against the release_34 branch in llvm-mirror). With this patch the attached reg-alloc-test.ll file doesn’t fail with the “LLVM ERROR: ran out of registers during register allocation” error any longer. I haven’t run any llvm tests to make sure this patch doesn’t break anything else but as far as I can see it doesn’t at least break anything when used in RoboVM.

Is it too late to get this into 3.4.2? Is there anything else I can do to make it easier for you to include this patch in 3.4.2 or the next possible point release?

reg-alloc-regression.patch (10.8 KB)

reg-alloc-test.ll (19.9 KB)

I think we have located the revision which fixes this regression: r206094
(or commit 6bb00df in llvm-mirror on GitHub). I have attached a patch which
can be applied to the current release_34 branch (tested against the
release_34 branch in llvm-mirror). With this patch the attached
reg-alloc-test.ll file doesn't fail with the "LLVM ERROR: ran out of
registers during register allocation" error any longer. I haven't run any
llvm tests to make sure this patch doesn't break anything else but as far
as I can see it doesn't at least break anything when used in RoboVM.

Is it too late to get this into 3.4.2? Is there anything else I can do to
make it easier for you to include this patch in 3.4.2 or the next possible
point release?

Hi Niklas,

Unfortunately, it is too late to get this into 3.4.2. With the 3.5
release coming soon, I wasn't planning on doing another 3.4 point
release.

I took a quick look at the patch, would it be possible to work around
this by adding a value to the FeatureString when invoking LLVM?

-Tom

Hi Tom,

I'm not opposed to doing more 3.4.x releases, we just need to make sure
that if we continue the 3.4.x series that each release has fixes with
a broad enough appeal that it justifies the time spent by the community
testers.

I suspect that we could automate most of the release process with bots, so
if we could find a good solution for this, then it would be much easier to
keep stable branches going longer, since we wouldn't need to rely on the
time of human testers.

Once 3.5 is released, then 3.5.x will become more interesting to me than
3.4.x, so I'm not sure I will have the time to keep the 3.4.x series
going. However, if someone else would like to volunteer to continue
managing 3.4.x releases, I will not object.

-Tom

Hi Niklas,

The attached patch has nothing to do with register allocation. r206094 changes how cpu auto-detection is done. I believe it’s now the responsibility of the tools (e.g. llc) to detect the cpu. My guess is the proper fix is on the RoboVM side to handle the change.

Jim, can you confirm?

Evan

Yep, quite right, Evan. Any regalloc differences due to that patch are purely coincidence;

-Jim

I suspect that we could automate most of the release process with bots, so
if we could find a good solution for this, then it would be much easier to
keep stable branches going longer, since we wouldn't need to rely on the
time of human testers.

Assuming nothing goes wrong, as with 3.4.2, it's not such a big pain
to test and release to each one of us, testers. I heard some people do
the release test on buildbots (by monitoring the release branch
commits, not trunk). While that's a good idea, the main problems are
hardware availability (for those of us that don't have too many
devices to test, and trunk is more important), and making sure that
all bots (check, self-hosting, LNT) are validated (for whatever
meaning of validation) before the binaries are uploaded on the SFTP
site.

These are non-trivial constraints.

Once 3.5 is released, then 3.5.x will become more interesting to me than
3.4.x, so I'm not sure I will have the time to keep the 3.4.x series
going. However, if someone else would like to volunteer to continue
managing 3.4.x releases, I will not object.

I agree this is a good idea. I'm not too interested in point releases,
myself, but I'll keep testing them. However, being a release manager
is no easy task, and I wouldn't force that on anyone just to keep it
up. I'd happily test 3.4.3 if anyone else decides to be the 3.4.x
release manager from now on, though.

cheers,
--renato

Thanks guys! I did some experimenting and it turns out that if I just set a value for CPU in the call to LLVMCreateTargetMachine() the “ran out of registers” error disappears. Easy fix!

BTW, if it’s up to the tools to do the auto-detection I guess llc in 3.4 is broken then since it fails too?

Glad to hear things are back to working for you. Most excellent.

llc is an interesting case because it used to do auto-detection, but was change to not do so. It’s purpose is as a test and development driver for LLVM backend work, and for that it’s very important that runs of the tool on different host machines generate the same results. CPU auto-detection is at strong cross purposes to that. Compiler drivers that are meant to be end-user facing have a different set of priorities and so different solutions are better.

-Jim