[PATCH] D150577: [CodeGen] Fix for MachineBasicBlock::rfindDebugLoc(instr_rend())

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D150577#4346213 <https://reviews.llvm.org/D150577#4346213>, @Orlando wrote:

> Whether it's better to return an empty `DebugLoc` (because the iterator is invalid) or scan forward for the next one (because `rend` points to somewhere before the start, and `rfindDebugLoc` finds the "this _or next_" filtered `DebugLoc`) I am not entirely sure as I think both make sense. What are your thoughts on this? I am happy to accept this as it is, but I want to add some other debug-info reviewers in case anyone has a stronger opinion.

I think I'd prefer one of these two alternatives:

1. Allow passing an end/rend iterator as currently proposed in this patch.
2. Add code comments explaining that the iterators must point to a MachineInst (end()/rend() is not supported). And then add asserts to verify that.

The reason why I went for proposing alternative 1 was to make it possible to find the last DebugLoc in the MBB using `findPrevDebugLoc(instr_end())`. If we disallow passing the end iterator, or if consider a third alternative to return an empty `DebugLoc` in such situations, then we'd need to add a new `findDebugLocBackward` helper to support that use case (or the user would need to explicitly check the last MI before using `findPrevDebugLoc`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150577