[PATCH] D43324: WIP: [Utils] Salvage debug info of DCE'ed extractvalue instructions

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/Transforms/Utils/Local.cpp:1605
+    unsigned FieldOffset = 0;
+    for (unsigned I = 0, E = FieldIndices.size(); I < E; ++I) {
+      Type *IndexedTy = ExtractValueInst::getIndexedType(
----------------
aprantl wrote:
> Why does this loop go all the way to FieldIndices.size()?
This is overloading `I` which makes the code harder to read.


================
Comment at: lib/Transforms/Utils/Local.cpp:1605
+    unsigned FieldOffset = 0;
+    for (unsigned I = 0, E = FieldIndices.size(); I < E; ++I) {
+      Type *IndexedTy = ExtractValueInst::getIndexedType(
----------------
aprantl wrote:
> aprantl wrote:
> > Why does this loop go all the way to FieldIndices.size()?
> This is overloading `I` which makes the code harder to read.
got it.. this is iterating over the field indices of the extract_value instruction, not over the data type! I was confused by this.


================
Comment at: test/Transforms/InstCombine/debuginfo-variables.ll:125
+; CHECK-LABEL: @test_extractvalue_4(
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata %ty2 %S, metadata !92, metadata !DIExpression(DW_OP_constu, 8, DW_OP_shr, DW_OP_stack_value)), !dbg !100
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata %ty2 %S, metadata !94, metadata !DIExpression(DW_OP_constu, 64, DW_OP_shr, DW_OP_stack_value)), !dbg !101
----------------
Hard-coding the metadata numbering (metadata !92, !dbg !100) seems to be really fragile, and probably doesn't add anything to the test.


https://reviews.llvm.org/D43324