[PATCH] D78107: [CSInfo][MIPS] Call delay slot support in DwarfDebug

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:859-860
+    // TODO: Add support for targets with delay slots (see: beginInstruction).
+    if (TT.isMIPS())
+      return true;
+    return false;
----------------
vsk wrote:
> dstenb wrote:
> > Do you know what would happen if you didn't have this guard, and enabled call site information for another target with delay slots?
> > 
> > If it is something that is relatively easy to catch, or perhaps not very serious, I think I'd prefer to have this code target-agnostic (i.e. just `MI.hasDelaySlot`), and just leave it up to the developer enabling call site information for the target to make sure that there are no delay slot quirks that need to be considered.
> + 1
+1, but I think we should at least make an assertion describing what we currently expect, e.g. bundled call instruction should be mips-call-bundle like:
  call {
     delay slot
    }
  label



================
Comment at: llvm/test/DebugInfo/MIR/Mips/dbg-call-site-delay-slot-interpretation-64bit.mir:93
+  !20 = !DILocation(line: 0, scope: !13)
+  !21 = !DILocation(line: 3, column: 7, scope: !13)
+  !22 = !DILocation(line: 4, column: 17, scope: !13)
----------------
Please use only one `DILocation` per scope.
e.g. attach every {!21, !22, !23, !24} dbg! to !21



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

https://reviews.llvm.org/D78107