RegisterCoalescer: Fix iterating through use operands.

[hgreving]

Summary

Fixes a minor bug when trying to iterate through use operands when
updating debug use operands.

Extends a test to include above.

[arsenm]

llvm/lib/CodeGen/RegisterCoalescer.cpp

1560

Can you just use the explicit iterators and preincrement instead of saving all the operands?

[hgreving]

Use iterator preincrement instead.

[jmorse]

Just so that I understand clearly, the fault here was modifying operands while using range-iteration over the same collection? Was the symptom a crash, or missing output? (Thanks for the catch).

llvm/test/DebugInfo/MIR/X86/regcoalescer.mir

45

Should this not lead to an additional DBG_VALUE in output, and an additional CHECK line for it?

[hgreving]

Second CHECK line.
clang-format.

[hgreving]

In D104576#2828828, @jmorse wrote:

Just so that I understand clearly, the fault here was modifying operands while using range-iteration over the same collection? Was the symptom a crash, or missing output? (Thanks for the catch).

Yes, the iterator was invalidated while updating it. I am not familiar with this particular part of the register coalescer, but stumbled over it when debugging. I think it's a common mistake, thanks for reviewing.

[jmorse]

Cheers, LGTM.

llvm/lib/CodeGen/RegisterCoalescer.cpp

bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
   }
 
   LLVM_DEBUG(dbgs() << "Remat: " << NewMI);
   ++NumReMats;
 
   // If the virtual SrcReg is completely eliminated, update all DBG_VALUEs
   // to describe DstReg instead.
   if (MRI->use_nodbg_empty(SrcReg)) {
-    for (MachineOperand &UseMO : MRI->use_operands(SrcReg)) {
+    for (MachineRegisterInfo::use_iterator UI = MRI->use_begin(SrcReg);
+         UI != MRI->use_end();) {
+      MachineOperand &UseMO = *UI++;
       MachineInstr *UseMI = UseMO.getParent();
       if (UseMI->isDebugValue()) {
         if (Register::isPhysicalRegister(DstReg))
           UseMO.substPhysReg(DstReg, *TRI);
         else
           UseMO.setReg(DstReg);
         // Move the debug value directly after the def of the rematerialized
         // value in DstReg.

llvm/test/DebugInfo/MIR/X86/regcoalescer.mir

--- |
   !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
   !12 = !{i32 2, !"Debug Info Version", i32 3}
   !13 = !{i32 1, !"wchar_size", i32 4}
   !14 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 12, type: !15, isLocal: false, isDefinition: true, scopeLine: 12, isOptimized: true, unit: !0, retainedNodes: !17)
   !15 = !DISubroutineType(types: !16)
   !16 = !{!7}
   !17 = !{!18}
   !18 = !DILocalVariable(name: "bazinga", scope: !14, file: !1, line: 13, type: !7)
-  !19 = !DILocation(line: 14, column: 11, scope: !14)
-  !20 = !DILocation(line: 13, column: 7, scope: !14)
-  !21 = !DILocation(line: 16, column: 3, scope: !14)
+  !19 = !DILocalVariable(name: "bazinga2", scope: !14, file: !1, line: 13, type: !7)
+  !20 = !DILocation(line: 14, column: 11, scope: !14)
+  !21 = !DILocation(line: 13, column: 7, scope: !14)
+  !22 = !DILocation(line: 16, column: 3, scope: !14)
 
 ...
 ---
 name:            main
 alignment:       16
 registers:
   - { id: 0, class: gr32, preferred-register: '' }
 body:             |
   bb.0.entry:
-    %0 = MOV32r0 implicit-def dead $eflags, debug-location !19
-    DBG_VALUE %0, _, !18, !DIExpression(), debug-location !20
-    $eax = COPY killed %0, debug-location !21
-    RET 0, killed $eax, debug-location !21
+    %0 = MOV32r0 implicit-def dead $eflags, debug-location !20
+    DBG_VALUE %0, _, !18, !DIExpression(), debug-location !21
+    DBG_VALUE %0, _, !19, !DIExpression(), debug-location !22
+    $eax = COPY killed %0, debug-location !22
+    RET 0, killed $eax, debug-location !22
 
 ...
 
 # CHECK: $eax = MOV32r0
 # CHECK-NEXT: DBG_VALUE $eax
+# CHECK-NEXT: DBG_VALUE $eax