Fix crash on inline asm with 64bit matching input in 32bit GPR

[thopre]

Summary

Add support for inline assembly with matching input operand that do not naturally go in the register class it is constrained to (eg. double in a 32-bit GPR). Note that regular input is already handled by existing code.

[thopre]

Rename testcase and test both soft and hard float ABI

[efriedma]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7222

I don't follow this... don't we need to allocate virtual registers anyway? And how is this relevant to this patch?

7471–7472

Why is this "OpInfo.ConstraintType == TargetLowering::C_Register" here, but "RefOpInfo.ConstraintType == TargetLowering::C_RegisterClass" later?

[thopre]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7222

The virtual register is allocated later at line 7599 with the patch applied when "looping over all of the inputs, copying the operand values into the appropriate registers and processing the output regs" (dixit comment). That code was already there before any of my patch and I decided to keep that code structure.

It is relevant to this patch because prior to it GetRegistersForValue was not called for matching input since it was only called for C_RegisterClass constraints. It is now called for matching input as well since that is where is the logic to deal with mismatch between register constraint size and the operand type.

7471–7472

I didn't write these two checks but from the comment above the goal seems to distinguish between constraints for specific register (C_Register) from general register constraint (C_RegisterClass). Allocating registers for the former before the latter avoids allocating register X to a general register constraint and later dealing with a constraint asking for X specifically when there was a register Y which the general constraint would have been happy with.

[efriedma]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7471–7472

Sorry, I wasn't clear; why is the first check checking OpInfo, and the second checking RefOpInfo?

[thopre]

Fix logic for input matching a register constraint for a specific register and add tests for it. I've verified that the code for this logic is executed by those new testcases and that the testcase failed without the fix.

[thopre]

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

7471–7472

Good catch! I've fixed this and added testcases for this cases and confirmed that they failed when using OpInfo.

[efriedma]

LGTM

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

 }
 
 /// GetRegistersForValue - Assign registers (virtual or physical) for the
 /// specified operand.  We prefer to assign virtual registers, to allow the
 /// register allocator to handle the assignment process.  However, if the asm
 /// uses features that we can't model on machineinstrs, we have SDISel do the
 /// allocation.  This produces generally horrible, but correct, code.
 ///
-///   OpInfo describes the operand.
+///   OpInfo describes the operand
+///   RefOpInfo describes the matching operand if any, the operand otherwise
 static void GetRegistersForValue(SelectionDAG &DAG, const TargetLowering &TLI,
-                                 const SDLoc &DL,
-                                 SDISelAsmOperandInfo &OpInfo) {
+                                 const SDLoc &DL, SDISelAsmOperandInfo &OpInfo,
+                                 SDISelAsmOperandInfo &RefOpInfo) {
   LLVMContext &Context = *DAG.getContext();
 
   MachineFunction &MF = DAG.getMachineFunction();
   SmallVector<unsigned, 4> Regs;
   const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
 
   // If this is a constraint for a single physreg, or a constraint for a
   // register class, find it.
   std::pair<unsigned, const TargetRegisterClass *> PhysReg =
-      TLI.getRegForInlineAsmConstraint(&TRI, OpInfo.ConstraintCode,
-                                       OpInfo.ConstraintVT);
+      TLI.getRegForInlineAsmConstraint(&TRI, RefOpInfo.ConstraintCode,
+                                       RefOpInfo.ConstraintVT);
 
   unsigned NumRegs = 1;
   if (OpInfo.ConstraintVT != MVT::Other) {
     // If this is a FP operand in an integer register (or visa versa), or more
     // generally if the operand value disagrees with the register class we plan
     // to stick it in, fix the operand type.
     //
     // If this is an input value, the bitcast to the new type is done now.
if ((OpInfo.Type == InlineAsm::isOutput ||
               DAG.getNode(ISD::BITCAST, DL, RegVT, OpInfo.CallOperand);
         OpInfo.ConstraintVT = RegVT;
       }
     }
 
     NumRegs = TLI.getNumRegisters(Context, OpInfo.ConstraintVT);
   }
 
+  // No need to allocate a matching input constraint since the constraint it's
+  // matching to has already been allocated.
+  if (OpInfo.isMatchingInputConstraint())
+    return;
+
   MVT RegVT;
   EVT ValueVT = OpInfo.ConstraintVT;
 
   // If this is a constraint for a specific physical register, like {r17},
   // assign it now.
   if (unsigned AssignedReg = PhysReg.first) {
     const TargetRegisterClass *RC = PhysReg.second;
     if (OpInfo.ConstraintVT == MVT::Other)
if (OpInfo.ConstraintType == TargetLowering::C_Memory &&
       // There is no longer a Value* corresponding to this operand.
       OpInfo.CallOperandVal = nullptr;
 
       // It is now an indirect operand.
       OpInfo.isIndirect = true;
     }
 
     // If this constraint is for a specific register, allocate it before
     // anything else.
-    if (OpInfo.ConstraintType == TargetLowering::C_Register)
-      GetRegistersForValue(DAG, TLI, getCurSDLoc(), OpInfo);
+    SDISelAsmOperandInfo &RefOpInfo =
+        OpInfo.isMatchingInputConstraint()
+            ? ConstraintOperands[OpInfo.getMatchedOperand()]
+            : ConstraintOperands[i];
+    if (RefOpInfo.ConstraintType == TargetLowering::C_Register)
+      GetRegistersForValue(DAG, TLI, getCurSDLoc(), OpInfo, RefOpInfo);
   }
 
   // Third pass - Loop over all of the operands, assigning virtual or physregs
   // to register class operands.
   for (unsigned i = 0, e = ConstraintOperands.size(); i != e; ++i) {
     SDISelAsmOperandInfo &OpInfo = ConstraintOperands[i];
+    SDISelAsmOperandInfo &RefOpInfo =
+        OpInfo.isMatchingInputConstraint()
+            ? ConstraintOperands[OpInfo.getMatchedOperand()]
+            : ConstraintOperands[i];
 
     // C_Register operands have already been allocated, Other/Memory don't need
     // to be.
-    if (OpInfo.ConstraintType == TargetLowering::C_RegisterClass)
-      GetRegistersForValue(DAG, TLI, getCurSDLoc(), OpInfo);
+    if (RefOpInfo.ConstraintType == TargetLowering::C_RegisterClass)
+      GetRegistersForValue(DAG, TLI, getCurSDLoc(), OpInfo, RefOpInfo);
   }
 
   // AsmNodeOperands - The operands for the ISD::INLINEASM node.
   std::vector<SDValue> AsmNodeOperands;
   AsmNodeOperands.push_back(SDValue());  // reserve space for input chain
   AsmNodeOperands.push_back(DAG.getTargetExternalSymbol(
       IA->getAsmString().c_str(), TLI.getPointerTy(DAG.getDataLayout())));
 

test/CodeGen/ARM/inline-asm-operand-implicit-cast.ll

 define arm_aapcscc double @zerobits_double_soft() #0 {
 ; CHECK-LABEL: zerobits_double_soft
 ; CHECK: mov r0, #0
 ; CHECK-NEXT: mov r1, #0
   %1 = tail call double asm "mov ${0:Q}, #0\0Amov ${0:R}, #0", "=&r"()
   ret double %1
 }
 
+; Check support for returning a float in GPR with matching float input with
+; soft float ABI
+define arm_aapcscc float @flt_gpr_matching_in_op_soft(float %f) #0 {
+; CHECK-LABEL: flt_gpr_matching_in_op_soft
+; CHECK: mov r0, r0
+  %1 = call float asm "mov $0, $1", "=&r,0"(float %f)
+  ret float %1
+}
+
+; Check support for returning a double in GPR with matching double input with
+; soft float ABI
+define arm_aapcscc double @dbl_gpr_matching_in_op_soft(double %d) #0 {
+; CHECK-LABEL: dbl_gpr_matching_in_op_soft
+; CHECK: mov r1, r0
+  %1 = call double asm "mov ${0:R}, ${1:Q}", "=&r,0"(double %d)
+  ret double %1
+}
+
+; Check support for returning a float in specific GPR with matching float input
+; with soft float ABI
+define arm_aapcscc float @flt_gpr_matching_spec_reg_in_op_soft(float %f) #0 {
+; CHECK-LABEL: flt_gpr_matching_spec_reg_in_op_soft
+; CHECK: mov r3, r3
+  %1 = call float asm "mov $0, $1", "=&{r3},0"(float %f)
+  ret float %1
+}
+
+; Check support for returning a double in specific GPR with matching double
+; input with soft float ABI
+define arm_aapcscc double @dbl_gpr_matching_spec_reg_in_op_soft(double %d) #0 {
+; CHECK-LABEL: dbl_gpr_matching_spec_reg_in_op_soft
+; CHECK: mov r3, r2
+  %1 = call double asm "mov ${0:R}, ${1:Q}", "=&{r2},0"(double %d)
+  ret double %1
+}
+
 attributes #0 = { nounwind "target-features"="+d16,+vfp2,+vfp3,-fp-only-sp" "use-soft-float"="true" }
 
 
 ; Check support for returning a float in GPR with hard float ABI
 define float @zerobits_float_hard() #1 {
 ; CHECK-LABEL: zerobits_float_hard
 ; CHECK: mov r0, #0
 ; CHECK: vmov s0, r0
   %1 = tail call float asm "mov ${0}, #0", "=&r"()
   ret float %1
 }
 
 ; Check support for returning a double in GPR with hard float ABI
 define double @zerobits_double_hard() #1 {
 ; CHECK-LABEL: zerobits_double_hard
 ; CHECK: mov r0, #0
 ; CHECK-NEXT: mov r1, #0
 ; CHECK: vmov d0, r0, r1
   %1 = tail call double asm "mov ${0:Q}, #0\0Amov ${0:R}, #0", "=&r"()
   ret double %1
 }
 
+; Check support for returning a float in GPR with matching float input with
+; hard float ABI
+define float @flt_gpr_matching_in_op_hard(float %f) #1 {
+; CHECK-LABEL: flt_gpr_matching_in_op_hard
+; CHECK: vmov r0, s0
+; CHECK: mov r0, r0
+; CHECK: vmov s0, r0
+  %1 = call float asm "mov $0, $1", "=&r,0"(float %f)
+  ret float %1
+}
+
+; Check support for returning a double in GPR with matching double input with
+; hard float ABI
+define double @dbl_gpr_matching_in_op_hard(double %d) #1 {
+; CHECK-LABEL: dbl_gpr_matching_in_op_hard
+; CHECK: vmov r0, r1, d0
+; CHECK: mov r1, r0
+; CHECK: vmov d0, r0, r1
+  %1 = call double asm "mov ${0:R}, ${1:Q}", "=&r,0"(double %d)
+  ret double %1
+}
+
+; Check support for returning a float in specific GPR with matching float
+; input with hard float ABI
+define float @flt_gpr_matching_spec_reg_in_op_hard(float %f) #1 {
+; CHECK-LABEL: flt_gpr_matching_spec_reg_in_op_hard
+; CHECK: vmov r3, s0
+; CHECK: mov r3, r3
+; CHECK: vmov s0, r3
+  %1 = call float asm "mov $0, $1", "=&{r3},0"(float %f)
+  ret float %1
+}
+
+; Check support for returning a double in specific GPR with matching double
+; input with hard float ABI
+define double @dbl_gpr_matching_spec_reg_in_op_hard(double %d) #1 {
+; CHECK-LABEL: dbl_gpr_matching_spec_reg_in_op_hard
+; CHECK: vmov r2, r3, d0
+; CHECK: mov r3, r2
+; CHECK: vmov d0, r2, r3
+  %1 = call double asm "mov ${0:R}, ${1:Q}", "=&{r2},0"(double %d)
+  ret double %1
+}
+
 attributes #1 = { nounwind "target-features"="+d16,+vfp2,+vfp3,-fp-only-sp" "use-soft-float"="false" }