Page MenuHomePhabricator

[X86] Convert vXi1 vectors to xmm/ymm/zmm types via getRegisterTypeForCallingConv rather than using CCPromoteToType in the td file
ClosedPublic

Authored by craig.topper on Feb 25 2020, 9:15 PM.

Details

Summary

Previously we tried to promote these to xmm/ymm/zmm by promoting
in the X86CallingConv.td file. But this breaks when we run out
of xmm/ymm/zmm registers and need to fall back to memory. We end
up trying to create a non-sensical scalar to vector. This lead
to an assertion. The new tests in avx512-calling-conv.ll all
trigger this assertion.

Since we really want to treat these types like we do on avx2,
it seems better to promote them before the calling convention
code gets involved. Except when the calling convention is one
that passes the vXi1 type in a k register.

The changes in avx512-regcall-Mask.ll are because we indicated
that xmm/ymm/zmm types should be passed indirectly for the
Win64 ABI before we go to the common lines that promoted the
vXi1 types. This caused the promoted types to be picked up by
the default calling convention code. Now we promote them earlier
so they get passed indirectly as though they were xmm/ymm/zmm.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 25 2020, 9:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 25 2020, 9:15 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

@rnk Any comments? It makes sense to me but I don't know a lot about this area tbh.

rnk accepted this revision.Wed, Mar 4, 1:34 PM

@rnk Any comments? It makes sense to me but I don't know a lot about this area tbh.

Honestly, the conventions for passing vectors that don't fit into XMM regs are beyond me.

I just had a suggestion for tightening up the code, but otherwise, it seems fine to me.

llvm/lib/Target/X86/X86ISelLowering.cpp
2169

Can this be factored to share the logic? It looks structurally similar, and you could do return {MTV::vNiM, 2} and then have the caller pick the field that matters, type or number of registers.

This revision is now accepted and ready to land.Wed, Mar 4, 1:34 PM
This revision was automatically updated to reflect the committed changes.