[PATCH] D150788: [AMDGPU][GlobalISel] Rematerialze constants with different regbank

[Mirko Brkusanin via Phabricator via llvm-commits]

mbrkusanin added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp:250-261
+  const unsigned OpIdx = RepairPt.getOpIdx();
+  OpdMapper.createVRegs(OpIdx);
+
+  // Clone the instruction but update the regbank.
+  MachineInstr *NewMI = OldMI->getMF()->CloneMachineInstr(OldMI);
+  NewMI->getOperand(0).setReg(*OpdMapper.getVRegs(OpIdx).begin());
+  LLVM_DEBUG(dbgs() << "Clone with new regbank: " << *OldMI << '\n');
----------------
Regbank's MIRBuilder does not have CSEInfo so this will crate new G_(F)CONSTANT for each use. I tried an idea to avoid that where InsertPoint for rematerializations would be changed to always be after old G_(F)CONSTANT, so we would only need to check instruction after the current/old one if it is a copy with the appropriate regbank (created earlier). This did make .mir test look shorter in some cases but it did not have any real improvements for final ISA, only shuffled instructions.


================
Comment at: llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp:527
+         DefMI->getOpcode() == TargetOpcode::G_FCONSTANT) &&
+        !MO.isDef() && MRI.getType(OpReg).getSizeInBits() <= 32) {
+      // Constants can be rematerialzed.
----------------
Maybe this should be a target hook? Lit tests from other targets are not affected by this change, but check for size<32 is because of lack of v_mov_b64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150788