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

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 15 18:23:49 PST 2018

vsk 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:
> 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.
I'll add a comment.

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
aprantl wrote:
> Hard-coding the metadata numbering (metadata !92, !dbg !100) seems to be really fragile, and probably doesn't add anything to the test.
It tests that we don't apply debug values to the wrong variables, which seemed worthwhile to me. Can we keep the numbering until it makes test updates harder than they need to be? Or would you rather simply delete them now?


More information about the llvm-commits mailing list