[PATCH] D47097: [WIP][DebugInfo] Preserve scope in auto generated StoreInst

2018-05-21 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:2062
 EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+ApplyDebugLocation::CreateArtificial(*this);
+  }

There are two issues here:
1) ApplyDebugLocation is a RAII helper, which means that it installs the proper 
debug location in its constructor, and restores the old debug location in its 
destructor. Since you are not assigning the result of the static constructor 
(CreateArtificial) to anything, the ApplyDebugLocation instance is immediately 
destroyed, so it's a no-op.
2) This is being applied in the wrong place, because it sets a debug location 
*after* the store is emitted. The right location needs to be applied before the 
store or alloca are emitted.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47097: [WIP][DebugInfo] Preserve scope in auto generated StoreInst

2018-05-21 Thread Anastasis via Phabricator via cfe-commits 
gramanas updated this revision to Diff 147840.
gramanas marked an inline comment as done.
gramanas added a comment.

Update according to the comments


Repository:
  rC Clang

https://reviews.llvm.org/D47097

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/debug-info-preserve-scope.c


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+
+// CHECK: ![[dbgLocForStore]] = !DILocation(line: 0
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -2057,8 +2057,10 @@
   }
 
   // Store the initial value into the alloca.
-  if (DoStore)
+  if (DoStore) {
 EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+ApplyDebugLocation::CreateArtificial(*this);
+  }
 
   setAddrOfLocalVar(, DeclPtr);
 


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+
+// CHECK: ![[dbgLocForStore]] = !DILocation(line: 0
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -2057,8 +2057,10 @@
   }
 
   // Store the initial value into the alloca.
-  if (DoStore)
+  if (DoStore) {
 EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+ApplyDebugLocation::CreateArtificial(*this);
+  }
 
   setAddrOfLocalVar(, DeclPtr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47097: [WIP][DebugInfo] Preserve scope in auto generated StoreInst

2018-05-21 Thread Anastasis via Phabricator via cfe-commits 
gramanas updated this revision to Diff 147838.
gramanas added a comment.

- Apply debug location


Repository:
  rC Clang

https://reviews.llvm.org/D47097

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/debug-info-preserve-scope.c


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck 
%s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -2057,8 +2057,10 @@
   }
 
   // Store the initial value into the alloca.
-  if (DoStore)
+  if (DoStore) {
 EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+ApplyDebugLocation::CreateArtificial(*this);
+  }
 
   setAddrOfLocalVar(, DeclPtr);
 


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -2057,8 +2057,10 @@
   }
 
   // Store the initial value into the alloca.
-  if (DoStore)
+  if (DoStore) {
 EmitStoreOfScalar(ArgVal, lv, /* isInitialization */ true);
+ApplyDebugLocation::CreateArtificial(*this);
+  }
 
   setAddrOfLocalVar(, DeclPtr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47097: [WIP][DebugInfo] Preserve scope in auto generated StoreInst

2018-05-21 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

I think CodeGenFunction::EmitParmDecl is the right place to set up an 
ApplyDebugLocation instance. You can look at CodeGenFunction::EmitAutoVarInit 
for an example of how to use ApplyDebugLocation.




Comment at: test/CodeGen/debug-info-preserve-scope.c:1
+// RUN: %clang_cc1 -O0 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck 
%s
+

The -O0 flag isn't needed here.



Comment at: test/CodeGen/debug-info-preserve-scope.c:11
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+

To check that we set the right location on the store, you might add:
`; CHECK: ![[dbgLocForStore]] = !DILocation(line: 0`


Repository:
  rC Clang

https://reviews.llvm.org/D47097



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47097: [WIP][DebugInfo] Preserve scope in auto generated StoreInst

2018-05-18 Thread Anastasis via Phabricator via cfe-commits 
gramanas created this revision.
gramanas added a reviewer: vsk.
Herald added subscribers: JDevlieghere, aprantl.

In this test there is a store instruction generated by clang
for the function argument `int b` where the debug info is missing.

The goal of this patch is to instruct clang to add an artificial
location to auto generated store instructions using
ApplyDebugLocation::CreateArtificial() so that we can
preserve the scope information of the dbg metadata.


Repository:
  rC Clang

https://reviews.llvm.org/D47097

Files:
  test/CodeGen/debug-info-preserve-scope.c


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck 
%s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -O0 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 %b, i32* %b.addr, align 4, !dbg ![[dbgLocForStore:[0-9]+]]
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits