[PATCH] D156852: [AMDGPU] Use inreg for hint to preload kernel arguments

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:922
 
+static void addPreloadKernArgHint(Function &F) {
+  for (unsigned I = 0;
----------------
This isn't really gaining value by being in the attributor as it stands. Are you planning on a more sophisticated user analysis to select the starting point?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:924
+  for (unsigned I = 0;
+       I < F.arg_size() && I < std::min(KernargPreloadCount.getValue(), 16u);
+       ++I) {
----------------
Assuming 1 register : 1 argument which is not the case.

Also the user SGPR count went up (in gfx10 I think?) so you should query the number for that


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:981
           A.getOrCreateAAFor<AAAMDWavesPerEU>(IRPosition::function(F));
+        } else if (CC == CallingConv::AMDGPU_KERNEL && !F.arg_empty()) {
+          addPreloadKernArgHint(F);
----------------
Don't need the arg_empty check, the implementation handles it anyway and it's uncommon


================
Comment at: llvm/test/CodeGen/AMDGPU/preload-kernal-args-inreg-hints.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a -amdgpu-attributor -S < %s | FileCheck -check-prefix=NO-PRELOAD %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a -amdgpu-kernarg-preload-count=1 -amdgpu-attributor -S < %s | FileCheck -check-prefix=PRELOAD-1 %s
----------------
Use -passes


================
Comment at: llvm/test/CodeGen/AMDGPU/preload-kernal-args-inreg-hints.ll:235
+}
+
+declare void @func(ptr) #0
----------------
Needs some exotic types. i8, <2 x half>, <3 x half> odd bit sized scalars, odd bit sized element vectors, odd vectors, structs, arrays and nested structs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156852