[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia requested changes to this revision.
Anastasia added a comment.
This revision now requires changes to proceed.

I feel that to progress further on this change, it would be good to get details about the use cases and the limitations first.

However, if there is sufficient evidence of the need to extend clang builtins with language address spaces I am not convinced the approach here is suitable as:

1. It doesn't allow uses of language builtins with language address spaces generically as mapping to the target address spaces is not portable. In general there can also be generic buitins that are normally mapped to native LLVM intrinsics (not target intrinsics!) shared among multiple targets that might also benefit from having this implemented in a target agnostic way. Such intrinsics could be shares for example between PTX and SPIR-V targets as there is quite some overlap in the functionality.
2. It has much wider impact on the language semantics then just allowing language address spaces being used in builtins i.e. it results in implicit conversions more broadly. This might not be desirable evolution and we might need to reach some consensus with more languages or targets using the address spaces in order to proceed with such change. In fact current title and description doesn't adequately reflect the impact of the change.

Has extending the builtin definition syntax been considered for this problem? That seems like a more natural and fairly localized change. For example the syntax in https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Builtins.def#L65 could be changed to:

  // * -> pointer (optionally followed by an address space number for target address space
  //               or by 00 and a number for language address space as it is set in LangAS, if no
  //               address space is specified than any address space will be accepted)

Note that we will likely need to set LangAS entry values explicitly which would make maintaing the enum slightly more painful but it doesn't seem like a concern.

If the only use case we have right now is ability to specify generic address space in kernel-like langauges we could also just reserve a special `00` number in address space field of the buitin prototype description  for `generic address space` and leave full support as a future work.

If we understand the use case better we might be able to look at other alternatives too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382