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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to