This is an archive of the discontinued LLVM Phabricator instance.

[Mips] Fix argument lowering for illegal vector types (PR63608)
ClosedPublic

Authored by nikic on Jul 4 2023, 7:09 AM.

Details

Summary

The Mips MSA ABI requires that legal vector types are passed in scalar registers in packed representation. E.g. a type like v16i8 would be passed as two i64.

The implementation attempts to do the same for illegal vectors with non-power-of-two element counts or non-power-of-two element types. However, the SDAG argument lowering code doesn't support this, and it is not easy to extend it to support this (we would have to deal with situations like passing v7i18 as two i64 values).

This patch instead opts to restrict the special argument lowering to only vectors with power-of-two elements and round element types. Everything else is lowered naively, that is by passing each element in promoted registers. This matches the general argument lowering, which also gives up on doing anything smart ones non-power-of-two vectors get involved.

Fixes https://github.com/llvm/llvm-project/issues/63608.

Diff Detail

Event Timeline

nikic created this revision.Jul 4 2023, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2023, 7:09 AM
nikic requested review of this revision.Jul 4 2023, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2023, 7:09 AM

Not seeing any active maintainers for MIPS, so mostly adding people who might be familiar with calling convention handling in general.

arsenm added a comment.Jul 6 2023, 3:05 PM

However, the SDAG argument lowering code doesn't support this,

How? I thought that was working reasonably OK these days

llvm/test/CodeGen/Mips/cconv/illegal-vectors.ll
20

Missing tests for the return half. Also the caller side

xen0n added a subscriber: xen0n.Jul 10 2023, 11:32 PM

However, the SDAG argument lowering code doesn't support this,

How? I thought that was working reasonably OK these days

If I remember right, TargetLoweringBase::getVectorTypeBreakdown doesn't support this.

nikic updated this revision to Diff 539936.Jul 13 2023, 3:48 AM

Add return and call-site arg+return test variants.

nikic added a comment.Jul 13 2023, 3:54 AM

However, the SDAG argument lowering code doesn't support this,

How? I thought that was working reasonably OK these days

MIPS uses an unusual calling convention where vectors (even legal vectors) get passed in GPR registers in packed representation (i.e. multiple vector elements per GPR register). Outside of MIPS, we should usually either pass in legalized vector registers, or element-wise in GPR registers.

I initially wanted to support non-pow2 vectors that follow the usual MIPS vector ABI and implemented this in https://gist.github.com/nikic/a52f764ee0ba211ddfd98a50e3df009d. This basically adds support for combination of bitcast and widening/narrowing. The problem is that when we get to non-pow2 element sizes, this becomes a lot less straightforward. E.g. v7i18 needs to be passed as two i64 registers, but v7i18 and v2i64 don't even have the same size, so we can't bitcast between them. We'd have to do something like bitcast v2i64 to i128 first, then truncate to i126, then bitcast to v7i18. It doesn't seem worthwhile to me to add this complexity to SDAG lowering, if we don't have to match any particular ABI for the non-pow2 cases.

arsenm accepted this revision.Jul 21 2023, 9:59 AM
This revision is now accepted and ready to land.Jul 21 2023, 9:59 AM
This revision was landed with ongoing or failed builds.Jul 24 2023, 3:07 AM
This revision was automatically updated to reflect the committed changes.