This is an archive of the discontinued LLVM Phabricator instance.

[SDAG] Fix pow2 assumption when splitting vectors
ClosedPublic

Authored by critson on Jun 8 2021, 2:55 AM.

Details

Summary

When reducing vector builds to shuffles it possible that
the DAG combiner may try to extract invalid subvectors.

This happens as the existing code assumes vectors will be power
of 2 sizes, which is already untrue, but becomes more noticable
with v6 and v7 types.
Specifically the existing code assumes that half PowerOf2Ceil of
a given vector index will fit twice into a given vector.

Diff Detail

Event Timeline

critson created this revision.Jun 8 2021, 2:55 AM
critson requested review of this revision.Jun 8 2021, 2:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2021, 2:55 AM

test case?

I am not sure if I can trigger this without v6 types, as the current non-pow2 types v3 and v5 are not divisible by 2.

Without this change simply adding v6i32 types will cause an assertion failure while compiling: llvm/test/CodeGen/X86/pr44976.ll

craig.topper added inline comments.Jun 9 2021, 10:01 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
19366

I think you can use getVectorNumElements(). These should be fixed vectors I think.

critson updated this revision to Diff 351120.Jun 10 2021, 3:54 AM
critson marked an inline comment as done.
  • Replace getVectorMinNumElements with getVectorNumElements

test case?

I am not sure if I can trigger this without v6 types, as the current non-pow2 types v3 and v5 are not divisible by 2.

Without this change simply adding v6i32 types will cause an assertion failure while compiling: llvm/test/CodeGen/X86/pr44976.ll

OK - if this per-emptively fixes an existing test regression then that's fine.

RKSimon accepted this revision.Jun 10 2021, 5:34 AM

LGTM

This revision is now accepted and ready to land.Jun 10 2021, 5:34 AM
This revision was landed with ongoing or failed builds.Jun 10 2021, 4:59 PM
This revision was automatically updated to reflect the committed changes.