[PATCH] D124843: AMDGPU: Add G_AMDGPU_MAD_64_32 instructions

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3168
+  case AMDGPU::G_AMDGPU_MAD_I64_I32:
+    return selectG_AMDGPU_MAD_64_32(I);
   case TargetOpcode::G_INTTOPTR:
----------------
nhaehnle wrote:
> arsenm wrote:
> > Don't the selector patterns work? This one should work without much fuss
> Which selector patterns do you mean? SelectionDAG has custom code for this as well because of the vcc_out (see AMDGPUDAGToDAGISel::SelectMAD_64_32).
Oh right, I forgot about the VCC output as usual (the original CI manual didn't mention it and I still haven't caught up)


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1641
+
+  bool IsUnsigned = MI.getOpcode() == AMDGPU::G_AMDGPU_MAD_U64_U32;
+  LLT S1 = LLT::scalar(1);
----------------
Most places invert this and use IsSigned (saves a few characters I guess)


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1649-1651
+    if (MachineInstr *Def = getDefIgnoringCopies(Src2, MRI)) {
+      if (Def->getOpcode() == TargetOpcode::G_CONSTANT &&
+          Def->getOperand(1).getCImm()->isZeroValue())
----------------
Can you use m_ZeroInt?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1680-1692
+    if (!DstOnValu) {
+      const TargetRegisterClass *Constrained =
+          constrainGenericRegister(DstHi, AMDGPU::VGPR_32RegClass, MRI);
+      (void)Constrained;
+      assert(Constrained && "Failed to constrain readfirstlane src reg");
+
+      Register SMulHi = MRI.createVirtualRegister(&AMDGPU::SReg_32RegClass);
----------------
Can you use constrainOpWithReadfirstlane?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1773
+      Carry = B.buildConstant(CarryType, 0).getReg(0);
+      MRI.setRegBank(Carry, CarryBank);
+    }
----------------
You could use ApplyRegBankMapping to avoid all of these setRegBank calls


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124843/new/

https://reviews.llvm.org/D124843