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

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added a comment.

Looks good once the IR parser accepts calls to non-AS0 pointers.



================
Comment at: lib/AsmParser/LLParser.cpp:1238
+    }
+
     if (Val->getType() == Ty) return Val;
----------------
I believe loading a global in a different address space should still be an error, only calling is fine.


================
Comment at: lib/AsmParser/LLParser.cpp:2682
   // If we have the value in the symbol table or fwd-ref table, return it.
   if (Val) {
     if (Val->getType() == Ty) return Val;
----------------
bjope wrote:
> I think we need the same patch as in GetGlobalVal here (line 1228).
> I have a test case with function pointers that did not work otherwise. Here is an extract of the IR:
> 
> ```
> define void @f2(i16 %n, void (i16) addrspace(40)* %f) addrspace(40) #0 !dbg !16 {
> entry:
>   %n.addr = alloca i16, align 1
>   %f.addr = alloca void (i16) addrspace(40)*, align 1
>   %gap = alloca i16, align 1
>   store i16 %n, i16* %n.addr, align 1, !tbaa !9
>   store void (i16) addrspace(40)* %f, void (i16) addrspace(40)** %f.addr, align 1, !tbaa !17
>   %0 = bitcast i16* %gap to i8*, !dbg !19
>   call void @llvm.lifetime.start.p0i8(i64 1, i8* %0) #3, !dbg !19
>   %1 = load i16, i16* %n.addr, align 1, !dbg !20, !tbaa !9
>   %div = sdiv i16 %1, 2, !dbg !21
>   store i16 %div, i16* %gap, align 1, !dbg !22, !tbaa !9
>   br label %for.cond, !dbg !23
> 
> for.cond:                                         ; preds = %for.inc, %entry
>   %2 = load i16, i16* %gap, align 1, !dbg !24, !tbaa !9
>   %cmp = icmp sgt i16 %2, 0, !dbg !25
>   br i1 %cmp, label %for.body, label %for.end, !dbg !26
> 
> for.body:                                         ; preds = %for.cond
>   %3 = load void (i16) addrspace(40)*, void (i16) addrspace(40)** %f.addr, align 1, !dbg !27, !tbaa !17
>   call void %3(i16 8), !dbg !28
>   br label %for.inc, !dbg !29
> 
> ```
> 
> And I got this error (unless I added the same patch as in GetGlobalVal):
> 
> ```
> opt: ./func_ptr.ll:43:13: error: '%3' defined with type 'void (i16) addrspace(40)*'
>   call void %3(i16 8), !dbg !28
>             ^
> ```
Yes, we run into the same issue with CHERI and in our fork I added a hack to simply accept any pointer in AS200.

However, I believe this should only be okay for call instructions since we really don't want any implicit conversions from AS200 to AS0 pointers.


https://reviews.llvm.org/D37054