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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

I've applied your patches to my downstream repo, trying to figure out how much of our own patches for having different address space for functions that can be removed and still getting existing test cases to work when only having your patches.
Lots of new things happens now when there are addrspace on all function definitions. Our old implementation kind of added the addrspace information when fetching function pointers from the symbol table instead of adding addrspace when creating the functions. So I still have a couple of issues to analyse. I think that I at least tracked down one problem with calls to a function given a local function pointer variable (see my inline comment in LLParser.cpp).

There are no testcases included here with ll-files including calls/bitcasts etc with non-zero addrspace, so it is a little bit tricky to understand how far I'm supposed to get by only using your set of patches.
Are you testing this with some other out-of-tree target?



================
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;
----------------
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
            ^
```


https://reviews.llvm.org/D37054