GlobalISel: Lower G_WRITE_REGISTER

[arsenm]

ping

[aemerson]

llvm/test/CodeGen/AMDGPU/GlobalISel/write_register.ll

3

I don't know how I feel about this. Perhaps just add an extra run line to the existing test?

Since you're removing something from arm64 fallback, could you add a small test for arm64 as well?

[arsenm]

llvm/test/CodeGen/AMDGPU/GlobalISel/write_register.ll

3

This is how all of the other tests I've been sharing do it. I'm not sure I want to start leaking global isel run lines into the general test population

[arsenm]

Restore aarch64 test. Since this was moved to opt-in lowering, this does fail, just in a different place

[arsenm]

c5c1bb33747a556a4d3cd12eeaba9146e7c6068f

llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h

public:
   LegalizeResult lowerUnmergeValues(MachineInstr &MI);
   LegalizeResult lowerShuffleVector(MachineInstr &MI);
   LegalizeResult lowerDynStackAlloc(MachineInstr &MI);
   LegalizeResult lowerExtract(MachineInstr &MI);
   LegalizeResult lowerInsert(MachineInstr &MI);
   LegalizeResult lowerSADDO_SSUBO(MachineInstr &MI);
   LegalizeResult lowerBswap(MachineInstr &MI);
   LegalizeResult lowerBitreverse(MachineInstr &MI);
-  LegalizeResult lowerReadRegister(MachineInstr &MI);
+  LegalizeResult lowerReadWriteRegister(MachineInstr &MI);
 
 private:
   MachineRegisterInfo &MRI;
   const LegalizerInfo &LI;
   /// To keep track of changes made by the LegalizerHelper.
   GISelChangeObserver &Observer;
 };
 

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

case Intrinsic::sideeffect:
     return true;
   case Intrinsic::read_register: {
     Value *Arg = CI.getArgOperand(0);
     MIRBuilder
         .buildInstr(TargetOpcode::G_READ_REGISTER, {getOrCreateVReg(CI)}, {})
         .addMetadata(cast<MDNode>(cast<MetadataAsValue>(Arg)->getMetadata()));
     return true;
   }
+  case Intrinsic::write_register: {
+    Value *Arg = CI.getArgOperand(0);
+    MIRBuilder.buildInstr(TargetOpcode::G_WRITE_REGISTER)
+      .addMetadata(cast<MDNode>(cast<MetadataAsValue>(Arg)->getMetadata()))
+      .addUse(getOrCreateVReg(*CI.getArgOperand(1)));
+    return true;
+  }
   }
   return false;
 }
 
 bool IRTranslator::translateInlineAsm(const CallInst &CI,
                                       MachineIRBuilder &MIRBuilder) {
   const InlineAsm &IA = cast<InlineAsm>(*CI.getCalledValue());
   if (!IA.getConstraintString().empty())

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

case G_EXTRACT:
     return lowerExtract(MI);
   case G_INSERT:
     return lowerInsert(MI);
   case G_BSWAP:
     return lowerBswap(MI);
   case G_BITREVERSE:
     return lowerBitreverse(MI);
   case G_READ_REGISTER:
-    return lowerReadRegister(MI);
+  case G_WRITE_REGISTER:
+    return lowerReadWriteRegister(MI);
   }
 }
 
 LegalizerHelper::LegalizeResult LegalizerHelper::fewerElementsVectorImplicitDef(
     MachineInstr &MI, unsigned TypeIdx, LLT NarrowTy) {
   SmallVector<Register, 2> DstRegs;
 
   unsigned NarrowSize = NarrowTy.getSizeInBits();
LegalizerHelper::lowerBitreverse(MachineInstr &MI) {
   // -> [(val & 0xAAAAAAAA) >> 1] & [(val << 1) & 0xAAAAAAAA]
   SwapN(1, Dst, MIRBuilder, Swap2, APInt::getSplat(Size, APInt(8, 0xAA)));
 
   MI.eraseFromParent();
   return Legalized;
 }
 
 LegalizerHelper::LegalizeResult
-LegalizerHelper::lowerReadRegister(MachineInstr &MI) {
-  Register Dst = MI.getOperand(0).getReg();
-  const LLT Ty = MRI.getType(Dst);
-  const MDString *RegStr = cast<MDString>(
-    cast<MDNode>(MI.getOperand(1).getMetadata())->getOperand(0));
-
+LegalizerHelper::lowerReadWriteRegister(MachineInstr &MI) {
   MachineFunction &MF = MIRBuilder.getMF();
   const TargetSubtargetInfo &STI = MF.getSubtarget();
   const TargetLowering *TLI = STI.getTargetLowering();
-  Register Reg = TLI->getRegisterByName(RegStr->getString().data(), Ty, MF);
-  if (!Reg.isValid())
+
+  bool IsRead = MI.getOpcode() == TargetOpcode::G_READ_REGISTER;
+  int NameOpIdx = IsRead ? 1 : 0;
+  int ValRegIndex = IsRead ? 0 : 1;
+
+  Register ValReg = MI.getOperand(ValRegIndex).getReg();
+  const LLT Ty = MRI.getType(ValReg);
+  const MDString *RegStr = cast<MDString>(
+    cast<MDNode>(MI.getOperand(NameOpIdx).getMetadata())->getOperand(0));
+
+  Register PhysReg = TLI->getRegisterByName(RegStr->getString().data(), Ty, MF);
+  if (!PhysReg.isValid())
     return UnableToLegalize;
 
-  MIRBuilder.buildCopy(Dst, Reg);
+  if (IsRead)
+    MIRBuilder.buildCopy(ValReg, PhysReg);
+  else
+    MIRBuilder.buildCopy(PhysReg, ValReg);
+
   MI.eraseFromParent();
   return Legalized;
 }

llvm/test/CodeGen/AArch64/GlobalISel/arm64-fallback.ll

define i64 @atomic_ops(i64* %addr) {
   store atomic i64 0, i64* %addr unordered, align 8
   %res = load atomic i64, i64* %addr seq_cst, align 8
   ret i64 %res
 }
 
 ; Make sure we don't mess up metadata arguments.
 declare void @llvm.write_register.i64(metadata, i64)
 
-; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to translate instruction: call: ' call void @llvm.write_register.i64(metadata !0, i64 0)' (in function: test_write_register_intrin)
+; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to legalize instruction: G_WRITE_REGISTER !0, %0:_(s64) (in function: test_write_register_intrin)
 ; FALLBACK-WITH-REPORT-ERR: warning: Instruction selection used fallback path for test_write_register_intrin
 ; FALLBACK-WITH-REPORT-LABEL: test_write_register_intrin:
 define void @test_write_register_intrin() {
   call void @llvm.write_register.i64(metadata !{!"sp"}, i64 0)
   ret void
 }
 
 @_ZTIi = external global i8*

llvm/test/CodeGen/AMDGPU/GlobalISel/write_register.ll

+; Runs original SDAG test with -global-isel
+; RUN: llc -global-isel -march=amdgcn -mcpu=bonaire -enable-misched=0 -verify-machineinstrs < %S/../write_register.ll | FileCheck -enable-var-scope %S/../write_register.ll