[AArch64] Avoid selecting XZR inline ASM memory operand

[kongyi]

Summary

Restricting register class to PointerRegClass for memory operands.

Also fix the PointerRegClass for AArch64 from GPR64 to GPR64sp, since XZR cannot hold a memory pointer while SP is.

Fixes PR33134.

[efriedma]

Is this really the best place to solve this issue?

PowerPC has a similar problem (r0 can't be used for memory addresses); the PPC backend solves it in PPCDAGToDAGISel::SelectInlineAsmMemoryOperand.

For non-memory inline asm operands, SelectionDAGBuilder::visitInlineAsm has code to constrain the register classes.

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

161 ↗

(On Diff #105226)

Let me see if I follow this correctly. At this point, either isMachineOpcode is true, or the opcode is one of the ones listed in the switch in InstrEmitter::EmitSpecialNode (ISD::MERGE_VALUES, ISD::TokenFactor, ISD::CopyToReg, ISD::CopyFromReg, ISD::EH_LABEL, ISD::LIFETIME_START, ISD::LIFETIME_END, and ISD::INLINEASM). For machine opcodes, we use getRegClass() to get the register class. For inline asm, we use this new code (roughly equivalent to MachineInstr::getRegClassConstraint) to get the register class. For the other opcodes, the register class doesn't matter...?

InstrEmitter::AddRegisterOperand has similar code to retrieve a regclass for an operand; do we also need to special-case inline asm there?

[kongyi]

In D34999#802781, @efriedma wrote:

Is this really the best place to solve this issue?

PowerPC has a similar problem (r0 can't be used for memory addresses); the PPC backend solves it in PPCDAGToDAGISel::SelectInlineAsmMemoryOperand.

For non-memory inline asm operands, SelectionDAGBuilder::visitInlineAsm has code to constrain the register classes.

Thanks, this is a more suitable place to implement the logic.

[efriedma]

LGTM, with one minor comment.

test/CodeGen/AArch64/asm-zero-address.ll

9 ↗

(On Diff #106704)

Maybe put the test in test/CodeGen/AArch64/arm64-inline-asm.ll instead of its own file?

llvm/trunk/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

 bool AArch64DAGToDAGISel::SelectInlineAsmMemoryOperand(
     const SDValue &Op, unsigned ConstraintID, std::vector<SDValue> &OutOps) {
   switch(ConstraintID) {
   default:
     llvm_unreachable("Unexpected asm memory constraint");
   case InlineAsm::Constraint_i:
   case InlineAsm::Constraint_m:
   case InlineAsm::Constraint_Q:
-    // Require the address to be in a register.  That is safe for all AArch64
-    // variants and it is hard to do anything much smarter without knowing
-    // how the operand is used.
-    OutOps.push_back(Op);
+    // We need to make sure that this one operand does not end up in XZR, thus
+    // require the address to be in a PointerRegClass register.
+    const TargetRegisterInfo *TRI = Subtarget->getRegisterInfo();
+    const TargetRegisterClass *TRC = TRI->getPointerRegClass(*MF);
+    SDLoc dl(Op);
+    SDValue RC = CurDAG->getTargetConstant(TRC->getID(), dl, MVT::i64);
+    SDValue NewOp =
+        SDValue(CurDAG->getMachineNode(TargetOpcode::COPY_TO_REGCLASS,
+                                       dl, Op.getValueType(),
+                                       Op, RC), 0);
+    OutOps.push_back(NewOp);
     return false;
   }
   return true;
 }
 
 /// SelectArithImmed - Select an immediate value that can be represented as
 /// a 12-bit value shifted left by either 0 or 12.  If so, return true with
 /// Val set to the 12-bit value and Shift set to the shifter operand.

llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.cpp

 
 bool AArch64RegisterInfo::isConstantPhysReg(unsigned PhysReg) const {
   return PhysReg == AArch64::WZR || PhysReg == AArch64::XZR;
 }
 
 const TargetRegisterClass *
 AArch64RegisterInfo::getPointerRegClass(const MachineFunction &MF,
                                       unsigned Kind) const {
-  return &AArch64::GPR64RegClass;
+  return &AArch64::GPR64spRegClass;
 }
 
 const TargetRegisterClass *
 AArch64RegisterInfo::getCrossCopyRegClass(const TargetRegisterClass *RC) const {
   if (RC == &AArch64::CCRRegClass)
     return &AArch64::GPR64RegClass; // Only MSR & MRS copy NZCV.
   return RC;
 }

llvm/trunk/test/CodeGen/AArch64/arm64-inline-asm.ll

 }
 
 define void @test_inline_modifier_a(i8* %ptr) nounwind {
   ; CHECK-LABEL: test_inline_modifier_a:
   tail call void asm sideeffect "prfm pldl1keep, ${0:a}\0A", "r"(i8* %ptr)
   ; CHECK: prfm pldl1keep, [x0]
   ret void
 }
+
+; PR33134
+define void @test_zero_address() {
+entry:
+; CHECK-LABEL: test_zero_address
+; CHECK: mov {{x[0-9]+}}, xzr
+; CHECK: ldr {{x[0-9]+}}, {{[x[0-9]+]}}
+  tail call i32 asm sideeffect "ldr $0, $1 \0A", "=r,*Q"(i32* null)
+  ret void
+}