r205651 - DebugInfo: PR19298: function local const variables duplicated in the root scope

[Eric Christopher]

On Fri, Apr 4, 2014 at 3:23 PM, David Blaikie <dblaikie at gmail.com> wrote:
> Hmm - this might not be the right long-term fix.
>
> We still have basically the same bug for a global const int:
>
> const int i = 3;
> void func(int*);
> int main() {
>   func(&i); // references the global variable
>   return i; // references the Constant
> }
>
> from this Clang describes two global variables and LLVM produces debug
> info describing both...
>
> I suppose what we want is to emit the constant location description
> when the variable itself is not emitted (such as when the address
> doesn't escape) and otherwise emit the global memory location....
> perhaps a simple frontend lookup-and-replacement would suffice. Except
> in cases where the global variable is emitted initially and then
> optimized away by LLVM...
>
> oh, and we don't put the constant in the right namespace - but I'll
> probably just fix that immediately, even if the broader issue still
> stands :/
>

Hrm. I think, ideally at least, what we'd want is for the variable
emission to be cached (as we've discussed offline, just repeating for
public benefit).

Thanks for the patch though. :)

-eric

> On Fri, Apr 4, 2014 at 1:56 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> Author: dblaikie
>> Date: Fri Apr  4 15:56:17 2014
>> New Revision: 205651
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=205651&view=rev
>> Log:
>> DebugInfo: PR19298: function local const variables duplicated in the root scope
>>
>> See the comment for CodeGenFunction::tryEmitAsConstant that describes
>> how in some contexts (lambdas) we must not emit references to the
>> variable, but instead use the constant directly - because of this we end
>> up emitting a constant for the variable, as well as emitting the
>> variable itself.
>>
>> Should we just skip putting the variable on the stack at all and omit
>> the debug info for the constant? It's not clear to me - what if the
>> address of the local is taken?
>>
>> Modified:
>>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>     cfe/trunk/test/CodeGenCXX/debug-info.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205651&r1=205650&r2=205651&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Apr  4 15:56:17 2014
>> @@ -3227,6 +3227,9 @@ void CGDebugInfo::EmitGlobalVariable(con
>>    // Do not use DIGlobalVariable for enums.
>>    if (Ty.getTag() == llvm::dwarf::DW_TAG_enumeration_type)
>>      return;
>> +  // Do not emit separate definitions for function local const/statics.
>> +  if (isa<FunctionDecl>(VD->getDeclContext()))
>> +    return;
>>    llvm::DIGlobalVariable GV = DBuilder.createStaticVariable(
>>        Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init,
>>        getOrCreateStaticDataMemberDeclarationOrNull(cast<VarDecl>(VD)));
>>
>> Modified: cfe/trunk/test/CodeGenCXX/debug-info.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info.cpp?rev=205651&r1=205650&r2=205651&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/debug-info.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/debug-info.cpp Fri Apr  4 15:56:17 2014
>> @@ -51,11 +51,6 @@ namespace VirtualBase {
>>    }
>>  }
>>
>> -void foo() {
>> -  const wchar_t c = L'x';
>> -  wchar_t d = c;
>> -}
>> -
>>  namespace b5249287 {
>>  template <typename T> class A {
>>    struct B;
>> @@ -88,6 +83,13 @@ foo func(foo f) {
>>  // CHECK: [[FUNC:![0-9]*]] = {{.*}} metadata !"_ZN7pr147634funcENS_3fooE", i32 {{[0-9]*}}, metadata [[FUNC_TYPE:![0-9]*]], {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func]
>>  }
>>
>> +void foo() {
>> +  const wchar_t c = L'x';
>> +  wchar_t d = c;
>> +}
>> +
>> +// CHECK-NOT: ; [ DW_TAG_variable ] [c]
>> +
>>  namespace pr9608 { // also pr9600
>>  struct incomplete;
>>  incomplete (*x)[3];
>> @@ -96,9 +98,11 @@ incomplete (*x)[3];
>>  // CHECK: [[INCARRAY]] = {{.*}}metadata !"_ZTSN6pr960810incompleteE", metadata {{![0-9]*}}, i32 0, null, null, null} ; [ DW_TAG_array_type ] [line 0, size 0, align 0, offset 0] [from _ZTSN6pr960810incompleteE]
>>  }
>>
>> -// For some reason the argument for PR14763 ended up all the way down here
>> +// For some reason function arguments ended up down here
>>  // CHECK: = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]], {{.*}}, metadata !"[[FOO]]", i32 8192, i32 0} ; [ DW_TAG_arg_variable ] [f]
>>
>> +// CHECK: ; [ DW_TAG_auto_variable ] [c]
>> +
>>  namespace pr16214 {
>>  struct a {
>>    int i;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits