[Transforms][SROA] Skip uses of allocas where the type is scalable
ClosedPublic
Actions

Authored by c-rhodes on Aug 11 2020, 5:56 AM.

Details

Reviewers

efriedma
sdesmalen
cameron.mcinally
david-arm
rengolin

Commits

rG511d5aaca37a: [Transforms][SROA] Skip uses of allocas where the type is scalable

Summary

When splitting an alloca of a fixed-length vector (VLS) and making a
slice for each load and store, skip uses where the VLS is bitcasted to a
scalable vector (VLA).

This is relevant in the implementation of the 'arm_sve_vector_bits'
attribute that is used to define VLS types. See D85128 for more
information.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

c-rhodes created this revision.Aug 11 2020, 5:56 AM

Herald added a reviewer: rengolin. · View Herald TranscriptAug 11 2020, 5:56 AM

Herald added a project: Restricted Project. · View Herald Transcript

Herald added subscribers: hiraditya, kristof.beyls. · View Herald Transcript

c-rhodes requested review of this revision.Aug 11 2020, 5:56 AM

Harbormaster completed remote builds in B67885: Diff 284680.Aug 11 2020, 6:24 AM

david-arm added inline comments.Aug 11 2020, 9:00 AM

llvm/test/Transforms/SROA/scalable-vectors.ll
54	I think this is undefined behaviour for anything other than 512 fixed length vectors. Is it possible to amend the opt line to specify the fixed length vector size?

c-rhodes added inline comments.Aug 11 2020, 11:39 AM

llvm/test/Transforms/SROA/scalable-vectors.ll
54	I think this is undefined behaviour for anything other than 512 fixed length vectors. Is it possible to amend the opt line to specify the fixed length vector size? Good point, I can add the `-aarch64-sve-vector-bits-min=` flag.

Specify -aarch64-sve-vector-bits-min=512 in test.

c-rhodes marked an inline comment as done.Aug 11 2020, 12:05 PM

c-rhodes added inline comments.

llvm/test/Transforms/SROA/scalable-vectors.ll
54	I think this is undefined behaviour for anything other than 512 fixed length vectors. Is it possible to amend the opt line to specify the fixed length vector size? Good point, I can add the -aarch64-sve-vector-bits-min= flag. I've added new check lines for these tests that specify `-aarch64-sve-vector-bits-min=512`. I also tried specifying it in an attribute group but I wasn't sure it was having any effect since it didn't complain when I put garbage in there.

LGTM

This revision is now accepted and ready to land.Aug 11 2020, 12:18 PM

I also tried specifying it in an attribute group but I wasn't sure it was having any effect since it didn't complain when I put garbage in there.

Err, right... aarch64-sve-vector-bits-min is a backend flag; it's not going to have any effect on SROA. (I'm not sure what effect it could have, anyway.) So probably the extra run lines aren't useful.

In D85725#2211150, @efriedma wrote:

I also tried specifying it in an attribute group but I wasn't sure it was having any effect since it didn't complain when I put garbage in there.

Err, right... aarch64-sve-vector-bits-min is a backend flag; it's not going to have any effect on SROA. (I'm not sure what effect it could have, anyway.) So probably the extra run lines aren't useful.

Ah ok, I'll remove them then.

Closed by commit rG511d5aaca37a: [Transforms][SROA] Skip uses of allocas where the type is scalable (authored by c-rhodes). · Explain WhyAug 12 2020, 2:36 AM

This revision was automatically updated to reflect the committed changes.

c-rhodes added a commit: rG511d5aaca37a: [Transforms][SROA] Skip uses of allocas where the type is scalable.