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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 16 15:55:41 PDT 2023


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRABranchDistance.cpp:36
+class GCNPreRABranchDistance : public MachineFunctionPass {
+  /// BasicBlockInfo - Information about the offset and size of a single
+  /// basic block.
----------------
crobeck wrote:
> arsenm wrote:
> > This is all copied from BranchRelaxation?
> > 
> > I think you're trying to be far too precise for tracking this. This is going to be far too fuzzy given you have no idea what spills are going to be inserted. I think expected ~10 bytes per instruction without bothering to compute actual instruction sizes is enough and see if that's in the neighborhood of the maximum branch distance 
> The distance calculation is mostly from BR. I'm not sure the actual method of determining what maximum branch distance use as the threshold should matter too much since it's just an empirical tuning factor. We lean toward always reserving the register by setting the default threshold somewhat high and can tune with the CL argument for individual cases where we don't want to reserve them. However you're right this distance calculation is probably overkill and could simpler.
I think you should go for fuzzier. Drop the block map and all that from BranchRelaxation. Just do something dead simple like number of instructions * 8 * fuzz ~= branch distance


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149775



More information about the llvm-commits mailing list