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

[Keno Fischer via llvm-commits]

loladiro added inline comments.

================
Comment at: lib/Transforms/Scalar/SROA.cpp:4032
@@ -4031,3 +4031,3 @@
       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,
----------------
aprantl wrote:
> Should we add an 
>  } else assert(Pieces.size() == 1&& "partition is as large as original alloca")
> ?
Sounds reasonable.

================
Comment at: test/Transforms/SROA/dbg-single-piece.ll:12
@@ +11,3 @@
+  %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
----------------
aprantl wrote:
> I think we should also check for the sroa'd alloca and verify that its size matches the size of the OP_bit_piece.
The problem is that it gets mem2reged so won't be in the final IR.

================
Comment at: test/Transforms/SROA/dbg-single-piece.ll:13
@@ +12,3 @@
+  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)
----------------
aprantl wrote:
> checking for the hardcoded !dbg !8 is just asking for trouble :-)
Yep, sorry.

================
Comment at: test/Transforms/SROA/dbg-single-piece.ll:14
@@ +13,3 @@
+; 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*
----------------
aprantl wrote:
> 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.
> 
I have a comment at the very top. Do you want me to move it here?


Repository:
  rL LLVM

http://reviews.llvm.org/D16187