Hello folks – FYI I gave a presentation at EuroLLVM illustrating the ideas behind the proposal above, and exploring where the costs are, I’ll link to that from here when the videos are up,
I’ve started uploading our patches to Phabricator, forming a stack based on [0]. If you look at the five patches based on that, you’ll see the extent of the invasiveness of these changes: basically turning a bunch of things that were instruction pointers into iterators. I don’t think this is especially remarkable as it’s effectively a spelling change, although you might say that the iterator form is slightly more verbose.
Technically these changes are NFC (aside from D151419 itself) – however they won’t be NFC in the context of a debug-info-intrinsic free LLVM, because they will be transmitting more information about the debug-info context through the BasicBlock::iterator object once that happens. This raises an annoying question of how do we go about testing these changes. So far I’ve been comparing the output of a compile with no intrinsics, with one that does use intrinsics, to determine differences from the “correct” output: however this isn’t a long term strategy as we want to get rid of intrinsics entirely. It’s also infeasible to write a test for each changed code path because there are going to be hundreds of them.
If we’re willing to accept that with these instruction API changes the only way to insert instructions is going to be with an iterator, then the changes I’ve linked to don’t need to be tested: they’re just the first tranche of changes needed in replacing one set of APIs with another. That’s my prefered direction of travel / interpretation – no-one has yet objected to the idea of requiring insertion with a BasicBlock::iterator, or the need to store an extra bit in the iterator class. If you have an objection to this idea, please do speak up now!
Testing for the correct placement of variable location information is an open problem as we don’t have comprehensive testing of debug-info in the context of every optimisation transformation.
Going forwards: the next set of patches to put in that stack will be for a naive implementation of storing variable assignment information outside of instructions. I’d prefer to avoid premature optimisation at this stage – we’re still optimising it, but the validity of the approach can be tested / evaluated before patches to make it fast are iterated on and landed. The final set of patches are per-pass extra instrumentation needed for scenarios where debug-info has special handling (i.e. inlining etc).