[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

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

10

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

include/llvm/CodeGen/SelectionDAGNodes.h

switch (NodeType) {
         return true;
     }
   }
 
   /// Test if this node has a post-isel opcode, directly
   /// corresponding to a MachineInstr opcode.
   bool isMachineOpcode() const { return NodeType < 0; }
 
+  /// Test if this node is an inline ASM.
+  bool isInlineAsm() const { return NodeType == ISD::INLINEASM; }
+
   /// This may only be called if isMachineOpcode returns
   /// true. It returns the MachineInstr opcode value that the node's opcode
   /// corresponds to.
   unsigned getMachineOpcode() const {
     assert(isMachineOpcode() && "Not a MachineInstr opcode!");
     return ~NodeType;
   }
 

lib/CodeGen/SelectionDAG/InstrEmitter.cpp

for (SDNode *User : Node->uses()) {
         for (unsigned i = 0, e = User->getNumOperands(); i != e; ++i) {
           SDValue Op = User->getOperand(i);
           if (Op.getNode() != Node || Op.getResNo() != ResNo)
             continue;
           MVT VT = Node->getSimpleValueType(Op.getResNo());
           if (VT == MVT::Other || VT == MVT::Glue)
             continue;
           Match = false;
+          {
+            const TargetRegisterClass *RC = nullptr;
+
             if (User->isMachineOpcode()) {
               const MCInstrDesc &II = TII->get(User->getMachineOpcode());
-            const TargetRegisterClass *RC = nullptr;
-            if (i+II.getNumDefs() < II.getNumOperands()) {
+              if (i + II.getNumDefs() < II.getNumOperands()) {
                 RC = TRI->getAllocatableClass(
-                TII->getRegClass(II, i+II.getNumDefs(), TRI, *MF));
+                    TII->getRegClass(II, i + II.getNumDefs(), TRI, *MF));
               }
+            }
+
+            // Constrain register class to target pointer class if it's a memory
+            // register in inline asm.
+            if (User->isInlineAsm()) {
+              unsigned FlagIdx;
+              unsigned Flags;
+              unsigned NumOps;
+              // Look up the flags for the register. See InlineAsm.h.
+              for (FlagIdx = InlineAsm::Op_FirstOperand; FlagIdx < e;
+                   FlagIdx += NumOps) {
+                Flags = User->getConstantOperandVal(FlagIdx);
+                NumOps = 1 + InlineAsm::getNumOperandRegisters(Flags);
+                if (FlagIdx + NumOps > i)
+                  break;
+              }
+              Flags = User->getConstantOperandVal(FlagIdx);
+              if (InlineAsm::isMemKind(Flags)) {
+                RC = TRI->getPointerRegClass(*MF);
+              }
+            }
             if (!UseRC)
               UseRC = RC;
             else if (RC) {
               const TargetRegisterClass *ComRC =
                 TRI->getCommonSubClass(UseRC, RC, VT.SimpleTy);
               // If multiple uses expect disjoint register classes, we emit
               // copies in AddRegisterOperand.
               if (ComRC)

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;
 }

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

+; RUN: llc < %s -mtriple=aarch64-eabi | FileCheck %s
+
+define void @test() {
+entry:
+; 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
+}