[PATCH] D91428: Add support for multiple program address spaces

pmatos abandoned this revision. pmatos added a comment. In D91428#2413292 , @pmatos wrote: > Thanks, @arichardson and @jrtc27 for your comments. > I am definitely surprised to find that if you explicitly mark the call with > the address space, this

[PATCH] D91428: Add support for multiple program address spaces

pmatos added a comment. Thanks, @arichardson and @jrtc27 for your comments. I am definitely surprised to find that if you explicitly mark the call with the address space, this patch is not required. At first look, this RFC is not required any more but I need sometime to investigate further. If

[PATCH] D91428: Add support for multiple program address spaces

jrtc27 added a comment. Currently `P0-P1` is valid and results in the program address space being 1, but this patch changes the semantics of that. How sure are you that nothing will break? I do not like the idea of reusing existing valid syntax to mean something else; if you want to introduce

[PATCH] D91428: Add support for multiple program address spaces

arichardson added a comment. The change to rename getProgramAddressSpace getDefaultProgramAddressSpace seems fine to me since it matches the GlobalsAddressSpace. By the way, your test already seems to work (if you add an explicit `call addrspace(1) void %ref()` for llc:

[PATCH] D91428: Add support for multiple program address spaces

arichardson requested changes to this revision. arichardson added a comment. I don't see why you would need multiple program address spaces to support calls to other address spaces. You can already do the following: define i32 @foo(i32) addrspace(1) { %ret = add i32 %0, 1 ret i32

[PATCH] D91428: Add support for multiple program address spaces

pmatos added a comment. The RFC has now been sent to the mailing list: http://lists.llvm.org/pipermail/llvm-dev/2020-November/146723.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91428/new/ https://reviews.llvm.org/D91428

[PATCH] D91428: Add support for multiple program address spaces

tlively added a comment. @arsenm, Are you suggesting that we just relax the verification rules to allow calling function pointers to arbitrary address spaces without needing any changes to the data layout string? That seems fine to me, but the data layout string solution does allow targets to

[PATCH] D91428: Add support for multiple program address spaces

arsenm added a comment. While the IR definitely should support mixing functions with different address spaces, I don't see the point of encoding multiple of these in the datalayout Comment at: llvm/include/llvm/IR/DataLayout.h:125-129 + /// Vector of address spaces that can

[PATCH] D91428: Add support for multiple program address spaces

jdoerfert added a subscriber: tianshilei1992. jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. I'll be on the lookout for the RFC. There, and in an updated commit message, you have to provide more details.

[PATCH] D91428: Add support for multiple program address spaces

pmatos updated this revision to Diff 305521. pmatos added a comment. Fix type check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91428/new/ https://reviews.llvm.org/D91428 Files: clang/lib/CodeGen/CGException.cpp

[PATCH] D91428: Add support for multiple program address spaces

pmatos updated this revision to Diff 305488. pmatos added a comment. Ensure the program address spaces vector doesn't contain duplicates. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91428/new/ https://reviews.llvm.org/D91428 Files:

[PATCH] D91428: Add support for multiple program address spaces

tlively added a comment. I think this is a good direction overall, and I'm glad the code doesn't become any messier with this change. I do think it would be good to also email llvm-dev about this change to get general feedback and make sure it doesn't require a full RFC.

[PATCH] D91428: Add support for multiple program address spaces

pmatos added a comment. This is WIP - there a still a few test failures but I am happy to start getting comments on this. This is in preparation for implementation of reference types for the WebAssembly backend that requires functions to be able to live in multiple address spaces.

[PATCH] D91428: Add support for multiple program address spaces

pmatos created this revision. pmatos added a reviewer: tlively. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jdoerfert, hiraditya, dschuff. Herald added projects: clang, LLVM. pmatos requested review of this revision. Herald added a subscriber: aheejin. Allows for multiple