[llvm-commits] [llvm] r106075 - in /llvm/trunk: include/llvm/Module.h lib/Analysis/DebugInfo.cpp lib/CodeGen/AsmPrinter/DwarfDebug.cpp lib/VMCore/Module.cpp

[Chris Lattner]

On Jun 15, 2010, at 5:53 PM, Devang Patel wrote:

> Author: dpatel
> Date: Tue Jun 15 19:53:55 2010
> New Revision: 106075
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=106075&view=rev
> Log:
> Use separate named MDNode to hold each function's local variable info.
> This speeds up local variable handling in DwarfDebug.

Ok.

> +++ llvm/trunk/include/llvm/Module.h Tue Jun 15 19:53:55 2010
> @@ -326,6 +326,7 @@
>   /// specified name. This method returns null if a NamedMDNode with the 
>   /// specified name is not found.
>   NamedMDNode *getNamedMetadata(StringRef Name) const;
> +  NamedMDNode *getNamedMetadataUsingTwine(Twine Name) const;

Twine's should always be taken by const reference, not by copy.  Also, please just name it getNamedMetadata, not getNamedMetadataUsingTwine.  To solve the ambiguity problems you got, just remove the stringref version.

> +++ llvm/trunk/lib/Analysis/DebugInfo.cpp Tue Jun 15 19:53:55 2010
> @@ -1053,8 +1053,12 @@
>     // The optimizer may remove local variable. If there is an interest
>     // to preserve variable info in such situation then stash it in a
>     // named mdnode.
..
> +    DISubprogram Fn(getDISubprogram(Context));
> +    const Twine FnLVName = Twine("llvm.dbg.lv.", Fn.getName());
> +    NamedMDNode *FnLocals = M.getNamedMetadataUsingTwine(FnLVName);

This is just barely safe, but is really dangerous.  Please use:

    NamedMDNode *FnLocals = M.getNamedMetadata(Twine("llvm.dbg.lv.")+Fn.getName());

The temporaries are destroyed at the end of the statement.


> +    if (!FnLocals)
> +      FnLocals = NamedMDNode::Create(VMContext, FnLVName, NULL, 0, &M);

Just pass in the same expression to form the twine here as well.

> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Jun 15 19:53:55 2010
> @@ -2259,8 +2259,9 @@
>   }
> 
>   // Collect info for variables that were optimized out.
> -  if (NamedMDNode *NMD = 
> -      MF->getFunction()->getParent()->getNamedMetadata("llvm.dbg.lv")) {
> +  const Twine FnLVName = Twine("llvm.dbg.lv.", MF->getFunction()->getName());
> +  if (NamedMDNode *NMD =
> +      MF->getFunction()->getParent()->getNamedMetadataUsingTwine(FnLVName)) {

Same deal as above.

-Chris