[M68k] Support inline asm operands w/ simple constraints

[myhsu]

Summary

This patch adds supports for inline assembly operands and some simple
operand constraints, including register and constant operands.

[nickdesaulniers]

LGTM but please review the linter warnings.

clang/lib/Basic/Targets/M68k.cpp

145

did you mean to drop support for 'f'?

llvm/test/CodeGen/M68k/inline-asm.ll

36

You probably should add tests for all newly added constraints with valid input.

[myhsu]

Fixed lint warning and added more test cases

clang/lib/Basic/Targets/M68k.cpp

145

To drop it for now. I believe this constraint was added by mistake since we barely has any support for floating point in the backend. I think it's a good idea to remove this constraint from the frontend so that it will not be misused until floating point implementation got matured.

[nickdesaulniers]

Looking good @myhsu . Also, I got your LLVM book recently! You'll need to sign it for me at the next llvm conf in person.

clang/lib/Basic/Targets/M68k.cpp

197–199

return std::string("^") + std::string(Constraint++, 2);

clang/test/Sema/inline-asm-validate-m68k.c

11

Tests look good. Would you mind please adding an asm for each function that doesn't require an expected-error? ie. is valid input?

Also, volatile keyword is not necessary when there are no output operands. See also: https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile.

52

do we need tests for lowercase a, r, or d?

llvm/lib/Target/M68k/M68kAsmPrinter.cpp

57–59

If DL is only needed for this lone case, consider using {} for the case and sinking the def closer to the use.

[myhsu]

• Add test cases for valid constraints
• Fix formatting

[myhsu]

In D102585#2769887, @nickdesaulniers wrote:

Looking good @myhsu . Also, I got your LLVM book recently! You'll need to sign it for me at the next llvm conf in person.

Thank you :-) Hope you will like it.
I'll start to practice my autograph too.

clang/test/Sema/inline-asm-validate-m68k.c

52

Actually I think r is already a built-in constraint letter for register. I'm adding test cases for a and d though.

clang/lib/Basic/Targets/M68k.h

public:
   M68kTargetInfo(const llvm::Triple &Triple, const TargetOptions &);
 
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
   ArrayRef<Builtin::Info> getTargetBuiltins() const override;
   bool hasFeature(StringRef Feature) const override;
   ArrayRef<const char *> getGCCRegNames() const override;
   ArrayRef<TargetInfo::GCCRegAlias> getGCCRegAliases() const override;
+  std::string convertConstraint(const char *&Constraint) const override;
   bool validateAsmConstraint(const char *&Name,
                              TargetInfo::ConstraintInfo &info) const override;
   const char *getClobbers() const override;
   BuiltinVaListKind getBuiltinVaListKind() const override;
   bool setCPU(const std::string &Name) override;
 };
 
 } // namespace targets
 } // namespace clang
 
 #endif

clang/lib/Basic/Targets/M68k.cpp

 #include "M68k.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/TargetParser.h"
+#include <cstdint>
 #include <cstring>
+#include <limits>
 
 namespace clang {
 namespace targets {
 
 M68kTargetInfo::M68kTargetInfo(const llvm::Triple &Triple,
                                const TargetOptions &)
     : TargetInfo(Triple) {
 
ArrayRef<TargetInfo::GCCRegAlias> M68kTargetInfo::getGCCRegAliases() const {
   return None;
 }
 
 bool M68kTargetInfo::validateAsmConstraint(
     const char *&Name, TargetInfo::ConstraintInfo &info) const {
   switch (*Name) {
   case 'a': // address register
   case 'd': // data register
-  case 'f': // floating point register
     info.setAllowsRegister();
     return true;
-  case 'K': // the constant 1
-  case 'L': // constant -1^20 .. 1^19
-  case 'M': // constant 1-4:
+  case 'I': // constant integer in the range [1,8]
+    info.setRequiresImmediate(1, 8);
     return true;
+  case 'J': // constant signed 16-bit integer
+    info.setRequiresImmediate(std::numeric_limits<int16_t>::min(),
+                              std::numeric_limits<int16_t>::max());
+    return true;
+  case 'K': // constant that is NOT in the range of [-0x80, 0x80)
+    info.setRequiresImmediate();
+    return true;
+  case 'L': // constant integer in the range [-8,-1]
+    info.setRequiresImmediate(-8, -1);
+    return true;
+  case 'M': // constant that is NOT in the range of [-0x100, 0x100]
+    info.setRequiresImmediate();
+    return true;
+  case 'N': // constant integer in the range [24,31]
+    info.setRequiresImmediate(24,31);
+    return true;
+  case 'O': // constant integer 16
+    info.setRequiresImmediate(16);
+    return true;
+  case 'P': // constant integer in the range [8,15]
+    info.setRequiresImmediate(8,15);
+    return true;
+  case 'C':
+    ++Name;
+    switch (*Name) {
+    case '0': // constant integer 0
+      info.setRequiresImmediate(0);
+      return true;
+    case 'i': // constant integer
+    case 'j': // integer constant that doesn't fit in 16 bits
+      info.setRequiresImmediate();
+      return true;
+    default:
+      break;
+    }
+    break;
+  default:
+    break;
   }
-  // FIXME: Support all constraints like 'N', 'O', 'P', 'R'
   return false;
 }
 
+std::string M68kTargetInfo::convertConstraint(const char *&Constraint) const {
+  if (*Constraint == 'C') {
+    // Two-character constraint; add "^" hint for later parsing
+    std::string R = std::string("^") + std::string(Constraint, 2);
+    ++Constraint;
+    return R;
+  }
+
+  return std::string(1, *Constraint);
+}
+
 const char *M68kTargetInfo::getClobbers() const {
   // FIXME: Is this really right?
   return "";
 }
 
 TargetInfo::BuiltinVaListKind M68kTargetInfo::getBuiltinVaListKind() const {
   return TargetInfo::VoidPtrBuiltinVaList;
 }
 
 } // namespace targets
 } // namespace clang

clang/test/Sema/inline-asm-validate-m68k.c

+// RUN: %clang_cc1 -triple m68k -fsyntax-only -verify %s
+
+// This file tests constraint rules that are simple enough to be
+// validated by frontend
+
+void I() {
+  static const int BelowMin = 0;
+  static const int AboveMax = 9;
+  asm volatile ("" :: "I"(BelowMin)); // expected-error{{value '0' out of range for constraint 'I'}}
+  asm volatile ("" :: "I"(AboveMax)); // expected-error{{value '9' out of range for constraint 'I'}}
+}
+
+void J() {
+  static const int BelowMin = -0x8001;
+  static const int AboveMax = 0x8000;
+  asm volatile ("" :: "J"(BelowMin)); // expected-error{{value '-32769' out of range for constraint 'J'}}
+  asm volatile ("" :: "J"(AboveMax)); // expected-error{{value '32768' out of range for constraint 'J'}}
+}
+
+void L() {
+  static const int BelowMin = -9;
+  static const int AboveMax = 0;
+  asm volatile ("" :: "L"(BelowMin)); // expected-error{{value '-9' out of range for constraint 'L'}}
+  asm volatile ("" :: "L"(AboveMax)); // expected-error{{value '0' out of range for constraint 'L'}}
+}
+
+void N() {
+  static const int BelowMin = 23;
+  static const int AboveMax = 32;
+  asm volatile ("" :: "N"(BelowMin)); // expected-error{{value '23' out of range for constraint 'N'}}
+  asm volatile ("" :: "N"(AboveMax)); // expected-error{{value '32' out of range for constraint 'N'}}
+}
+
+void O() {
+  // Valid only if it's 16
+  static const int IncorrectVal = 18;
+  asm volatile ("" :: "O"(IncorrectVal)); // expected-error{{value '18' out of range for constraint 'O'}}
+}
+
+void P() {
+  static const int BelowMin = 7;
+  static const int AboveMax = 16;
+  asm volatile ("" :: "P"(BelowMin)); // expected-error{{value '7' out of range for constraint 'P'}}
+  asm volatile ("" :: "P"(AboveMax)); // expected-error{{value '16' out of range for constraint 'P'}}
+}
+
+void C0() {
+  // Valid only if it's 0
+  static const int IncorrectVal = 1;
+  asm volatile ("" :: "C0"(IncorrectVal)); // expected-error{{value '1' out of range for constraint 'C0'}}
+}

llvm/lib/Target/M68k/M68kAsmPrinter.h

 
 class M68kSubtarget;
 class M68kMachineFunctionInfo;
 
 class LLVM_LIBRARY_VISIBILITY M68kAsmPrinter : public AsmPrinter {
 
   void EmitInstrWithMacroNoAT(const MachineInstr *MI);
 
+  void printOperand(const MachineInstr *MI, int opNum, raw_ostream &OS);
+
 public:
   const M68kSubtarget *Subtarget;
   const M68kMachineFunctionInfo *MMFI;
   std::unique_ptr<M68kMCInstLower> MCInstLowering;
 
   explicit M68kAsmPrinter(TargetMachine &TM,
                           std::unique_ptr<MCStreamer> Streamer)
       : AsmPrinter(TM, std::move(Streamer)) {
     Subtarget = static_cast<M68kTargetMachine &>(TM).getSubtargetImpl();
   }
 
   StringRef getPassName() const override { return "M68k Assembly Printer"; }
 
   virtual bool runOnMachineFunction(MachineFunction &MF) override;
 
+  bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
+                       const char *ExtraCode, raw_ostream &OS) override;
+
   void emitInstruction(const MachineInstr *MI) override;
   void emitFunctionBodyStart() override;
   void emitFunctionBodyEnd() override;
   void emitStartOfAsmFile(Module &M) override;
   void emitEndOfAsmFile(Module &M) override;
 };
 } // namespace llvm
 
 #endif

llvm/lib/Target/M68k/M68kAsmPrinter.cpp

 //===----------------------------------------------------------------------===//
 
 // TODO Conform to Motorola ASM syntax
 
 #include "M68kAsmPrinter.h"
 
 #include "M68k.h"
 #include "M68kMachineFunction.h"
+#include "MCTargetDesc/M68kInstPrinter.h"
 #include "TargetInfo/M68kTargetInfo.h"
 
 #include "llvm/Support/TargetRegistry.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "m68k-asm-printer"
 
 bool M68kAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
   MMFI = MF.getInfo<M68kMachineFunctionInfo>();
   MCInstLowering = std::make_unique<M68kMCInstLower>(MF, *this);
   AsmPrinter::runOnMachineFunction(MF);
   return true;
 }
 
+void M68kAsmPrinter::printOperand(const MachineInstr *MI, int opNum,
+                                  raw_ostream &OS) {
+  const DataLayout &DL = getDataLayout();
+  const MachineOperand &MO = MI->getOperand(opNum);
+  switch (MO.getType()) {
+  case MachineOperand::MO_Register:
+    OS << "%" << M68kInstPrinter::getRegisterName(MO.getReg());
+    break;
+  case MachineOperand::MO_Immediate:
+    OS << '#' << MO.getImm();
+    break;
+  case MachineOperand::MO_MachineBasicBlock:
+    MO.getMBB()->getSymbol()->print(OS, MAI);
+    break;
+  case MachineOperand::MO_GlobalAddress:
+    PrintSymbolOperand(MO, OS);
+    break;
+  case MachineOperand::MO_BlockAddress:
+    GetBlockAddressSymbol(MO.getBlockAddress())->print(OS, MAI);
+    break;
+  case MachineOperand::MO_ConstantPoolIndex:
+    OS << DL.getPrivateGlobalPrefix() << "CPI" << getFunctionNumber() << '_'
+       << MO.getIndex();
+    break;
+  default:
+    llvm_unreachable("not implemented");
+  }
+}
+
+bool M68kAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
+                                     const char *ExtraCode, raw_ostream &OS) {
+  // Print the operand if there is no operand modifier.
+  if (!ExtraCode || !ExtraCode[0]) {
+    printOperand(MI, OpNo, OS);
+    return false;
+  }
+
+  // Fallback to the default implementation.
+  return AsmPrinter::PrintAsmOperand(MI, OpNo, ExtraCode, OS);
+}
+
 void M68kAsmPrinter::emitInstruction(const MachineInstr *MI) {
   switch (MI->getOpcode()) {
   default: {
     if (MI->isPseudo()) {
       LLVM_DEBUG(dbgs() << "Pseudo opcode(" << MI->getOpcode()
                         << ") found in EmitInstruction()\n");
       llvm_unreachable("Cannot proceed");
     }

llvm/lib/Target/M68k/M68kISelLowering.h

SDValue getPICJumpTableRelocBase(SDValue Table,
                                    SelectionDAG &DAG) const override;
 
   /// This returns the relocation base for the given PIC jumptable,
   /// the same as getPICJumpTableRelocBase, but as an MCExpr.
   const MCExpr *getPICJumpTableRelocBaseExpr(const MachineFunction *MF,
                                              unsigned JTI,
                                              MCContext &Ctx) const override;
 
+  ConstraintType getConstraintType(StringRef ConstraintStr) const override;
+
+  std::pair<unsigned, const TargetRegisterClass *>
+  getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
+                               StringRef Constraint, MVT VT) const override;
+
+  // Lower operand with C_Immediate and C_Other constraint type
+  void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
+                                    std::vector<SDValue> &Ops,
+                                    SelectionDAG &DAG) const override;
+
   MachineBasicBlock *
   EmitInstrWithCustomInserter(MachineInstr &MI,
                               MachineBasicBlock *MBB) const override;
 
 private:
   unsigned GetAlignedArgumentStackSize(unsigned StackSize,
                                        SelectionDAG &DAG) const;
 

llvm/lib/Target/M68k/M68kISelLowering.cpp

 }
 
 // NOTE This only used for MachineJumpTableInfo::EK_LabelDifference32 entries
 const MCExpr *M68kTargetLowering::getPICJumpTableRelocBaseExpr(
     const MachineFunction *MF, unsigned JTI, MCContext &Ctx) const {
   return MCSymbolRefExpr::create(MF->getJTISymbol(JTI, Ctx), Ctx);
 }
 
+M68kTargetLowering::ConstraintType
+M68kTargetLowering::getConstraintType(StringRef Constraint) const {
+  if (Constraint.size() > 0) {
+    switch (Constraint[0]) {
+    case 'a':
+    case 'd':
+      return C_RegisterClass;
+    case 'I':
+    case 'J':
+    case 'K':
+    case 'L':
+    case 'M':
+    case 'N':
+    case 'O':
+    case 'P':
+      return C_Immediate;
+    case 'C':
+      if (Constraint.size() == 2)
+        switch (Constraint[1]) {
+        case '0':
+        case 'i':
+        case 'j':
+          return C_Immediate;
+        default:
+          break;
+        }
+      break;
+    default:
+      break;
+    }
+  }
+
+  return TargetLowering::getConstraintType(Constraint);
+}
+
+void M68kTargetLowering::LowerAsmOperandForConstraint(SDValue Op,
+                                                      std::string &Constraint,
+                                                      std::vector<SDValue> &Ops,
+                                                      SelectionDAG &DAG) const {
+  SDValue Result;
+
+  if (Constraint.size() == 1) {
+    // Constant constraints
+    switch (Constraint[0]) {
+    case 'I':
+    case 'J':
+    case 'K':
+    case 'L':
+    case 'M':
+    case 'N':
+    case 'O':
+    case 'P': {
+      auto *C = dyn_cast<ConstantSDNode>(Op);
+      if (!C)
+        return;
+
+      int64_t Val = C->getSExtValue();
+      switch (Constraint[0]) {
+      case 'I': // constant integer in the range [1,8]
+        if (Val > 0 && Val <= 8)
+          break;
+        return;
+      case 'J': // constant signed 16-bit integer
+        if (isInt<16>(Val))
+          break;
+        return;
+      case 'K': // constant that is NOT in the range of [-0x80, 0x80)
+        if (Val < -0x80 || Val >= 0x80)
+          break;
+        return;
+      case 'L': // constant integer in the range [-8,-1]
+        if (Val < 0 && Val >= -8)
+          break;
+        return;
+      case 'M': // constant that is NOT in the range of [-0x100, 0x100]
+        if (Val < -0x100 || Val >= 0x100)
+          break;
+        return;
+      case 'N': // constant integer in the range [24,31]
+        if (Val >= 24 && Val <= 31)
+          break;
+        return;
+      case 'O': // constant integer 16
+        if (Val == 16)
+          break;
+        return;
+      case 'P': // constant integer in the range [8,15]
+        if (Val >= 8 && Val <= 15)
+          break;
+        return;
+      default:
+        llvm_unreachable("Unhandled constant constraint");
+      }
+
+      Result = DAG.getTargetConstant(Val, SDLoc(Op), Op.getValueType());
+      break;
+    }
+    default:
+      break;
+    }
+  }
+
+  if (Constraint.size() == 2) {
+    switch (Constraint[0]) {
+    case 'C':
+      // Constant constraints start with 'C'
+      switch (Constraint[1]) {
+      case '0':
+      case 'i':
+      case 'j': {
+        auto *C = dyn_cast<ConstantSDNode>(Op);
+        if (!C)
+          break;
+
+        int64_t Val = C->getSExtValue();
+        switch (Constraint[1]) {
+        case '0': // constant integer 0
+          if (!Val)
+            break;
+          return;
+        case 'i': // constant integer
+          break;
+        case 'j': // integer constant that doesn't fit in 16 bits
+          if (!isInt<16>(C->getSExtValue()))
+            break;
+          return;
+        default:
+          llvm_unreachable("Unhandled constant constraint");
+        }
+
+        Result = DAG.getTargetConstant(Val, SDLoc(Op), Op.getValueType());
+        break;
+      }
+      default:
+        break;
+      }
+      break;
+    default:
+      break;
+    }
+  }
+
+  if (Result.getNode()) {
+    Ops.push_back(Result);
+    return;
+  }
+
+  TargetLowering::LowerAsmOperandForConstraint(Op, Constraint, Ops, DAG);
+}
+
+std::pair<unsigned, const TargetRegisterClass *>
+M68kTargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
+                                                 StringRef Constraint,
+                                                 MVT VT) const {
+  if (Constraint.size() == 1) {
+    switch (Constraint[0]) {
+    case 'r':
+    case 'd':
+      switch (VT.SimpleTy) {
+      case MVT::i8:
+        return std::make_pair(0U, &M68k::DR8RegClass);
+      case MVT::i16:
+        return std::make_pair(0U, &M68k::DR16RegClass);
+      case MVT::i32:
+        return std::make_pair(0U, &M68k::DR32RegClass);
+      default:
+        break;
+      }
+      break;
+    case 'a':
+      switch (VT.SimpleTy) {
+      case MVT::i16:
+        return std::make_pair(0U, &M68k::AR16RegClass);
+      case MVT::i32:
+        return std::make_pair(0U, &M68k::AR32RegClass);
+      default:
+        break;
+      }
+      break;
+    default:
+      break;
+    }
+  }
+
+  return TargetLowering::getRegForInlineAsmConstraint(TRI, Constraint, VT);
+}
+
 /// Determines whether the callee is required to pop its own arguments.
 /// Callee pop is necessary to support tail calls.
 bool M68k::isCalleePop(CallingConv::ID CallingConv, bool IsVarArg,
                        bool GuaranteeTCO) {
   return false;
 }
 
 // Return true if it is OK for this CMOV pseudo-opcode to be cascaded

llvm/test/CodeGen/M68k/inline-asm.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=m68k < %s -o - | FileCheck %s
+
+; This function is verifying constant constraints that can NOT
+; be easily checked by Clang. For example, 'K' and 'M' are both
+; constraints for values that are outside certain numerical range.
+define void @constant_constraints() {
+; CHECK-LABEL: constant_constraints:
+; CHECK:         .cfi_startproc
+; CHECK-NEXT:  ; %bb.0: ; %entry
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #-129, %d1
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #128, %d1
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #-257, %d1
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #256, %d1
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #-32769, %d1
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #32768, %d1
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    rts
+entry:
+  call void asm sideeffect "move.l $0, %d1", "K"(i32 -129)
+  call void asm sideeffect "move.l $0, %d1", "K"(i32 128)
+  call void asm sideeffect "move.l $0, %d1", "M"(i32 -257)
+  call void asm sideeffect "move.l $0, %d1", "M"(i32 256)
+  call void asm sideeffect "move.l $0, %d1", "^Cj"(i32 -32769)
+  call void asm sideeffect "move.l $0, %d1", "^Cj"(i32 32768)
+  ret void
+}
+
+define void @register_constraints() {
+; CHECK-LABEL: register_constraints:
+; CHECK:         .cfi_startproc
+; CHECK-NEXT:  ; %bb.0: ; %entry
+; CHECK-NEXT:    sub.l #4, %sp
+; CHECK-NEXT:    .cfi_def_cfa_offset -8
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #94, %d0
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    move.l %d0, (0,%sp)
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #87, %d0
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    move.l %d0, (0,%sp)
+; CHECK-NEXT:    ;APP
+; CHECK-NEXT:    move.l #66, %a0
+; CHECK-NEXT:    ;NO_APP
+; CHECK-NEXT:    move.l %a0, (0,%sp)
+; CHECK-NEXT:    add.l #4, %sp
+; CHECK-NEXT:    rts
+entry:
+  %out = alloca i32, align 4
+  %0 = call i32 asm sideeffect "move.l #94, $0", "=r"()
+  store i32 %0, i32* %out, align 4
+  %1 = call i32 asm sideeffect "move.l #87, $0", "=d"()
+  store i32 %1, i32* %out, align 4
+  %2 = call i32 asm sideeffect "move.l #66, $0", "=a"()
+  store i32 %2, i32* %out, align 4
+  ret void
+}
+