[GlobalISel][InlineAsm] Add support for basic input operand constraints

[paquette]

llvm/include/llvm/CodeGen/GlobalISel/InlineAsmLowering.h

38

Doxygen formatting for Ops?

\p Ops

Also can you add Doxygen comments for Val and Constraint?

40

• Can this return a bool? It may be useful to know if this succeeds or fails.
• Can Constraint be a StringRef?

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

277

Missing newline in debug string.

431

Should this newline be here? clang-format?

481

Can you use a range-based for loop here?

[kschwarz]

Address review comments

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

481

I've removed the loop entirely, and return if the number of registers is > 1. In that case we have to unpack the value first, which was missing here.

[kschwarz]

Rebase

[arsenm]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

461

Extra newline

470

Extra newline

483

Missing return false?

582–583

Changing the extension type based on the byte-sizedness doesn't really make sense to me

[kschwarz]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

582–583

I admit I simply followed what is done in the legalizer for extending constants (https://reviews.llvm.org/D70922).

Do you have a suggestion how the condition should look like instead?

[arsenm]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

582–583

I think we generally just sign extend immediate operands like this. There's no legalization or register here, so you shouldn't even need to look at the LLT

[kschwarz]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

582–583

Yes, but it looks like inline asm boolean constants should behave differently (https://reviews.llvm.org/D60224)?

[kschwarz]

Formatting issues, add missing return false;

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

461

Is there a way to catch these formatting issues with clang-format? It doesn't remove those superfluous newlines for me

[arsenm]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

461

I think it just doesn't implement this

582–583

That seems convoluted. That's at least checking explicitly for a bit width of 1, not isByteSized

[kschwarz]

Change boolean constant extension logic to just query the type's bit width

[arsenm]

LGTM

[paquette]

This is causing assertion failures on this bot:

http://green.lab.llvm.org/green/job/test-suite-verify-machineinstrs-aarch64-O0-g/7316/

[kschwarz]

I'll have a look into it now. Thanks for letting me know!

[kschwarz]

I did not handle direct memory operands that need to be "indirectified" in this patch.
https://reviews.llvm.org/D79955 adds an early return for now so we don't run into the assertion while working on supporting that feature.

llvm/include/llvm/CodeGen/GlobalISel/InlineAsmLowering.h

 #define LLVM_CODEGEN_GLOBALISEL_INLINEASMLOWERING_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include <functional>
 
 namespace llvm {
 class CallBase;
 class MachineIRBuilder;
+class MachineOperand;
 class Register;
 class TargetLowering;
 class Value;
 
 class InlineAsmLowering {
   const TargetLowering *TLI;
 
   virtual void anchor();
 
 public:
   bool lowerInlineAsm(MachineIRBuilder &MIRBuilder, const CallBase &CB,
                       std::function<ArrayRef<Register>(const Value &Val)>
                           GetOrCreateVRegs) const;
 
+  /// Lower the specified operand into the Ops vector.  If it is invalid, don't
+  /// add anything to Ops.
+  virtual void lowerAsmOperandForConstraint(Value *Val, std::string &Constraint,
+                                            std::vector<MachineOperand> &Ops,
+                                            MachineIRBuilder &MIRBuilder) const;
+
 protected:
   /// Getter for generic TargetLowering class.
   const TargetLowering *getTLI() const { return TLI; }
 
   /// Getter for target specific TargetLowering class.
   template <class XXXTargetLowering> const XXXTargetLowering *getTLI() const {
     return static_cast<const XXXTargetLowering *>(TLI);
   }

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

bool InlineAsmLowering::lowerInlineAsm(
   const DataLayout &DL = F.getParent()->getDataLayout();
 
   MachineRegisterInfo *MRI = MIRBuilder.getMRI();
 
   TargetLowering::AsmOperandInfoVector TargetConstraints =
       TLI->ParseConstraints(DL, MF.getSubtarget().getRegisterInfo(), Call);
 
   ExtraFlags ExtraInfo(Call);
+  unsigned ArgNo = 0; // ArgNo - The argument of the CallInst.
   unsigned ResNo = 0; // ResNo - The result number of the next output.
   for (auto &T : TargetConstraints) {
     ConstraintOperands.push_back(GISelAsmOperandInfo(T));
     GISelAsmOperandInfo &OpInfo = ConstraintOperands.back();
 
     // Compute the value type for each operand.
     if (OpInfo.Type == InlineAsm::isInput ||
         (OpInfo.Type == InlineAsm::isOutput && OpInfo.isIndirect)) {
 
-      LLVM_DEBUG(dbgs() << "Input operands and indirect output operands are "
-                           "not supported yet\n");
+      OpInfo.CallOperandVal = const_cast<Value *>(Call.getArgOperand(ArgNo++));
+
+      if (const auto *BB = dyn_cast<BasicBlock>(OpInfo.CallOperandVal)) {
+        LLVM_DEBUG(dbgs() << "Basic block input operands not supported yet");
+        return false;
+      }
+
+      Type *OpTy = OpInfo.CallOperandVal->getType();
+
+      // If this is an indirect operand, the operand is a pointer to the
+      // accessed type.
+      if (OpInfo.isIndirect) {
+        PointerType *PtrTy = dyn_cast<PointerType>(OpTy);
+        if (!PtrTy)
+          report_fatal_error("Indirect operand for inline asm not a pointer!");
+        OpTy = PtrTy->getElementType();
+      }
+
+      // FIXME: Support aggregate input operands
+      if (!OpTy->isSingleValueType()) {
+        LLVM_DEBUG(
+            dbgs() << "Aggregate input operands are not supported yet\n");
         return false;
+      }
+
+      OpInfo.ConstraintVT = TLI->getValueType(DL, OpTy, true).getSimpleVT();
 
     } else if (OpInfo.Type == InlineAsm::isOutput && !OpInfo.isIndirect) {
       assert(!Call.getType()->isVoidTy() && "Bad inline asm!");
       if (StructType *STy = dyn_cast<StructType>(Call.getType())) {
         OpInfo.ConstraintVT =
             TLI->getSimpleValueType(DL, STy->getElementType(ResNo));
       } else {
         assert(ResNo == 0 && "Asm only has one result!");
case InlineAsm::isOutput:
                           getImplRegState(Register::isPhysicalRegister(Reg)));
         }
 
         // Remember this output operand for later processing
         OutputOperands.push_back(OpInfo);
       }
 
       break;
-    case InlineAsm::isInput:
+    case InlineAsm::isInput: {
+      if (OpInfo.isMatchingInputConstraint()) {
+        LLVM_DEBUG(dbgs() << "Tied input operands not supported yet\n");
+        return false;
+      }
+
+      if (OpInfo.ConstraintType == TargetLowering::C_Other &&
+          OpInfo.isIndirect) {
+        LLVM_DEBUG(dbgs() << "Indirect input operands with unknown constraint "
+                             "not supported yet\n");
+        return false;
+      }
+
+      if (OpInfo.ConstraintType == TargetLowering::C_Immediate ||
+          OpInfo.ConstraintType == TargetLowering::C_Other) {
+
+        std::vector<MachineOperand> Ops;
+        lowerAsmOperandForConstraint(OpInfo.CallOperandVal,
+                                     OpInfo.ConstraintCode, Ops, MIRBuilder);
+
+        if (Ops.empty()) {
+          LLVM_DEBUG(dbgs() << "Don't support constraint: "
+                            << OpInfo.ConstraintCode << " yet\n");
+          return false;
+        }
+
+        // Add information to the INLINEASM node to know about this input.
+        unsigned OpFlags =
+            InlineAsm::getFlagWord(InlineAsm::Kind_Imm, Ops.size());
+        Inst.addImm(OpFlags);
+        Inst.add(Ops);
+
+        break;
+      }
+
+      if (OpInfo.ConstraintType == TargetLowering::C_Memory) {
+        assert(OpInfo.isIndirect && "Operand must be indirect to be a mem!");
+
+        unsigned ConstraintID =
+            TLI->getInlineAsmMemConstraint(OpInfo.ConstraintCode);
+        unsigned OpFlags = InlineAsm::getFlagWord(InlineAsm::Kind_Mem, 1);
+        OpFlags = InlineAsm::getFlagWordForMem(OpFlags, ConstraintID);
+        Inst.addImm(OpFlags);
+        ArrayRef<Register> SourceRegs =
+            GetOrCreateVRegs(*OpInfo.CallOperandVal);
+        assert(
+            SourceRegs.size() == 1 &&
+            "Expected the memory input to fit into a single virtual register");
+        Inst.addReg(SourceRegs[0]);
+        break;
+      }
+
+      assert((OpInfo.ConstraintType == TargetLowering::C_RegisterClass ||
+              OpInfo.ConstraintType == TargetLowering::C_Register) &&
+             "Unknown constraint type!");
+
+      if (OpInfo.isIndirect) {
+        LLVM_DEBUG(dbgs() << "Can't handle indirect register inputs yet "
+                             "for constraint '"
+                          << OpInfo.ConstraintCode << "'\n");
+
         return false;
+      }
+
+      // Copy the input into the appropriate registers.
+      if (OpInfo.Regs.empty()) {
+        LLVM_DEBUG(
+            dbgs()
+            << "Couldn't allocate input register for register constraint\n");
+
+        return false;
+      }
+
+      unsigned NumRegs = OpInfo.Regs.size();
+      ArrayRef<Register> SourceRegs = GetOrCreateVRegs(*OpInfo.CallOperandVal);
+      assert(NumRegs == SourceRegs.size() &&
+             "Expected the number of input registers to match the number of "
+             "source registers");
+
+      unsigned Flag = InlineAsm::getFlagWord(InlineAsm::Kind_RegUse, NumRegs);
+      Inst.addImm(Flag);
+      for (unsigned i = 0; i < NumRegs; i++) {
+        MIRBuilder.buildCopy(OpInfo.Regs[i], SourceRegs[0]);
+        Inst.addReg(OpInfo.Regs[i]);
+      }
+
+      break;
+    }
+
     case InlineAsm::isClobber: {
 
       unsigned NumRegs = OpInfo.Regs.size();
       if (NumRegs > 0) {
         unsigned Flag =
             InlineAsm::getFlagWord(InlineAsm::Kind_Clobber, NumRegs);
         Inst.addImm(Flag);
 
for (unsigned int i = 0, e = ResRegs.size(); i < e; i++) {
     case TargetLowering::C_Unknown:
       LLVM_DEBUG(dbgs() << "Unexpected unknown constraint\n");
       return false;
     }
   }
 
   return true;
 }
+
+void InlineAsmLowering::lowerAsmOperandForConstraint(
+    Value *Val, std::string &Constraint, std::vector<MachineOperand> &Ops,
+    MachineIRBuilder &MIRBuilder) const {
+  if (Constraint.length() > 1)
+    return;
+
+  char ConstraintLetter = Constraint[0];
+  switch (ConstraintLetter) {
+  default:
+    return;
+  case 'i': // Simple Integer or Relocatable Constant
+    if (ConstantInt *CI = dyn_cast<ConstantInt>(Val)) {
+      assert(CI->getBitWidth() <= 64 &&
+             "expected immediate to fit into 64-bits");
+      LLT Ty = getLLTForType(*Val->getType(), MIRBuilder.getDataLayout());
+      unsigned ExtOpc =
+          Ty.isByteSized() ? TargetOpcode::G_SEXT : TargetOpcode::G_ZEXT;
+      int64_t ExtVal = ExtOpc == TargetOpcode::G_SEXT ? CI->getSExtValue()
+                                                      : CI->getZExtValue();
+
+      Ops.push_back(MachineOperand::CreateImm(ExtVal));
+      return;
+    }
+    return;
+  }
+}

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-inline-asm.ll

define zeroext i8 @test_register_output_trunc(i8* %src) nounwind {
   ; CHECK:   [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[COPY1]](s32)
   ; CHECK:   [[ZEXT:%[0-9]+]]:_(s32) = G_ZEXT [[TRUNC]](s8)
   ; CHECK:   $w0 = COPY [[ZEXT]](s32)
   ; CHECK:   RET_ReallyLR implicit $w0
 entry:
   %0 = tail call i8 asm "mov ${0:w}, 32", "=r"() nounwind
   ret i8 %0
 }
+
+define void @test_input_register_imm() {
+  ; CHECK-LABEL: name: test_input_register_imm
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 42
+  ; CHECK:   [[COPY:%[0-9]+]]:gpr64common = COPY [[C]](s64)
+  ; CHECK:   INLINEASM &"mov x0, $0", 1 /* sideeffect attdialect */, 9 /* reguse */, [[COPY]]
+  ; CHECK:   RET_ReallyLR
+  call void asm sideeffect "mov x0, $0", "r"(i64 42)
+  ret void
+}
+
+; Make sure that boolean immediates are properly (zero) extended.
+define i32 @test_boolean_imm_ext() {
+  ; CHECK-LABEL: name: test_boolean_imm_ext
+  ; CHECK: bb.1.entry:
+  ; CHECK:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
+  ; CHECK:   INLINEASM &"#TEST 42 + ${0:c} - .\0A\09", 9 /* sideeffect mayload attdialect */, 13 /* imm */, 1
+  ; CHECK:   $w0 = COPY [[C]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+entry:
+  tail call void asm sideeffect "#TEST 42 + ${0:c} - .\0A\09", "i"(i1 true)
+  ret i32 1
+}
+
+define void @test_input_imm() {
+  ; CHECK-LABEL: name: test_input_imm
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   INLINEASM &"mov x0, $0", 9 /* sideeffect mayload attdialect */, 13 /* imm */, 42
+  ; CHECK:   RET_ReallyLR
+  call void asm sideeffect "mov x0, $0", "i"(i64 42)
+  ret void
+}
+
+define zeroext i8 @test_input_register(i8* %src) nounwind {
+  ; CHECK-LABEL: name: test_input_register
+  ; CHECK: bb.1.entry:
+  ; CHECK:   liveins: $x0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+  ; CHECK:   [[COPY1:%[0-9]+]]:gpr64common = COPY [[COPY]](p0)
+  ; CHECK:   INLINEASM &"ldtrb ${0:w}, [$1]", 0 /* attdialect */, 589834 /* regdef:GPR32common */, def %1, 9 /* reguse */, [[COPY1]]
+  ; CHECK:   [[COPY2:%[0-9]+]]:_(s32) = COPY %1
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[COPY2]](s32)
+  ; CHECK:   [[ZEXT:%[0-9]+]]:_(s32) = G_ZEXT [[TRUNC]](s8)
+  ; CHECK:   $w0 = COPY [[ZEXT]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+entry:
+  %0 = tail call i8 asm "ldtrb ${0:w}, [$1]", "=r,r"(i8* %src) nounwind
+  ret i8 %0
+}
+
+define i32 @test_memory_constraint(i32* %a) nounwind {
+  ; CHECK-LABEL: name: test_memory_constraint
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   liveins: $x0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+  ; CHECK:   INLINEASM &"ldr $0, $1", 8 /* mayload attdialect */, 589834 /* regdef:GPR32common */, def %1, 196622 /* mem:m */, [[COPY]](p0)
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY %1
+  ; CHECK:   $w0 = COPY [[COPY1]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %1 = tail call i32 asm "ldr $0, $1", "=r,*m"(i32* %a)
+  ret i32 %1
+}