[PATCH] D81437: [CGP] Reset the debug location when promoting zext(s)

[Davide Italiano via Phabricator via llvm-commits]

davide updated this revision to Diff 271234.
davide added a comment.

Address Vedant's feedback.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81437/new/

https://reviews.llvm.org/D81437

Files:
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/test/DebugInfo/X86/zextload.ll
  llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll


Index: llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/CodeGenPrepare/X86/promoted-zext-debugloc.ll
@@ -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)
Index: llvm/test/DebugInfo/X86/zextload.ll
===================================================================
--- llvm/test/DebugInfo/X86/zextload.ll
+++ llvm/test/DebugInfo/X86/zextload.ll
@@ -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"
Index: llvm/lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -2648,6 +2648,9 @@
   };
 
   /// Build a zero extension instruction.
+  /// CGP does not check if instructions would 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 ZExtBuilder : public TypePromotionAction {
     Value *Val;
 
@@ -2658,6 +2661,7 @@
     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 @@
   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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81437.271234.patch
Type: text/x-patch
Size: 4345 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200616/279fe6f8/attachment.bin>