[PATCH] D152536: [DWARF] Allow op-index in line number programs

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:1253-1254
+  //
+  // TODO: This function, and its users, needs to be update to return multiple
+  // rows for bundles with multiple op-indexes.
   DWARFDebugLine::Row Row;
----------------
dstenb wrote:
> StephenTozer wrote:
> > Any idea about the complexity of implementing this? It seems to me that most users either 1) already want a range, in which case this function can just return a pair of `{firstIndex, lastIndex}` and the caller can determine the range, or 2) want the info for a specific operation, in which case an OpIndex needs to be passed. I've not personally done anything with VLIW architectures but I'm assuming it's possible for different operations in the same instruction to have different source locations and even be in different inline frames, in which case it would be necessary to update a dozen or so callsites to pass an OpIndex through - seems like it would still be fairly simple in terms of code though.
> I have some downstream WIP patches that starts addressing 1). I suspect it is mainly a lot of mechanical reworking of the code to propagate that information to the tools, but I can't show my working yet. I am not sure in how many cases 2) would be needed, but it seems that that for example could be useful if, whatever you are running a test on, can pinpoint which individual operation in a bundle that triggered a crash/interrupt.
> 
> There are some user interface decisions that need to be made, e.g. what the output for llvm-addr2line --inlines should look like when printing an address with multiple op-indexes. The locations for such addresses can, as you say, originate from different inline frames.
> 
> My hope is that, following some framework changes, mostly in this file, we can gradually enable proper op-index support in tools. Until we get to that point with a tool/option we should just be able to pick one of the locations if handling an address with multiple op-indexes.
I have some concerns about returning information that could be misleading (by misattributing the source location of an operation), since that can be worse than getting no information at all. I think it would be a suitable stopgap to disable those tools for line tables with `MaxOperationsPerInst > 1`, since that ensures that we are no worse off in any case. Open to other options though, or a justification for allowing the "incorrect" output.


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

https://reviews.llvm.org/D152536