[PATCH] D37054: Require address space to be specified when creating functions (2/3)

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: docs/LangRef.rst:772
 
+If an explicit address space is not given, it will default to zero.
+
----------------
arichardson wrote:
> Just wondering if it would be better to put functions without an explicit address space in the program address space?
> I think this may be more consistent since creating a new `FunctionType` without an address space will put it in the program address space and not address space zero. 
> I am not sure if there are any backwards compatibility constraints here since IR generated by an older version will never have the program address space in the datalayout string so they should also end up in address space zero.
> 
> Putting everything in the program address space should make it easier for us to keep the existing IR testcases once we expect all functions to be in address space 200.
That sounds interesting. I'll probably try to do that for our out-of-tree target. Because we currently have two different frontends (one based on clang). With this commit the clang frontend will start to add explicit address spaces on functions, but it will take some time for me to adapt the other frontend to do the same in all situations.

The result would be quite similar to what we have out-of-tree today. We have patched both frontends to set a non-zero program address space in the DataLayout. But we do not set the address space explicitly on function declarations.

Without having the AS explicit when parsing the LL file we need to parse the DataLayout string first, to know about the targets default program address space when parsing the functions declarations. I think that is one problem  those explicit address space annotations was supposed to help out with. So I'm not sure if it is feasible for the upstream LLVM trunk. However, normally the DataLayout is in the beginning of the .ll-file (no idea about .bc).


Repository:
  rL LLVM

https://reviews.llvm.org/D37054