[PATCH] D47857: [RISCV] Add machine function pass to merge base + offset

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/RISCVMergeBaseOffset.cpp:143
+  assert((TailAdd.getOpcode() == RISCV::ADD) && "Expected ADD instruction!");
+  for (MachineOperand &Op : TailAdd.operands()) {
+    if (Op.isDef())
----------------
sabuasal wrote:
> asb wrote:
> > Given that we know the MI is an Add, is looping over the operands the clearest way of handling this?
> I need to find the input argument that is coming from the offset creation; which can be a LUI or ADDI followed by LUI as explained in the comment above the function. 
> 
> Do you think there is a cleaner way to do it?
I was imagining removing the loop with something like the below:


```
 assert((TailAdd.getOpcode() == RISCV::ADD) && "Expected ADD instruction!");
  unsigned Rs = TailAdd.getOperand(1).getReg();
  unsigned Rt = TailAdd.getOperand(2).getReg();
  unsigned Reg = Rs == GAReg ? Rt : Rs;

  // Can't fold if the register has more than one use.
  if (!MRI->hasOneUse(Reg))
    return false;
  // This can point to an ADDI or a LUI:
  MachineInstr &OffsetTail = *MRI->getVRegDef(Reg);
  if (OffsetTail.getOpcode() == RISCV::ADDI) {
    // The offset value has non zero bits in both %hi and %lo parts.
    // Detect an ADDI that feeds from a LUI instruction.
    MachineOperand &AddiImmOp = OffsetTail.getOperand(2);
    if (AddiImmOp.getTargetFlags() != RISCVII::MO_None)
      return false;
    int64_t OffLo = AddiImmOp.getImm();
    MachineInstr &OffsetLui =
        *MRI->getVRegDef(OffsetTail.getOperand(1).getReg());
    MachineOperand &LuiImmOp = OffsetLui.getOperand(1);
    if (OffsetLui.getOpcode() != RISCV::LUI ||
        LuiImmOp.getTargetFlags() != RISCVII::MO_None ||
        !MRI->hasOneUse(OffsetLui.getOperand(0).getReg()))
      return false;
    int64_t OffHi = OffsetLui.getOperand(1).getImm();
    Offset = (OffHi << 12) + OffLo;
    LLVM_DEBUG(dbgs() << "  Offset Instrs: " << OffsetTail
                      << "                 " << OffsetLui);
    DeadInstrs.insert(&OffsetTail);
    DeadInstrs.insert(&OffsetLui);
    return true;
  } else if (OffsetTail.getOpcode() == RISCV::LUI) {
    // The offset value has all zero bits in the lower 12 bits. Only LUI
    // exists.
    LLVM_DEBUG(dbgs() << "  Offset Instr: " << OffsetTail);
    Offset = OffsetTail.getOperand(1).getImm() << 12;
    DeadInstrs.insert(&OffsetTail);
    return true;
  }
  return false;
```


https://reviews.llvm.org/D47857