This is an archive of the discontinued LLVM Phabricator instance.

[mips] Don't create nested CALLSEQ_START..CALLSEQ_END nodes.
ClosedPublic

Authored by sdardis on Mar 9 2018, 3:21 AM.

Details

Summary

For the MIPS O32 ABI, the current call lowering logic naively lowers each
call, creating the reserved argument area to hold the spill areas a0..a3 and
the outgoing parameter area if one is required at each call site.

In the case of a sufficently large byval argument, a call to memcpy is used
to write the start+16 to end into the outgoing parameter area. This is done
with the CALLSEQ_START..CALLSEQ_END of the callee. The CALLSEQ nodes are
responsible for performing the necessary stack adjustments.

Since the O32/N32/N64 MIPS ABIs do not have red-zone and writing below the
stack pointer and reading the values back is unpredictable, the call to memcpy
cannot be hoisted out of the callee's CALLSEQ nodes.

However, for the O32 ABI requires the reserved argument area for functions
which have parameters. The naive lowering of calls will then create nested
CALLSEQ sequences. For N32 and N64 these nodes are also created, but with
zero stack adjustments as those ABIs do not have a reserved argument area.

This patch addresses the correctness issue by recognizing the special case
of lowering a byval argument that uses memcpy. By recognizing that the
incoming chain already has a CALLSEQ_START node on it when calling memcpy,
the CALLSEQ nodes are not created. For the N32 and N64 ABIs, this is not an
issue, as no stack adjustment has to be performed.

For the O32 ABI, the correctness reasoning is different. In the case of a
sufficently large byval argument, registers a0..a3 are going to be used for
the callee's arguments, mandating the creation of the reserved argument area.
The call to memcpy in the naive case will also create its own reserved
argument area. However, since the reserved argument area consists of undefined
values, both calls can use the same reserved argument area.

Diff Detail

Repository
rL LLVM

Event Timeline

sdardis created this revision.Mar 9 2018, 3:21 AM
sdardis edited the summary of this revision. (Show Details)Mar 9 2018, 3:22 AM
abeserminji accepted this revision.Mar 9 2018, 8:16 AM

LGTM with nit

test/CodeGen/Mips/cconv/byval.ll
354 ↗(On Diff #137719)

extra new line

This revision is now accepted and ready to land.Mar 9 2018, 8:16 AM
atanasyan accepted this revision.Mar 9 2018, 9:04 AM

LGTM

lib/Target/Mips/MipsISelLowering.cpp
2955 ↗(On Diff #137719)

nit: s/where where/where/

This revision was automatically updated to reflect the committed changes.