[PATCH] D64500: [DebugInfo] Remove call sites when eliminating unreachable blocks

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D64500#1580217 <https://reviews.llvm.org/D64500#1580217>, @NikolaPrica wrote:

> @dstenb The patch looks good! LGTM! Thanks for working on this!
>
> > Wdyt of addressing this problem in a more general way, e.g. in MachineInstruction::eraseFromParent itself?
> >  edit: I meant to write MachineBasicBlock::eraseFromParent, but I think the point still stands.
>
> That would be great and we would not need to worry about validity of the `CallSitesInfo`. But the thing is that we have a problem with the `MI` range deletion from the `MBB` (see for example `MachineOutliner::outline` or version before this feature for `TargetInstrInfo::ReplaceTailWithBranchTo`). Such deletions cannot be generalized. For now, there are just two of such cases but in further we may stumble across more.
>  The whole idea behind this assertion and update is to evade generalization in the destructor of the `MachineInst` (which is the safest but requires certain overhead) and to use the update only when necessary. Similarly will be with the `eraseFromParent`, it will require certain overhead.
>  Please let us know if you have an idea how to overcome those constraints.


Thanks for humoring me :). I haven't actively worked in this area, so I may be missing important/subtle details here.

But ISTM that range deletion within MachineBasicBlock can be handled generically as well ('if any MI in the range is a call ...'). It doesn't seem too far-fetched to teach all APIs that unlink an MI from the instruction list to update call site info. Perhaps this would incur a little extra overhead if, say, an MI is unlinked and subsequently re-inserted somewhere else. In this case there might be two updates instead of one. However, I'd argue that the cost is likely justified if it makes the Machine* APIs safe to use from a debug info correctness perspective. Has this already been tried, and been shown to be too expensive in compile-time benchmarking?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64500