[llvm-commits] llvm-gcc part of PR1017

[Chris Lattner]

On Apr 25, 2007, at 7:39 AM, Anton Korobeynikov wrote:

> Hello, Everyone.
>
> Please find attached llvm-gcc patch for symbol aliasing support. Now
> libstdc++ can be build as shared library on x86/linux, stripping 10M
> from the llvm-gcc distribution.
>
> This fixes: PR843, PR1006, PR1017.

Ooh, cool.

A running example:

int X() alias "Y";


+  // Get or create LLVM global for our alias.
+  GlobalValue *V = cast<GlobalValue>DECL_LLVM(decl);

Two things: first please use ()'s around DECL_LLVM.  I assume this  
works because DECL_LLVM expands to being surrounded by parens, but  
this looks very strange.

Second, does this create a decl for "X" or for "Y"?  I think it  
creates a global for X, which you later hack out and replace with a  
GlobalAlias instance.  This is ugly because you're creating a global  
or function, only to remove it.  Would it be possible to make  
make_decl_llvm create the GlobalAlias for you directly?



+  // Query SymTab for aliasee
+  const char* AliaseeName = IDENTIFIER_POINTER(target);
+  if (!Aliasee) {
+    if (isa<Function>(V)) {
+      Aliasee = TheModule->getFunction(AliaseeName);
+    } else if (isa<GlobalVariable>(V)) {
+      Aliasee = TheModule->getNamedGlobal(AliaseeName);
+    } else if (isa<GlobalAlias>(V)) {
+      Aliasee = TheModule->getNamedAlias(AliaseeName);
+    }
+  }

This can be made simpler.  Try something like this:
   Aliasee = TheModule->getValueSymbolTable().lookup(AliaseeName);


+  // Last resort. Query for name set via __asm__
..
+    if (isa<Function>(V)) {
+      Aliasee = TheModule->getFunction(starred);
+    } else if (isa<GlobalVariable>(V)) {
+      Aliasee = TheModule->getNamedGlobal(starred);
+    } else if (isa<GlobalAlias>(V)) {
+      Aliasee = TheModule->getNamedAlias(starred);
+    }

Likewise.


+  if (!Aliasee)
+    if (stage) {
+      TheModule->dump();
+      error ("%J%qD aliased to undefined symbol %qE",
+             decl, decl, target);
+    } else
+      return -1;

This probably shouldn't dump TheModule if it is a user error.  :)  If  
a user shouldn't be able to trigger this, turn this into an assert.

+  std::string savedName = V->getName();
+  GlobalAlias* GA = new GlobalAlias(Aliasee->getType(), Linkage,  
savedName,
+                                    Aliasee, TheModule);
..
+  V->eraseFromParent();
+  GA->setName(savedName);

The name gymnastics would be more efficient as:

   GlobalAlias* GA = new GlobalAlias(Aliasee->getType(), Linkage, "",  
Aliasee, TheModule);
   GA->takeName(V);

but hopefully you can eliminate them entirely by fixing make_decl_llvm.



+++ b/gcc/varasm.c	Wed Apr 25 17:03:17 2007 +0400
@@ -4645,17 +4647,17 @@ find_decl_and_mark_needed (tree decl, tr
    if (!cgraph_global_info_ready)
      {
        if (TREE_CODE (decl) == FUNCTION_DECL)
-	{
-	  fnode = cgraph_node_for_asm (target);
-	  if (fnode == NULL)
-	    vnode = cgraph_varpool_node_for_asm (target);
-	}
+        {
+          fnode = cgraph_node_for_asm (target);
+          if (fnode == NULL)
+            vnode = cgraph_varpool_node_for_asm (target);
+        }

this looks like indentation changes, please remove from the diff.   
There are several others in this file.


Otherwise, looks great!

-Chris