[PATCH] D47541: Allow creating llvm::Function in non-zero address spaces

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added inline comments.


================
Comment at: lib/AsmParser/LLParser.cpp:1256
+    return Function::Create(FT, GlobalValue::ExternalWeakLinkage,
+                            PTy->getAddressSpace(), Name, M);
   else
----------------
bjope wrote:
> My comments in checkValidVariableType are related to a test case that looked something like this:
> 
> ```
> ; RUN: llc -o /dev/null %s
> 
> define void @main() {
> entry:
>   %0 = tail call i32 @llvm.phx.aload(i16* null)
>   ...
> }
> 
> declare i32 @llvm.phx.aload(i16* nocapture) addrspace(40)
> ```
> 
> And when creating the fwd ref for `@llvm.phx.aload.i32.p21i16` when parsing the call it will be given the default addrspace zero. As far as I can see that will happen here even if we have specified `"Px"` in the datalayout string. So I'm getting errors when parsing the declare since it will find the function from the forward reference having a different address space.
> 
> 
> If I understand correctly this special handling of IsCall in checkValidVariableType would have accepted the missing addrspace notation in the call if it wasn't a fwd ref (e.g. if I had declared `@llvm.phx.aload.i32.p21i16` before `@main`). But with the order above we create the function with addrspace zero here, and then when we get to the declare we get into trouble.
> 
> So even with alterntative (1) and (4) from the discussion in checkValidVariableType I think we would need to use the ProgramAddressSpace here when creating the Function. Or am I missing something in my analysis?
Yes, that's exactly the issue that I ran into after applying the patch to our fork. I've currently added a horrible hack to always allow address space 200 declarations, but I'm trying to find a better solution.

I am tempted to use option 2 since we were previously using an addrspacecast in every function call and that broke stuff like always_inline at -O0. A new syntax for the addressspace could probably avoid that but I'm not sure if that is the best solution.

I wonder if just setting the address space for foward declaration address space to -1 and fixing it up once we have the data layout would work. I'll try this approach and see if it works.


Repository:
  rL LLVM

https://reviews.llvm.org/D47541