[CGP] Reset the debug location when promoting zext(s).
authorDavide Italiano <ditaliano@apple.com>
Wed, 17 Jun 2020 17:29:47 +0000 (10:29 -0700)
committerDavide Italiano <ditaliano@apple.com>
Wed, 17 Jun 2020 18:13:13 +0000 (11:13 -0700)
When the zext gets promoted, it used to retain the original location,
which pessimizes the debugging experience causing an unexpected
jump in stepping at -Og.

Fixes https://bugs.llvm.org/show_bug.cgi?id=46120 (which also
contains a full C repro).

Differential Revision:  https://reviews.llvm.org/D81437

llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/test/DebugInfo/X86/zextload.ll
llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll [new file with mode: 0644]

index 935fc95..f01a1fe 100644 (file)
@@ -2443,6 +2443,9 @@ namespace {
 /// This class provides transaction based operation on the IR.
 /// Every change made through this class is recorded in the internal state and
 /// can be undone (rollback) until commit is called.
+/// CGP does not check if instructions could be speculatively executed when
+/// moved. Preserving the original location would pessimize the debugging
+/// experience, as well as negatively impact the quality of sample PGO.
 class TypePromotionTransaction {
   /// This represents the common interface of the individual transaction.
   /// Each class implements the logic for doing one specific modification on
@@ -2658,6 +2661,7 @@ class TypePromotionTransaction {
     ZExtBuilder(Instruction *InsertPt, Value *Opnd, Type *Ty)
         : TypePromotionAction(InsertPt) {
       IRBuilder<> Builder(InsertPt);
+      Builder.SetCurrentDebugLocation(DebugLoc());
       Val = Builder.CreateZExt(Opnd, Ty, "promoted");
       LLVM_DEBUG(dbgs() << "Do: ZExtBuilder: " << *Val << "\n");
     }
@@ -5798,16 +5802,8 @@ bool CodeGenPrepare::optimizeExt(Instruction *&Inst) {
   if (canFormExtLd(SpeculativelyMovedExts, LI, ExtFedByLoad, HasPromoted)) {
     assert(LI && ExtFedByLoad && "Expect a valid load and extension");
     TPT.commit();
-    // Move the extend into the same block as the load
+    // Move the extend into the same block as the load.
     ExtFedByLoad->moveAfter(LI);
-    // CGP does not check if the zext would be speculatively executed when moved
-    // to the same basic block as the load. Preserving its original location
-    // would pessimize the debugging experience, as well as negatively impact
-    // the quality of sample pgo. We don't want to use "line 0" as that has a
-    // size cost in the line-table section and logically the zext can be seen as
-    // part of the load. Therefore we conservatively reuse the same debug
-    // location for the load and the zext.
-    ExtFedByLoad->setDebugLoc(LI->getDebugLoc());
     ++NumExtsMoved;
     Inst = ExtFedByLoad;
     return true;
index d076fa9..7a48c54 100644 (file)
@@ -23,7 +23,7 @@
 
 ; CHECK-LABEL: @test
 ; CHECK:   [[LOADVAL:%[0-9]+]] = load i32, i32* %ptr, align 4, !dbg [[DEBUGLOC:![0-9]+]]
-; CHECK-NEXT:                    zext i32 [[LOADVAL]] to i64, !dbg [[DEBUGLOC]]
+; CHECK-NEXT:                    zext i32 [[LOADVAL]] to i64
 ; CHECK:   [[DEBUGLOC]] = !DILocation(line: 3
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll b/llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll
new file mode 100644 (file)
index 0000000..f76e101
--- /dev/null
@@ -0,0 +1,37 @@
+; RUN: opt < %s -codegenprepare -S -mtriple=x86_64-unknown-unknown | FileCheck %s --match-full-lines
+  
+; Make sure the promoted zext doesn't get a debug location associated.
+; CHECK: %promoted = zext i8 %t to i64
+
+define void @patatino(i8* %p, i64* %q, i32 %b, i32* %addr) !dbg !6 {
+entry:
+  %t = load i8, i8* %p, align 1, !dbg !8
+  %zextt = zext i8 %t to i32, !dbg !9
+  %add = add nuw i32 %zextt, %b, !dbg !10
+  %add2 = add nuw i32 %zextt, 12, !dbg !11
+  store i32 %add, i32* %addr, align 4, !dbg !12
+  %s = zext i32 %add2 to i64, !dbg !13
+  store i64 %s, i64* %q, align 4, !dbg !14
+  ret void, !dbg !15
+}
+
+!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: "a2.ll", directory: "/")
+!2 = !{}
+!3 = !{i32 8}
+!4 = !{i32 0}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "patatino", linkageName: "patatino", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 1, column: 1, scope: !6)
+!9 = !DILocation(line: 2, column: 1, scope: !6)
+!10 = !DILocation(line: 3, column: 1, scope: !6)
+!11 = !DILocation(line: 4, column: 1, scope: !6)
+!12 = !DILocation(line: 5, column: 1, scope: !6)
+!13 = !DILocation(line: 6, column: 1, scope: !6)
+!14 = !DILocation(line: 7, column: 1, scope: !6)
+!15 = !DILocation(line: 8, column: 1, scope: !6)