This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Widening the result of INSERT_SUBVECTOR
ClosedPublic

Authored by CarolineConcatto on Oct 20 2021, 3:07 PM.

Details

Summary

Widens the result and first input vector because they have the same size.
The subvector to be inserted is widened in the operand widen function.

Diff Detail

Event Timeline

CarolineConcatto requested review of this revision.Oct 20 2021, 3:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 20 2021, 3:07 PM
sdesmalen added inline comments.Oct 21 2021, 12:59 AM
llvm/test/CodeGen/AArch64/sve-insert-vector.ll
425

This should be v1?

CarolineConcatto edited reviewers, added: dmgreen; removed: greened.Oct 21 2021, 1:24 AM
david-arm added inline comments.Oct 21 2021, 1:29 AM
llvm/test/CodeGen/AArch64/sve-insert-vector.ll
407

Is it worth having an equivalent one for floats, i.e.

define <vscale x 3 x float> @insert_nxv3f32_nxv2f32(<vscale x 2 x float> %sv0)

?

407

Also, I wonder if it's worth having a test where we also pass in a <vscale x 3 x i32> vector, i.e.

define <vscale x 3 x i32> @insert_nxv3i32_nxv2i32_2(<vscale x 3 x i32> %sv0, <vscale x 2 x i32> %sv1) {
  %v0 = call <vscale x 3 x i32> @llvm.experimental.vector.insert.nxv3i32.nxv2i32(<vscale x 3 x i32> %sv0, <vscale x 2 x i32> %sv1, i64 0)
  ret <vscale x 3 x i32> %v0

}

I realise this may be covered by one of the tests below, but I just wanted to make sure that a DAG combine wasn't hiding a bug somewhere that's all.

  • Fix return value in the test insert_nxv6i32_nxv2i32
  • Add new tests insert_nxv3i32_nxv2i32_2 and insert_nxv3f32_nxv2f32
CarolineConcatto marked 3 inline comments as done.Oct 21 2021, 2:29 AM
david-arm added inline comments.Oct 25 2021, 6:43 AM
llvm/test/CodeGen/AArch64/sve-insert-vector.ll
466

Hi @CarolineConcatto, did you mean to pass in %sv1 here instead?

  • Fix insert_nxv6i32_nxv2i32 to use sv0 and sv1 as input instead of only sv0
CarolineConcatto marked an inline comment as done.Oct 27 2021, 2:32 AM
CarolineConcatto added inline comments.
llvm/test/CodeGen/AArch64/sve-insert-vector.ll
466

Thank you David!

sdesmalen accepted this revision.Oct 27 2021, 2:40 AM

LGTM with nit addressed! Thanks @CarolineConcatto

llvm/test/CodeGen/AArch64/sve-insert-vector.ll
428

nit: please add nounwind attributes to the functions to avoid all the .cfi directives.

This revision is now accepted and ready to land.Oct 27 2021, 2:40 AM
david-arm accepted this revision.Oct 27 2021, 2:41 AM

LGTM! Thanks for making the changes @CarolineConcatto. :)

CarolineConcatto retitled this revision from [SelectionDAG] Add widen result function for insert subvector to [SelectionDAG] Widening the result of INSERT_SUBVECTOR.Oct 27 2021, 5:10 AM
CarolineConcatto edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.
CarolineConcatto marked an inline comment as done.