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

[Nikola Tesic via Phabricator via llvm-commits]

ntesic marked an inline comment as done.
ntesic 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;
----------------
dstenb wrote:
> 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?
@atanasyan Thanks for suggestions.
@dstenb This was leftover from an old implementation. Case of ADDiu with frame index operand, should be considered separately. We can omit it for now and mark it with TODO.


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

https://reviews.llvm.org/D78108