This is an archive of the discontinued LLVM Phabricator instance.

[mips][FastISel] Handle functions which return i1, i8 or i16.
ClosedPublic

Authored by vkalintiris on Dec 22 2014, 4:09 PM.

Details

Summary

Allow MIPS FastISel to handle functions which return i1/i8/i16 signed/unsigned.

Diff Detail

Repository
rL LLVM

Event Timeline

rkotler updated this revision to Diff 17579.Dec 22 2014, 4:09 PM
rkotler retitled this revision from to Mips fast-isel - handle functions which return i8 or i6 ..
rkotler updated this object.
rkotler edited the test plan for this revision. (Show Details)
rkotler added a reviewer: dsanders.
rkotler added subscribers: rfuhler, Unknown Object (MLST).
vkalintiris commandeered this revision.Apr 16 2015, 4:54 AM
vkalintiris added a reviewer: rkotler.
dsanders edited edge metadata.Apr 22 2015, 3:43 AM

Testcase?

lib/Target/Mips/MipsFastISel.cpp
1121–1124 ↗(On Diff #17579)

Type promotion should be handled by RetCC_Mips where it can be used by both Fast ISel and SelectionDAG. I suggest adding 'CCIfType<[i1, i8, i16], CCPromoteToType<i32>>' to RetCC_MipsO32.

I'll explain how to run the ABI tester to verify the change off-list.

1448–1449 ↗(On Diff #17579)

This ought to be a test for integer types <= 32bits. For example, it's entirely possible to support i2 sign-extension to i11, or i25 zero-extension to i29. However, this will require emitIntZExt() and emitIntSExt*() to be updated to use shl/srl and shl/sra pairs, and in the case of emitIntZExt() also calculate the andi mask where appropriate.

That should be done in a later patch so no changes required at the moment.

vkalintiris retitled this revision from Mips fast-isel - handle functions which return i8 or i6 . to [mips][FastISel] Handle functions which return i1, i8 or i16..
vkalintiris updated this object.
vkalintiris edited edge metadata.

Handle type promotion for return values through RetCC_Mips.

Thanks. It still needs a testcase though. You may be able to re-use the calling convention test at test/CodeGen/Mips/cconv/return.ll if support for loads is good enough.

Added a testcase and modified the patch to handle ret values for i1/i8/i16
when they don't require sign/zero extension.

You may be able to re-use the calling convention test at test/CodeGen/Mips/cconv/return.ll if support for loads is good enough.

There's not any test for FastISel outside from the test/CodeGen/Mips/Fast-ISel directory. That's why I added the tests in the Fast-ISel directory. In the future, I would like to rename the test/CodeGen/Mips/Fast-ISel/retabi.ll file to ret.ll as we check essentially for the ret IR instruction.

dsanders accepted this revision.Apr 29 2015, 3:54 AM
dsanders edited edge metadata.

LGTM

You may be able to re-use the calling convention test at test/CodeGen/Mips/cconv/return.ll if support for loads is good enough.

There's not any test for FastISel outside from the test/CodeGen/Mips/Fast-ISel directory. That's why I added the tests in the Fast-ISel directory. In the future, I would like to rename the test/CodeGen/Mips/Fast-ISel/retabi.ll file to ret.ll as we check essentially for the ret IR instruction.

That's true at the moment, but in the long term I'd like to merge the ABI tests for FastISel with the ABI tests for SelectionDAG. I'd be surprised if they produce significantly different output for such simple tests.

test/CodeGen/Mips/Fast-ISel/retabi.ll
34 ↗(On Diff #24605)

It's mandatory to use $2 so $[[REG_S:[0-9]+]] should check for $2 instead. Similar for the other new test cases.

This revision is now accepted and ready to land.Apr 29 2015, 3:54 AM
This revision was automatically updated to reflect the committed changes.