This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Introduce unaligned-vector-mem feature
ClosedPublic

Authored by reames on Apr 27 2023, 12:22 PM.

Details

Summary

This allows us to model and thus test transforms which are legal only when a vector load with less than element alignment are supported. This was originally part of D126085, but was split out as we didn't have a good example of such a transform. As can be seen in the test diffs, we have the recently added concat_vector(loads) -> strided_load transform (from D147713) which now benefits from the unaligned support.

While making this change, I realized that we actually *do* support unaligned vector loads and stores of all types via conversion to i8 element type. For contiguous loads and stores, we actually already implement this in the backend - though we don't tell the optimizer that. For indexed, lowering to i8 requires complicated addressing. For indexed and segmented, we'd have to use indexed. All around, doesn't seem worthwhile pursuing, but makes for an interesting observation.

Diff Detail

Unit TestsFailed

Event Timeline

reames created this revision.Apr 27 2023, 12:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 12:22 PM
reames requested review of this revision.Apr 27 2023, 12:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2023, 12:22 PM
reames planned changes to this revision.Apr 27 2023, 12:25 PM
This comment was removed by reames.
reames requested review of this revision.Apr 27 2023, 12:45 PM

(Sent myself on a wild goose chase around the unaligned_access elf attribute. We'd discussed it in the original review, and landed without. I thought we'd later decided to add it, but I can't find evidence of that in tree or in history. So, it appears I just misremembered the outcome.)

craig.topper added a comment.EditedApr 27 2023, 2:26 PM

While making this change, I realized that we actually *do* support unaligned vector loads and stores of all types via conversion to i8 element type. For contiguous loads and stores, we actually already implement this in the backend - though we don't tell the optimizer that. For indexed, lowering to i8 requires complicated addressing. For indexed and segmented, we'd have to use indexed. All around, doesn't seem worthwhile pursuing, but makes for an interesting observation.

I don't think we support unaligned using i8 for contiguous masked load/store or VP load/store.

craig.topper accepted this revision.Apr 27 2023, 10:01 PM
craig.topper added inline comments.
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-strided-load-combine.ll
410

Might want to tweak this comment to mention unaligned-vector-mem

This revision is now accepted and ready to land.Apr 27 2023, 10:01 PM
luke accepted this revision.Apr 28 2023, 2:03 AM

While making this change, I realized that we actually *do* support unaligned vector loads and stores of all types via conversion to i8 element type. For contiguous loads and stores, we actually already implement this in the backend - though we don't tell the optimizer that. For indexed, lowering to i8 requires complicated addressing. For indexed and segmented, we'd have to use indexed. All around, doesn't seem worthwhile pursuing, but makes for an interesting observation.

I don't think we support unaligned using i8 for contiguous masked load/store or VP load/store.

You're correct about masking, and I didn't look at VP. Masking could be handled, but isn't today. For the moment, I just updated the comment in the commit and source.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-strided-load-combine.ll
410

Deleted the comment. Swear I did that before posting the patch, but seems not.

This revision was landed with ongoing or failed builds.Apr 28 2023, 8:28 AM
This revision was automatically updated to reflect the committed changes.