[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9381
+      M.getTarget().getTargetOpts().CodeObjectVersion != 500) {
+    F->addFnAttr("amdgpu-no-hostcall-ptr");
+  }
----------------
scott.linder wrote:
> arsenm wrote:
> > sameerds wrote:
> > > The frontend does not need to worry about this attribute. See the comment in the MetadataStreamer. A worthwhile check would be to generate an error if we are able to detect that some hostcall service is being used in OpenCL on code-object-v4 or lower. None exists right now, but we should add the check if such services show up. But those checks are likely to be in a different place. For example, enabling asan on OpenCL for code-object-v4 should result in an error in the place where asan commandline options are parsed.
> > Should be all opencl, not just kernels. Also < instead of !=?
> > The frontend does not need to worry about this attribute. See the comment in the MetadataStreamer. 
> 
> The reason I went this route is that otherwise the MetadataStreamer can only go off of the presence of the OCL printf metadata to infer that hostcall argument metadata is invalid and will be ignored by the runtime. Ideally, the MetadataStreamer or Verifier or something would actually //require// that a module does not have both OCL printf metadata and functions which are not "amdgpu-no-hostcall-ptr", but I didn't go that far as it would break old IR/bitcode that relies on the implicit behavior.
> 
> I'm OK with removing this code, and the rest of the patch remains essentially unchanged. We will still have an implicit rule based on code-object-version and presence of printf metadata, and at least conceptually you will still be able to compile OCL for pre-V5 and end up with hostcall argument metadata. That case is benign if the runtime just ignores it, but it still seems needlessly relaxed when we can just add the correct attribute in Clang codegen.
> 
> > Should be all opencl, not just kernels. Also < instead of !=?
> 
> Yes, my mistake, I'll update this
+1 for using < instead of !=

However,  device library is written in OpenCL language. There are functions using hostcall buffer. We cannot mark all OpenCL functions with this attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951