Page MenuHomePhabricator

[AArch64][SVE] Don't crash on pre-legalized types in extload combine.
ClosedPublic

Authored by ab on Thu, May 26, 3:06 PM.

Details

Summary

This was assuming the vector types were MVTs, but they don't have to be.

Note that the concrete output of the test isn't very useful, since it's dominated by nonsensical calling convention lowering for the weird types (ideas for a less gross test welcome!)

An alternative to consider is whether it makes sense to try this that early, but we get to this from the generic ext-load combines, so I guess it's fine on that side

Diff Detail

Event Timeline

ab created this revision.Thu, May 26, 3:06 PM
ab requested review of this revision.Thu, May 26, 3:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptThu, May 26, 3:06 PM
paulwalker-arm accepted this revision.Thu, May 26, 4:27 PM

Thanks for the fix. I don't mind the test since as you say, the exact output is not relevant.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
5313

I kind of wondered if this should only require the vector element type to be simple but looking at MachineValueType.h the important vector types upto 2048bit are simple so I doubt it matters.

This revision is now accepted and ready to land.Thu, May 26, 4:27 PM

@ab What's your plans for submitting this fix. I only ask because others have hit the same issue, as shown by https://github.com/llvm/llvm-project/issues/55866.

This revision was landed with ongoing or failed builds.Thu, Jun 9, 10:33 AM
This revision was automatically updated to reflect the committed changes.
ab added a comment.Thu, Jun 9, 10:33 AM

Yep, committed c68b469e0788, thanks!