[SystemZ] Use valid base/index registers for memory constraints

[• zhanjunl]

Summary

Inline asm memory constraints can have the base or index register be assigned to %r0 right now.
Make sure that we assign only ADDR64 registers to the base and index.

[uweigand]

Hmm, I guess that makes sense. I'm wondering why most other targets don't have to do anything similar ...

I am a bit concerned about the ISD::Register tests. I assume this is to catch the case where no base and/or no index register are actually used; in those cases selectBDXAddr sets Base and/or Index to CurDAG->getRegister(0, VT). In those cases, leaving this as-is is fine. (Mentioning this in a comment would be good, though.) However, can there be other instances of ISD::Register? Can this e.g. be hard-coded by a user somehow? What if there is a hard-coded register, which is *not* suitable for use in addressing?

[• zhanjunl]

In D23367#518195, @uweigand wrote:

Hmm, I guess that makes sense. I'm wondering why most other targets don't have to do anything similar ...

I am a bit concerned about the ISD::Register tests. I assume this is to catch the case where no base and/or no index register are actually used; in those cases selectBDXAddr sets Base and/or Index to CurDAG->getRegister(0, VT). In those cases, leaving this as-is is fine. (Mentioning this in a comment would be good, though.) However, can there be other instances of ISD::Register? Can this e.g. be hard-coded by a user somehow? What if there is a hard-coded register, which is *not* suitable for use in addressing?

I'm not sure of the other targets, but PowerPC has to do something similar because they have the same restrictions (r0 cannot be used in addresses).

My thinking is that if there's a hard coded register, then we should let it go through, flaws and all. So for the example of a user hard coding the register, we should be doing exactly what the user wants. If the user specifies r0 as the base/index register, then r0 should be what we use for that register, even if it's wrong. LLVM will throw an error when it detects that anyway.

[uweigand]

In D23367#518229, @zhanjunl wrote:

My thinking is that if there's a hard coded register, then we should let it go through, flaws and all. So for the example of a user hard coding the register, we should be doing exactly what the user wants. If the user specifies r0 as the base/index register, then r0 should be what we use for that register, even if it's wrong. LLVM will throw an error when it detects that anyway.

OK, fair enough. Patch LGTM, thanks!

lib/Target/SystemZ/SystemZISelDAGToDAG.cpp

case InlineAsm::Constraint_m:
     // Accept an address with a long displacement and an index.
     // m works the same as T, as this is the most general case.
     Form = SystemZAddressingMode::FormBDXNormal;
     DispRange = SystemZAddressingMode::Disp20Only;
     break;
   }
 
   if (selectBDXAddr(Form, DispRange, Op, Base, Disp, Index)) {
+    const TargetRegisterClass *TRC =
+      Subtarget->getRegisterInfo()->getPointerRegClass(*MF);
+    SDLoc DL(Base);
+    SDValue RC = CurDAG->getTargetConstant(TRC->getID(), DL, MVT::i32);
+
+    // Make sure that the base address doesn't go into %r0.
+    // If it's a TargetFrameIndex or a fixed register, we shouldn't do anything.
+    if (Base.getOpcode() != ISD::TargetFrameIndex &&
+        Base.getOpcode() != ISD::Register) {
+      Base =
+        SDValue(CurDAG->getMachineNode(TargetOpcode::COPY_TO_REGCLASS,
+                                       DL, Base.getValueType(),
+                                       Base, RC), 0);
+    }
+
+    // Make sure that the index register isn't assigned to %r0 either.
+    if (Index.getOpcode() != ISD::Register) {
+      Index =
+        SDValue(CurDAG->getMachineNode(TargetOpcode::COPY_TO_REGCLASS,
+                                       DL, Index.getValueType(),
+                                       Index, RC), 0);
+    }
+
     OutOps.push_back(Base);
     OutOps.push_back(Disp);
     OutOps.push_back(Index);
     return false;
   }
 
   return true;
 }

test/CodeGen/SystemZ/asm-02.ll

; CHECK: br %r14
   %addr = inttoptr i64 %addi to i64 *
   call void asm "blah $0", "=*R" (i64 *%addr)
   ret void
 }
 
 ; Check that LAY is used if there is an index but the displacement is too large
 define void @f7(i64 %base, i64 %index) {
 ; CHECK-LABEL: f7:
-; CHECK: lay %r0, 4096(%r3,%r2)
-; CHECK: blah 0(%r0)
+; CHECK: lay %r1, 4096(%r3,%r2)
+; CHECK: blah 0(%r1)
 ; CHECK: br %r14
   %add = add i64 %base, 4096
   %addi = add i64 %add, %index
   %addr = inttoptr i64 %addi to i64 *
   call void asm "blah $0", "=*R" (i64 *%addr)
   ret void
 }