This is an archive of the discontinued LLVM Phabricator instance.

[IR][Legalizations] Widen illegal deinterleave and interleave vectors
Needs ReviewPublic

Authored by CarolineConcatto on Mar 9 2023, 8:59 AM.



To make legalization easier, the operands and outputs have the same size for
these ISD Nodes. When legalizing the results in WidenVecRes_VECTOR_DEINTERLEAVE
the operands are legalized to the same size as the outputs.
The ISD Node has two output/results, therefore the legalizing functions update
both results/outputs.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 8:59 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
CarolineConcatto requested review of this revision.Mar 9 2023, 8:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 8:59 AM
  • Remove old function header
Matt added a subscriber: Matt.Mar 20 2023, 8:44 AM
MattDevereau added inline comments.

I'm not sure if i'm missing something but if i remove this whole block and run check-all tests I don't see anything failing. Is this missing a test case?




This can be defined next to where it's used. Is PackedVT or LegalTY a better name for this? If InVT is nxv3i32 then WideVT = TLI.getTypeToTransformTo(Ctx, InVT); will return nxv4i32 which I found quite surprising


The way i'm reading this if statement is "before we had to widen InVT to WideTV". If we do something like:

bool InVTMustWiden = (TLI.getTypeAction(Ctx, InVT) == TargetLowering::TypeWidenVector);
if (InVTMustWiden) {
  WideVT = TLI.getTypeToTransformTo(Ctx, InVT);
if (InVTMustWiden) {
  OutVT = TLI.getValueType(DAG.getDataLayout(), I.getType());

Do you think that makes it easier to understand?

CarolineConcatto marked an inline comment as done.

-Address review comments

CarolineConcatto edited the summary of this revision. (Show Details)May 30 2023, 12:42 AM
craig.topper added inline comments.

Unrelated change?


GetWidenedVector does a map lookup, but there's no way WidenVec could be in the map. Won't this fail?