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

[David Stenberg via Phabricator via llvm-commits]

dstenb added inline comments.


================
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;
----------------
atanasyan wrote:
> 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;
> ```
Oh, and the `getReg()` invocation will trigger an assert/produce incorrect data in the `Sop1.isFI()` case! What is the expected behavior if the operand is a frame-index?


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

https://reviews.llvm.org/D78108