rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/TargetInfo.cpp:8816 + // FIXME: Should use byref when promoting pointers in structs, but this + // requires adding implementing the coercion. + if (!getContext().getLangOpts().OpenCL && LTy == OrigLTy && ---------------- arsenm wrote: > rjmccall wrote: > > arsenm wrote: > > > rjmccall wrote: > > > > I don't see why you'd use `byref` when promoting pointers in structs. > > > > Maybe it works as a hack with your backend, but it seems *extremely* > > > > special-case and should not be hacked into the general infrastructure. > > > The whole point is to reinterpret the address space of the pointers in > > > memory since we know if it's a kernel argument it has to be an > > > addrspace(1) pointer or garbage. We can't infer the address space of a > > > generic pointer loaded from memory. > > > > > > byref doesn't change that, it just makes the fact that these are passed > > > in memory explicit > > `byref` is interpreted by your backend passes as an instruction that the > > argument value is actually the address of an object that's passed to the > > kernel by value, so you need to expand the memory in the kernel argument > > marshalling. Why would that be something you'd want to trigger when > > passing a struct with a pointer in it? You're not going to recursively > > copy and pass down the pointee values of those pointers. > Because all arguments are really passed byref, we're just not at the point > yet where we can switch all IR arguments to use byref for all arguments. All > of the relevant properties are really always on the in-memory value. > > The promotion this is talking about is really orthogonal to the IR mechanism > used for passing kernel arguments. This promotion is because the language > only exposes generic pointers. In the context of a pointer inside a struct > passed as a kernel argument, we semantically know the address space of any > valid pointers must be global. You could not produce a valid generic pointer > from another address space here. The pointers/structs are still the same size > and layout, but coercing the in-memory address space is semantically more > useful to the optimizer I understand that the promotion is orthogonal to the IR mechanism used for passing kernel arguments, which is exactly why I'm asking why there's a comment saying that we should "use byref when promoting pointers in struct", because I have no idea what that's supposed to mean when the pointer is just a part of the value being passed. It sounds like what you want is to maybe customize the code that's emitted to copy a byref parameter into a parameter variable when the parameter type is a struct containing a pointer you want to promote. But that doesn't really have anything to do with `byref`; if you weren't using `byref`, you'd still want a similar customization when creating the parameter variable. So it seems to me that the comment is still off-target. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79744/new/ https://reviews.llvm.org/D79744 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
