Page MenuHomePhabricator

[X86] Better support for the MCU psABI
ClosedPublic

Authored by mkuper on Nov 29 2015, 4:31 AM.

Details

Summary

This adds support for the MCU psABI in a way different from r251223 and r251224, basically reverting most of these two patches.
The problem with the approach taken in r251223 is that it only handled libcalls that originated from the backend. However, the midend also inserts quite a few libcalls and - with good reason - assumes these use the platform's default calling convention.

The previous patch tried to insert inregs when necessary both in the FE and, somewhat hackily, in the CG. This patch (and its clang companion patch) goes in a different direction. It defines a new default calling convention for the MCU, which doesn't use inreg marking at all, similarly to what, say, x86-64 does.

Diff Detail

Repository
rL LLVM

Event Timeline

mkuper updated this revision to Diff 41347.Nov 29 2015, 4:31 AM
mkuper retitled this revision from to [X86] Better support for the MCU psABI.
mkuper updated this object.
mkuper added reviewers: rnk, rafael, DavidKreitzer.
mkuper added a subscriber: llvm-commits.

The clang part of this patch is D15055 .

lib/Target/X86/X86CallingConv.h
46 ↗(On Diff #41347)

I am not a big fan of this, but I couldn't think of a better to do it (and the idea here is stolen from similar code in the ARM codegen, although the set of constraints there is a bit different).
Other suggestions are welcome.

lib/Target/X86/X86FastISel.cpp
1105 ↗(On Diff #41347)

This is actually not an MCU-related issue - this code should fire for x86 regardless of bitness and windows-ness. The DAG-based selection does the right thing, but FastISel didn't. Of course, since the MCU is non-windows 32-bit, it also hits this.

ping?

I'd really appreciate a sanity check for this (and D15055 ).

mkuper updated this revision to Diff 43603.Dec 24 2015, 7:06 AM

Fixed a TODO in the previous version on the patch.
It now properly supports fp128 (which is an optional type in the MCU psABI).

rnk accepted this revision.Dec 27 2015, 8:15 PM
rnk edited edge metadata.

Seems reasonable.

lib/Target/X86/X86CallingConv.h
46 ↗(On Diff #43603)

Given that it's what ARM does, let's go with it.

lib/Target/X86/X86CallingConv.td
603–604 ↗(On Diff #43603)

ECX is an allocatable argument register, so this will not implement 'nest' properly. Either ignoring 'nest' completely or giving a fatal error seems like better behavior than having code that looks like it's trying to implement something.

lib/Target/X86/X86FastISel.cpp
1105 ↗(On Diff #43603)

Aha, I changed SDag to do the right thing here, but not fastisel. Oops.

test/CodeGen/X86/mcu-abi.ll
64–66 ↗(On Diff #43603)

Idle thought: We should teach LLVM that memcpy returns its first argument, so that we don't have to spill in this kind of situation. :)

This revision is now accepted and ready to land.Dec 27 2015, 8:15 PM

Thanks, Reid!

lib/Target/X86/X86CallingConv.td
603–604 ↗(On Diff #43603)

Right, thanks.

test/CodeGen/X86/mcu-abi.ll
64–66 ↗(On Diff #43603)

Yeah. Interesting. :-)

This revision was automatically updated to reflect the committed changes.