[PATCH] D82363: [DebugInfo] Add new instruction and expression operator for variadic debug values

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer marked an inline comment as done.
StephenTozer added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:503
+  }
+  ArrayRef<MachineOperand *> getDebugOperandsForReg(Register Reg) {
+    assert(isDebugValue() && "Tried to get debug operands for non-debug_value");
----------------
djtodoro wrote:
> Can we use templates to avoid duplicated code here for `getDebugOperandsForReg()`?
We can, as long as we use a static function to hold the common code (if there's a way to do so without a static function then I'd be happy to go with that instead); the solution looks something like this:

```
  template <typename Operand, typename Instruction>
  static ArrayRef<Operand *> getDebugOperandsForReg(Instruction *MI, Register Reg) {
    assert(MI->isDebugValue() && "Tried to get debug operands for non-debug_value");
    SmallVector<Operand *, 2> Ops;
    for (Operand &Op : MI->debug_operands()) {
      if (Op.isReg() && Op.getReg() == Reg)
        Ops.push_back(&Op);
    }
    return Ops;
  }
  ArrayRef<const MachineOperand *> getDebugOperandsForReg(Register Reg) const {
	  return MachineInstr::getDebugOperandsForReg<const MachineOperand, const MachineInstr>(this, Reg);
  }
  ArrayRef<MachineOperand *> getDebugOperandsForReg(Register Reg) {
	  return MachineInstr::getDebugOperandsForReg<MachineOperand, MachineInstr>(this, Reg);
  }
```

Does this look good? It removes the duplication, it's just a bit more verbose and leaves an otherwise useless static function hanging around, unless it's moved to a private block (which is also fine but reduces readability by moving it far away from the public functions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82363