This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Remove invalid TypeSize conversion from WidenVecOp_BITCAST.
ClosedPublic

Authored by paulwalker-arm on Jun 8 2022, 10:20 AM.

Diff Detail

Event Timeline

paulwalker-arm created this revision.Jun 8 2022, 10:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 10:20 AM
paulwalker-arm requested review of this revision.Jun 8 2022, 10:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 10:20 AM

For full disclosure, I'm not 100% sure but believe the nxv1#64 output is possibly wrong. This is true for the existing and new tests. I don't think this is the fault of this or previous bitcast work but rather an issue with how we're lowering ISD::INSERT_SUBVECTOR for these types. It looks suspiciously like we're treating ISD::INSERT_SUBVECTOR nxv2i64 undef, nxv1i64 data, i32 0 as a NOP.

This is certainly at odds with how we handle the other "unpacked" types but I don't think we've ever really agreed how <vscale x 1 x should be represented at the register level.

efriedma accepted this revision.Jun 8 2022, 4:52 PM

LGTM

This is certainly at odds with how we handle the other "unpacked" types but I don't think we've ever really agreed how <vscale x 1 x should be represented at the register level.

In memory, `<vscale x 1 x is tightly packed. There are no legal <vscale x 1 x types on AArch64, so target-independent legalization will eliminate them.

So the question you're asking is really how <vscale x 1 x are passed across functions, and in SelectionDAG, across basic blocks. That's entirely determined by getCopyFromPartsVector()/getCopyToPartsVector() in SelectionDAGBuilder, I think. I'm fine with the current state, but if you want to mess with it, we can, I guess. I don't think it directly interacts with anything else.

This revision is now accepted and ready to land.Jun 8 2022, 4:52 PM

Thanks for the information @efriedma. I'll likely need to do a fuller <vscale x 1 x investigation to close out the remaining legalisation issues as I think the current strategy may well not be a good fit.

This revision was landed with ongoing or failed builds.Jun 11 2022, 3:57 AM
This revision was automatically updated to reflect the committed changes.