[PATCH] D88423: Fix llvm-link assert failure in BitCodeWriter

[Richard Smith - zygoloid via Phabricator via llvm-commits]

rsmith added a comment.

Text IR printing is still incorrect



================
Comment at: llvm/test/Linker/sret-types.ll:1
+; RUN: llvm-link %s %p/Inputs/sret-types-1.ll | llvm-dis | FileCheck %s
+
----------------
dblaikie wrote:
> rsmith wrote:
> > Simpler testcase (from https://reviews.llvm.org/D88241#2299465):
> > 
> > ```
> > $ cat c.ll
> > define void @f(i8* sret({i64})) { ret void }
> > $ ./build/bin/opt - -o /tmp/tmp.ir # or anything that converts text IR to bitcode
> > ```
> > 
> > There appear to be two issues here: the linker is not properly linking types inside attributes, and the value enumerator doesn't number types inside attributes. (I'm not sure if the linker relies on the value enumerator; maybe this patch fixes both issues, maybe not.)
> If you have a chance, could you try this patch with both the test cases you posted over there? See if it does address both issues?
This patch does *not* fix the failure of `llvm-link` to link together the modules properly. Indeed, this test fails if we:

```
-; RUN: llvm-link %s %p/Inputs/sret-types-1.ll | llvm-dis | FileCheck %s
+; RUN: llvm-link %s %p/Inputs/sret-types-1.ll -S -o - | FileCheck %s
```

... because it generates incorrect output that looks like:

```
; ModuleID = 'llvm-link'
source_filename = "llvm-link"

%v = type { i32 }

define void @g(%v* %0) {
  ret void
}

define void @f(%v* sret(%"type 0x2c18b30") %0) {
  ret void
}
```

So I guess there are actually *three* different bugs here: fixing the issue for the bitcode writer (as I can confirm this patch does) does not fix the corresponding issue for the textual IR printing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88423/new/

https://reviews.llvm.org/D88423