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

Vasileios Kalintiris Vasileios.Kalintiris at imgtec.com
Mon Mar 23 05:57:13 PDT 2015


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.


================
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,
----------------
Out of curiosity, these constraints were missing when I applied the patch with `arc patch D8414`. Do you have any idea why?

================
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();
----------------
Given these asserts, it would be nice to assert also that `OpNum + 1` a valid operand number in `getOperand()`

http://reviews.llvm.org/D8414

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






More information about the llvm-commits mailing list