[llvm-commits] Make library interface pass for review

[Chris Lattner]

On May 8, 2010, at 1:46 PM, David Chisnall wrote:

> Hi,
> 
> Does anyone object if I commit the attached diff?  It's a work-in-progress pass that prepares the bitcode used to compile a shared library for use by the inliner.  After running the pass, the resulting bitcode can be linked into code that links against the library and used for inlining and any other related optimisations.
> 
> In theory, it strips out anything that depends on globals that are not exposed outside of the module and marks everything as available_externally.  In practice, there are almost certainly some cases that I've missed (reports welcome)...
> 
> David

A few requests:

Please document this in Passes.html


+++ lib/Transforms/IPO/MakeRuntimeLibraryInterface.cpp	(revision 0)
@@ -0,0 +1,201 @@
+#include "llvm/Pass.h"
+#include "llvm/Function.h"
+#include "llvm/Module.h"
+#include "llvm/LLVMContext.h"
+#include "llvm/Instructions.h"
+#include "llvm/Constants.h"
+#include "llvm/GlobalVariable.h"

You need a file header commend, please prune out the redundant includes you don't need.


+namespace
+{
+  class MakeRuntimeLibraryInterface : public ModulePass 
+  {

Please put the { on the same line, conforming to the predominate style.

+  for (Module::iterator I=M.begin(), E=M.end() ;
+      I!=E ; ++I) {

Please use whitespace like the rest of llvm :).  You don't need braces if the for loop contains a single statement.

Please remove ProcessGlobals / IteratorMethod until and when they are needed.

+  switch (V->getLinkage()) {
+    case GlobalValue::ExternalLinkage:
+    case GlobalValue::DLLExportLinkage:
+    case GlobalValue::ExternalWeakLinkage:
+    case GlobalValue::CommonLinkage:
+      V->setLinkage(GlobalValue::AvailableExternallyLinkage);
+      return true;
+    case GlobalValue::AvailableExternallyLinkage:
+      // Already done
+    case GlobalValue::WeakAnyLinkage:
+    case GlobalValue::WeakODRLinkage:
+    case GlobalValue::LinkOnceAnyLinkage:
+    case GlobalValue::LinkOnceODRLinkage:
+      // Copy link-once and weak stuff
+    case GlobalValue::AppendingLinkage:
+    case GlobalValue::InternalLinkage:
+    case GlobalValue::PrivateLinkage:
+    case GlobalValue::LinkerPrivateLinkage:
+    case GlobalValue::DLLImportLinkage:
+      // Ignore internal symbols here.
+      return false;

Use predicates (like hasLocalLinkage) instead of enumerating all of this stuff.  Also, you should be able to handle linkonce_odr and weak_odr.  Since you have several *different* places where your reasoning about the same linkage types, you should probably write your own predicates as a static inline function in your file, instead of duplicating the logic all over.


+        case GlobalValue::LinkOnceODRLinkage:
+        case GlobalValue::LinkOnceAnyLinkage:
+          G->setLinkage(GlobalValue::ExternalWeakLinkage);

I don't think that external weak is what you're looking for here.

Please resend this for another iteration after you incorporate these changes,

-Chris