This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel] Legalization and ISel support for load/stores of vectors of pointers
ClosedPublic

Authored by aemerson on Apr 10 2019, 12:35 PM.

Details

Summary

Loads and store of values with type like <2 x p0> currently don't get imported because SelectionDAG has no knowledge of pointer types. To leverage the existing support for vector load/stores, we can bitcast the value to have s64 element types instead. We do this as a custom legalization.

This patch also adds support for general loads of <2 x s64>, and relaxes some type conditions on selecting G_BITCAST.

Diff Detail

Repository
rL LLVM

Event Timeline

aemerson created this revision.Apr 10 2019, 12:35 PM
arsenm added a subscriber: arsenm.Apr 10 2019, 2:03 PM
arsenm added inline comments.
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
218–220 ↗(On Diff #194571)

The downside of this is it now breaks any non-0 address pointers, where before they would just work. You might want to add another case for arbitrary address spaces

aemerson marked an inline comment as done.Apr 10 2019, 3:29 PM
aemerson added inline comments.
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
218–220 ↗(On Diff #194571)

Not sure what you mean, how does this break non-zero addrspace pointers? This legalization rule only applies to the 0 addrspace p0 type.

aemerson marked an inline comment as done.Apr 10 2019, 4:28 PM
aemerson added inline comments.
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
218–220 ↗(On Diff #194571)

Ah, I see what you mean. New patch incoming.

aemerson updated this revision to Diff 194611.Apr 10 2019, 4:46 PM

Updated patch to not use the custom legalization for non-p0 vectors.

paquette accepted this revision.Apr 11 2019, 10:26 AM

I think this looks fine now that you've addressed @arsenm's comment.

This revision is now accepted and ready to land.Apr 11 2019, 10:26 AM
arsenm added inline comments.Apr 11 2019, 10:44 AM
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
218–220 ↗(On Diff #194571)

In this case you have to watch out for both the loaded type, as well as the actual pointer type. For the actual load address space the current SelectionDAG behavior means that any non-0 address space pointer type works for most targets since the default load patterns ignore the mem operand's address space.

aemerson marked an inline comment as done.Apr 11 2019, 11:07 AM
aemerson added inline comments.
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
218–220 ↗(On Diff #194571)

Ok I see the point about the current imported patterns, but this new patch doesn't break anything w.r.t that right?

arsenm added inline comments.Apr 11 2019, 11:10 AM
llvm/lib/Target/AArch64/AArch64LegalizerInfo.cpp
218–220 ↗(On Diff #194571)

For the pointer type, I think that was and remains broken. I would expect these to fail to select and hit the fallback

This revision was automatically updated to reflect the committed changes.