To finalise the "RemoveDIs" work removing debug intrinsics, we're updating call sites that insert instructions to use iterators instead. This set of changes are those where it's not immediately obvious that just calling getIterator to fetch an iterator is correct, and one or two places where more than one line needs to change. Overall the same rule holds though: iterators generated for the start of a block such as getFirstNonPHIIt need to be passed into insert/move methods without being unwrapped/rewrapped, everything else can use getIterator.
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. A number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can just replace the pointer-returning version with an iterator-returning version, hopefully without much/any disruption. Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all call-sites. There are no concerns about the iterators returned being converted to Instruction*'s and losing the debug-info bit: because the methods skip debug intrinsics, the iterator head bit is always false anyway.
With the introduction of CmpPredicate in 51a895a (IR: introduce struct with CmpInst::Predicate and samesign), PatternMatch is one of the first key pieces of infrastructure that must be updated to match a CmpInst respecting samesign information. Implement this change to Cmp-matchers. This is a preparatory step in migrating the codebase over to CmpPredicate. Since we no functional changes are desired at this stage, we have chosen not to migrate CmpPredicate::operator==(CmpPredicate) calls to use CmpPredicate::getMatching(), as that would have visible impact on tests that are not yet written: instead, we call CmpPredicate::operator==(Predicate), preserving the old behavior, while also inserting a few FIXME comments for follow-ups.
As part of the effort to rename the DbgRecord classes, this patch renames the widely-used functions that operate on DbgRecords but refer to DbgValues or DPValues in their names to refer to DbgRecords instead; all such functions are defined in one of `BasicBlock.h`, `Instruction.h`, and `DebugProgramInstruction.h`. This patch explicitly does not change the names of any comments or variables, except for where they use the exact name of one of the renamed functions. The reason for this is reviewability; this patch can be trivially examined to determine that the only changes are direct string substitutions and any results from clang-format responding to the changed line lengths. Future patches will cover renaming variables and comments, and then renaming the classes themselves.
Original commit message below; asan complained about this commit because it transpires that the final comparison with CurrentI is in fact a comparison of a pointer that has been freed. This seems to work fine most of the time, but using the iterator for such an instruction causes the freed instruction memory to be accessed, causing a use-after-free. The fix is to perform the comparison as an instruction, not an iterator. [NFC][DebugInfo][RemoveDIs] Use iterators to insert in callsite-splitting (#74455) This patch gets call site splitting to use iterators for insertion rather than instruction pointers. When we switch on non-instr debug-info this becomes significant, as the iterators are going to signal whether or not a position is before or after debug-info. NFC as this isn't going to affect the output of any existing test.
This reverts commit 34cdc913214fd9561b6ec8d535bd3d0313772cb5. Two buildbots say this is bad: https://lab.llvm.org/buildbot/#/builders/265/builds/861 https://lab.llvm.org/buildbot/#/builders/168/builds/17272
This patch gets call site splitting to use iterators for insertion rather than instruction pointers. When we switch on non-instr debug-info this becomes significant, as the iterators are going to signal whether or not a position is before or after debug-info. NFC as this isn't going to affect the output of any existing test.
With intrinsics representing debug-info, we just clone all the intrinsics when inlining a function and don't think about it any further. With non-instruction debug-info however we need to be a bit more careful and manually move the debug-info from one place to another. For the most part, this means keeping a "cursor" during block cloning of where we last copied debug-info from, and performing debug-info copying whenever we successfully clone another instruction. There are several utilities in LLVM for doing this, all of which now need to manually call cloneDebugInfo. The testing story for this is not well covered as we could rely on normal instruction-cloning mechanisms to do all the hard stuff. Thus, I've added a few tests to explicitly test dbg.value behaviours, ahead of them becoming not-instructions.