Page MenuHomePhabricator

[mips] Tighten FastISel restrictions
ClosedPublic

Authored by sdardis on Aug 23 2016, 3:24 AM.

Details

Summary

LLVM PR/29052 highlighted that FastISel for MIPS attempted to lower
arguments assuming that it was using the paired 32bit registers to
perform operations for f64. This mode of operation is not supported
for MIPSR6.

This patch resolves the reported issue by adding additional checks
for unsupported floating point unit configuration.

Thanks to mike.k for reporting this issue!

Diff Detail

Event Timeline

sdardis updated this revision to Diff 68964.Aug 23 2016, 3:24 AM
sdardis retitled this revision from to [mips] Tighten FastISel restrictions.
sdardis updated this object.
sdardis added a reviewer: vkalintiris.
sdardis set the repository for this revision to rL LLVM.
sdardis added a subscriber: llvm-commits.
seanbruno accepted this revision.Aug 23 2016, 2:21 PM
seanbruno added a reviewer: seanbruno.

For what its worth, this makes the clang enabled build of FreeBSD MIPS64 no longer require any of the fast-isel options that I have been using for the last few weeks.

This revision is now accepted and ready to land.Aug 23 2016, 2:21 PM
vkalintiris requested changes to this revision.Aug 30 2016, 8:24 AM
vkalintiris edited edge metadata.

The code changes LGTM but the test case doesn't look like it is testing the right thing.

test/CodeGen/Mips/Fast-ISel/double-arg.ll
2–3

I'm not sure why we need this RUN test. I would expect a not llc -mattr=fp64 ... test in order to make sure that for 64bit FPU registers we don't follow the FastISel path.

4–5

This test case is covered by check-disabled-mcpus.ll as we don't support MIPS R6 for FastISel.

This revision now requires changes to proceed.Aug 30 2016, 8:24 AM
sdardis updated this revision to Diff 69866.Aug 31 2016, 8:44 AM
sdardis edited edge metadata.
sdardis removed rL LLVM as the repository for this revision.

Address review comments on test, fix a comment in test.

vkalintiris accepted this revision.Sep 1 2016, 5:24 AM
vkalintiris edited edge metadata.

Thanks! LGTM.

This revision is now accepted and ready to land.Sep 1 2016, 5:24 AM
sdardis closed this revision.Sep 6 2016, 5:52 AM

Thanks, committed rL280706.