[PATCH] [mips] Distinguish 'R', 'ZC', and 'm' inline assembly memory constraint.

Daniel Sanders daniel.sanders at imgtec.com
Tue Mar 24 04:30:34 PDT 2015


In http://reviews.llvm.org/D8414#145029, @vkalintiris wrote:

> LGTM. I've noticed that we don't check for negative offsets in this patch, in http://reviews.llvm.org/D8435 and in http://reviews.llvm.org/D8440. That's fine with me, given that those tests would essentially test `selectAddrFrameIndexOffset()` and not the logic of the memory constraints, I just wanted to make sure that you're aware of that.


I've added a test for -4 in the commit.


================
Comment at: include/llvm/IR/InlineAsm.h:248-258
@@ -247,13 +247,13 @@
     Constraint_Q,
     Constraint_R,
     Constraint_S,
     Constraint_T,
     Constraint_X,
     Constraint_Um,
     Constraint_Un,
     Constraint_Uq,
     Constraint_Us,
     Constraint_Ut,
     Constraint_Uv,
     Constraint_Uy,
     Constraint_Z,
----------------
vkalintiris wrote:
> Out of curiosity, these constraints were missing when I applied the patch with `arc patch D8414`. Do you have any idea why?
It's because they were gradually added by the various target-specific patches and not all of them have been committed yet.

================
Comment at: lib/Target/Mips/MipsAsmPrinter.cpp:545-546
@@ +544,4 @@
+  const MachineOperand &OffsetMO = MI->getOperand(OpNum + 1);
+  assert(BaseMO.isReg() && "Unexpected base pointer for inline asm memory operand.");
+  assert(OffsetMO.isImm() && "Unexpected offset for inline asm memory operand.");
+  int Offset = OffsetMO.getImm();
----------------
vkalintiris wrote:
> Given these asserts, it would be nice to assert also that `OpNum + 1` a valid operand number in `getOperand()`
I've added this assert in the commit

http://reviews.llvm.org/D8414

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list