[PATCH] D18683: Fix bug with duplicate struct types in Linker

[Rafael Ávila de Espíndola via llvm-commits]

rafael added a comment.

So, modifying the test a bit I found that if the dst is initially

  %struct.T1 = type { i8 }
  %struct.T2 = type { i8 }
  declare void @callee1(%struct.T1* %x)
  declare void @callee2(%struct.T2* %x)
  define void @foo1(%struct.T1* %x) {
    call void @callee1(%struct.T1* %x)
    ret void
  }
  define void @foo2(%struct.T2* %x) {
    call void @callee2(%struct.T2* %x)
    ret void
  }

and the source is

  %struct.T1 = type { i8 }
  %struct.T2 = type { i8 }
  define void @callee1(%struct.T1* %x) {
    ret void
  }
  define void @callee2(%struct.T2* %x) {
    ret void
  }

The linker produces

  %struct.T1 = type { i8 }
  %struct.T2 = type { i8 }
  define void @foo1(%struct.T1* %x) {
    call void @callee1(%struct.T1* %x)
    ret void
  }
  define void @foo2(%struct.T2* %x) {
    call void @callee2(%struct.T2* %x)
    ret void
  }
  define void @callee1(%struct.T1* %x) {
    ret void
  }
  define void @callee2(%struct.T2* %x) {
    ret void
  }

I.E.: it correctly avoids merging the types.

If I remove the call to callee2  and its declaration in the destination,  we merge the types, but the module is still valid:

  %struct.T1 = type { i8 }
  %struct.T2 = type { i8 }
  define void @foo1(%struct.T1* %x) {
    call void @callee1(%struct.T1* %x)
    ret void
  }
  define void @foo2(%struct.T2* %x) {
    ret void
  }
  define void @callee1(%struct.T1* %x) {
    ret void
  }
  define void @callee2(%struct.T1* %x) {
    ret void
  }

or if I delete the call to callee1 instead:

  %struct.T1 = type { i8 }
  %struct.T2 = type { i8 }
  define void @foo1(%struct.T1* %x) {
    ret void
  }
  define void @foo2(%struct.T2* %x) {
    call void @callee2(%struct.T2* %x)
    ret void
  }
  define void @callee1(%struct.T2* %x) {
    ret void
  }
  define void @callee2(%struct.T2* %x) {
    ret void
  }

What I don't think is valid is the assumption that you can lookup types by name in the destination and assume their use in newly linked code.


================
Comment at: unittests/Linker/LinkModulesTest.cpp:346
@@ +345,3 @@
+
+  // Check that we can call both callee1 and callee2 still.
+  // Since T1 and T2 were defined in Src, it should be valid to call functions
----------------
rafael wrote:
> Looks like this got out of date.There are no calls in the IR in this test.
Sorry, I now noticed that you create the calls after linking.

Can the calls be there before?


http://reviews.llvm.org/D18683