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

[dsanders]

Summary

Previous behaviour of 'R' and 'm' has been preserved for now. They will be
improved in subsequent commits.

The offset permitted by ZC varies according to the subtarget since it is
intended to match the restrictions of the pref, ll, and sc instructions.

The restrictions on these instructions are:

• For microMIPS: 12-bit signed offset.
• For Mips32r6/Mips64r6: 9-bit signed offset.
• Otherwise: 16-bit signed offset.

[vkalintiris]

LGTM. I've noticed that we don't check for negative offsets in this patch, in D8435 and in 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.

include/llvm/IR/InlineAsm.h

248–258

Out of curiosity, these constraints were missing when I applied the patch with arc patch D8414. Do you have any idea why?

lib/Target/Mips/MipsAsmPrinter.cpp

545–546

Given these asserts, it would be nice to assert also that OpNum + 1 a valid operand number in getOperand()

[vkalintiris]

Forgot to explicitly mark the revision as accepted in my previous comment.

[dsanders]

In D8414#145029, @vkalintiris wrote:

LGTM. I've noticed that we don't check for negative offsets in this patch, in D8435 and in 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.

include/llvm/IR/InlineAsm.h

248–258

It's because they were gradually added by the various target-specific patches and not all of them have been committed yet.

lib/Target/Mips/MipsAsmPrinter.cpp

545–546

I've added this assert in the commit

include/llvm/IR/InlineAsm.h

enum : uint32_t {
     // constraint codes for all targets.
     Constraint_Unknown = 0,
     Constraint_es,
     Constraint_i,
     Constraint_m,
     Constraint_o,
     Constraint_v,
     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,
+    Constraint_ZC,
     Constraint_Zy,
     Constraints_Max = Constraint_Zy,
     Constraints_ShiftAmount = 16,
 
     Flag_MatchingOperand = 0x80000000
   };
   
   static unsigned getFlagWord(unsigned Kind, unsigned NumOps) {

lib/Target/Mips/MipsAsmPrinter.cpp

bool MipsAsmPrinter::isBlockOnlyReachableByFallthrough(const MachineBasicBlock*
   MachineBasicBlock::const_iterator I = Pred->end();
   while (I != Pred->begin() && !(--I)->isTerminator()) ;
 
   return !I->isBarrier();
 }
 
 // Print out an operand for an inline asm expression.
 bool MipsAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
-                                     unsigned AsmVariant,const char *ExtraCode,
+                                     unsigned AsmVariant, const char *ExtraCode,
                                      raw_ostream &O) {
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1] != 0) return true; // Unknown modifier.
 
     const MachineOperand &MO = MI->getOperand(OpNum);
     switch (ExtraCode[0]) {
     default:
bool MipsAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
   printOperand(MI, OpNum, O);
   return false;
 }
 
 bool MipsAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
                                            unsigned OpNum, unsigned AsmVariant,
                                            const char *ExtraCode,
                                            raw_ostream &O) {
-  int Offset = 0;
+  const MachineOperand &BaseMO = MI->getOperand(OpNum);
+  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();
+
   // Currently we are expecting either no ExtraCode or 'D'
   if (ExtraCode) {
     if (ExtraCode[0] == 'D')
-      Offset = 4;
+      Offset += 4;
     else
       return true; // Unknown modifier.
+    // FIXME: M = high order bits
+    // FIXME: L = low order bits
   }
 
-  const MachineOperand &MO = MI->getOperand(OpNum);
-  assert(MO.isReg() && "unexpected inline asm memory operand");
-  O << Offset << "($" << MipsInstPrinter::getRegisterName(MO.getReg()) << ")";
+  O << Offset << "($" << MipsInstPrinter::getRegisterName(BaseMO.getReg()) << ")";
 
   return false;
 }
 
 void MipsAsmPrinter::printOperand(const MachineInstr *MI, int opNum,
                                   raw_ostream &O) {
   const DataLayout *DL = TM.getDataLayout();
   const MachineOperand &MO = MI->getOperand(opNum);

lib/Target/Mips/MipsISelDAGToDAG.cpp

else
     DEBUG(ResNode->dump(CurDAG));
   DEBUG(errs() << "\n");
   return ResNode;
 }
 
 bool MipsDAGToDAGISel::
 SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID,
                              std::vector<SDValue> &OutOps) {
-  assert(ConstraintID == InlineAsm::Constraint_m &&
-         "unexpected asm memory constraint");
+  // All memory constraints can at least accept raw pointers.
+  switch(ConstraintID) {
+  default:
+    llvm_unreachable("Unexpected asm memory constraint");
+  case InlineAsm::Constraint_i:
+  case InlineAsm::Constraint_m:
+  case InlineAsm::Constraint_R:
+  case InlineAsm::Constraint_ZC:
     OutOps.push_back(Op);
     return false;
   }
+  return true;
+}

lib/Target/Mips/MipsISelLowering.h

private:
     /// being processed is 'm'.
     void LowerAsmOperandForConstraint(SDValue Op,
                                       std::string &Constraint,
                                       std::vector<SDValue> &Ops,
                                       SelectionDAG &DAG) const override;
 
     unsigned getInlineAsmMemConstraint(
         const std::string &ConstraintCode) const override {
-      // FIXME: Map different constraints differently.
-      return InlineAsm::Constraint_m;
+      if (ConstraintCode == "R")
+        return InlineAsm::Constraint_R;
+      else if (ConstraintCode == "ZC")
+        return InlineAsm::Constraint_ZC;
+      return TargetLowering::getInlineAsmMemConstraint(ConstraintCode);
     }
 
     bool isLegalAddressingMode(const AddrMode &AM, Type *Ty) const override;
 
     bool isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const override;
 
     EVT getOptimalMemOpType(uint64_t Size, unsigned DstAlign,
                             unsigned SrcAlign,

lib/Target/Mips/MipsISelLowering.cpp

switch (Constraint[0]) {
       case 'c':
       case 'l':
       case 'x':
         return C_RegisterClass;
       case 'R':
         return C_Memory;
     }
   }
+
+  if (Constraint == "ZC")
+    return C_Memory;
+
   return TargetLowering::getConstraintType(Constraint);
 }
 
 /// Examine constraint type and operand type and determine a weight value.
 /// This object must already have been set up with the operand type
 /// and the current alternative constraint selected.
 TargetLowering::ConstraintWeight
 MipsTargetLowering::getSingleConstraintMatchWeight(

lib/Target/Mips/MipsSEISelDAGToDAG.h

bool selectAddrRegReg(SDValue Addr, SDValue &Base,
                         SDValue &Offset) const override;
 
   bool selectAddrDefault(SDValue Addr, SDValue &Base,
                          SDValue &Offset) const override;
 
   bool selectIntAddr(SDValue Addr, SDValue &Base,
                      SDValue &Offset) const override;
 
+  bool selectAddrRegImm9(SDValue Addr, SDValue &Base,
+                         SDValue &Offset) const;
+
   bool selectAddrRegImm10(SDValue Addr, SDValue &Base,
                           SDValue &Offset) const;
 
   bool selectAddrRegImm12(SDValue Addr, SDValue &Base,
                           SDValue &Offset) const;
 
+  bool selectAddrRegImm16(SDValue Addr, SDValue &Base,
+                          SDValue &Offset) const;
+
   bool selectIntAddrMM(SDValue Addr, SDValue &Base,
                        SDValue &Offset) const override;
 
   bool selectIntAddrLSL2MM(SDValue Addr, SDValue &Base,
                            SDValue &Offset) const override;
 
   bool selectIntAddrMSA(SDValue Addr, SDValue &Base,
                         SDValue &Offset) const override;
private:
 
   std::pair<bool, SDNode*> selectNode(SDNode *Node) override;
 
   void processFunctionAfterISel(MachineFunction &MF) override;
 
   // Insert instructions to initialize the global base register in the
   // first MBB of the function.
   void initGlobalBaseReg(MachineFunction &MF);
+
+  bool SelectInlineAsmMemoryOperand(const SDValue &Op,
+                                    unsigned ConstraintID,
+                                    std::vector<SDValue> &OutOps) override;
 };
 
 FunctionPass *createMipsSEISelDag(MipsTargetMachine &TM);
 
 }
 
 #endif

lib/Target/Mips/MipsSEISelDAGToDAG.cpp

 }
 
 bool MipsSEDAGToDAGISel::selectIntAddr(SDValue Addr, SDValue &Base,
                                        SDValue &Offset) const {
   return selectAddrRegImm(Addr, Base, Offset) ||
     selectAddrDefault(Addr, Base, Offset);
 }
 
+bool MipsSEDAGToDAGISel::selectAddrRegImm9(SDValue Addr, SDValue &Base,
+                                           SDValue &Offset) const {
+  if (selectAddrFrameIndex(Addr, Base, Offset))
+    return true;
+
+  if (selectAddrFrameIndexOffset(Addr, Base, Offset, 9))
+    return true;
+
+  return false;
+}
+
 bool MipsSEDAGToDAGISel::selectAddrRegImm10(SDValue Addr, SDValue &Base,
                                             SDValue &Offset) const {
   if (selectAddrFrameIndex(Addr, Base, Offset))
     return true;
 
   if (selectAddrFrameIndexOffset(Addr, Base, Offset, 10))
     return true;
 
   return false;
 }
 
 /// Used on microMIPS Load/Store unaligned instructions (12-bit offset)
 bool MipsSEDAGToDAGISel::selectAddrRegImm12(SDValue Addr, SDValue &Base,
                                             SDValue &Offset) const {
   if (selectAddrFrameIndex(Addr, Base, Offset))
     return true;
 
   if (selectAddrFrameIndexOffset(Addr, Base, Offset, 12))
     return true;
 
   return false;
 }
 
+bool MipsSEDAGToDAGISel::selectAddrRegImm16(SDValue Addr, SDValue &Base,
+                                            SDValue &Offset) const {
+  if (selectAddrFrameIndex(Addr, Base, Offset))
+    return true;
+
+  if (selectAddrFrameIndexOffset(Addr, Base, Offset, 16))
+    return true;
+
+  return false;
+}
+
 bool MipsSEDAGToDAGISel::selectIntAddrMM(SDValue Addr, SDValue &Base,
                                          SDValue &Offset) const {
   return selectAddrRegImm12(Addr, Base, Offset) ||
     selectAddrDefault(Addr, Base, Offset);
 }
 
 bool MipsSEDAGToDAGISel::selectIntAddrLSL2MM(SDValue Addr, SDValue &Base,
                                              SDValue &Offset) const {
case ISD::BUILD_VECTOR: {
     return std::make_pair(true, Res);
   }
 
   }
 
   return std::make_pair(false, nullptr);
 }
 
+bool MipsSEDAGToDAGISel::
+SelectInlineAsmMemoryOperand(const SDValue &Op, unsigned ConstraintID,
+                             std::vector<SDValue> &OutOps) {
+  SDValue Base, Offset;
+
+  switch(ConstraintID) {
+  default:
+    llvm_unreachable("Unexpected asm memory constraint");
+  // All memory constraints can at least accept raw pointers.
+  case InlineAsm::Constraint_i:
+  case InlineAsm::Constraint_m:
+  case InlineAsm::Constraint_R:
+    OutOps.push_back(Op);
+    OutOps.push_back(CurDAG->getTargetConstant(0, MVT::i32));
+    return false;
+  case InlineAsm::Constraint_ZC:
+    // ZC matches whatever the pref, ll, and sc instructions can handle for the
+    // given subtarget.
+    if (Subtarget->inMicroMipsMode()) {
+      // On microMIPS, they can handle 12-bit offsets.
+      if (selectAddrRegImm12(Op, Base, Offset)) {
+        OutOps.push_back(Base);
+        OutOps.push_back(Offset);
+        return false;
+      }
+    } else if (Subtarget->hasMips32r6()) {
+      // On MIPS32r6/MIPS64r6, they can only handle 9-bit offsets.
+      if (selectAddrRegImm9(Op, Base, Offset)) {
+        OutOps.push_back(Base);
+        OutOps.push_back(Offset);
+        return false;
+      }
+    } else if (selectAddrRegImm16(Op, Base, Offset)) {
+      // Prior to MIPS32r6/MIPS64r6, they can handle 16-bit offsets.
+      OutOps.push_back(Base);
+      OutOps.push_back(Offset);
+      return false;
+    }
+    // In all cases, 0-bit offsets are acceptable.
+    OutOps.push_back(Op);
+    OutOps.push_back(CurDAG->getTargetConstant(0, MVT::i32));
+    return false;
+  }
+  return true;
+}
+
 FunctionPass *llvm::createMipsSEISelDag(MipsTargetMachine &TM) {
   return new MipsSEDAGToDAGISel(TM);
 }

test/CodeGen/Mips/inlineasm_constraint_ZC.ll

+; RUN: llc -march=mipsel -mcpu=mips32r6 < %s | FileCheck %s -check-prefix=ALL -check-prefix=09BIT
+; RUN: llc -march=mipsel -mattr=+micromips < %s | FileCheck %s -check-prefix=ALL -check-prefix=12BIT
+; RUN: llc -march=mipsel < %s | FileCheck %s -check-prefix=ALL -check-prefix=16BIT
+
+@data = global [8193 x i32] zeroinitializer
+
+define void @ZC(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 0))
+
+  ; ALL: lw $[[BASEPTR:[0-9]+]], %got(data)(
+  ; ALL: #APP
+  ; ALL: lw $1, 0($[[BASEPTR]])
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_4(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_4:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 1))
+
+  ; ALL: lw $[[BASEPTR:[0-9]+]], %got(data)(
+  ; ALL: #APP
+  ; ALL: lw $1, 4($[[BASEPTR]])
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_252(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_252:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 63))
+
+  ; ALL: lw $[[BASEPTR:[0-9]+]], %got(data)(
+  ; ALL: #APP
+  ; ALL: lw $1, 252($[[BASEPTR]])
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_256(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_256:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 64))
+
+  ; ALL: lw $[[BASEPTR:[0-9]+]], %got(data)(
+
+  ; 09BIT: addiu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], 256
+
+  ; ALL: #APP
+
+  ; 09BIT: lw $1, 0($[[BASEPTR2]])
+  ; 12BIT: lw $1, 256($[[BASEPTR]])
+  ; 16BIT: lw $1, 256($[[BASEPTR]])
+
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_2044(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_2044:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 511))
+
+  ; ALL: lw $[[BASEPTR:[0-9]+]], %got(data)(
+
+  ; 09BIT: addiu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], 2044
+
+  ; ALL: #APP
+
+  ; 09BIT: lw $1, 0($[[BASEPTR2]])
+  ; 12BIT: lw $1, 2044($[[BASEPTR]])
+  ; 16BIT: lw $1, 2044($[[BASEPTR]])
+
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_2048(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_2048:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 512))
+
+  ; ALL: lw $[[BASEPTR:[0-9]+]], %got(data)(
+
+  ; 09BIT: addiu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], 2048
+  ; 12BIT: addiu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], 2048
+
+  ; ALL: #APP
+
+  ; 09BIT: lw $1, 0($[[BASEPTR2]])
+  ; 12BIT: lw $1, 0($[[BASEPTR2]])
+  ; 16BIT: lw $1, 2048($[[BASEPTR]])
+
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_32764(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_32764:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 8191))
+
+  ; ALL-DAG: lw $[[BASEPTR:[0-9]+]], %got(data)(
+
+  ; 09BIT: addiu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], 32764
+  ; 12BIT: addiu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], 32764
+
+  ; ALL: #APP
+
+  ; 09BIT: lw $1, 0($[[BASEPTR2]])
+  ; 12BIT: lw $1, 0($[[BASEPTR2]])
+  ; 16BIT: lw $1, 32764($[[BASEPTR]])
+
+  ; ALL: #NO_APP
+
+  ret void
+}
+
+define void @ZC_offset_32768(i32 *%p) nounwind {
+entry:
+  ; ALL-LABEL: ZC_offset_32768:
+
+  call void asm sideeffect "lw $$1, $0", "*^ZC,~{$1}"(i32* getelementptr inbounds ([8193 x i32], [8193 x i32]* @data, i32 0, i32 8192))
+
+  ; ALL-DAG: lw $[[BASEPTR:[0-9]+]], %got(data)(
+  ; ALL-DAG: ori $[[T0:[0-9]+]], $zero, 32768
+
+  ; 09BIT: addu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], $[[T0]]
+  ; 12BIT: addu16 $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], $[[T0]]
+  ; 16BIT: addu $[[BASEPTR2:[0-9]+]], $[[BASEPTR]], $[[T0]]
+
+  ; ALL: #APP
+  ; ALL: lw $1, 0($[[BASEPTR2]])
+  ; ALL: #NO_APP
+
+  ret void
+}