This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Use getVectorElementCount inside visitINSERT_SUBVECTOR
ClosedPublic

Authored by joechrisellis on Dec 7 2020, 1:44 AM.

Diff Detail

Event Timeline

joechrisellis created this revision.Dec 7 2020, 1:44 AM
joechrisellis requested review of this revision.Dec 7 2020, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 7 2020, 1:44 AM

Uploaded this patch without tests for now. I had a go at writing some tests that invoke the modified codepath here, but I couldn't come up with anything small that would produce a TypeSize-related warnings.

david-arm added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
21109

You can use getVectorMinNumElements() here if you want?

llvm/test/CodeGen/AArch64/dag-combine-insert-subvector.ll
28

The insert intrinsic expects a <vscale x 8 x double> here I think, right? i.e. nxv2f64.nxv8f64

joechrisellis marked an inline comment as done.

Address @david-arm's comments.

  • Fix use of incorrect intrinsic.
  • Use getVectorMinElements() instead of getVectorElementCount().getKnownMinValue().
sdesmalen added inline comments.Jan 8 2021, 6:20 AM
llvm/test/CodeGen/AArch64/dag-combine-insert-subvector.ll
10

It's more robust not to test for specific warnings, but just check no warnings are emitted at all. When the time comes that the warning becomes an error, all tests are still guaranteed to pass.

joechrisellis marked an inline comment as done.

Address @sdesmalen's comment.

  • Do not test for specific warnings, but for all warnings.
This revision is now accepted and ready to land.Jan 11 2021, 5:46 AM
This revision was landed with ongoing or failed builds.Jan 11 2021, 6:15 AM
This revision was automatically updated to reflect the committed changes.