[PATCH] Kill ModuleProvider and ghost linkage

Fixes http://llvm.org/PR5737 and http://llvm.org/PR5735 by inverting the relationship between Modules and ModuleProviders. Because the "ModuleProvider" simply materializes GlobalValues now, and doesn't provide modules, it's renamed to "GVMaterializer". Code that used to need a ModuleProvider to materialize Functions can now materialize the Functions directly. Functions no longer use a magic linkage to record that they're materializable; they simply ask the GVMaterializer.

Because the C ABI must never change, we can't remove LLVMModuleProviderRef or the functions that refer to it. Instead, because Module now exposes the same functionality ModuleProvider used to, we store a Module* in any LLVMModuleProviderRef and translate in the wrapper methods. We could make this slightly cleaner if we just typedef'ed LLVMModuleProviderRef to LLVMModuleRef, but that would break programs that currently refer to LLVMOpaqueModuleProvider* directly. Is that ok?

Tested with `make BUILD_EXAMPLES=1 && make check-lit`

Saves ~130 lines of code.

[nlewycky1, 2010-01-23 02:08:16]

TODO: I have not yet considered what it means to receive LLVMSetLinkage(...
LLVMGhostLinkage) from a C ABI prorgam.

http://codereview.appspot.com/193064/diff/1/28
File lib/Linker/LinkArchives.cpp (right):

http://codereview.appspot.com/193064/diff/1/28#newcode164
lib/Linker/LinkArchives.cpp:164: Module* aModule = *I;
You need to delete the module at some point (the auto_ptr used to do it for
you). Why not continue using an auto_ptr<Module> here?

http://codereview.appspot.com/193064/diff/1/38
File lib/VMCore/Core.cpp (right):

http://codereview.appspot.com/193064/diff/1/38#newcode1970
lib/VMCore/Core.cpp:1970: delete reinterpret_cast<Module*>(MP);
You implemented unwrap() for LLVMMModuleProviderRef.

http://codereview.appspot.com/193064/diff/1/44
File lib/VMCore/Verifier.cpp (right):

http://codereview.appspot.com/193064/diff/1/44#newcode418
lib/VMCore/Verifier.cpp:418: GV.isMaterializable() ||
It has to have a valid linkage type, now that being unmaterialized doesn't break
the linkage type.

http://codereview.appspot.com/193064/diff/1/44#newcode652
lib/VMCore/Verifier.cpp:652: F.hasExternalWeakLinkage() || F.isMaterializable(),
Materializable is no longer a linkage type.

[Jeffrey Yasskin, 2010-01-25 19:19:27]

http://codereview.appspot.com/193064/diff/1/28
File lib/Linker/LinkArchives.cpp (right):

http://codereview.appspot.com/193064/diff/1/28#newcode164
lib/Linker/LinkArchives.cpp:164: Module* aModule = *I;
On 2010/01/23 02:08:17, nlewycky1 wrote:
> You need to delete the module at some point (the auto_ptr used to do it for
> you). Why not continue using an auto_ptr<Module> here?

Archive::findModulesDefiningSymbols keeps ownership of the Modules it returns.
Note that the code never deleted the ModuleProviders in the set before, it just
cleared their modules. I could call aModule->dropAllReferences() to free up
memory, but that seems like a sketchy thing to do to an object we don't own.

http://codereview.appspot.com/193064/diff/1/38
File lib/VMCore/Core.cpp (right):

http://codereview.appspot.com/193064/diff/1/38#newcode1970
lib/VMCore/Core.cpp:1970: delete reinterpret_cast<Module*>(MP);
On 2010/01/23 02:08:17, nlewycky1 wrote:
> You implemented unwrap() for LLVMMModuleProviderRef.

Thanks, fixed.

http://codereview.appspot.com/193064/diff/1/44
File lib/VMCore/Verifier.cpp (right):

http://codereview.appspot.com/193064/diff/1/44#newcode418
lib/VMCore/Verifier.cpp:418: GV.isMaterializable() ||
On 2010/01/23 02:08:17, nlewycky1 wrote:
> It has to have a valid linkage type, now that being unmaterialized doesn't
break
> the linkage type.

This assert is checking that declarations have one of the few linkages
compatible with a missing definition. Materializable GVs may have any linkage
because the definition is present but elsewhere, so this assert shouldn't fire
on them. I've moved isMaterializable up to just after isDeclaration to avoid
implying that it's a linkage.

http://codereview.appspot.com/193064/diff/1/44#newcode652
lib/VMCore/Verifier.cpp:652: F.hasExternalWeakLinkage() || F.isMaterializable(),
On 2010/01/23 02:08:17, nlewycky1 wrote:
> Materializable is no longer a linkage type.

I've moved this check into its own if branch before isDeclaration. I'm not sure
that's the right thing to do.

[Jeffrey Yasskin, 2010-01-26 23:44:33]

Please take a look. The major APi changes are in Module.h, GVMaterializer.h
(moved from ModuleProvider.h), and GlobalValue.h.

The current version of the patch is at
http://codereview.appspot.com/download/issue193064_2077.diff but I'll update the
Rietveld issue instead of announcing new patches as I address comments.

[clattner_apple.com, 2010-01-27 01:19:29]

On Jan 26, 2010, at 3:44 PM, jyasskin@gmail.com wrote:

> Please take a look. The major APi changes are in Module.h,
> GVMaterializer.h (moved from ModuleProvider.h), and GlobalValue.h.
>
> The current version of the patch is at
> http://codereview.appspot.com/download/issue193064_2077.diff but I'll
> update the Rietveld issue instead of announcing new patches as I  
> address
> comments.

Looks pretty nice to me!  Major cleanup,

-Chris