[PATCH] D78108: [CSInfo][MIPS] Describe parameter value loaded by ADDiu instruction

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan accepted this revision.
atanasyan added a comment.
This revision is now accepted and ready to land.

Looks good overall, just a few nits inline.



================
Comment at: llvm/lib/Target/Mips/MipsInstrInfo.cpp:850
+      DIExpression::get(MI.getMF()->getFunction().getContext(), {});
+  int64_t Offset;
+  // TODO: Special MIPS instructions that need to be described separately.
----------------
You can join declaration and initialization.


================
Comment at: llvm/lib/Target/Mips/MipsInstrInfo.cpp:869
+                                                   Register Reg) const {
+  int64_t Offset = 0;
+
----------------
You can join declaration and initialization.


================
Comment at: llvm/lib/Target/Mips/MipsInstrInfo.cpp:883
+    const MachineOperand &Sop2 = MI.getOperand(2);
+    // Value is sum of register and immediate. Immediate value value could be
+    // global string address which is not supported.
----------------
The second "value" in this comment is redundant.


================
Comment at: llvm/lib/Target/Mips/MipsInstrInfo.cpp:885
+    // global string address which is not supported.
+    if (!Dop.isReg() || (!Sop1.isReg() && !Sop1.isFI()) || !Sop2.isImm())
+      return None;
----------------
I prefer to reverse this condition something like this. I think, in this case it's more clear what we need from the operands.
```
if (Dop.isReg() && (Sop1.isReg() || Sop1.isFI()) && Sop2.isImm())
  return RegImmPair{Sop1.getReg(), Sop2.getImm()};

return None;
```


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

https://reviews.llvm.org/D78108