[PATCH] D16061: [Utils] Insert DW_OP_bit_piece when only describing part of the variable

[Keno Fischer via llvm-commits]

loladiro added inline comments.

================
Comment at: lib/Transforms/Utils/Local.cpp:1061
@@ +1060,3 @@
+    const DataLayout &DL = DDI->getModule()->getDataLayout();
+    NewDIExpr.push_back(DL.getTypeSizeInBits(ExtendedArg->getType())); // Size
+    Builder.insertDbgValueIntrinsic(ExtendedArg, 0, DIVar,
----------------
aprantl wrote:
> It's not clear to me that a sign/zero-extended value always indicates that the value is a subset of the variable. Is that guaranteed? Note that the Verifier checks that a piece does not cover the entire variable.
Yes, I think that's guaranteed. sext/zext guarantee that they expand from a smaller to a larger value, and if the IR was correct before, then the extended value would cover the entire variable, so this should be smaller. One case that does occur to me however, is where the expression already contains a DW_OP_bit_piece. In that case we'll have to update the existing one.

================
Comment at: test/Transforms/Util/split-bit-piece.ll:1
@@ +1,2 @@
+; Checks that SROA inserts a proper bit piece expression if it only describes
+; part of a variable
----------------
aprantl wrote:
> SORA can create bit pieces, but in this case I assume it's Local that is supposed to generate the bit piece?
Well, SORA calls the Local util, but I'll make the comment more clear.

================
Comment at: test/Transforms/Util/split-bit-piece.ll:14
@@ +13,3 @@
+  %v2 = alloca i64, align 8
+  store i32 %hs, i32* %hs.addr, align 4
+; CHECK: call void @llvm.dbg.value({{[^,]*}}, i64 0, metadata !{{[0-9]+}}, metadata ![[EXPR:[0-9]+]])
----------------
aprantl wrote:
> Please also check for the fist argument of the dbg.value so it is clear what the assumptions about the lowering are. This will help us keeping the test useful in the future.
Ok.


Repository:
  rL LLVM

http://reviews.llvm.org/D16061