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

[Corbin Robeck via Phabricator via llvm-commits]

crobeck marked an inline comment as done.
crobeck added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:2606-2609
+  Register LongBranchReservedReg = MFI->getLongBranchReservedReg();
+  if (!Scav && LongBranchReservedReg != AMDGPU::NoRegister) {
+    Scav = LongBranchReservedReg;
+  }
----------------
crobeck wrote:
> Pierre-vh wrote:
> > If I understand correctly, `LongBranchReservedReg` is always reserved so could we just avoid running the scavenger when it's set and just use it directly? Otherwise it's just a "wasted" reserved register
> > 
> > + nit: no `{}` for a one-line if body
> > + very small nit: you could also just use `Register()` as the value for LongBranchReservedReg I think, it avoids the need to check against NoRegister. IMO it's more intuitive as you can just do `if(Register LongBranchReservedReg = MFI->getLongBranchReservedReg())` to check for its presence
> LongBranchReservedReg is only set if the PreRABranchDistance pass sets it because it finds a long branch. Otherwise it is just set to null.
> 
> Formatting of LongBranchReservedReg != AMDGPU::NoRegister seemed consistent with the surrounding code but no strong feelings there. Open to changing.
Updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149775