[GlobalISel][InlineAsm] Fix buildCopy for inputs

[Petar.Avramovic]

Summary

Check that input size matches size of destination reg class.
Attempt to extend input size when needed.

[kschwarz]

Hi @Petar.Avramovic,

we noticed that this extension is also missing for the non-tied register input operands. I added the opposite logic, i.e. truncating of output operands, but forgot about extending the inputs.
Would you mind adding that handling in this patch too, as it is basically the same?
If you feel like it should be in a separate commit, I can also provide a patch for it. Thanks!

[paquette]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

240

Would MachineIRBuilder::buildExtOrTrunc work here?

If not, maybe it would make sense to move this to MachineIRBuilder for the sake of consistency?

[Petar.Avramovic]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

240

The destination is vreg with reg class and source is generic vreg with LLT, it would not work for anyext since it requires both source and dest to be generic virtual registers. Here we anyext to new generic vreg with same size as Dst and then copy to Dst.
MachineIRBuilder specializes for generic vregs so should it be something like: "buildExtOrTruncToVRegWithRegClass" ? Should I also cover vectors? I don't know if there is a way to know LLT of vector type that would fit into reg class.

[paquette]

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

240

I see.

Considering that difference, I think this is fine. We can refactor later if it turns out to be a good idea.

As for vectors, I think that would be better in a follow-up.

254

Can we check MRI->getType(Src).isValid() before here? I think we always want to check this, right?

e.g.

auto SrcTy = MRI->getType(Src);
if (!SrcTy.isValid()) {
    LLVM_DEBUG(dbgs() << "Source type for copy is not valid\n");
    return false;
}

if (DstSize < SrcSize) {
  ...
}

// Attempt to anyext small scalar sources.
if (DstSize > SrcSize) {
  ...
}

...

[Petar.Avramovic]

Move isValid check on top.

[paquette]

LGTM

[jrtc27]

I believe this has broken AArch64 (-global-isel not required due to defaults, but it's a GlobalISel bug so should force it nonetheless):

; RUN: llc -mtriple=aarch64 -global-isel < %s

define i64 @foo() nounwind {
  %1 = tail call i64 asm "", "={x2},0"(i64 0) nounwind
  ret i64 %1
}

fails with:

llc: /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/include/llvm/CodeGen/Register.h:79: static unsigned int llvm::Register::virtReg2Index(unsigned int): Assertion `isVirtualRegister(Reg) && "Not a virtual register"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.      Program arguments: /local/scratch/jrtc4/cheri/llvm-project-upstream/build/bin/llc -mtriple=aarch64 -O0
1.      Running pass 'Function Pass Manager' on module '<stdin>'.
2.      Running pass 'IRTranslator' on function '@foo'
 #0 0x00007fb5bc5e281b llvm::sys::PrintStackTrace(llvm::raw_ostream&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/Support/Unix/Signals.inc:564:0
 #1 0x00007fb5bc5e28ae PrintStackTraceSignalHandler(void*) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/Support/Unix/Signals.inc:625:0
 #2 0x00007fb5bc5e064b llvm::sys::RunSignalHandlers() /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/Support/Signals.cpp:68:0
 #3 0x00007fb5bc5e2198 SignalHandler(int) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/Support/Unix/Signals.inc:406:0
 #4 0x00007fb5bba4efd0 (/lib/x86_64-linux-gnu/libc.so.6+0x3efd0)
 #5 0x00007fb5bba4ef47 raise /build/glibc-2ORdQG/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #6 0x00007fb5bba508b1 abort /build/glibc-2ORdQG/glibc-2.27/stdlib/abort.c:81:0
 #7 0x00007fb5bba4042a __assert_fail_base /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:89:0
 #8 0x00007fb5bba404a2 (/lib/x86_64-linux-gnu/libc.so.6+0x304a2)
 #9 0x00007fb5bb387afa llvm::Register::virtReg2Index(unsigned int) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/include/llvm/CodeGen/Register.h:79:0
#10 0x00007fb5bb388ad1 llvm::VirtReg2IndexFunctor::operator()(unsigned int) const /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/include/llvm/CodeGen/TargetRegisterInfo.h:1152:0
#11 0x00007fb5bb389c4f llvm::IndexedMap<std::pair<llvm::PointerUnion<llvm::TargetRegisterClass const*, llvm::RegisterBank const*>, llvm::MachineOperand*>, llvm::VirtReg2IndexFunctor>::operator[](unsigned int) const /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/include/llvm/ADT/IndexedMap.h:51:0
#12 0x00007fb5bb4003d5 llvm::MachineRegisterInfo::getRegClass(llvm::Register) const /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/include/llvm/CodeGen/MachineRegisterInfo.h:632:0
#13 0x00007fb5bb3fe0ec llvm::InlineAsmLowering::lowerInlineAsm(llvm::MachineIRBuilder&, llvm::CallBase const&, std::function<llvm::ArrayRef<llvm::Register> (llvm::Value const&)>) const /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp:459:0
#14 0x00007fb5bb3ca04a llvm::IRTranslator::translateInlineAsm(llvm::CallBase const&, llvm::MachineIRBuilder&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:1701:0
#15 0x00007fb5bb3ca744 llvm::IRTranslator::translateCall(llvm::User const&, llvm::MachineIRBuilder&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:1759:0
#16 0x00007fb5bb3ce383 llvm::IRTranslator::translate(llvm::Instruction const&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/include/llvm/IR/Instruction.def:209:0
#17 0x00007fb5bb3d0f97 llvm::IRTranslator::runOnMachineFunction(llvm::MachineFunction&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2511:0
#18 0x00007fb5bfd0d33f llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/CodeGen/MachineFunctionPass.cpp:73:0
#19 0x00007fb5bcc07ee8 llvm::FPPassManager::runOnFunction(llvm::Function&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1587:0
#20 0x00007fb5bcc08191 llvm::FPPassManager::runOnModule(llvm::Module&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1633:0
#21 0x00007fb5bcc085b9 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1702:0
#22 0x00007fb5bcc035d1 llvm::legacy::PassManagerImpl::run(llvm::Module&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:614:0
#23 0x00007fb5bcc08e45 llvm::legacy::PassManager::run(llvm::Module&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/lib/IR/LegacyPassManager.cpp:1830:0
#24 0x000055c7388da197 compileModule(char**, llvm::LLVMContext&) /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/tools/llc/llc.cpp:653:0
#25 0x000055c7388d8077 main /local/scratch/jrtc4/cheri/llvm-project-upstream/llvm/tools/llc/llc.cpp:360:0
#26 0x00007fb5bba31b97 __libc_start_main /build/glibc-2ORdQG/glibc-2.27/csu/../csu/libc-start.c:344:0
#27 0x000055c7388d727a _start (/local/scratch/jrtc4/cheri/llvm-project-upstream/build/bin/llc+0x2527a)

This is on commit 161882816540fc011554e4a820ab896278491b6a, so after D83763 too, but the test also fails on the 11 release branch, which does not have that revision, only this one.

Credit to @sorear for providing a minimal C test case that I then analysed further.

[jrtc27]

Actually the code in the backtrace points at D82651, not this. I did however see buildAnyextOrCopy in a backtrace when playing around to get that minimal case, which is what brought me to this revision. Investigating further, it seems that this is because of D82651 not fully implementing this case, and future revisions not fixing that. Prior to that revision, inline asm would hit reportTranslationError and then fall back to SelectionDAG due to isGlobalISelAbortEnabled being true (provided -global-isel wasn't passed to override that, as is done in my test, but not the original C), but now, since it claims to be implemented but not correctly, it instead asserts deep down and is unable to fall back to SelectionDAG.

Thus, please do one of:

1. Gate inline asm lowering behind an experimental flag so that the old behaviour of using SelectionDAG when inline asm is involved is restored

2. Ensure full support for all the inline asm constructs

3. Be very cautious in what inline asm is accepted, calling reportTranslationError eagerly if it looks like anything might go awry

4. Revert the set of inline asm commits (seems a bit unnecessary given the other options, though maybe the right option for 11.x if not doing option 1?)

or something else appropriate. But this regression is affecting the 11.x release and hit by real-world code (in this case, seL4 on AArch64 compiled at -O0).

llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp

static void computeConstraintToUse(const TargetLowering *TLI,
   }
 }
 
 static unsigned getNumOpRegs(const MachineInstr &I, unsigned OpIdx) {
   unsigned Flag = I.getOperand(OpIdx).getImm();
   return InlineAsm::getNumOperandRegisters(Flag);
 }
 
+static bool buildAnyextOrCopy(Register Dst, Register Src,
+                              MachineIRBuilder &MIRBuilder) {
+  const TargetRegisterInfo *TRI =
+      MIRBuilder.getMF().getSubtarget().getRegisterInfo();
+  MachineRegisterInfo *MRI = MIRBuilder.getMRI();
+
+  unsigned SrcSize = TRI->getRegSizeInBits(Src, *MRI);
+  unsigned DstSize = TRI->getRegSizeInBits(Dst, *MRI);
+  if (DstSize < SrcSize) {
+    LLVM_DEBUG(dbgs() << "Input can't fit in destination reg class\n");
+    return false;
+  }
+  // Attempt to anyext small scalar sources.
+  if (DstSize > SrcSize) {
+    if (!MRI->getType(Src).isValid() || !MRI->getType(Src).isScalar()) {
+      LLVM_DEBUG(dbgs() << "Can't extend input to size of destination"
+                           " reg class\n");
+      return false;
+    }
+    Src = MIRBuilder.buildAnyExt(LLT::scalar(DstSize), Src).getReg(0);
+  }
+  MIRBuilder.buildCopy(Dst, Src);
+  return true;
+}
+
 bool InlineAsmLowering::lowerInlineAsm(
     MachineIRBuilder &MIRBuilder, const CallBase &Call,
     std::function<ArrayRef<Register>(const Value &Val)> GetOrCreateVRegs)
     const {
   const InlineAsm *IA = cast<InlineAsm>(Call.getCalledOperand());
 
   /// ConstraintOperands - Information about all of the constraints.
   GISelAsmOperandInfoVector ConstraintOperands;
case InlineAsm::isInput: {
         unsigned DefRegIdx = InstFlagIdx + 1;
         Register Def = Inst->getOperand(DefRegIdx).getReg();
 
         // Copy input to new vreg with same reg class as Def
         const TargetRegisterClass *RC = MRI->getRegClass(Def);
         ArrayRef<Register> SrcRegs = GetOrCreateVRegs(*OpInfo.CallOperandVal);
         assert(SrcRegs.size() == 1 && "Single register is expected here");
         Register Tmp = MRI->createVirtualRegister(RC);
-        MIRBuilder.buildCopy(Tmp, SrcRegs[0]);
+        if (!buildAnyextOrCopy(Tmp, SrcRegs[0], MIRBuilder))
+          return false;
 
         // Add Flag and input register operand (Tmp) to Inst. Tie Tmp to Def.
         unsigned UseFlag = InlineAsm::getFlagWord(InlineAsm::Kind_RegUse, 1);
         unsigned Flag = InlineAsm::getFlagWordForMatchingOp(UseFlag, DefIdx);
         Inst.addImm(Flag);
         Inst.addReg(Tmp);
         Inst->tieOperands(DefRegIdx, Inst->getNumOperands() - 1);
         break;
case InlineAsm::isInput: {
       if (NumRegs > 1) {
         LLVM_DEBUG(dbgs() << "Input operands with multiple input registers are "
                              "not supported yet\n");
         return false;
       }
 
       unsigned Flag = InlineAsm::getFlagWord(InlineAsm::Kind_RegUse, NumRegs);
       Inst.addImm(Flag);
-      MIRBuilder.buildCopy(OpInfo.Regs[0], SourceRegs[0]);
+      if (!buildAnyextOrCopy(OpInfo.Regs[0], SourceRegs[0], MIRBuilder))
+        return false;
       Inst.addReg(OpInfo.Regs[0]);
       break;
     }
 
     case InlineAsm::isClobber: {
 
       unsigned NumRegs = OpInfo.Regs.size();
       if (NumRegs > 0) {

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

define i32 @test_memory_constraint(i32* %a) nounwind {
   ; CHECK:   [[COPY:%[0-9]+]]:_(p0) = COPY $x0
   ; CHECK:   INLINEASM &"ldr $0, $1", 8 /* mayload attdialect */, 655370 /* 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
 }
+
+define i16 @test_anyext_input() {
+  ; CHECK-LABEL: name: test_anyext_input
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   [[C:%[0-9]+]]:_(s16) = G_CONSTANT i16 1
+  ; CHECK:   [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[C]](s16)
+  ; CHECK:   [[COPY:%[0-9]+]]:gpr32common = COPY [[ANYEXT]](s32)
+  ; CHECK:   INLINEASM &"", 1 /* sideeffect attdialect */, 655370 /* regdef:GPR32common */, def %0, 9 /* reguse */, [[COPY]]
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY %0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY1]](s32)
+  ; CHECK:   [[ANYEXT1:%[0-9]+]]:_(s32) = G_ANYEXT [[TRUNC]](s16)
+  ; CHECK:   $w0 = COPY [[ANYEXT1]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %1 = call i16 asm sideeffect "", "=r,r"(i16 1)
+  ret i16 %1
+}
+
+define i16 @test_anyext_input_with_matching_constraint() {
+  ; CHECK-LABEL: name: test_anyext_input_with_matching_constraint
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   [[C:%[0-9]+]]:_(s16) = G_CONSTANT i16 1
+  ; CHECK:   [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[C]](s16)
+  ; CHECK:   [[COPY:%[0-9]+]]:gpr32common = COPY [[ANYEXT]](s32)
+  ; CHECK:   INLINEASM &"", 1 /* sideeffect attdialect */, 655370 /* regdef:GPR32common */, def %0, 2147483657 /* reguse tiedto:$0 */, [[COPY]](tied-def 3)
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY %0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY1]](s32)
+  ; CHECK:   [[ANYEXT1:%[0-9]+]]:_(s32) = G_ANYEXT [[TRUNC]](s16)
+  ; CHECK:   $w0 = COPY [[ANYEXT1]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %1 = call i16 asm sideeffect "", "=r,0"(i16 1)
+  ret i16 %1
+}