Page MenuHomePhabricator

[GlobalISel][CallLowering] Add support for splitting types according to calling conventions
Needs ReviewPublic

Authored by aemerson on Tue, Aug 13, 3:39 PM.

Details

Summary

On AArch64, s128 types have to be split into s64 GPRs when passed as arguments. This change adds the generic support in call lowering for dealing with multiple registers, for incoming and outgoing args. Support for splitting for return types not yet implemented.

Diff Detail

Event Timeline

aemerson created this revision.Tue, Aug 13, 3:39 PM

I'm somewhat confused by what the responsibilities of handleAssignments are vs. target code. I already have multiple register breakdowns without this patch in AMDGPU

I'm somewhat confused by what the responsibilities of handleAssignments are vs. target code. I already have multiple register breakdowns without this patch in AMDGPU

IIUC calling convention specified register splits should be generic, and it's handled in generic code by SelectionDAG. Whether handleAssignments is the best place for I'm not sure, but I don't think targets should have to deal with this individually. In arm64 I think we already have some target call lowering code that should really be generic.

paquette added inline comments.Wed, Aug 21, 9:18 AM
llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
198–200

obligatory braces nit

206

I think a high-level explanation of what we're going to do when we have an incoming vs outgoing handler would be useful.

220

Can we have a comment here explaining what's going on?

225–227

I think that this could use a comment too.

232

Why less than 2?

232–235

braces

285

Pull Args[I].Regs.size() into a variable with a meaningful name?

llvm/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll
1

Why are we falling back now?

aemerson marked 2 inline comments as done.Wed, Aug 21, 3:51 PM
aemerson added inline comments.
llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
232

Because this is a code path that hasn't been implemented or tested yet. That is, if we're dealing with an outgoing arg, and we failed to assign a location (at the beginning of handleAssignments), and the calling convention says it doesn't need to be split (NumParts < 2), then we don't know what else to do.

llvm/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll
1

We're not, I'm just adding this flag to ensure we error if we do fall back at all.

aemerson updated this revision to Diff 216514.Wed, Aug 21, 4:10 PM

Address comments.

t.p.northover added inline comments.Thu, Aug 22, 6:35 AM
llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
196

It looks like you only use the ability here to change elements, not add/remove them? I think that's a MutableArrayRef use-case.

llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
165

It looks like you only modify elements in place? If so, that's MutableArrayRef's thing.

260

Why only setSplit for outgoing args?

290

Assertion?

aemerson marked an inline comment as done.Thu, Aug 22, 11:09 AM
aemerson added inline comments.
llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
260

No good reason, I'll add it.

aemerson marked an inline comment as done.Thu, Aug 22, 11:15 AM
aemerson added inline comments.
llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h
196

Right, but there's no conversion for SmallVectors to MutableArrayRef, only SmallVectorImpl.

aemerson updated this revision to Diff 216683.Thu, Aug 22, 11:58 AM

Add split flags to incoming args and assertions.