Page MenuHomePhabricator

[RISCV] Add codegen support for ilp32f, ilp32d, lp64f, and lp64d ("hard float") ABIs
ClosedPublic

Authored by asb on Mar 14 2019, 4:38 AM.

Details

Summary

This patch adds support for the RISC-V hard float ABIs, building on top of rL355771, which added basic target-abi parsing and MC layer support. It also builds on some re-organisations and expansion of the upstream ABI and calling convention tests which were recently committed directly upstream.

A number of aspects of the RISC-V float hard float ABIs require frontend support (e.g. flattening of structs and passing int+fp for fp+fp structs in a pair of registers), and will be addressed in a Clang patch.

As can be seen from the tests, it would be worthwhile extending RISCVMergeBaseOffsets to handle constant pool as well as global accesses.

Diff Detail

Repository
rL LLVM

Event Timeline

asb created this revision.Mar 14 2019, 4:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2019, 4:38 AM

Thanks a lot for this Alex.

Looks good to me. I only have a minor comment about testing varargs for hard-float ABIs as well.

lib/Target/RISCV/RISCVISelLowering.cpp
986–987 ↗(On Diff #190601)

Using IsFixed here adds a new path that we may want to have covered as well.

I think this can be achieved by adding a call to a varargs function passing several double arguments in test files calling-conv-ilp32-ilp32f-ilp32d-common.ll and calling-conv-lp64-lp64f-lp64d-common.ll.

What do you think?

bero awarded a token.Mar 19 2019, 7:15 AM
asb updated this revision to Diff 192978.Mar 30 2019, 9:00 AM
asb marked an inline comment as done.

@rogfer01: Thanks for the review. You're right that we could do with coverage for varargs under the hard float ABIs. I've modifed test/CodeGen/RISCV/vararg.ll so it provides common coverage for vararg passing for these ABIs (which of course should be the same as the integer ABIs). How does this patch look now?

rogfer01 accepted this revision.Mar 30 2019, 9:33 AM

Thanks for the update Alex, certainly vararg.ll is a better test to update than my earlier suggestions.

I have no other question so this lgtm.

Thanks again!

This revision is now accepted and ready to land.Mar 30 2019, 9:33 AM
This revision was automatically updated to reflect the committed changes.