[cfe-dev] [cfe-commits] r67438 - in /cfe/trunk/lib/CodeGen: CGExpr.cpp CodeGenModule.cpp CodeGenModule.h

[Daniel Dunbar]

On 3/21/09, Chris Lattner <sabre at nondot.org> wrote:
> Author: lattner
> Date: Sat Mar 21 02:48:31 2009
> New Revision: 67438
>
> URL: http://llvm.org/viewvc/llvm-project?rev=67438&view=rev
> Log:
> simplify and comment some code better.  Make BindRuntimeGlobals
> more optimistic that it will work (optimizing for the common case).

I may be missing something, but wouldn't this be cleaner if we just
provided a Module method to return a reference to the entry for a
name? This would avoiding the shuffling with names and only do one
lookup.

 - Daniel
> Modified:
>     cfe/trunk/lib/CodeGen/CGExpr.cpp
>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>     cfe/trunk/lib/CodeGen/CodeGenModule.h
>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=67438&r1=67437&r2=67438&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Sat Mar 21 02:48:31 2009
> @@ -29,6 +29,7 @@
>  /// block.
>  llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(const llvm::Type *Ty,
>                                                      const char *Name) {
> +  // FIXME: Should not pass name if names are disabled in IRBuilder.
>    return new llvm::AllocaInst(Ty, 0, Name, AllocaInsertPt);
>  }
>
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=67438&r1=67437&r2=67438&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sat Mar 21 02:48:31 2009
> @@ -81,32 +81,37 @@
>        continue;
>      }
>
> -    // See if there is a conflict against a function.
> +    // See if there is a conflict against a function by setting the name
> and
> +    // seeing if we got the desired name.
> +    GV->setName(Name);
> +    if (GV->isName(Name.c_str()))
> +      continue;  // Yep, it worked!
> +
> +    GV->setName(""); // Zap the bogus name until we work out the conflict.
>      llvm::GlobalValue *Conflict = TheModule.getNamedValue(Name);
> -    if (Conflict) {
> -      // Decide which version to take. If the conflict is a definition
> -      // we are forced to take that, otherwise assume the runtime
> -      // knows best.
> -
> -      // FIXME: This will fail phenomenally when the conflict is the
> -      // wrong type of value. Just bail on it for now. This should
> -      // really reuse something inside the LLVM Linker code.
> -      assert(GV->getValueID() == Conflict->getValueID() &&
> -             "Unable to resolve conflict between globals of different
> types.");
> -      if (!Conflict->isDeclaration()) {
> -        llvm::Value *Casted =
> -          llvm::ConstantExpr::getBitCast(Conflict, GV->getType());
> -        GV->replaceAllUsesWith(Casted);
> -        GV->eraseFromParent();
> -      } else {
> -        GV->takeName(Conflict);
> -        llvm::Value *Casted =
> -          llvm::ConstantExpr::getBitCast(GV, Conflict->getType());
> -        Conflict->replaceAllUsesWith(Casted);
> -        Conflict->eraseFromParent();
> -      }
> -    } else
> -      GV->setName(Name);
> +    assert(Conflict && "Must have conflicted!");
> +
> +    // Decide which version to take. If the conflict is a definition
> +    // we are forced to take that, otherwise assume the runtime
> +    // knows best.
> +
> +    // FIXME: This will fail phenomenally when the conflict is the
> +    // wrong type of value. Just bail on it for now. This should
> +    // really reuse something inside the LLVM Linker code.
> +    assert(GV->getValueID() == Conflict->getValueID() &&
> +           "Unable to resolve conflict between globals of different
> types.");
> +    if (!Conflict->isDeclaration()) {
> +      llvm::Value *Casted =
> +        llvm::ConstantExpr::getBitCast(Conflict, GV->getType());
> +      GV->replaceAllUsesWith(Casted);
> +      GV->eraseFromParent();
> +    } else {
> +      GV->takeName(Conflict);
> +      llvm::Value *Casted =
> +        llvm::ConstantExpr::getBitCast(GV, Conflict->getType());
> +      Conflict->replaceAllUsesWith(Casted);
> +      Conflict->eraseFromParent();
> +    }
>    }
>  }
>
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=67438&r1=67437&r2=67438&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Sat Mar 21 02:48:31 2009
> @@ -94,7 +94,7 @@
>    /// emitted. Note that the entries in this map are the actual
>    /// globals and therefore may not be of the same type as the decl,
>    /// they should be bitcasted on retrieval. Also note that the
> -  /// globals are keyed on their source name, not the global name
> +  /// globals are keyed on their source mangled name, not the global name
>    /// (which may change with attributes such as asm-labels).  This key
>    /// to this map should be generated using getMangledName().
>    llvm::DenseMap<const char*, llvm::GlobalValue*> GlobalDeclMap;
> @@ -141,6 +141,9 @@
>    /// strings. This value has type int * but is actually an Obj-C class
> pointer.
>    llvm::Constant *CFConstantStringClassRef;
>
> +  /// BuiltinFunctions - This is the cached set of Function*'s that have
> been
> +  /// created for each builtin, indexed by the Builtin ID.  This is null if
> the
> +  /// Function* has not yet been created.
>    std::vector<llvm::Value *> BuiltinFunctions;
>  public:
>    CodeGenModule(ASTContext &C, const LangOptions &Features, llvm::Module
> &M,
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>