This is an archive of the discontinued LLVM Phabricator instance.

[SROA] Also insert a bit piece expression if only one piece is needed
ClosedPublic

Authored by loladiro on Jan 14 2016, 6:50 AM.

Details

Summary

If SROA creates only one piece (e.g. because the other is not needed), it still needs to create a bit_piece expression if that bit piece is smaller than the original size of the alloca.

Diff Detail

Repository
rL LLVM

Event Timeline

loladiro updated this revision to Diff 44878.Jan 14 2016, 6:50 AM
loladiro retitled this revision from to [SROA] Also insert a bit piece expression if only one piece is needed.
loladiro updated this object.
loladiro added a reviewer: aprantl.
loladiro set the repository for this revision to rL LLVM.
loladiro added a subscriber: llvm-commits.
aprantl accepted this revision.Jan 14 2016, 9:09 AM
aprantl edited edge metadata.

Thanks. LGTM with minor comments addressed.

lib/Transforms/Scalar/SROA.cpp
4032

Should we add an
} else assert(Pieces.size() == 1&& "partition is as large as original alloca")
?

test/Transforms/SROA/dbg-single-piece.ll
12

I think we should also check for the sroa'd alloca and verify that its size matches the size of the OP_bit_piece.

13

checking for the hardcoded !dbg !8 is just asking for trouble :-)

14

Comment?
; If SROA creates only one piece (e.g. because the other is not needed), it still needs to create a bit_piece expression if that bit piece is smaller than the original size of the alloca.

This revision is now accepted and ready to land.Jan 14 2016, 9:09 AM
loladiro added inline comments.Jan 14 2016, 9:12 AM
lib/Transforms/Scalar/SROA.cpp
4032

Sounds reasonable.

test/Transforms/SROA/dbg-single-piece.ll
12

The problem is that it gets mem2reged so won't be in the final IR.

13

Yep, sorry.

14

I have a comment at the very top. Do you want me to move it here?

aprantl added inline comments.Jan 14 2016, 9:18 AM
test/Transforms/SROA/dbg-single-piece.ll
12

I see. Maybe add CHECK-NOT: dbg.value before and after to ensure we actually hit the case where there is only one alloca?

14

My bad! I didn't see it. That said, moving it closer to the CHECK might be a good idea.

This revision was automatically updated to reflect the committed changes.