[PATCH] D100270: [debug-info] MemCpyOpt should merge the debug location when replace multiple block-local instruction with a new memset

Yuanbo Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 11 12:29:22 PDT 2021


yuanboli233 created this revision.
yuanboli233 added reviewers: vsk, jmorse, dblaikie, djtodoro.
yuanboli233 added a project: debug-info.
Herald added a subscriber: hiraditya.
yuanboli233 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The bug is proposed here: https://bugs.llvm.org/show_bug.cgi?id=49270
Nobody has commented on the bug yet, but I think it clearly deviates from the description in our LLVM how-to-update-debug-info guide here: https://llvm.org/docs/HowToUpdateDebugInfo.html#when-to-merge-instruction-locations

"(Merges should be applied when) peephole optimizations which combine multiple instructions together, like (add (mul A B) C) => llvm.fma.f32(A, B, C)".

Multiple store instructions in the same basic block are merged into one memset instruction:

In the original ll file:

  %arrayidx = getelementptr inbounds i32, i32* %P, i64 1, !dbg !13
    call void @llvm.dbg.value(metadata i32* %arrayidx, metadata !9, metadata !DIExpression()), !dbg !13
    store i32 0, i32* %arrayidx, align 4, !dbg !14
    %add.ptr = getelementptr inbounds i32, i32* %P, i64 2, !dbg !15
    call void @llvm.dbg.value(metadata i32* %add.ptr, metadata !11, metadata !DIExpression()), !dbg !15
    %0 = bitcast i32* %add.ptr to i8*, !dbg !16
    call void @llvm.dbg.value(metadata i8* %0, metadata !12, metadata !DIExpression()), !dbg !16
    tail call void @llvm.memset.p0i8.i64(i8* %0, i8 0, i64 11, i1 false), !dbg !17
    ret void, !dbg !18

The ll file after apply -memcpyopt pass:

  %arrayidx = getelementptr inbounds i32, i32* %P, i64 1, !dbg !13
    call void @llvm.dbg.value(metadata i32* %arrayidx, metadata !9, metadata !DIExpression()), !dbg !13
    %add.ptr = getelementptr inbounds i32, i32* %P, i64 2, !dbg !14
    call void @llvm.dbg.value(metadata i32* %add.ptr, metadata !11, metadata !DIExpression()), !dbg !14
    %0 = bitcast i32* %add.ptr to i8*, !dbg !15
    call void @llvm.dbg.value(metadata i8* %0, metadata !12, metadata !DIExpression()), !dbg !15
    %1 = bitcast i32* %arrayidx to i8*, !dbg !16
    call void @llvm.memset.p0i8.i64(i8* align 4 %1, i8 0, i64 15, i1 false), !dbg !17
    ret void, !dbg !16

There are two stores: "stores i32 0, i32* %arrayidx, align 4, !dbg !14" and "tail call void @llvm.memset.p0i8.i64(i8* %0, i8 0, i64 11, i1 false), !dbg !17".

They are merged into one memset instruction " call void @llvm.memset.p0i8.i64(i8* align 4 %1, i8 0, i64 15, i1 false), !dbg !17".

The new memset instruction only takes the debug location from one of the merged instructions. However, it should take the merged debug location from the two instructions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100270

Files:
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/test/Transforms/MemCpyOpt/memset-merged-loc.ll


Index: llvm/test/Transforms/MemCpyOpt/memset-merged-loc.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/MemCpyOpt/memset-merged-loc.ll
@@ -0,0 +1,50 @@
+; RUN: opt -memcpyopt -S < %s | FileCheck %s
+; CHECK: !{{[0-9]+}} = !DILocation(line: 0, scope: !{{[0-9]+}})
+
+; Function Attrs: ssp
+define void @test3(i32* %P) #0 !dbg !6 {
+entry:
+  %arrayidx = getelementptr inbounds i32, i32* %P, i64 1, !dbg !13
+  call void @llvm.dbg.value(metadata i32* %arrayidx, metadata !9, metadata !DIExpression()), !dbg !13
+  store i32 0, i32* %arrayidx, align 4, !dbg !14
+  %add.ptr = getelementptr inbounds i32, i32* %P, i64 2, !dbg !15
+  call void @llvm.dbg.value(metadata i32* %add.ptr, metadata !11, metadata !DIExpression()), !dbg !15
+  %0 = bitcast i32* %add.ptr to i8*, !dbg !16
+  call void @llvm.dbg.value(metadata i8* %0, metadata !12, metadata !DIExpression()), !dbg !16
+  tail call void @llvm.memset.p0i8.i64(i8* %0, i8 0, i64 11, i1 false), !dbg !17
+  ret void, !dbg !18
+}
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #1
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+attributes #0 = { ssp }
+attributes #1 = { argmemonly nofree nosync nounwind willreturn writeonly }
+attributes #2 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!3, !4}
+!llvm.module.flags = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "bugpoint-reduced-simplified.bc", directory: "/")
+!2 = !{}
+!3 = !{i32 6}
+!4 = !{i32 3}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "test3", linkageName: "test3", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+!7 = !DISubroutineType(types: !2)
+!8 = !{!9, !11, !12}
+!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
+!11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 3, type: !10)
+!12 = !DILocalVariable(name: "3", scope: !6, file: !1, line: 4, type: !10)
+!13 = !DILocation(line: 1, column: 1, scope: !6)
+!14 = !DILocation(line: 2, column: 1, scope: !6)
+!15 = !DILocation(line: 3, column: 1, scope: !6)
+!16 = !DILocation(line: 4, column: 1, scope: !6)
+!17 = !DILocation(line: 5, column: 1, scope: !6)
+!18 = !DILocation(line: 6, column: 1, scope: !6)
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -484,8 +484,14 @@
                                                    : Range.TheStores) dbgs()
                                               << *SI << '\n';
                dbgs() << "With: " << *AMemSet << '\n');
-    if (!Range.TheStores.empty())
-      AMemSet->setDebugLoc(Range.TheStores[0]->getDebugLoc());
+    if (!Range.TheStores.empty()) {
+      std::vector<const DILocation *> storeLocs;
+      for (auto storeInst : Range.TheStores) {
+        storeLocs.push_back(storeInst->getDebugLoc().get());
+      }
+      DebugLoc dl = DebugLoc(DILocation::getMergedLocations(storeLocs));
+      AMemSet->setDebugLoc(dl);
+    }
 
     if (MSSAU) {
       assert(LastMemDef && MemInsertPoint &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100270.336682.patch
Type: text/x-patch
Size: 3702 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210411/1047d29f/attachment.bin>


More information about the llvm-commits mailing list