[PATCH] D78108: [CSInfo][MIPS] Describe parameter value loaded by ADDiu instruction
Simon Atanasyan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 7 23:16:23 PDT 2020
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
More information about the llvm-commits
mailing list