[llvm] r203662 - Cloning a function now also clones its debug metadata if 'ModuleLevelChanges' is true.

Mishne, Alon alon.mishne at intel.com
Thu Mar 13 00:23:11 PDT 2014


Apologies, and thanks for the fix. Visual Studio reported nothing :-(

-----Original Message-----
From: Justin Bogner [mailto:justin at justinbogner.com] On Behalf Of Justin Bogner
Sent: Wednesday, March 12, 2014 19:06
To: Mishne, Alon
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm] r203662 - Cloning a function now also clones its debug metadata if 'ModuleLevelChanges' is true.

This broke the -Werror build for me, due to a signed compare warning in the unittest. Should be fixed in r203679.

Alon Mishne <alon.mishne at intel.com> writes:
> Author: amishne
> Date: Wed Mar 12 09:42:51 2014
> New Revision: 203662
>
> URL: http://llvm.org/viewvc/llvm-project?rev=203662&view=revLog:
> Cloning a function now also clones its debug metadata if 
> 'ModuleLevelChanges' is true.
>
> Modified:
>     llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
>     llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
>     llvm/trunk/unittests/Transforms/Utils/Cloning.cpp
>
> Modified: llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms
> /Utils/Cloning.h?rev=203662&r1=203661&r2=203662&view=diff=============
> =================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/Cloning.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Utils/Cloning.h Wed Mar 12 
> +++ 09:42:51 2014
> @@ -109,7 +109,7 @@ BasicBlock *CloneBasicBlock(const BasicB  /// 
> information about the cloned code if non-null.
>  ///
>  /// If ModuleLevelChanges is false, VMap contains no non-identity 
> GlobalValue -/// mappings.
> +/// mappings, and debug info metadata will not be cloned.
>  ///
>  Function *CloneFunction(const Function *F,
>                          ValueToValueMapTy &VMap,
>
> Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Cl
> oneFunction.cpp?rev=203662&r1=203661&r2=203662&view=diff==============
> ================================================================
> --- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Wed Mar 12 
> +++ 09:42:51 2014
> @@ -27,6 +27,7 @@
>  #include "llvm/IR/IntrinsicInst.h"
>  #include "llvm/IR/LLVMContext.h"
>  #include "llvm/IR/Metadata.h"
> +#include "llvm/IR/Module.h"
>  #include "llvm/Transforms/Utils/BasicBlockUtils.h"
>  #include "llvm/Transforms/Utils/Local.h"
>  #include "llvm/Transforms/Utils/ValueMapper.h"
> @@ -151,6 +152,60 @@ void llvm::CloneFunctionInto(Function *N
>                         TypeMapper, Materializer);  }
>  
> +// Find the MDNode which corresponds to the DISubprogram data that described F.
> +static MDNode* FindSubprogram(const Function *F, DebugInfoFinder 
> +&Finder) {
> +  for (DebugInfoFinder::iterator I = Finder.subprogram_begin(),
> +                                 E = Finder.subprogram_end();
> +       I != E; ++I) {
> +    DISubprogram Subprogram(*I);
> +    if (Subprogram.describes(F)) return Subprogram;
> +  }
> +  return NULL;
> +}
> +
> +// Add an operand to an existing MDNode. The new operand will be 
> +added at the // back of the operand list.
> +static void AddOperand(MDNode *Node, Value *Operand) {
> +  SmallVector<Value*, 16> Operands;
> +  for (unsigned i = 0; i < Node->getNumOperands(); i++) {
> +    Operands.push_back(Node->getOperand(i));
> +  }
> +  Operands.push_back(Operand);
> +  MDNode *NewNode = MDNode::get(Node->getContext(), Operands);
> +  Node->replaceAllUsesWith(NewNode);
> +}
> +
> +// Clone the module-level debug info associated with OldFunc. The 
> +cloned data // will point to NewFunc instead.
> +static void CloneDebugInfoMetadata(Function *NewFunc, const Function *OldFunc,
> +                            ValueToValueMapTy &VMap) {
> +  DebugInfoFinder Finder;
> +  Finder.processModule(*OldFunc->getParent());
> +
> +  const MDNode *OldSubprogramMDNode = FindSubprogram(OldFunc, 
> + Finder);  if (!OldSubprogramMDNode) return;
> +
> +  // Ensure that OldFunc appears in the map.
> +  // (if it's already there it must point to NewFunc anyway)  
> + VMap[OldFunc] = NewFunc;  DISubprogram 
> + NewSubprogram(MapValue(OldSubprogramMDNode, VMap));
> +
> +  for (DebugInfoFinder::iterator CUIter = Finder.compile_unit_begin(),
> +       CUEnd = Finder.compile_unit_end(); CUIter != CUEnd; ++CUIter) {
> +    DICompileUnit CU(*CUIter);
> +
> +    DIArray Subprograms(CU.getSubprograms());
> +
> +    // If the compile unit's function list contains the old function, it should
> +    // also contain the new one.
> +    for (unsigned i = 0; i < Subprograms.getNumElements(); i++) {
> +      if ((MDNode*)Subprograms.getElement(i) == OldSubprogramMDNode) {
> +        AddOperand(Subprograms, NewSubprogram);
> +      }
> +    }
> +  }
> +}
> +
>  /// CloneFunction - Return a copy of the specified function, but 
> without  /// embedding the function into another module.  Also, any 
> references specified  /// in the VMap are changed to refer to their 
> mapped value instead of the @@ -188,6 +243,9 @@ Function *llvm::CloneFunction(const Func
>        VMap[I] = DestI++;        // Add mapping to VMap
>      }
>  
> +  if (ModuleLevelChanges)
> +    CloneDebugInfoMetadata(NewF, F, VMap);
> +
>    SmallVector<ReturnInst*, 8> Returns;  // Ignore returns cloned.
>    CloneFunctionInto(NewF, F, VMap, ModuleLevelChanges, Returns, "", CodeInfo);
>    return NewF;
>
> Modified: llvm/trunk/unittests/Transforms/Utils/Cloning.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Ut
> ils/Cloning.cpp?rev=203662&r1=203661&r2=203662&view=diff==============
> ================================================================
> --- llvm/trunk/unittests/Transforms/Utils/Cloning.cpp (original)
> +++ llvm/trunk/unittests/Transforms/Utils/Cloning.cpp Wed Mar 12 
> +++ 09:42:51 2014
> @@ -7,15 +7,22 @@
>  //
>  
> //===-----------------------------------------------------------------
> -----===//
>  
> +#include "llvm/Transforms/Utils/Cloning.h"
> +#include "llvm/ADT/ArrayRef.h"
>  #include "llvm/ADT/STLExtras.h"
>  #include "llvm/ADT/SmallPtrSet.h"
>  #include "llvm/IR/Argument.h"
>  #include "llvm/IR/Constant.h"
> +#include "llvm/IR/DebugInfo.h"
> +#include "llvm/IR/DIBuilder.h"
>  #include "llvm/IR/Function.h"
>  #include "llvm/IR/IRBuilder.h"
> +#include "llvm/IR/InstIterator.h"
>  #include "llvm/IR/Instructions.h"
> +#include "llvm/IR/IntrinsicInst.h"
> +#include "llvm/IR/IRBuilder.h"
> +#include "llvm/IR/Module.h"
>  #include "llvm/IR/LLVMContext.h"
> -#include "llvm/Transforms/Utils/Cloning.h"
>  #include "gtest/gtest.h"
>  
>  using namespace llvm;
> @@ -173,4 +180,207 @@ TEST_F(CloneInstruction, Attributes) {
>    delete F2;
>  }
>  
> +class CloneFunc : public ::testing::Test {
> +protected:
> +  virtual void SetUp() {
> +    SetupModule();
> +    CreateOldFunc();
> +    CreateNewFunc();
> +    SetupFinder();
> +  }
> +
> +  virtual void TearDown() {
> +    delete Finder;
> +  }
> +
> +  void SetupModule() {
> +    M = new Module("", C);
> +  }
> +
> +  void CreateOldFunc() {
> +    FunctionType* FuncType = FunctionType::get(Type::getVoidTy(C), false);
> +    OldFunc = Function::Create(FuncType, GlobalValue::PrivateLinkage, "f", M);
> +    CreateOldFunctionBodyAndDI();
> +  }
> +
> +  void CreateOldFunctionBodyAndDI() {
> +    DIBuilder DBuilder(*M);
> +    IRBuilder<> IBuilder(C);
> +
> +    // Function DI
> +    DIFile File = DBuilder.createFile("filename.c", "/file/dir/");
> +    DIArray ParamTypes = DBuilder.getOrCreateArray(ArrayRef<Value*>());
> +    DICompositeType FuncType = DBuilder.createSubroutineType(File, ParamTypes);
> +    DICompileUnit CU = DBuilder.createCompileUnit(dwarf::DW_LANG_C99,
> +        "filename.c", "/file/dir", "CloneFunc", false, "", 0);
> +
> +    DISubprogram Subprogram = DBuilder.createFunction(CU, "f", "f", File, 4,
> +        FuncType, true, true, 3, 0, false, OldFunc);
> +
> +    // Function body
> +    BasicBlock* Entry = BasicBlock::Create(C, "", OldFunc);
> +    IBuilder.SetInsertPoint(Entry);
> +    DebugLoc Loc = DebugLoc::get(3, 2, Subprogram);
> +    IBuilder.SetCurrentDebugLocation(Loc);
> +    AllocaInst* Alloca = IBuilder.CreateAlloca(IntegerType::getInt32Ty(C));
> +    IBuilder.SetCurrentDebugLocation(DebugLoc::get(4, 2, Subprogram));
> +    Value* AllocaContent = IBuilder.getInt32(1);
> +    Instruction* Store = IBuilder.CreateStore(AllocaContent, Alloca);
> +    IBuilder.SetCurrentDebugLocation(DebugLoc::get(5, 2, Subprogram));
> +    Instruction* Terminator = IBuilder.CreateRetVoid();
> +
> +    // Create a local variable around the alloca
> +    DIType IntType = DBuilder.createBasicType("int", 32, 0,
> +        dwarf::DW_ATE_signed);
> +    DIVariable Variable = DBuilder.createLocalVariable(
> +      dwarf::DW_TAG_auto_variable, Subprogram, "x", File, 5, IntType, true);
> +    DBuilder.insertDeclare(Alloca, Variable, Store);
> +    DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, Terminator);
> +    // Finalize the debug info
> +    DBuilder.finalize();
> +
> +
> +    // Create another, empty, compile unit
> +    DIBuilder DBuilder2(*M);
> +    DBuilder2.createCompileUnit(dwarf::DW_LANG_C99,
> +        "extra.c", "/file/dir", "CloneFunc", false, "", 0);
> +    DBuilder2.finalize();
> +  }
> +
> +  void CreateNewFunc() {
> +    ValueToValueMapTy VMap;
> +    NewFunc = CloneFunction(OldFunc, VMap, true, NULL);
> +    M->getFunctionList().push_back(NewFunc);
> +  }
> +
> +  void SetupFinder() {
> +    Finder = new DebugInfoFinder();
> +    Finder->processModule(*M);
> +  }
> +
> +  LLVMContext C;
> +  Function* OldFunc;
> +  Function* NewFunc;
> +  Module* M;
> +  DebugInfoFinder* Finder;
> +};
> +
> +// Test that a new, distinct function was created.
> +TEST_F(CloneFunc, NewFunctionCreated) {
> +  EXPECT_NE(OldFunc, NewFunc);
> +}
> +
> +// Test that a new subprogram entry was added and is pointing to the 
> +new // function, while the original subprogram still points to the old one.
> +TEST_F(CloneFunc, Subprogram) {
> +  unsigned SubprogramCount = Finder->subprogram_count();
> +  EXPECT_EQ(2, SubprogramCount);
> +
> +  DebugInfoFinder::iterator Iter = Finder->subprogram_begin();  
> + DISubprogram Sub1(*Iter);  EXPECT_TRUE(Sub1.Verify());  Iter++;  
> + DISubprogram Sub2(*Iter);  EXPECT_TRUE(Sub2.Verify());
> +
> +  EXPECT_TRUE(Sub1.getFunction() == OldFunc && Sub2.getFunction() == NewFunc
> +           || Sub1.getFunction() == NewFunc && Sub2.getFunction() == 
> +OldFunc); }
> +
> +// Test that the new subprogram entry was not added to the CU which 
> +doesn't // contain the old subprogram entry.
> +TEST_F(CloneFunc, SubprogramInRightCU) {
> +  EXPECT_EQ(2, Finder->compile_unit_count());
> +
> +  DebugInfoFinder::iterator Iter = Finder->compile_unit_begin();
> +  DICompileUnit CU1(*Iter);
> +  EXPECT_TRUE(CU1.Verify());
> +  Iter++;
> +  DICompileUnit CU2(*Iter);
> +  EXPECT_TRUE(CU2.Verify());
> +  EXPECT_TRUE(CU1.getSubprograms().getNumElements() == 0
> +           || CU2.getSubprograms().getNumElements() == 0); }
> +
> +// Test that instructions in the old function still belong to it in 
> +the // metadata, while instruction in the new function belong to the new one.
> +TEST_F(CloneFunc, InstructionOwnership) {
> +  inst_iterator OldIter = inst_begin(OldFunc);
> +  inst_iterator OldEnd = inst_end(OldFunc);
> +  inst_iterator NewIter = inst_begin(NewFunc);
> +  inst_iterator NewEnd = inst_end(NewFunc);
> +  while (OldIter != OldEnd && NewIter != NewEnd) {
> +    Instruction& OldI = *OldIter;
> +    Instruction& NewI = *NewIter;
> +    EXPECT_NE(&OldI, &NewI);
> +
> +    EXPECT_EQ(OldI.hasMetadata(), NewI.hasMetadata());
> +    if (OldI.hasMetadata()) {
> +      const DebugLoc& OldDL = OldI.getDebugLoc();
> +      const DebugLoc& NewDL = NewI.getDebugLoc();
> +
> +      // Verify that the debug location data is the same
> +      EXPECT_EQ(OldDL.getLine(), NewDL.getLine());
> +      EXPECT_EQ(OldDL.getCol(), NewDL.getCol());
> +      
> +      // But that they belong to different functions
> +      DISubprogram OldSubprogram(OldDL.getScope(C));
> +      DISubprogram NewSubprogram(NewDL.getScope(C));
> +      EXPECT_TRUE(OldSubprogram.Verify());
> +      EXPECT_TRUE(NewSubprogram.Verify());
> +      EXPECT_EQ(OldFunc, OldSubprogram.getFunction());
> +      EXPECT_EQ(NewFunc, NewSubprogram.getFunction());
> +    }
> +
> +    ++OldIter;
> +    ++NewIter;
> +  }
> +  EXPECT_EQ(OldEnd, OldIter);
> +  EXPECT_EQ(NewEnd, NewIter);
> +}
> +
> +// Test that the arguments for debug intrinsics in the new function 
> +were // properly cloned TEST_F(CloneFunc, DebugIntrinsics) {
> +  inst_iterator OldIter = inst_begin(OldFunc);
> +  inst_iterator OldEnd = inst_end(OldFunc);
> +  inst_iterator NewIter = inst_begin(NewFunc);
> +  inst_iterator NewEnd = inst_end(NewFunc);
> +  while (OldIter != OldEnd && NewIter != NewEnd) {
> +    Instruction& OldI = *OldIter;
> +    Instruction& NewI = *NewIter;
> +    if (DbgDeclareInst* OldIntrin = dyn_cast<DbgDeclareInst>(&OldI)) {
> +      DbgDeclareInst* NewIntrin = dyn_cast<DbgDeclareInst>(&NewI);
> +      EXPECT_TRUE(NewIntrin);
> +
> +      // Old address must belong to the old function
> +      EXPECT_EQ(OldFunc, cast<AllocaInst>(OldIntrin->getAddress())->
> +                         getParent()->getParent());
> +      // New address must belong to the new function
> +      EXPECT_EQ(NewFunc, cast<AllocaInst>(NewIntrin->getAddress())->
> +                         getParent()->getParent());
> +
> +      // Old variable must belong to the old function
> +      EXPECT_EQ(OldFunc, DISubprogram(DIVariable(OldIntrin->getVariable())
> +                         .getContext()).getFunction());
> +      // New variable must belong to the New function
> +      EXPECT_EQ(NewFunc, DISubprogram(DIVariable(NewIntrin->getVariable())
> +                         .getContext()).getFunction());
> +    } else if (DbgValueInst* OldIntrin = dyn_cast<DbgValueInst>(&OldI)) {
> +      DbgValueInst* NewIntrin = dyn_cast<DbgValueInst>(&NewI);
> +      EXPECT_TRUE(NewIntrin);
> +
> +      // Old variable must belong to the old function
> +      EXPECT_EQ(OldFunc, DISubprogram(DIVariable(OldIntrin->getVariable())
> +                         .getContext()).getFunction());
> +      // New variable must belong to the New function
> +      EXPECT_EQ(NewFunc, DISubprogram(DIVariable(NewIntrin->getVariable())
> +                         .getContext()).getFunction());
> +    }
> +
> +    ++OldIter;
> +    ++NewIter;
> +  }
> +}
> +
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.





More information about the llvm-commits mailing list