[llvm-branch-commits] [llvm] 6b2e4c5 - [DebugInfo][InstrRef] Don't break up ret-sequences on debug-info instrs

[Jeremy Morse via llvm-branch-commits]

Author: Jeremy Morse
Date: 2021-07-29T15:08:13+01:00
New Revision: 6b2e4c5a58d7f75f52b740bd7d96dc3a00b7ef6c

URL: https://github.com/llvm/llvm-project/commit/6b2e4c5a58d7f75f52b740bd7d96dc3a00b7ef6c
DIFF: https://github.com/llvm/llvm-project/commit/6b2e4c5a58d7f75f52b740bd7d96dc3a00b7ef6c.diff

LOG: [DebugInfo][InstrRef] Don't break up ret-sequences on debug-info instrs

When we have a terminator sequence (i.e. a tailcall or return),
MIIsInTerminatorSequence is used to work out where the preceding ABI-setup
instructions end, i.e. the parts that were glued to the terminator
instruction. This allows LLVM to split blocks safely without having to
worry about ABI stuff.

The function only ignores DBG_VALUE instructions, meaning that the two
debug instructions I recently added can end terminator sequences early,
causing various MachineVerifier errors. This patch promotes the test for
debug instructions from "isDebugValue" to "isDebugInstr", thus avoiding any
debug-info interfering with this function.

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

(cherry picked from commit 8612417e5a54cfef941ab45de55e48b4a0c4e8b4)

Added: 
    llvm/test/DebugInfo/ARM/instr-ref-tcreturn.ll

Modified: 
    llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 1415cce3b1dff..09627ee6a1645 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -1660,7 +1660,7 @@ static bool MIIsInTerminatorSequence(const MachineInstr &MI) {
     // physical registers if there is debug info associated with the terminator
     // of our mbb. We want to include said debug info in our terminator
     // sequence, so we return true in that case.
-    return MI.isDebugValue();
+    return MI.isDebugInstr();
 
   // We have left the terminator sequence if we are not doing one of the
   // following:

diff  --git a/llvm/test/DebugInfo/ARM/instr-ref-tcreturn.ll b/llvm/test/DebugInfo/ARM/instr-ref-tcreturn.ll
new file mode 100644
index 0000000000000..f86e35061dc22
--- /dev/null
+++ b/llvm/test/DebugInfo/ARM/instr-ref-tcreturn.ll
@@ -0,0 +1,66 @@
+; RUN: llc %s -o - -stop-after=finalize-isel -verify-machineinstrs -experimental-debug-variable-locations | FileCheck %s
+
+; In the sequence below, the sdiv is converted to a function call to __divsi3,
+; which is then tail call optimised. The dbg.value is suddenly stuck between
+; terminators, and the corresponding DBG_INSTR_REF is forced-placed to be
+; immediately before the TCRETURN.
+; However, with the function having the sspstrong attribute, we then try to
+; peel apart the terminator sequence, DBG_INSTR_REF is interpreted as being
+; a "real" instruction, and the stack check is inserted at that point rather
+; than before the copies-to-physreg setting up the call. This breaks the
+; code, and MachineVerifier complains.
+;
+; Check that the tail sequence is stack-protected, and split at the correct
+; position, ignoring the DBG_INSTR_REF
+
+target datalayout = "e-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+target triple = "thumbv7-apple-ios7.0.0"
+
+; CHECK-LABEL:  bb.0.entry:
+; CHECK:        LOAD_STACK_GUARD
+
+; CHECK-LABEL:  bb.2.entry:
+; CHECK:        tBL {{.*}} &__stack_chk_fail,
+
+; CHECK-LABEL:  bb.1.entry:
+; CHECK:          $r0 = COPY %0
+; CHECK-NEXT:     $r1 = COPY %1
+; CHECK-NEXT:     DBG_INSTR_REF 1, 0
+; CHECK-NEXT:     TCRETURNdi &__divsi3, 0, implicit $sp, implicit $r0, implicit $r1
+
+declare i1 @ext()
+
+define i32 @test(i32 %a1, i32 %a2) #1 !dbg !5 {
+entry:
+  %foo = alloca i32, i32 %a1
+  %bool = call i1 @ext()
+  %res = sdiv i32 %a1, %a2
+  call void @llvm.dbg.value(metadata i32 %a1, metadata !13, metadata !DIExpression()), !dbg !16
+  ret i32 %res
+}
+
+attributes #1 = {sspstrong}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !1, producer: "Swift", isOptimized: true, runtimeVersion: 5, emissionKind: FullDebug)
+!1 = !DIFile(filename: "foo.swift", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"Swift Minor Version", i8 3}
+!5 = distinct !DISubprogram(name: "n0", linkageName: "n1", scope: !7, file: !6, line: 86, type: !8, scopeLine: 86, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!6 = !DIFile(filename: "bar.swift", directory: "")
+!7 = !DIModule(scope: null, name: "Swift")
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !12}
+!10 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Int", scope: !7, file: !11, size: 32, elements: !2, runtimeLang: DW_LANG_Swift, identifier: "$i1")
+!11 = !DIFile(filename: "f1.swift", directory: "")
+!12 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "n2", scope: !7, file: !6, size: 32, elements: !2, runtimeLang: DW_LANG_Swift, identifier: "n3")
+!13 = !DILocalVariable(name: "n4", scope: !14, file: !1, line: 89, type: !15)
+!14 = distinct !DILexicalBlock(scope: !5, file: !6, line: 86, column: 34)
+!15 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !10)
+!16 = !DILocation(line: 89, column: 9, scope: !14)