[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-11 Thread Bob Haarman via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312965: [codeview] omit debug locations for nested exprs 
unless column info enabled (authored by inglorion).

Changed prior to commit:
  https://reviews.llvm.org/D37529?vs=114715&id=114716#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37529

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.h
  cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -3553,6 +3553,10 @@
   return ConstantAddress(GV, Alignment);
 }
 
+bool CodeGenModule::getExpressionLocationsEnabled() const {
+  return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+}
+
 QualType CodeGenModule::getObjCFastEnumerationStateType() {
   if (ObjCFastEnumerationStateType.isNull()) {
 RecordDecl *D = Context.buildImplicitRecord("__objcFastEnumerationState");
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -97,6 +97,10 @@
   }
 
   OriginalLocation = CGF->Builder.getCurrentDebugLocation();
+
+  if (OriginalLocation && !DI->CGM.getExpressionLocationsEnabled())
+return;
+
   if (TemporaryLocation.isValid()) {
 DI->EmitLocation(CGF->Builder, TemporaryLocation);
 return;
Index: cfe/trunk/lib/CodeGen/CodeGenModule.h
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.h
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h
@@ -513,6 +513,9 @@
   /// Finalize LLVM code generation.
   void Release();
 
+  /// Return true if we should emit location information for expressions.
+  bool getExpressionLocationsEnabled() const;
+
   /// Return a reference to the configured Objective-C runtime.
   CGObjCRuntime &getObjCRuntime() {
 if (!ObjCRuntime) createObjCRuntime();
Index: cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
+++ cfe/trunk/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+  static Foo create();
+  void func();
+  int *begin();
+  int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+  // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+  // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+  int i = 1, b = 0, c = 0;
+  // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+  while (i > 0) {
+b = bar(a, b);
+--i;
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-11 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114715.
inglorion added a comment.

renamed get{Nested,}ExpressionLocationsEnabled and moved it into 
CodeGetModule.cpp


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+  static Foo create();
+  void func();
+  int *begin();
+  int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+  // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+  // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+  int i = 1, b = 0, c = 0;
+  // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+  while (i > 0) {
+b = bar(a, b);
+--i;
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+
+  for (i = 0; i < 1; i++) {
+b = bar(a, b);
+c = qux(a, c);
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+
+  if (a < b) {
+int t = a;
+a = b;
+b = t;
+  }
+  // NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+
+  int d = onearg(
+  noargs());
+  // NONEST: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DECLD:[0-9]+]]
+  // NONEST: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
+  // NESTED: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DNOARGS:[0-9]+]]
+  // NESTED: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD:[0-9]+]]
+  // N

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-11 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.h:32
 #include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"

majnemer wrote:
> Any reason to do this? I'd just keep getNestedExpressionLocationsEnabled in 
> the .cpp file.
+1, I think we can continue to expect that ApplyDebugLocation will be used for 
expression emission and CodeGenFunction::EmitStopPoint (or whatever we rename 
it to) will be called for statements in CGStmt.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-11 Thread David Majnemer via Phabricator via cfe-commits 
majnemer added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.h:32
 #include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"

Any reason to do this? I'd just keep getNestedExpressionLocationsEnabled in the 
.cpp file.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-11 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/lib/CodeGen/CodeGenModule.h:517
+  /// Return true if we should emit location information for nested 
expressions.
+  bool getNestedExpressionLocationsEnabled() const {
+return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;

I'd drop the "nested" part of this name. Statements are also nested, in a 
sense. A for loop is a statement, and it has child statements. This is really 
about expression locations vs. statement expressions.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-08 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114465.
inglorion added a comment.

Of course, ApplyDebugLocation is also a perfectly legitimate way to add a debug 
location to nodes that are not nested inside nodes that already have a 
location. I updated the diff so that we do end up applying the location in such 
cases.


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+  static Foo create();
+  void func();
+  int *begin();
+  int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+  // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+  // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+  int i = 1, b = 0, c = 0;
+  // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+  while (i > 0) {
+b = bar(a, b);
+--i;
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+
+  for (i = 0; i < 1; i++) {
+b = bar(a, b);
+c = qux(a, c);
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+
+  if (a < b) {
+int t = a;
+a = b;
+b = t;
+  }
+  // NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+
+  int d = onearg(
+  noargs());
+  // NONEST: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DECLD:[0-9]+]]
+  // NONEST: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
+  // NESTED: call i32 @

Re: [PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-08 Thread Zachary Turner via cfe-commits 
Well, if they worked I wasn't going to say we needed to add tests for them,
i just wanted to make sure they work before we move onto something else.
In any case, lgtm

On Fri, Sep 8, 2017 at 4:43 PM Bob Haarman via Phabricator <
revi...@reviews.llvm.org> wrote:

> inglorion updated this revision to Diff 114463.
> inglorion added a comment.
>
> added examples suggested by @zturner, verified step over and step into
> specific behavior matches MSVC, and added tests for them
>
>
> https://reviews.llvm.org/D37529
>
> Files:
>   clang/lib/CodeGen/CGDebugInfo.cpp
>   clang/lib/CodeGen/CodeGenModule.h
>   clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-08 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114463.
inglorion added a comment.

added examples suggested by @zturner, verified step over and step into specific 
behavior matches MSVC, and added tests for them


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-std=c++11 -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-std=c++11 -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+class Foo {
+public:
+  static Foo create();
+  void func();
+  int *begin();
+  int *end();
+};
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+int onearg(int x);
+int noargs();
+int noargs1();
+Foo range(int x);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+  // NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+  // NONEST: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[LOC]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // NESTED: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[BAR]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+  // COLUMNS: store i32 {{.*}}, i32* %a,{{.*}} !dbg ![[DECLA:[0-9]+]]
+
+  int i = 1, b = 0, c = 0;
+  // NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+  // NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+  // COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+  // COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+
+  while (i > 0) {
+b = bar(a, b);
+--i;
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+
+  for (i = 0; i < 1; i++) {
+b = bar(a, b);
+c = qux(a, c);
+  }
+  // NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+  // NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+  // COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+
+  if (a < b) {
+int t = a;
+a = b;
+b = t;
+  }
+  // NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+  // COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+
+  int d = onearg(
+  noargs());
+  // NONEST: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DECLD:[0-9]+]]
+  // NONEST: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD]]
+  // NONEST: store i32 %{{[^,]+}}, i32* %d,{{.*}} !dbg ![[DECLD]]
+  // NESTED: call i32 @{{.*}}noargs{{.*}}, !dbg ![[DNOARGS:[0-9]+]]
+  // NESTED: call i32 @{{.*}}onearg{{.*}}, !dbg ![[DECLD:[0-9]+]]

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-07 Thread Zachary Turner via Phabricator via cfe-commits 
zturner added a comment.

@inglorion : Since we're already in this code anyway, should we at least do due 
diligence and check the basics, even if only manually?  As in, at least we 
should check in MSVC if any of those other types of examples that I came up 
with present a problem.  Otherwise we're going to be back here fixing this 
again in a few weeks.  And it doesn't seem like it would be that hard to whip 
up some sample code with 10 or 12 cases and just do a sanity check that they 
all work.  If they don't, we can then decide whether it should be fixed in this 
patch, or a bug can be filed to track it.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-07 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114299.
inglorion added a comment.

removed compound statement; that was only in there to double check that the 
debugger stops on the first child statement, but that's true with or without 
this change


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// COLUMNS: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// COLUMNS: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// COLUMNS: ret i32 {{.*}}, !dbg !
+// COLUMNS: ![[BAR]] = !DILocation(
+// COLUMNS: ![[BAZ]] = !DILocation(
+// COLUMNS: ![[QUX]] = !DILocation(
+// COLUMNS: ![[ILOC]] = !DILocation(
+// COLUMNS: ![[BLOC]] = !DILocation(
+// COLUMNS: ![[CLOC]] = !DILocation(
+// COLUNMS: ![[RETSUB]] = !DILocation(
+// COLUMNS: ![[RETMUL]

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-07 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion added a comment.

@zturner: I am still thinking about your comment about other cases to test. My 
concern is that there are very many possible combinations.

I'm actually not too concerned about not exactly matching cl's behavior in 
every single case. The difference in behavior here is us emitting a debug 
location for an expression that doesn't get its own debug location from cl. In 
general, I think having more fine-grained information is good, so I don't think 
differing in this way is a problem. That is, unless we end up breaking 
functionality in the debugger (Visual Studio). The behavior I know of we can 
end up breaking this way is step into specific, which appears to require 
multiple calls that are associated with a single debug location.

I think, at a minimum, the test case should cover a scenario where we would 
normally like to emit some debug locations, but need to elide them if we want 
to be compatible with Visual Studio. I also think it makes sense to include 
some cases where we want the behavior to be the same whether or not we're 
targeting MS compatibility. That way, we can verify that we aren't throwing 
away too much information. Beyond that, I feel there are diminishing returns. 
To avoid going too far down that path, I would like to start with a relatively 
small test case (as I've done), fix the bug that prompted me to write this 
code, and then add additional tests if we find out there are other cases where 
people care strongly about the granularity of the debug locations we emit. Does 
that sound reasonable?


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-07 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114292.
inglorion added a comment.

Following conversation with @rnk, I managed to whittle this down to a very 
small change that seems to do what we need. Step into specific works and single 
stepping through the program behaves similarly whether compiled with clang-cl 
or cl.


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,115 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// COLUMNS: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// COLUMNS: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// COLUMNS: ret i32 {{.*}}, !dbg !
+// COLUMNS: ![[BAR]] = !DILocation(
+// COLUMNS: ![[BAZ]] = !DILocation(
+// COLUMNS: ![[QUX]] = !DILocation(
+// COLUMNS: ![[ILOC]] = !DILocation(
+// COLUMNS: ![[BLOC]] = !DILocation(
+// COLUMNS: ![[CLOC]] =

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion planned changes to this revision.
inglorion added a comment.

rnk and I talked about a different approach. The idea is to explicitly emit 
locations in some cases (e.g. inside compound statements, the braces of for 
loops, ...), and otherwise emit locations only when emitting column info or 
emitting non-codeview debug info. That may lead to more elegant code. I'll give 
it a try later.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114097.
inglorion marked 5 inline comments as done.
inglorion added a comment.

I limited the change to only calls, returns, and declarations. I also
updated the test case to include a multi-variable declaration, a while
loop, a for loop, and an if statement (after verifying the behavior in
the debugger, compared to MSVC). I discovered that there is a
difference between the generated info for DWARF with or without
-dwarf-column-info, so I included that in the test, too. I also made a
couple of minor changes that were suggested.


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Zachary Turner via Phabricator via cfe-commits 
zturner added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);

inglorion wrote:
> zturner wrote:
> > inglorion wrote:
> > > zturner wrote:
> > > > Can you make a function called `int foo()` and make this `int a = 
> > > > bar(foo(), y) + ...`
> > > Yes. Why? To test an additional level of nesting?
> > Yes.  for that matter, even better would be if this call to `foo()` spans 
> > multiple lines.  Right here you've got a single statement which spans 
> > multiple lines, but no individual sub-expression spans multiple lines.  And 
> > FWICT there is no nesting at all, since + is just a builtin operator.
> The nesting here is the calls to bar, baz, and qux inside the declaration of 
> a. The old behavior would emit separate locations for each of the calls, the 
> new behavior annotates each of the calls with the same location as the 
> declaration. This causes the debugger to stop only once for the entire 
> statement when using step over, and makes step into specific work.
Sure, but does that execute the same codepath as:

```
int a = bar(
  baz());

a = bar(baz());

for (const auto &X : foo(bar()))

for (int x = foo(); x < bar(); baz(x))

if (foo() && bar())

if (createObject().func())
```

etc?  And what if one or more of these functions get inlined?  For example, 
what if you have:

```
int a = foo(bar(baz()), bar(buzz()));
```



https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Zachary Turner via Phabricator via cfe-commits 
zturner added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.h:65
   llvm::MDNode *CurInlinedAt = nullptr;
+  bool LocationEnabled = true;
   llvm::DIType *VTablePtrType = nullptr;

Can you move this line up to put it next to another bool?  Not a huge deal, but 
might as well pack the class members.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);

Can you make a function called `int foo()` and make this `int a = bar(foo(), y) 
+ ...`


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);

zturner wrote:
> inglorion wrote:
> > zturner wrote:
> > > Can you make a function called `int foo()` and make this `int a = 
> > > bar(foo(), y) + ...`
> > Yes. Why? To test an additional level of nesting?
> Yes.  for that matter, even better would be if this call to `foo()` spans 
> multiple lines.  Right here you've got a single statement which spans 
> multiple lines, but no individual sub-expression spans multiple lines.  And 
> FWICT there is no nesting at all, since + is just a builtin operator.
The nesting here is the calls to bar, baz, and qux inside the declaration of a. 
The old behavior would emit separate locations for each of the calls, the new 
behavior annotates each of the calls with the same location as the declaration. 
This causes the debugger to stop only once for the entire statement when using 
step over, and makes step into specific work.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion created this revision.
Herald added a subscriber: aprantl.

Microsoft Visual Studio expects debug locations to correspond to
statements. We used to emit locations for expressions nested inside statements.
This would confuse the debugger, causing it to stop multiple times on the
same line and breaking the "step into specific" feature. This change inhibits
the emission of debug locations for nested expressions when emitting CodeView
debug information, unless column information is enabled.

Fixes PR34312.


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: store i32 {{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);
+  return a -
+ (y * z);
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -29,6 +29,7 @@
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SanitizerBlacklist.h"
 #include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -70,7 +71,6 @@
 class ValueDecl;
 class VarDecl;
 class LangOptions;
-class CodeGenOptions;
 class HeaderSearchOptions;
 class PreprocessorOptions;
 class DiagnosticsEngine;
@@ -513,6 +513,11 @@
   /// Finalize LLVM code generation.
   void Release();
 
+  /// Return true if we should emit location information for nested expressions.
+  bool getNestedExpressionLocationsEnabled() const {
+return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+  }
+
   /// Return a reference to the configured Objective-C runtime.
   CGObjCRuntime &getObjCRuntime() {
 if (!ObjCRuntime) createObjCRuntime();
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -87,6 +87,7 @@
 class BlockByrefInfo;
 class BlockFlags;
 class BlockFieldFlags;
+class InhibitDebugLocation;
 class RegionCodeGenTy;
 class TargetCodeGenInfo;
 struct OMPTaskDataTy;
@@ -2525,6 +2526,14 @@
   // Statement Emission
   //======//
 
+  /// EmitStmtStopPoint - Emit a statement stoppoint if we are emitting debug
+  /// info.
+  /// If getNestedExpressionLocationsEnabled() returns false, this also inhibits
+  /// the emission of debug location information until the returned
+  /// InhibitDebugLocation
+  /// object is destroyed.
+  InhibitDebugLocation EmitStmtStopPoint(const Stmt *S);
+
   /// EmitStopPoint - Emit a debug stoppoint if we are emitting debug info.
   void EmitStopPoint(const Stmt *S);
 
Index: clang/lib/CodeGen/CGStmt.cpp
==

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+  }
+  return IDL;
+}

inglorion wrote:
> rnk wrote:
> > Does MSVC accept this? I think it will emit the copy ctor call in an -O0 
> > build.
> I wrote this thinking that the right thing would happen under copy elision 
> (there is only one object, move constructor isn't called, and the destructor 
> only runs once) and without copy elision (there are two objects, move 
> constructor is called, destructor is run for both objects but is a no-op for 
> the moved-from object). If that's not the case, how would you rewrite this to 
> do the right thing?
FWIW, the test passes with MSVC in a Debug build, too.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Zachary Turner via Phabricator via cfe-commits 
zturner added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);

inglorion wrote:
> zturner wrote:
> > Can you make a function called `int foo()` and make this `int a = 
> > bar(foo(), y) + ...`
> Yes. Why? To test an additional level of nesting?
Yes.  for that matter, even better would be if this call to `foo()` spans 
multiple lines.  Right here you've got a single statement which spans multiple 
lines, but no individual sub-expression spans multiple lines.  And FWICT there 
is no nesting at all, since + is just a builtin operator.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]

inglorion wrote:
> rnk wrote:
> > This is pretty painful to test. :(
> > 
> > If we let ourselves do an asm test, .cv_loc is printed with some nice 
> > assembly comments that make this testing easy.
> Would you like me to write an asm test in addition to / instead of this one?
Oops, I thought I wrote this comment and deleted it before hitting send, but 
apparently not. No, let's stick with the IR test.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a subscriber: echristo.
rnk added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:38
 
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+  InhibitDebugLocation IDL;

"Stop point" is a hold-over from the llvm.dbg.stoppoint, which doesn't exist 
anymore:
http://releases.llvm.org/2.6/docs/SourceLevelDebugging.html#format_common_stoppoint

@echristo renamed CGDebugInfo::EmitStopPoint to EmitLocation long ago, and we 
should do the same for CodeGenFunction::EmitStopPoint. I'd like to have 
something like `EmitStmtLocation` and `EmitExprLocation`.



Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+  }
+  return IDL;
+}

Does MSVC accept this? I think it will emit the copy ctor call in an -O0 build.



Comment at: clang/lib/CodeGen/CGStmt.cpp:145
 
   case Stmt::IfStmtClass:   EmitIfStmt(cast(*S)); 
break;
   case Stmt::WhileStmtClass:EmitWhileStmt(cast(*S));   
break;

Doesn't this end up recursing? Won't InhibitDebugLocation prevent us from 
applying the inner statement locations?



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]

This is pretty painful to test. :(

If we let ourselves do an asm test, .cv_loc is printed with some nice assembly 
comments that make this testing easy.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:48
+ (y * z);
+}

I'd like to see some compound statement tests: for, while, if, regular braces, 
etc.


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:45
+  }
+  return IDL;
+}

rnk wrote:
> Does MSVC accept this? I think it will emit the copy ctor call in an -O0 
> build.
I wrote this thinking that the right thing would happen under copy elision 
(there is only one object, move constructor isn't called, and the destructor 
only runs once) and without copy elision (there are two objects, move 
constructor is called, destructor is run for both objects but is a no-op for 
the moved-from object). If that's not the case, how would you rewrite this to 
do the right thing?



Comment at: clang/lib/CodeGen/CGStmt.cpp:145
 
   case Stmt::IfStmtClass:   EmitIfStmt(cast(*S)); 
break;
   case Stmt::WhileStmtClass:EmitWhileStmt(cast(*S));   
break;

rnk wrote:
> Doesn't this end up recursing? Won't InhibitDebugLocation prevent us from 
> applying the inner statement locations?
Yeah, this looks wrong. Let me get back to you with fixed code or an 
explanation of why it actually does the right thing.



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]

rnk wrote:
> This is pretty painful to test. :(
> 
> If we let ourselves do an asm test, .cv_loc is printed with some nice 
> assembly comments that make this testing easy.
Would you like me to write an asm test in addition to / instead of this one?



Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:44
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);

zturner wrote:
> Can you make a function called `int foo()` and make this `int a = bar(foo(), 
> y) + ...`
Yes. Why? To test an additional level of nesting?


https://reviews.llvm.org/D37529



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

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

2017-09-06 Thread Bob Haarman via Phabricator via cfe-commits 
inglorion updated this revision to Diff 114060.
inglorion added a comment.

removed accidentally left in include and reformatted mangled comment


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:-gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:-dwarf-column-info -emit-llvm -o - %s \
+// RUN:| FileCheck -check-prefix=NESTED %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: store i32 {{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+  baz(x, z) +
+  qux(y, z);
+  return a -
+ (y * z);
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -29,6 +29,7 @@
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SanitizerBlacklist.h"
 #include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -70,7 +71,6 @@
 class ValueDecl;
 class VarDecl;
 class LangOptions;
-class CodeGenOptions;
 class HeaderSearchOptions;
 class PreprocessorOptions;
 class DiagnosticsEngine;
@@ -513,6 +513,11 @@
   /// Finalize LLVM code generation.
   void Release();
 
+  /// Return true if we should emit location information for nested expressions.
+  bool getNestedExpressionLocationsEnabled() const {
+return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+  }
+
   /// Return a reference to the configured Objective-C runtime.
   CGObjCRuntime &getObjCRuntime() {
 if (!ObjCRuntime) createObjCRuntime();
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -87,6 +87,7 @@
 class BlockByrefInfo;
 class BlockFlags;
 class BlockFieldFlags;
+class InhibitDebugLocation;
 class RegionCodeGenTy;
 class TargetCodeGenInfo;
 struct OMPTaskDataTy;
@@ -2525,6 +2526,12 @@
   // Statement Emission
   //======//
 
+  /// EmitStmtStopPoint - Emit a statement stoppoint if we are emitting debug
+  /// info. If getNestedExpressionLocationsEnabled() returns false, this also
+  /// inhibits the emission of debug location information until the returned
+  /// InhibitDebugLocation object is destroyed.
+  InhibitDebugLocation EmitStmtStopPoint(const Stmt *S);
+
   /// EmitStopPoint - Emit a debug stoppoint if we are emitting debug info.
   void EmitStopPoint(const Stmt *S);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -35,8 +35,19 @@
 //  Statement Emission
 //===--===//
 
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+  InhibitDebugLocation IDL;
+  if (HaveInsertPoint()) {
+Emit