This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Add support for widening INSERT_SUBVECTOR operands
ClosedPublic

Authored by david-arm on May 14 2021, 8:16 AM.

Details

Summary

When attempting to return something like a <vscale x 1 x i32>
type from a function we end up trying to widen the vector by
inserting a <vscale x 1 x i32> subvector into an undefined
<vscale x 4 x i32> vector. However, during legalisation we
then attempt to widen the INSERT_SUBVECTOR operands and hit
an error in WidenVectorOperand.

This patch adds a new WidenVecOp_INSERT_SUBVECTOR function
that currently only supports inserting subvectors into undefined
vectors.

Diff Detail

Event Timeline

david-arm created this revision.May 14 2021, 8:16 AM
david-arm requested review of this revision.May 14 2021, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2021, 8:16 AM
frasercrmck added inline comments.May 17 2021, 3:42 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
4839

The index operand is required to be a constant. Perhaps we can be stricter here?

david-arm added inline comments.May 17 2021, 3:48 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
4839

Ah ok, you mean we can simply use cast<ConstantSDNode> here?

frasercrmck added inline comments.May 17 2021, 3:59 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
4839

Yeah, then even simplified to N->getConstantOperandVal(2) == 0. There's precedent for that elsewhere so it sounds reasonable to me.

david-arm updated this revision to Diff 346156.May 18 2021, 6:24 AM
  • Changed WidenVecOp_INSERT_SUBVECTOR to use getConstantOperandVal
david-arm marked 2 inline comments as done.May 18 2021, 6:24 AM
Matt added a subscriber: Matt.May 18 2021, 7:28 AM
sdesmalen added inline comments.May 19 2021, 1:27 AM
llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll
190 ↗(On Diff #346156)

Why is this test in sve-calling-convention-mixed.ll?

david-arm added inline comments.May 19 2021, 1:30 AM
llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll
190 ↗(On Diff #346156)

Ah, mainly because I wasn't sure where else to put it is the honest answer. :) Although I do see now that we only deal with scalar arguments and return types in this file so I can try to find somewhere else. I just wanted to avoid creating a new file for this one test that's all.

david-arm updated this revision to Diff 346369.May 19 2021, 2:37 AM
  • Moved a test to sve-vector-splat.ll
david-arm marked an inline comment as done.May 19 2021, 2:38 AM
sdesmalen accepted this revision.May 19 2021, 2:40 AM

LGTM, thanks!

This revision is now accepted and ready to land.May 19 2021, 2:40 AM
This revision was landed with ongoing or failed builds.May 20 2021, 2:37 AM
This revision was automatically updated to reflect the committed changes.