[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:933
+    // Check for incompatible attributes.
+    if (Arg.hasByRefAttr() || Arg.hasNestAttr())
+      break;
----------------
Could actually handle byref (and should, ideally all kernargs would use byref). If we consistently used byref, you could implement this by moving memory values to the arg list


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:942
+                      << "\" will attempt to preload " << KernargPreloadCount
+                      << " kernel arguemnts.\n");
+    LLVMContext &Ctx = F.getParent()->getContext();
----------------
Typo arguemnts


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:945
+    MDBuilder MDB(Ctx);
+    F.setMetadata("preload_kernel_args",
+                  MDNode::get(Ctx, MDB.createConstant(llvm::ConstantInt::get(
----------------
I don't understand why you need this metadata. You mark the arguments with inreg which provides the same information? You can still ignore the hint in the codegen


================
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
----------------
kerbowa wrote:
> arsenm wrote:
> > Use -passes
> I think it won't work for amdgpu-attributor with old pass manager?
Apparently it hasn't been ported yet


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156852