[PATCH] D153092: [Clang][CodeGen]`vtable`, `typeinfo` et al. are globals

Fri Jun 30 08:27:18 PDT 2023

yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

In D153092#4452251 <https://reviews.llvm.org/D153092#4452251>, @AlexVlx wrote:

> In D153092#4452070 <https://reviews.llvm.org/D153092#4452070>, @yaxunl wrote:
>
>> This could be a good chance to switch VT to constant address space instead of global address space. AFAIK if a target has global addr space they usually also has constant addr space since they usually support OpenCL or CUDA/HIP. Is there any reason we cannot introduce a CGM.ConstantGlobalsInt8PtrTy and use it for VT instead?
>
> I did give this some thought and the benefits are somewhat unclear to the point of being ultimately counterproductive. Note that these are already marked `constant`, which IIRC is / was going to be enough to get most of the benefits, at least on our back-end. Furthermore, the semantics of the constant address space are a bit weird in something like OpenCL e.g. `A pointer that points to the constant address space cannot be cast or implicitly converted to the generic address space.`. This would lead to weirdness when composing with CUDA / HIP, where `constant` is treated as `device`, which is to say global. IIRC, you are also meant to use magical interfaces to write into `constant` from the host, which a loader wouldn't necessarily do. Overall, I think that the OCL formulation of `constant` is actually meant to allow for relatively strange things like loading things into ROM or having different pointer types (be it width or canonicity). TL;DR, I am concerned that a target could validly have the `constant` addr space be disjoint from generic/flat, with no viable way to even cast between the two. We could say “yes, but this is not that `constant`”, but then if OCL ever starts supporting dynamic polymorphism it would get confusing.

I agree that constant address space may be target dependent about what can be put there. I also agree there is

In D153092#4452251 <https://reviews.llvm.org/D153092#4452251>, @AlexVlx wrote:

> In D153092#4452070 <https://reviews.llvm.org/D153092#4452070>, @yaxunl wrote:
>
>> This could be a good chance to switch VT to constant address space instead of global address space. AFAIK if a target has global addr space they usually also has constant addr space since they usually support OpenCL or CUDA/HIP. Is there any reason we cannot introduce a CGM.ConstantGlobalsInt8PtrTy and use it for VT instead?
>
> I did give this some thought and the benefits are somewhat unclear to the point of being ultimately counterproductive. Note that these are already marked `constant`, which IIRC is / was going to be enough to get most of the benefits, at least on our back-end. Furthermore, the semantics of the constant address space are a bit weird in something like OpenCL e.g. `A pointer that points to the constant address space cannot be cast or implicitly converted to the generic address space.`. This would lead to weirdness when composing with CUDA / HIP, where `constant` is treated as `device`, which is to say global. IIRC, you are also meant to use magical interfaces to write into `constant` from the host, which a loader wouldn't necessarily do. Overall, I think that the OCL formulation of `constant` is actually meant to allow for relatively strange things like loading things into ROM or having different pointer types (be it width or canonicity). TL;DR, I am concerned that a target could validly have the `constant` addr space be disjoint from generic/flat, with no viable way to even cast between the two. We could say “yes, but this is not that `constant`”, but then if OCL ever starts supporting dynamic polymorphism it would get confusing.

Sounds reasonable.

LGTM. Thanks.

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

https://reviews.llvm.org/D153092