[PATCH] D105839: [AMDGPU] Improve register computation for indirect calls

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:10
+/// \file
+/// \brief Analyzes the how many registers and other resources are used by
+/// functions. Takes callees into account.
----------------
Incorrect "the".


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:113-115
+      NonKernelMaxSGPRs = std::max(NonKernelMaxSGPRs, Info.NumExplicitSGPR);
+      NonKernelMaxVGPRs = std::max(NonKernelMaxVGPRs, Info.NumVGPR);
+      NonKernelMaxAGPRs = std::max(NonKernelMaxAGPRs, Info.NumAGPR);
----------------
These can be computed only if there is indirect call. You can wrap it in a function and call to avoid clutter.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:464
+          // Register usage of indirect calls gets handled later
+
+          Info.UsesVCC = true;
----------------
No need of newline.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:472
+          // usage etc. should be the cumulative usage of all callees.
+
+          MaxSGPR = std::max(I->second.NumExplicitSGPR - 1, MaxSGPR);
----------------
No need of newline.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h:10
+/// \file
+/// \brief Analyzes the how many registers and other resources are used by
+/// functions. Takes callees into account.
----------------
Please be more verbose.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h:15
+
+#ifndef LLVM_LIB_TARGET_AMDGPU_MDGPUPERFHINTANALYSIS_H
+#define LLVM_LIB_TARGET_AMDGPU_MDGPUPERFHINTANALYSIS_H
----------------
Incorrect inclusion header.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h:59
+
+  const SIFunctionResourceInfo &getResourceInfo(const Function *F) {
+    return CallGraphResourceInfo[F];
----------------
You make this as 'const' member function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105839