[PATCH] D49096: AMDGPU: Make hidden argument metadata consistent with amdgpu-implicitarg-num-bytes attribute

[Tony Tye via Phabricator via llvm-commits]

t-tye added inline comments.


================
Comment at: lib/Target/AMDGPU/MCTargetDesc/AMDGPUHSAMetadataStreamer.cpp:258
 
-  // TODO: What about other languages?
-  if (!Func.getParent()->getNamedMetadata("opencl.ocl.version"))
-    return;
-
-  auto &DL = Func.getParent()->getDataLayout();
-  auto Int64Ty = Type::getInt64Ty(Func.getContext());
-
-  emitKernelArg(DL, Int64Ty, ValueKind::HiddenGlobalOffsetX);
-  emitKernelArg(DL, Int64Ty, ValueKind::HiddenGlobalOffsetY);
-  emitKernelArg(DL, Int64Ty, ValueKind::HiddenGlobalOffsetZ);
-
-  auto Int8PtrTy = Type::getInt8PtrTy(Func.getContext(),
-                                      AMDGPUASI.GLOBAL_ADDRESS);
-
-  // Emit "printf buffer" argument if printf is used, otherwise emit dummy
-  // "none" argument.
-  if (Func.getParent()->getNamedMetadata("llvm.printf.fmts"))
-    emitKernelArg(DL, Int8PtrTy, ValueKind::HiddenPrintfBuffer);
-  else
-    emitKernelArg(DL, Int8PtrTy, ValueKind::HiddenNone);
-
-  // Emit "default queue" and "completion action" arguments if enqueue kernel is
-  // used, otherwise emit dummy "none" arguments.
-  if (Func.hasFnAttribute("calls-enqueue-kernel")) {
-    emitKernelArg(DL, Int8PtrTy, ValueKind::HiddenDefaultQueue);
-    emitKernelArg(DL, Int8PtrTy, ValueKind::HiddenCompletionAction);
-  } else {
-    emitKernelArg(DL, Int8PtrTy, ValueKind::HiddenNone);
-    emitKernelArg(DL, Int8PtrTy, ValueKind::HiddenNone);
-  }
+  emitHiddenKernelArgs(Func);
 }
----------------
arsenm wrote:
> t-tye wrote:
> > Just checking if this is correct for Mesa too?
> Clover doesn't use this attribute. I'm not sure what the current state is, but it follow some frankenstein ABI between HSA and what it was doing before. Since it isn't really maintained I don't think we need to worry too much about it
I thought Mesa was using this attribute and setting it to 16. But @kzhuravl  points out that this code is only called for the AMDHSA os and so it is not an issue.


Repository:
  rL LLVM

https://reviews.llvm.org/D49096