Question about TypeMapper::addTypeMapping in IRMover.cpp

I have a question about the following code in TypeMapper::addTypeMapping in IRMover.cpp:

if (!areTypesIsomorphic(DstTy, SrcTy)) {

:

:

} else {

// SrcTy and DstTy are recursively ismorphic. We clear names of SrcTy

// and all its descendants to lower amount of renaming in LLVM context

// Renaming occurs because we load all source modules to the same context

// and declaration with existing name gets renamed (i.e Foo → Foo.42).

// As a result we may get several different types in the destination

// module, which are in fact the same.

for (Type *Ty : SpeculativeTypes)

if (auto *STy = dyn_cast(Ty))

if (STy->hasName())

STy->setName("");

The above doesn’t account for the possibility SrcTy and DstTy can be the same pointer. When they are the same, effectively, the above ends up clearing the name of DstTy.

Is there an assumption that addTypeMapping should never be called with the same types? The code for areTypesIsomorphic does specifically handle the case where DstTy == SrcTy.

Background for the above question…

I am running into this problem where we have two modules sharing the same types and we are trying to link one module into the other. In our application, we have a common module with inline functions and several other modules call inline functions from this common module. This code was originally written with LLVM 3.4.2. At that time, the linker API had a PreserveSource mode which allowed for the common module to be preserved so that it can be linked into every other module. As we are now upgrading our LLVM version, we found that the current API takes ownership of the source module and destroys it after linking. We worked around this problem by cloning the common module before calling Linker::linkInModule. However the problem is that it still doesn’t work due:

Consider this function in the destination module:

declare void @foo(%struct.bar * %this1, i8 zeroext %s1) #0

and the corresponding function in the source module:

define linkonce_odr void @foo(%struct.bar* %this1, i8 zeroext %s1) #0 {

:

}

The linking doesn’t succeed (definition of foo doesn’t get copied into the destination module) because of the following code in IRLinker::linkGlobalValueProto:

Constant *C = NewGV;

// Only create a bitcast if necessary. In particular, with

// DebugTypeODRUniquing we may reach metadata in the destination module

// containing a GV from the source module, in which case SGV will be

// the same as DGV and NewGV, and TypeMap.get() will assert since it

// assumes it is being invoked on a type in the source module.

if (DGV && NewGV != SGV) {

C = ConstantExpr::getPointerBitCastOrAddrSpaceCast(

NewGV, TypeMap.get(SGV->getType()));

}

The above results in C to become something like (I suspect that the problem is with the TypeMap which causes a bit-cast to be created). C->dump() shows…

void (%“type 0x5d6c9c0”, i8) bitcast (void (%“type 0x5d6bf50”, i8) @foo to void (%“type 0x5d6c9c0”, i8))

This does not cast to a GlobalValue and IRLinker::materialize returns without copying the body of foo:

GlobalValue *New = dyn_cast(*NewProto);

if (!New)

return *NewProto;

llvm::verifyModule of the destination module then fails with the following error:

Global is external, but doesn’t have external or weak linkage!

void (%0*, i32)* @foo

Any comments on this? I think this issue is different from the one that was discussed in https://lists.llvm.org/pipermail/llvm-dev/2016-November/106949.html

/Riyaz