[llvm-commits] [llvm-gcc-4.2] r100531 - in /llvm-gcc-4.2/trunk/gcc: llvm-backend.cpp llvm-convert.cpp llvm-debug.cpp llvm-debug.h llvm-internal.h

[Stuart Hastings]

On Apr 6, 2010, at 11:49 AM, Devang Patel wrote:

> Stuart,
> 
> On Apr 6, 2010, at 10:19 AM, Stuart Hastings wrote:
> 
>> Author: stuart
>> Date: Tue Apr  6 12:19:47 2010
>> New Revision: 100531
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=100531&view=rev
>> Log:
>> Revise debug info machinery to digest nested functions and classes.
>> 
>> A certain GDB testsuite case (local.cc) has a function nested inside a
>> class nested inside another function.  GCC presents the innermost
>> function to llvm-convert first.  Heretofore, the debug info mistakenly
>> placed the inner function at module scope.  This patch walks the GCC
>> context links and instantiates the outer class and function so the
>> debug info is properly nested.  Radar 7426545.
>> 
>> Modified:
>>   llvm-gcc-4.2/trunk/gcc/llvm-backend.cpp
>>   llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
>>   llvm-gcc-4.2/trunk/gcc/llvm-debug.cpp
>>   llvm-gcc-4.2/trunk/gcc/llvm-debug.h
>>   llvm-gcc-4.2/trunk/gcc/llvm-internal.h

[snip]

>> /// performLateBackendInitialization - Set backend options that may only be
>> @@ -533,8 +535,6 @@
>> }
>> 
>> void llvm_lang_dependent_init(const char *Name) {
>> -  if (TheDebugInfo)
>> -    TheDebugInfo->Initialize();
> 
> What is the motivation behind this ?

[snip]

>> -TreeToLLVM::TreeToLLVM(tree fndecl) :
>> -    TD(getTargetData()), Builder(Context, *TheFolder) {
>> -  FnDecl = fndecl;
>> +TreeToLLVM::TreeToLLVM(tree decl) :
>> +  TD(getTargetData()), Builder(Context, *TheFolder) {
>> +  // If this isn't a FUNCITON_DECL, use only the source loc info from it.
>> +  FnDecl = (decl && TREE_CODE(decl) == FUNCTION_DECL) ? decl : NULL_TREE;
>>  Fn = 0;
>>  ReturnBB = UnwindBB = 0;
>>  ReturnOffset = 0;
>> 
>>  if (EmitDebugInfo()) {
>> -    expanded_location Location = expand_location(DECL_SOURCE_LOCATION (fndecl));
>> +    expanded_location Location = expand_location(DECL_SOURCE_LOCATION (decl));
>> 
>>    if (Location.file) {
>>      TheDebugInfo->setLocationFile(Location.file);
>> @@ -179,6 +180,7 @@
>>      TheDebugInfo->setLocationFile("<unknown file>");
>>      TheDebugInfo->setLocationLine(0);
>>    }
>> +    TheDebugInfo->Initialize();
>>  }
> 
> I do not think this is correct. DebugInfo::Initialize() should be called only once. Are you sure, you are not losing debug info with this change ?

You may be right.  I'll look into this.

>>  AllocaInsertionPoint = 0;
>> @@ -188,13 +190,25 @@
>>  FuncEHException = 0;
>>  FuncEHSelector = 0;
>>  FuncEHGetTypeID = 0;
>> -
>> -  assert(TheTreeToLLVM == 0 && "Reentering function creation?");
>> -  TheTreeToLLVM = this;
>> }
>> 
>> -TreeToLLVM::~TreeToLLVM() {
>> -  TheTreeToLLVM = 0;
>> +
>> +TreeToLLVM::~TreeToLLVM() {}
>> +
>> +TreeToLLVM *getTreeToLLVM(tree decl) {
>> +  // FIXME: should this static move into the TreeToLLVM class decl?
>> +  static std::map<tree_node *, TreeToLLVM * > FunctionMap;
> 
> Why not?

I put this here for expediency, and later realized it might be "more elegant" if this moved to the class.  It works fine as-is, so I guess this is a "preferred style" question.

stuart