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

[loladiro]

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.

[aprantl]

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.

[loladiro]

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]

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.

lib/Transforms/Scalar/SROA.cpp

     auto *Var = DbgDecl->getVariable();
     auto *Expr = DbgDecl->getExpression();
     DIBuilder DIB(*AI.getModule(), /*AllowUnresolved*/ false);
-    bool IsSplit = Pieces.size() > 1;
+    uint64_t AllocaSize = DL.getTypeSizeInBits(AI.getAllocatedType());
     for (auto Piece : Pieces) {
       // Create a piece expression describing the new partition or reuse AI's
       // expression if there is only one partition.
       auto *PieceExpr = Expr;
-      if (IsSplit || Expr->isBitPiece()) {
+      if (Piece.Size < AllocaSize || Expr->isBitPiece()) {
         // If this alloca is already a scalar replacement of a larger aggregate,
         // Piece.Offset describes the offset inside the scalar.
         uint64_t Offset = Expr->isBitPiece() ? Expr->getBitPieceOffset() : 0;

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

+; Checks that SROA still inserts a bit_piece expression, even if it produces only one piece
+; (as long as that piece is smaller than the whole thing)
+; RUN: opt -sroa %s -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+%foo = type { [8 x i8], [8 x i8] }
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+define void @_ZL18findInsertLocationPN4llvm17MachineBasicBlockENS_9SlotIndexERNS_13LiveIntervalsE() {
+entry:
+  %retval = alloca %foo, align 8
+  call void @llvm.dbg.declare(metadata %foo* %retval, metadata !1, metadata !7), !dbg !8
+; CHECK: call void @llvm.dbg.value(metadata %foo* undef, i64 0, metadata !1, metadata ![[BIT_PIECE:[0-9]+]]), !dbg !8
+; CHECK: ![[BIT_PIECE]] = !DIExpression(DW_OP_bit_piece, 64, 64)
+  %0 = bitcast %foo* %retval to i8*
+  %1 = getelementptr inbounds i8, i8* %0, i64 8
+  %2 = bitcast i8* %1 to %foo**
+  store %foo* undef, %foo** %2, align 8
+  ret void
+}
+
+attributes #0 = { nounwind readnone }
+
+!llvm.dbg.cu = !{}
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = !DILocalVariable(name: "I", scope: !2, file: !3, line: 947, type: !4)
+!2 = distinct !DISubprogram(name: "findInsertLocation", linkageName: "_ZL18findInsertLocationPN4llvm17MachineBasicBlockENS_9SlotIndexERNS_13LiveIntervalsE", scope: !3, file: !3, line: 937, isLocal: true, isDefinition: true, scopeLine: 938, flags: DIFlagPrototyped, isOptimized: true)
+!3 = !DIFile(filename: "none", directory: ".")
+!4 = !DICompositeType(tag: DW_TAG_class_type, name: "bundle_iterator<llvm::MachineInstr, llvm::ilist_iterator<llvm::MachineInstr> >", scope: !5, file: !3, line: 163, size: 128, align: 64, elements: !6, templateParams: !6, identifier: "_ZTSN4llvm17MachineBasicBlock15bundle_iteratorINS_12MachineInstrENS_14ilist_iteratorIS2_EEEE")
+!5 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "MachineBasicBlock", file: !3, line: 68, size: 1408, align: 64, identifier: "_ZTSN4llvm17MachineBasicBlockE")
+!6 = !{}
+!7 = !DIExpression()
+!8 = !DILocation(line: 947, column: 35, scope: !2)