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

[davide]

Summary

Full analysis in: https://bugs.llvm.org/show_bug.cgi?id=46120
tl;dr: avoid jumpiness in line tables

[probinson]

That's a pretty long test case for a one-line patch; there's no existing zext testcase where you can just add !dbg metadata?

[davide]

Let me try putting an assertion with the patch and see what I can do.

[davide]

Still looking for a reduced tescase but I noticed another bug, so I put another review online:
https://reviews.llvm.org/D81879

[davide]

I tried really hard to reduce this one but I failed. I guess I can either drop this review or I'm happy to hear suggestions. @probinson

[davide]

I might have found a way to reduce :)

[davide]

Updated diff/reduced testcase/found another bug/added context.

[vsk]

llvm/lib/CodeGen/CodeGenPrepare.cpp

5809

Might be worth moving parts of this comment near the TypePromotionTransaction definition, to justify why those clear the IRBuilder debug loc?

llvm/test/DebugInfo/X86/zextload.ll

26 ↗

(On Diff #271233)

{{$}}?

[davide]

Address Vedant's feedback.

[vsk]

Lgtm with minor nitpicks inline, thanks!

llvm/lib/CodeGen/CodeGenPrepare.cpp

2652

To quibble: s/would/could/ might be more fair, since we're conservatively dropping the location just in case the created instruction gets moved around after the fact -- it's just that this tends to happen a lot :). Also, maybe moving this to the TypePromotionAction class definition would serve as a more general warning?

[probinson]

Definitely better, thanks!

[davide]

Committed

commit 1cbaf847ab84f37d9044d867b5ed8ae646c6ebc9 (HEAD -> master, origin/master, origin/HEAD)
Author: Davide Italiano <[e-mail]>
Date:   Wed Jun 17 10:29:47 2020 -0700

    [CGP] Reset the debug location when promoting zext(s).
    
    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

     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");
     }

llvm/test/Transforms/CodeGenPrepare/X86/zext-promotion-debuginfo.ll

+; RUN: opt -codegenprepare -S %s | FileCheck %s --match-full-lines
+
+; Make sure we don't assign a location to the promoted zext.
+; CHECK: %promoted1 = zext i16 %l.0 to i32
+
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@b = local_unnamed_addr global i16 0, align 2, !dbg !0
+@f = local_unnamed_addr global i32 0, align 4, !dbg !18
+@h = local_unnamed_addr global i8 0, align 1, !dbg !20
+@d = local_unnamed_addr global i32 0, align 4, !dbg !15
+@i = local_unnamed_addr global i8 0, align 1, !dbg !23
+@c = local_unnamed_addr global i16 0, align 2, !dbg !13
+@a = local_unnamed_addr global i16 0, align 2, !dbg !8
+@g_35 = local_unnamed_addr global i16 0, align 2, !dbg !11
+
+define signext i16 @foo(i32 %j, i32 %k) local_unnamed_addr #0 !dbg !33 {
+entry:
+  call void @llvm.dbg.value(metadata i32 undef, metadata !37, metadata !DIExpression()), !dbg !39
+  call void @llvm.dbg.value(metadata i32 %k, metadata !38, metadata !DIExpression()), !dbg !39
+  %conv = trunc i32 %k to i16, !dbg !40
+  ret i16 %conv, !dbg !41
+}
+
+define i32 @main() local_unnamed_addr #1 !dbg !42 {
+entry:
+  call void @llvm.dbg.value(metadata i32 1, metadata !46, metadata !DIExpression()), !dbg !57
+  %0 = load i16, i16* @b, align 2, !dbg !58, !tbaa !59
+  %conv = sext i16 %0 to i64, !dbg !58
+  %1 = inttoptr i64 %conv to i32*, !dbg !58
+  call void @llvm.dbg.value(metadata i32* %1, metadata !48, metadata !DIExpression()), !dbg !57
+  %2 = load i32, i32* @f, align 4, !dbg !63, !tbaa !64
+  %tobool = icmp eq i32 %2, 0, !dbg !63
+  br i1 %tobool, label %if.end, label %if.then, !dbg !66
+
+if.then:                                          ; preds = %entry
+  %3 = load i8, i8* @h, align 1, !dbg !67, !tbaa !68
+  %inc = add i8 %3, 1, !dbg !67
+  store i8 %inc, i8* @h, align 1, !dbg !67, !tbaa !68
+  br label %if.end, !dbg !69
+
+if.end:                                           ; preds = %entry, %if.then
+  %l.0 = phi i16 [ 1, %if.then ], [ 0, %entry ]
+  call void @llvm.dbg.value(metadata i32 undef, metadata !46, metadata !DIExpression()), !dbg !57
+  %4 = load i16, i16* @g_35, align 2, !dbg !70, !tbaa !59
+  %conv2 = and i16 %4, %l.0, !dbg !70
+  store i16 %conv2, i16* @g_35, align 2, !dbg !70, !tbaa !59
+  %5 = load i32, i32* @d, align 4, !dbg !71, !tbaa !64
+  %conv3 = trunc i32 %5 to i8, !dbg !71
+  store i8 %conv3, i8* @i, align 1, !dbg !72, !tbaa !68
+  call void @llvm.dbg.value(metadata i16 %l.0, metadata !53, metadata !DIExpression()), !dbg !57
+  %6 = trunc i32 %5 to i16, !dbg !73
+  %conv6 = mul nuw i16 %l.0, %6, !dbg !73
+  store i16 %conv6, i16* @c, align 2, !dbg !74, !tbaa !59
+  %7 = load i16, i16* @a, align 2, !dbg !75, !tbaa !59
+  %tobool7 = icmp eq i16 %7, 0, !dbg !75
+  br i1 %tobool7, label %if.end13, label %if.then8, !dbg !76
+
+if.then8:                                         ; preds = %if.end
+  %conv11 = zext i16 %conv2 to i32, !dbg !77
+  %call = call signext i16 @foo(i32 undef, i32 %conv11), !dbg !78
+  %conv12 = sext i16 %call to i32, !dbg !79
+  store i32 %conv12, i32* %1, align 4, !dbg !80, !tbaa !64
+  br label %if.end13, !dbg !81
+
+if.end13:                                         ; preds = %if.end, %if.then8
+  ret i32 0, !dbg !82
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!28, !29, !30, !31}
+!llvm.ident = !{!32}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 1, type: !10, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project 43793b89a0792439588ba1b18f46a62d0b401cc7)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, retainedTypes: !5, globals: !7, nameTableKind: None, sysroot: "/")
+!3 = !DIFile(filename: "a.c", directory: "/Users/davide/work/build-debug/bin")
+!4 = !{}
+!5 = !{!6}
+!6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!7 = !{!8, !11, !0, !13, !15, !18, !20, !23, !25}
+!8 = !DIGlobalVariableExpression(var: !9, expr: !DIExpression())
+!9 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !10, isLocal: false, isDefinition: true)
+!10 = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+!11 = !DIGlobalVariableExpression(var: !12, expr: !DIExpression())
+!12 = distinct !DIGlobalVariable(name: "g_35", scope: !2, file: !3, line: 1, type: !10, isLocal: false, isDefinition: true)
+!13 = !DIGlobalVariableExpression(var: !14, expr: !DIExpression())
+!14 = distinct !DIGlobalVariable(name: "c", scope: !2, file: !3, line: 1, type: !10, isLocal: false, isDefinition: true)
+!15 = !DIGlobalVariableExpression(var: !16, expr: !DIExpression())
+!16 = distinct !DIGlobalVariable(name: "d", scope: !2, file: !3, line: 2, type: !17, isLocal: false, isDefinition: true)
+!17 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!18 = !DIGlobalVariableExpression(var: !19, expr: !DIExpression())
+!19 = distinct !DIGlobalVariable(name: "f", scope: !2, file: !3, line: 2, type: !17, isLocal: false, isDefinition: true)
+!20 = !DIGlobalVariableExpression(var: !21, expr: !DIExpression())
+!21 = distinct !DIGlobalVariable(name: "h", scope: !2, file: !3, line: 4, type: !22, isLocal: false, isDefinition: true)
+!22 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!23 = !DIGlobalVariableExpression(var: !24, expr: !DIExpression())
+!24 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 4, type: !22, isLocal: false, isDefinition: true)
+!25 = !DIGlobalVariableExpression(var: !26, expr: !DIExpression())
+!26 = distinct !DIGlobalVariable(name: "g", scope: !2, file: !3, line: 3, type: !27, isLocal: true, isDefinition: true)
+!27 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size: 64)
+!28 = !{i32 7, !"Dwarf Version", i32 4}
+!29 = !{i32 2, !"Debug Info Version", i32 3}
+!30 = !{i32 1, !"wchar_size", i32 4}
+!31 = !{i32 7, !"PIC Level", i32 2}
+!32 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project 43793b89a0792439588ba1b18f46a62d0b401cc7)"}
+!33 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 5, type: !34, scopeLine: 5, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !36)
+!34 = !DISubroutineType(types: !35)
+!35 = !{!10, !17, !17}
+!36 = !{!37, !38}
+!37 = !DILocalVariable(name: "j", arg: 1, scope: !33, file: !3, line: 5, type: !17)
+!38 = !DILocalVariable(name: "k", arg: 2, scope: !33, file: !3, line: 5, type: !17)
+!39 = !DILocation(line: 0, scope: !33)
+!40 = !DILocation(line: 6, column: 13, scope: !33)
+!41 = !DILocation(line: 6, column: 6, scope: !33)
+!42 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 9, type: !43, scopeLine: 9, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !45)
+!43 = !DISubroutineType(types: !44)
+!44 = !{!17}
+!45 = !{!46, !48, !50, !53, !54}
+!46 = !DILocalVariable(name: "l", scope: !47, file: !3, line: 11, type: !17)
+!47 = distinct !DILexicalBlock(scope: !42, file: !3, line: 10, column: 1)
+!48 = !DILocalVariable(name: "l_258", scope: !47, file: !3, line: 12, type: !49)
+!49 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !17, size: 64)
+!50 = !DILocalVariable(name: "m", scope: !51, file: !3, line: 15, type: !49)
+!51 = distinct !DILexicalBlock(scope: !52, file: !3, line: 14, column: 10)
+!52 = distinct !DILexicalBlock(scope: !47, file: !3, line: 13, column: 9)
+!53 = !DILocalVariable(name: "e", scope: !47, file: !3, line: 20, type: !10)
+!54 = !DILocalVariable(name: "l_305", scope: !55, file: !3, line: 23, type: !17)
+!55 = distinct !DILexicalBlock(scope: !56, file: !3, line: 22, column: 13)
+!56 = distinct !DILexicalBlock(scope: !47, file: !3, line: 22, column: 9)
+!57 = !DILocation(line: 0, scope: !47)
+!58 = !DILocation(line: 12, column: 18, scope: !47)
+!59 = !{!60, !60, i64 0}
+!60 = !{!"short", !61, i64 0}
+!61 = !{!"omnipotent char", !62, i64 0}
+!62 = !{!"Simple C/C++ TBAA"}
+!63 = !DILocation(line: 13, column: 9, scope: !52)
+!64 = !{!65, !65, i64 0}
+!65 = !{!"int", !61, i64 0}
+!66 = !DILocation(line: 13, column: 9, scope: !47)
+!67 = !DILocation(line: 13, column: 14, scope: !52)
+!68 = !{!61, !61, i64 0}
+!69 = !DILocation(line: 13, column: 13, scope: !52)
+!70 = !DILocation(line: 18, column: 8, scope: !47)
+!71 = !DILocation(line: 19, column: 9, scope: !47)
+!72 = !DILocation(line: 19, column: 7, scope: !47)
+!73 = !DILocation(line: 21, column: 10, scope: !47)
+!74 = !DILocation(line: 21, column: 7, scope: !47)
+!75 = !DILocation(line: 22, column: 9, scope: !56)
+!76 = !DILocation(line: 22, column: 9, scope: !47)
+!77 = !DILocation(line: 24, column: 45, scope: !55)
+!78 = !DILocation(line: 24, column: 20, scope: !55)
+!79 = !DILocation(line: 24, column: 19, scope: !55)
+!80 = !DILocation(line: 24, column: 17, scope: !55)
+!81 = !DILocation(line: 25, column: 5, scope: !55)
+!82 = !DILocation(line: 27, column: 1, scope: !42)