[PATCH] D149775: [AMDGPU] Reserve SGPR pair when long branches are present

[Corbin Robeck via Phabricator via llvm-commits]

crobeck marked 3 inline comments as done.
crobeck added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRALongBranchReg.cpp:116-117
+  constexpr bool ReserveHighestRegister = true;
+  Register LongBranchReservedReg = TRI->findUnusedRegister(
+      MRI, &AMDGPU::SGPR_64RegClass, MF, ReserveHighestRegister);
+  if (!LongBranchReservedReg) {
----------------
arsenm wrote:
> I don't see how this can fail unless you're using amdgpu-num-sgprs (and this is one of the reasons it should be removed)
It failed in some test cases where all the SGPRs were already used from inline asm


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRALongBranchReg.cpp:119
+  if (!LongBranchReservedReg) {
+    MFI->setLongBranchReservedReg(AMDGPU::NoRegister);
+    return false;
----------------
arsenm wrote:
> I would assume this is already noregister if this failed
> 
> I think you misunderstood what I was saying about reserve first and release later. I meant in the compile pipeline, not in the pass itself. You can find the reserved register in finalizeLowering, and then have this pass unset and un-reserve. I don't know if we have an exposed helper to clear reserved registers. You can do that as a follow up,
Yes, you're right I can remove that set function.

Got it. OK, that makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149775