[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d8f9c244074: [clang] Add -fdebug-default-version for 
specifying the default DWARF version (authored by dblaikie).

Changed prior to commit:
  https://reviews.llvm.org/D69822?vs=228250=228287#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,44 @@
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, or you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+int main(void) {
+  return 0;
+}
+
+// NOCODEVIEW-NOT: -gcodeview
+// CODEVIEW: "-gcodeview"
+
+// NODEBUGINFO-NOT: -debug-info-kind=
+
+// DWARF2: "-dwarf-version=2"
+// DWARF3: "-dwarf-version=3"
+// DWARF4: "-dwarf-version=4"
+// DWARF5: "-dwarf-version=5"
+
+// NOCODEVIEW-NOT: -gcodeview
+// NODWARF4-NOT: -dwarf-version=4
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,21 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 228250.
cmtice added a comment.

Fix small typo in comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, or you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+int main (void) {
+  return 0;
+}
+
+// NOCODEVIEW-NOT: -gcodeview
+// CODEVIEW: "-gcodeview"
+
+// NODEBUGINFO-NOT: -debug-info-kind=
+
+// DWARF2: "-dwarf-version=2"
+// DWARF3: "-dwarf-version=3"
+// DWARF4: "-dwarf-version=4"
+// DWARF5: "-dwarf-version=5"
+
+// NOCODEVIEW-NOT: -gcodeview
+// NODWARF4-NOT: -dwarf-version=4
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 2)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)
+<< A->getAsString(Args) << A->getValue();
+  return Value;
+}
+
 void 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Sigh, one last typo.  I'm happy otherwise.




Comment at: clang/test/Driver/debug-default-version.c:11
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.

s/of you/or you/


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 228144.
cmtice added a comment.

Remove quotes from -NOT tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+int main (void) {
+  return 0;
+}
+
+// NOCODEVIEW-NOT: -gcodeview
+// CODEVIEW: "-gcodeview"
+
+// NODEBUGINFO-NOT: -debug-info-kind=
+
+// DWARF2: "-dwarf-version=2"
+// DWARF3: "-dwarf-version=3"
+// DWARF4: "-dwarf-version=4"
+// DWARF5: "-dwarf-version=5"
+
+// NOCODEVIEW-NOT: -gcodeview
+// NODWARF4-NOT: -dwarf-version=4
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 2)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)
+<< A->getAsString(Args) << A->getValue();
+  return Value;
+}
+
 void 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

LG after `// NOCODEVIEW-NOT: "-gcodeview"` and `// NODWARF4-NOT: 
"-dwarf-version=4"` are fixed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 228130.
cmtice added a comment.

Fix NODEBUGINFO-NOT test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+int main (void) {
+  return 0;
+}
+
+// NOCODEVIEW-NOT: "-gcodeview"
+// CODEVIEW: "-gcodeview"
+
+// NODEBUGINFO-NOT: -debug-info-kind=
+
+// DWARF2: "-dwarf-version=2"
+// DWARF3: "-dwarf-version=3"
+// DWARF4: "-dwarf-version=4"
+// DWARF5: "-dwarf-version=5"
+
+// NOCODEVIEW-NOT: "-gcodeview"
+// NODWARF4-NOT: "-dwarf-version=4"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 2)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)
+<< A->getAsString(Args) << A->getValue();
+  return Value;
+}
+
 void 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/Driver/debug-default-version.c:37
+
+// NODEBUGINFO-NOT: "-debug-info-kind="
+

Same issue as with the dwarf-version below.

It's probably easier to remove the "" (FileCheck arguments aren't quoted - 
these quotes are treated like any other character/as part of the match - so 
omitting them means FileCheck can match -debug-info-kind= no matter what comes 
before/after it, rather than specifically looking for a " immediately after the 
'='). (you could remove all the " if you like, I don't think they add a lot to 
the tests here)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 228115.
cmtice added a comment.

Remove period from help text. Fix tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+int main (void) {
+  return 0;
+}
+
+// NOCODEVIEW-NOT: "-gcodeview"
+// CODEVIEW: "-gcodeview"
+
+// NODEBUGINFO-NOT: "-debug-info-kind="
+
+// DWARF2: "-dwarf-version=2"
+// DWARF3: "-dwarf-version=3"
+// DWARF4: "-dwarf-version=4"
+// DWARF5: "-dwarf-version=5"
+
+// NOCODEVIEW-NOT: "-gcodeview"
+// NODWARF4-NOT: "-dwarf-version=4"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 2)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)
+<< A->getAsString(Args) << A->getValue();
+  return Value;
+}
+
 void 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/debug-default-version.c:66
+// NOCODEVIEW-NOT: "-gcodeview"
+// NODWARF-NOT: "-dwarf-version="
+// NODWARF-NOT: "-dwarf-version="

This is not correct. I suppose this to be `"-dwarf-version=`.

The pattern `"-dwarf-version="` does not match `"-dwarf-version=5"`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Just a couple of typos, and I'm happy.  I agree with the other reviewers on the 
last needed test tweaks.




Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, 
Group, HelpText<"Default DWARF version to use, if a -g option caused 
DWARF debug info to be produced.">;
 def fdebug_prefix_map_EQ

HelpText does not want a final period.



Comment at: clang/test/Driver/debug-default-version.c:10
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.

indirectly


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/debug-default-version.c:9-17
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -### -target x86_64-apple-macosx10.11 -fdebug-default-version=5 
-g -S -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-apple-darwin14 -g -fdebug-default-version=4 
-S -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF4
+
+// RUN: %clang -### -target powerpc-unknown-openbsd -fdebug-default-version=4 
-g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4

dblaikie wrote:
> I'd probably skip these tests since overriding the platform default is 
> already tested above (for the linux platform default) and I'm not sure 
> there's much value add by testing that this overrides each platform default 
> separately (given the platform default override is pretty orthogonal to which 
> platform - it'd be pretty hard to introduce a bug that would break for one 
> platform but not for all of them)
We may need -Werror in some or all of the tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good to me - but maybe give it a day to see if anyone else has further 
thoughts/that you've addressed their feedback too.




Comment at: clang/test/Driver/debug-default-version.c:9-17
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -### -target x86_64-apple-macosx10.11 -fdebug-default-version=5 
-g -S -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-apple-darwin14 -g -fdebug-default-version=4 
-S -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF4
+
+// RUN: %clang -### -target powerpc-unknown-openbsd -fdebug-default-version=4 
-g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4

I'd probably skip these tests since overriding the platform default is already 
tested above (for the linux platform default) and I'm not sure there's much 
value add by testing that this overrides each platform default separately 
(given the platform default override is pretty orthogonal to which platform - 
it'd be pretty hard to introduce a bug that would break for one platform but 
not for all of them)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 227977.
cmtice added a comment.

Rename flag to -fdebug-default-version; add help text, mentioning DWARF. Update 
tests to use -### and add assembler tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,67 @@
+// RUN: %clang -### -c -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -### -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF4
+
+// RUN: %clang -### -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -target i386-pc-solaris -fdebug-default-version=4 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -c -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -### -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -x assembler -c -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -x assembler -c -o - %s -isysroot %t 2>&1 | FileCheck %s --check-prefix=DWARF4
+
+// RUN: %clang -### -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -target i386-pc-solaris -fdebug-default-version=4 -g -x assembler -c -o - %s 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice marked an inline comment as done.
cmtice added a comment.

Yes, I will change the flag name back to debug-default-version, and add help 
text mentioning dwarf.  I'm in the process of trying to re-do the test cases as 
required (I'm a bit new to this so it's taking me a bit to figure this out).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdwarf_default_version: Joined<["-"], "fdwarf-default-version=">, 
Group;
 def fdebug_prefix_map_EQ

probinson wrote:
> Probably should have HelpText, maybe something like this:
> "The default DWARF version to use, if a -g option causes DWARF debug info to 
> be produced"
> (it's probably not helpful to talk about overriding the target's default 
> version, I think)
Yep, I'd expect/be fine with all these options having something about DWARF in 
their long-form description.

@cmtice - would you mind switching this back to the original name & adding the 
relevant help text?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6171
+
+  if (DwarfVersion == 0)
+DwarfVersion = ParseDwarfDefaultVersion(getToolChain(), Args);

MaskRay wrote:
>   lang=cpp
>   if (DwarfVersion == 0) {
> DwarfVersion = ParseDwarfDefaultVersion(getToolChain(), Args);
> if (DwarfVersion == 0)
>   DwarfVersion = getToolChain().GetDefaultDwarfVersion();
>   }
> 
> This part constructs a -cc1as command line. It is not covered by a test. Can 
> you add one?
Hi @MaskRay , your version would have the unclaimed-option problem.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, 
Group;
 def fdebug_prefix_map_EQ

dblaikie wrote:
> probinson wrote:
> > If this is specifically the default DWARF version, I think the word "dwarf" 
> > ought to be in the option name.
> Can we haggle over this a bit?
> 
> My thinking behind -fdebug-default-version was consistency with other DWARF 
> related flags:
> -fdebug-compilation-dir
> -fdebug-info-for-profiling
> -fdebug-macro
> -fdebug-types-section
> -fdebug-ranges-base-address
> -fdebug-prefix-map
> 
> 
> We do have some -fdwarf:
> -fdwarf2-cfi-asm
> -fdwarf-directory-asm
> -fdwarf-exceptions
> 
> So I'm personally inclined to sticking with -fdebug* as being all DWARF 
> related/consistent there.
> 
> Thoughts?
> 
If the HelpText mentions DWARF, I'm okay with an `-fdebug` prefix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 227954.
cmtice added a comment.

Make second call to ParseDwarfDefaultVersion unconditional (to match the first 
and avoid errors with -Werror).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/dwarf-default-version.c

Index: clang/test/Driver/dwarf-default-version.c
===
--- /dev/null
+++ clang/test/Driver/dwarf-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdwarf-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdwarf-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdwarf-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdwarf-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdwarf-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdwarf-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdwarf-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdwarf-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target i386-pc-solaris -fdwarf-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdwarf-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdwarf-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// DWARF2: !{i32 2, !"Dwarf Version", i32 2}
+// DWARF3: !{i32 2, !"Dwarf Version", i32 3}
+// DWARF4: !{i32 2, !"Dwarf Version", i32 4}
+// DWARF5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDwarfDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDwarfDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdwarf_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

The test is in the right place, now it needs to behave more like other driver 
tests.  Sorry if it feels like I'm whaling on you, but the driver is a bit of a 
peculiar beast with an atypical testing mode.  Taming it is harder than it 
looks.




Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdwarf_default_version: Joined<["-"], "fdwarf-default-version=">, 
Group;
 def fdebug_prefix_map_EQ

Probably should have HelpText, maybe something like this:
"The default DWARF version to use, if a -g option causes DWARF debug info to be 
produced"
(it's probably not helpful to talk about overriding the target's default 
version, I think)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6172
+  if (DwarfVersion == 0)
+DwarfVersion = ParseDwarfDefaultVersion(getToolChain(), Args);
+

This is the path for parsing assembler options (when feeding clang a .s file) 
and I believe as written, the combination `-fdwarf-default-debug=2 -gdwarf-3` 
will yield an unused-option warning.   So I'm pretty sure the test doesn't 
cover this path.
You can fake an assembler driver test in the current test file by using `-x 
assembler` (especially if you use the `-###` tactic I mention in the comments 
there), or you can add a second test file with a .s extension. 



Comment at: clang/test/Driver/dwarf-default-version.c:1
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdwarf-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF3

As a rule, driver tests should pass `-###` which causes the driver to print the 
command lines instead of running the commands.  Then the checks would examine 
the command lines constructed by the driver, to verify they say what you want.
In this case, you'd look at the emitted `-dwarf-version` option and make sure 
it's what you want, and/or look for the option that says to do codeview.
The result is a test that is a more focused specifically on driver behavior, 
and also runs a bit faster (not starting any subprocesses etc).



Comment at: clang/test/Driver/dwarf-default-version.c:10
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.

indirectly



Comment at: clang/test/Driver/dwarf-default-version.c:23
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.

s/of/or/


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

For the record, I double-checked and if we use the flag and don't check for it 
(i.e. if we move the parsing inside the EmitDwarf is True block) then -Werror 
does indeed complain about an unused command line argument.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/dwarf-default-version.c:1
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdwarf-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF3

`-emit-llvm` -> `-###`. @probinson mentioned that we just need to test -cc1 
options.

We can use the  following to check that codeview is not emitted.
```
// NOCODEVIEW-NOT: "-gcodeview"
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3237
 
+// If the user specified a default DWARF version, that takes precedence
+// over the platform default.

lang=cpp
if (DefaultDWARFVersion) {
  // If the user specified a default DWARF version, that takes precedence
  // over the platform default.
  DWARFVersion = DefaultDWARFVersion;
} else {
  // Start with the platform default DWARF version.
  DWARFVersion = TC.GetDefaultDwarfVersion();
  assert(DWARFVersion && "toolchain default DWARF version must be nonzero");
}




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6171
+
+  if (DwarfVersion == 0)
+DwarfVersion = ParseDwarfDefaultVersion(getToolChain(), Args);

  lang=cpp
  if (DwarfVersion == 0) {
DwarfVersion = ParseDwarfDefaultVersion(getToolChain(), Args);
if (DwarfVersion == 0)
  DwarfVersion = getToolChain().GetDefaultDwarfVersion();
  }

This part constructs a -cc1as command line. It is not covered by a test. Can 
you add one?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

dblaikie:  The code, as written, parses the dwarf default version flag, whether 
or not DWARFVersion is 0, i.e. whether or not a -gdwarf-N flag was passed.  It 
only USES the value if there's no overriding value.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, 
Group;
 def fdebug_prefix_map_EQ

probinson wrote:
> If this is specifically the default DWARF version, I think the word "dwarf" 
> ought to be in the option name.
Can we haggle over this a bit?

My thinking behind -fdebug-default-version was consistency with other DWARF 
related flags:
-fdebug-compilation-dir
-fdebug-info-for-profiling
-fdebug-macro
-fdebug-types-section
-fdebug-ranges-base-address
-fdebug-prefix-map


We do have some -fdwarf:
-fdwarf2-cfi-asm
-fdwarf-directory-asm
-fdwarf-exceptions

So I'm personally inclined to sticking with -fdebug* as being all DWARF 
related/consistent there.

Thoughts?




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174
+DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+
   if (DwarfVersion == 0)

cmtice wrote:
> dblaikie wrote:
> > probinson wrote:
> > > I have to say, this sequence is a whole lot easier to follow than what's 
> > > up in RenderDebugOptions.  It would be nice if they were both so easy to 
> > > understand.
> > > 
> > > Although it makes me wonder, does `-fdebug-default-version` go unclaimed 
> > > here if there's a `-gdwarf-2` on the command line?
> > Once the "else if (DefaultDWARFVersion != 0)" clause is removed, and the 
> > ParseDebugDefaultVersion call is sunk down into the "if (EmitDwarf)" case - 
> > hopefully that'll simplify the RenderDebugOptions code enough to be 
> > reasonable?
> > 
> > I think the complication there is that "-g" can imply CodeView if nothing 
> > explicitly requests DWARF & the target is windows, whereas there's no 
> > support for that kind of CV fallback here? So some of that might be 
> > inherent.
> > 
> > & agreed - worth testing that -fdebug-default-version doesn't get a 
> > -Wunused warning if someone says "-fdebug-default-version 4 -gdwarf-2". 
> > @cmtice is that tested? You might be able to add -Werror to the existing 
> > test case's RUN lines to demonstrate that no such warnings are produced?
> The thought is that if the user actually chose a specific dwarf version, e.g. 
> -gdwarf-2, then that value should override the general default, specified 
> with -fdebug-default-version (soon to be 
> -fdwarf-default-version).
That's the desired behavior, yes (-gdwarf-2 overrides this new flag) - what 
@probinson is asking about is the behavior of -Wunused.

Clang's built-in flag handling does some work to warn on unused flags - it does 
this based on which flags are queried by the API.

The risk is that, because ParseDwarfDefaultVersion is only called under the "if 
(DwarfVersion == 0)" condition, which will be false if the user specified 
-gdwarf-2, for instance, then the ParseDwarfDefaultVersion won't be called & 
clang's unused flag handling may see the new flag as "unused" and warn about it.

Basically the question is (could you test this manually & confirm the behavior, 
and check that the automated tests cover this - which I think they can cover 
this with minimal effort by adding -Werror to some RUN lines in the tests), 
does this produce a warning:
clang -gdwarf-2 -fdebug-default-version=5 x.s

(& if it doesn't warn, it might be worth understanding why it doesn't, I guess, 
given the above hypothesis)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3231
+  unsigned DWARFVersion = 0;
+  unsigned DefaultDWARFVersion = ParseDwarfDefaultVersion(TC, Args);
   if (EmitDwarf) {

Can you move this down to the narrower scope:

  if (unsigned DefaultDWARFVersion = ParseDefaultVersion(TC, Args))
DWARFVersion = DefaultDWARFVersion;

(but maybe that runs into the -Wunused warning issues discussed in the other 
thread in this review)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

Ok, I think the upload was correct this time.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 227942.
cmtice added a comment.

re-try upload again.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/dwarf-default-version.c

Index: clang/test/Driver/dwarf-default-version.c
===
--- /dev/null
+++ clang/test/Driver/dwarf-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdwarf-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdwarf-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdwarf-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdwarf-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-linux-gnu -fdwarf-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdwarf-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdwarf-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=DWARF4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdwarf-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdwarf-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -target i386-pc-solaris -fdwarf-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdwarf-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdwarf-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// DWARF2: !{i32 2, !"Dwarf Version", i32 2}
+// DWARF3: !{i32 2, !"Dwarf Version", i32 3}
+// DWARF4: !{i32 2, !"Dwarf Version", i32 4}
+// DWARF5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDwarfDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDwarfDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdwarf_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

Hmmm...I'm having upload issues.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250
   // -gline-directives-only supported only for the DWARF debug info.
   if (DWARFVersion == 0 && DebugInfoKind == 
codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::NoDebugInfo;

dblaikie wrote:
> probinson wrote:
> > After setting DWARFVersion above, when nothing was requested, this looks 
> > peculiar.  Is `DWARFVersion == 0` a proxy for `-gcodeview` ?
> Yep, DWARFVersion shouldn't be set when none was requested - see discussion 
> above between myself & @cmtice - that should leave this code fine/correct.
DWARFVersion is (will be) only set when the debug info kind is DWARF, i.e. when 
EmitDwarf is True. I apologize for the confusion -- David Blaikie and I has 
some misunderstandings about the original implementation intent.  DWARFVersion 
== 0 ought only to result in codeview being generated/used if the user 
specified -g and codeview is the default debug format for the platform.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174
+DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+
   if (DwarfVersion == 0)

dblaikie wrote:
> probinson wrote:
> > I have to say, this sequence is a whole lot easier to follow than what's up 
> > in RenderDebugOptions.  It would be nice if they were both so easy to 
> > understand.
> > 
> > Although it makes me wonder, does `-fdebug-default-version` go unclaimed 
> > here if there's a `-gdwarf-2` on the command line?
> Once the "else if (DefaultDWARFVersion != 0)" clause is removed, and the 
> ParseDebugDefaultVersion call is sunk down into the "if (EmitDwarf)" case - 
> hopefully that'll simplify the RenderDebugOptions code enough to be 
> reasonable?
> 
> I think the complication there is that "-g" can imply CodeView if nothing 
> explicitly requests DWARF & the target is windows, whereas there's no support 
> for that kind of CV fallback here? So some of that might be inherent.
> 
> & agreed - worth testing that -fdebug-default-version doesn't get a -Wunused 
> warning if someone says "-fdebug-default-version 4 -gdwarf-2". @cmtice is 
> that tested? You might be able to add -Werror to the existing test case's RUN 
> lines to demonstrate that no such warnings are produced?
The thought is that if the user actually chose a specific dwarf version, e.g. 
-gdwarf-2, then that value should override the general default, specified with 
-fdebug-default-version (soon to be 
-fdwarf-default-version).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 227940.
cmtice added a comment.

Try uploading correct diff file?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/CodeGen/debug-default-version.c

Index: clang/test/CodeGen/debug-default-version.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,VER4
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target i386-pc-solaris -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// VER2: !{i32 2, !"Dwarf Version", i32 2}
+// VER3: !{i32 2, !"Dwarf Version", i32 3}
+// VER4: !{i32 2, !"Dwarf Version", i32 4}
+// VER5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 1)
+

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D69822#1734255 , @cmtice wrote:

> Made requested changes:
>
> - renamed option to be dwarf-specific
> - fixed spelling & blank line issues
> - only set version if emit-dwarf is true
> - move test to Driver directory
>
>   I *think* I got it all done...


It seems you uploaded the wrong diff, probably the original one...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 227910.
cmtice marked 9 inline comments as done.
cmtice added a comment.

Made requested changes:

- renamed option to be dwarf-specific
- fixed spelling & blank line issues
- only set version if emit-dwarf is true
- move test to Driver directory

I *think* I got it all done...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/CodeGen/debug-default-version.c

Index: clang/test/CodeGen/debug-default-version.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,VER4
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target i386-pc-solaris -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// VER2: !{i32 2, !"Dwarf Version", i32 2}
+// VER3: !{i32 2, !"Dwarf Version", i32 3}
+// VER4: !{i32 2, !"Dwarf Version", i32 4}
+// VER5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151
+  || Value > 5
+  || Value < 1)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)

dblaikie wrote:
> probinson wrote:
> > Clang doesn't support DWARF v1.  I expect nobody does at this point (DWARF 
> > 2 came out in 1993).
> You'd recommend/request moving this bound up to 2 (to be consistent with the 
> fact that clang supports -gdwarf-2 technically (even though we probably don't 
> produce fully conformant DWARFv2 these days, I would guess))?
It wants to permit the exact same range as the -gdwarf-N options.



Comment at: clang/test/CodeGen/debug-default-version.c:24
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf 
-S -emit-llvm -o - %s \

dblaikie wrote:
> probinson wrote:
> > Does that actually work?  Last I checked, DWARF and COFF didn't play 
> > nicely, but I admit that was quite a while ago.
> Existing test cases cover scenarios like this (see 
> clang/test/CodeGen/dwarf-version.c), so this seems consistent to preserve 
> that sort of functionality, however well it currently works.
Ok.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250
   // -gline-directives-only supported only for the DWARF debug info.
   if (DWARFVersion == 0 && DebugInfoKind == 
codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::NoDebugInfo;

probinson wrote:
> After setting DWARFVersion above, when nothing was requested, this looks 
> peculiar.  Is `DWARFVersion == 0` a proxy for `-gcodeview` ?
Yep, DWARFVersion shouldn't be set when none was requested - see discussion 
above between myself & @cmtice - that should leave this code fine/correct.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174
+DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+
   if (DwarfVersion == 0)

probinson wrote:
> I have to say, this sequence is a whole lot easier to follow than what's up 
> in RenderDebugOptions.  It would be nice if they were both so easy to 
> understand.
> 
> Although it makes me wonder, does `-fdebug-default-version` go unclaimed here 
> if there's a `-gdwarf-2` on the command line?
Once the "else if (DefaultDWARFVersion != 0)" clause is removed, and the 
ParseDebugDefaultVersion call is sunk down into the "if (EmitDwarf)" case - 
hopefully that'll simplify the RenderDebugOptions code enough to be reasonable?

I think the complication there is that "-g" can imply CodeView if nothing 
explicitly requests DWARF & the target is windows, whereas there's no support 
for that kind of CV fallback here? So some of that might be inherent.

& agreed - worth testing that -fdebug-default-version doesn't get a -Wunused 
warning if someone says "-fdebug-default-version 4 -gdwarf-2". @cmtice is that 
tested? You might be able to add -Werror to the existing test case's RUN lines 
to demonstrate that no such warnings are produced?



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151
+  || Value > 5
+  || Value < 1)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)

probinson wrote:
> Clang doesn't support DWARF v1.  I expect nobody does at this point (DWARF 2 
> came out in 1993).
You'd recommend/request moving this bound up to 2 (to be consistent with the 
fact that clang supports -gdwarf-2 technically (even though we probably don't 
produce fully conformant DWARFv2 these days, I would guess))?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D69822#1733269 , @dblaikie wrote:

> In D69822#1733262 , @probinson wrote:
>
> > The maze of twisty -g passages gets a new secret door.  Oh well.
>
>
> If you find this to be a significant complication, I'd really like to discuss 
> it further - I know there are some twisty things (one of the worst I created 
> when it comes to -fsplit-dwarf-inlining, -gsplit-dwarf, -gmlt combinations) - 
> but this seems pretty simple/tidy/orthogonal to me.


No, no, if we can at least keep the twistiness in one place (or two I suppose, 
one for C/C++ and one for asm) it's not a real problem.

Maybe a strategic helper function could be useful to make those two places more 
obviously the same.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, 
Group;
 def fdebug_prefix_map_EQ

If this is specifically the default DWARF version, I think the word "dwarf" 
ought to be in the option name.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3148
   // decision should be made in the driver as well though.
+  unsigned DefaultDWARFVersion = ParseDebugDefaultVersion(TC, Args);
   unsigned DWARFVersion = 0;

I would rather see all the DWARF version weirdness all in one place (i.e., 
please move this down to the rest of it) particularly given that the "default" 
DWARF version now means two different things (something from the command line, 
and something based on the toolchain).



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250
   // -gline-directives-only supported only for the DWARF debug info.
   if (DWARFVersion == 0 && DebugInfoKind == 
codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::NoDebugInfo;

After setting DWARFVersion above, when nothing was requested, this looks 
peculiar.  Is `DWARFVersion == 0` a proxy for `-gcodeview` ?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174
+DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+
   if (DwarfVersion == 0)

I have to say, this sequence is a whole lot easier to follow than what's up in 
RenderDebugOptions.  It would be nice if they were both so easy to understand.

Although it makes me wonder, does `-fdebug-default-version` go unclaimed here 
if there's a `-gdwarf-2` on the command line?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69822#1733262 , @probinson wrote:

> In D69822#1733243 , @dblaikie wrote:
>
> > In D69822#1733226 , @probinson 
> > wrote:
> >
> > > > Want to decouple setting the DWARF version from enabling/disabling 
> > > > generation of debug info.
> > >
> > > Um, why?
> >
> >
> > If you've got a big build system with various users opting in/out of DWARF 
> > and you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to 
> > your build system, because that'd turn on debug info in cases that don't 
> > need it - but it's easier to change the default than to modify all cases 
> > that enable dwarf across the codebase.
> >
> > Open to discussion about whether this is a good/bad idea - but it'd be 
> > useful for Google at least & seemed low-cost enough to go this route.
>
>
> Because you want the default to be based on your corporate environment rather 
> than the target platform.


Sure - if we know our users have access to a modern debugger, but we don't own 
a whole platform. You could imagine someone shipping clang+debugger in a 3rd 
party/non-platform IDE might want something like this too/similar to the Google 
situation. We can't dictate what's suitable for all of Linux, but for the 
subset of users that are using a specific toolchain/situation, we can.

> The maze of twisty -g passages gets a new secret door.  Oh well.

If you find this to be a significant complication, I'd really like to discuss 
it further - I know there are some twisty things (one of the worst I created 
when it comes to -fsplit-dwarf-inlining, -gsplit-dwarf, -gmlt combinations) - 
but this seems pretty simple/tidy/orthogonal to me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D69822#1733243 , @dblaikie wrote:

> In D69822#1733226 , @probinson wrote:
>
> > > Want to decouple setting the DWARF version from enabling/disabling 
> > > generation of debug info.
> >
> > Um, why?
>
>
> If you've got a big build system with various users opting in/out of DWARF 
> and you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to 
> your build system, because that'd turn on debug info in cases that don't need 
> it - but it's easier to change the default than to modify all cases that 
> enable dwarf across the codebase.
>
> Open to discussion about whether this is a good/bad idea - but it'd be useful 
> for Google at least & seemed low-cost enough to go this route.


Because you want the default to be based on your corporate environment rather 
than the target platform.  The maze of twisty -g passages gets a new secret 
door.  Oh well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69822#1733258 , @aprantl wrote:

> Since this sounds like it is hidden inside of some other tooling — is passing 
> the frontend option `-dwarf-version=5` not an option?


"hidden inside of some other tooling" - in the same sense as most build flags 
are passed by a build system, yes. But generally we (Google specifically, all 
of us LLVM/Clang users in general) try not to use implementation details like 
that.




Comment at: clang/test/CodeGen/debug-default-version.c:24
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf 
-S -emit-llvm -o - %s \

probinson wrote:
> Does that actually work?  Last I checked, DWARF and COFF didn't play nicely, 
> but I admit that was quite a while ago.
Existing test cases cover scenarios like this (see 
clang/test/CodeGen/dwarf-version.c), so this seems consistent to preserve that 
sort of functionality, however well it currently works.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Since this sounds like it is hidden inside of some other tooling — is passing 
the frontend option `-dwarf-version=5` not an option?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2002
 def gno_codeview_ghash : Flag<["-"], "gno-codeview-ghash">, 
Flags<[CoreOption]>;
+
 def ginline_line_tables : Flag<["-"], "ginline-line-tables">, 
Flags<[CoreOption]>;

Unexpected blank line here.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3237
 
+// If the user specified a default DWARF version, that takes precedent over
+// the platform default.

precedence



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151
+  || Value > 5
+  || Value < 1)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)

Clang doesn't support DWARF v1.  I expect nobody does at this point (DWARF 2 
came out in 1993).



Comment at: clang/test/CodeGen/debug-default-version.c:1
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER3

MaskRay wrote:
> This looks like a clangDriver change. Maybe move the test to test/Driver ?
+1.  It should be sufficient to use `-###` and check what gets passed as cc1 
options.



Comment at: clang/test/CodeGen/debug-default-version.c:24
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf 
-S -emit-llvm -o - %s \

Does that actually work?  Last I checked, DWARF and COFF didn't play nicely, 
but I admit that was quite a while ago.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69822#1733226 , @probinson wrote:

> > Want to decouple setting the DWARF version from enabling/disabling 
> > generation of debug info.
>
> Um, why?


If you've got a big build system with various users opting in/out of DWARF and 
you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to your 
build system, because that'd turn on debug info in cases that don't need it - 
but it's easier to change the default than to modify all cases that enable 
dwarf across the codebase.

Open to discussion about whether this is a good/bad idea - but it'd be useful 
for Google at least & seemed low-cost enough to go this route.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

> Want to decouple setting the DWARF version from enabling/disabling generation 
> of debug info.

Um, why?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;
 

cmtice wrote:
> dblaikie wrote:
> > Hmm, actually - why is this case necessary/what does it cover? I was hoping 
> > the case you added inside the "if (EmitDwarf)" case above would be all that 
> > was required (& the call to ParseDebugDefaultVersion would go inside there 
> > at the use, to reduce the variable scope/keep the code closer together).
> This covers the case where you are setting the default dwarf version without 
> actually turning on/off debug info at all (there's no -gdwarf or -g option, 
> so EmitDwarf is false).
Ah, perhaps I'm missing something - why should the DWARFVersion be set if 
EmitDwarf is false? No DWARF was requested, so the DWARFVersion probably 
shouldn't be set, I think?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 22.
cmtice added a comment.

Made requested formatting changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/CodeGen/debug-default-version.c

Index: clang/test/CodeGen/debug-default-version.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,VER4
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target i386-pc-solaris -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// VER2: !{i32 2, !"Dwarf Version", i32 2}
+// VER3: !{i32 2, !"Dwarf Version", i32 3}
+// VER4: !{i32 2, !"Dwarf Version", i32 4}
+// VER5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 1)
+

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGen/debug-default-version.c:2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER4

`VER3` -> `DWARF3` may be better, because you also check another debug info 
format codeview here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGen/debug-default-version.c:1
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER3

This looks like a clangDriver change. Maybe move the test to test/Driver ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
 DWARFVersion = ExplicitVersion;
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;

dblaikie wrote:
> Looks like this should be on a single line to conform to LLVM convention 
> (though might just be phabricator doing something weird)
> 
> If you can run clang-format over the change (not over the whole file) it 
> should fix up issues like this. (there's various clang-format editor 
> integrations - there's some google-internal documentation at go/clang-format 
> that'll explain how to setup an auto-save hook that'll clang-format the 
> changed lines so all your C++ code in the LLVM repository conforms to LLVM's 
> coding conventions (well, those that can be expressed by clang-format))
Oh, there's also clang/tools/clang-format/git-clang-format for formatting 
anything in a git revision range.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice marked 3 inline comments as done.
cmtice added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
 DWARFVersion = ExplicitVersion;
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;

dblaikie wrote:
> dblaikie wrote:
> > Looks like this should be on a single line to conform to LLVM convention 
> > (though might just be phabricator doing something weird)
> > 
> > If you can run clang-format over the change (not over the whole file) it 
> > should fix up issues like this. (there's various clang-format editor 
> > integrations - there's some google-internal documentation at 
> > go/clang-format that'll explain how to setup an auto-save hook that'll 
> > clang-format the changed lines so all your C++ code in the LLVM repository 
> > conforms to LLVM's coding conventions (well, those that can be expressed by 
> > clang-format))
> Oh, there's also clang/tools/clang-format/git-clang-format for formatting 
> anything in a git revision range.
Ok, will do.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;
 

dblaikie wrote:
> Hmm, actually - why is this case necessary/what does it cover? I was hoping 
> the case you added inside the "if (EmitDwarf)" case above would be all that 
> was required (& the call to ParseDebugDefaultVersion would go inside there at 
> the use, to reduce the variable scope/keep the code closer together).
This covers the case where you are setting the default dwarf version without 
actually turning on/off debug info at all (there's no -gdwarf or -g option, so 
EmitDwarf is false).



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6173-6175
+unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(), 
Args);
+DwarfVersion = DefaultDwarfVersion ? DefaultDwarfVersion
+   :getToolChain().GetDefaultDwarfVersion();

dblaikie wrote:
> Some clang-formatting required, but also could probably be written as:
> 
>   if (DwarfVersion == 0)
> DwarfVersion = ParseDebugDefaultVersion(...);
> 
>   if (DwarfVersion == 0)
> DwarfVersion = getToolChain().GetDefaultDwarfVersion();
> 
> To make these more symmetric?
Ok will do.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
 DWARFVersion = ExplicitVersion;
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;

Looks like this should be on a single line to conform to LLVM convention 
(though might just be phabricator doing something weird)

If you can run clang-format over the change (not over the whole file) it should 
fix up issues like this. (there's various clang-format editor integrations - 
there's some google-internal documentation at go/clang-format that'll explain 
how to setup an auto-save hook that'll clang-format the changed lines so all 
your C++ code in the LLVM repository conforms to LLVM's coding conventions 
(well, those that can be expressed by clang-format))



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;
 

Hmm, actually - why is this case necessary/what does it cover? I was hoping the 
case you added inside the "if (EmitDwarf)" case above would be all that was 
required (& the call to ParseDebugDefaultVersion would go inside there at the 
use, to reduce the variable scope/keep the code closer together).



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6173-6175
+unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(), 
Args);
+DwarfVersion = DefaultDwarfVersion ? DefaultDwarfVersion
+   :getToolChain().GetDefaultDwarfVersion();

Some clang-formatting required, but also could probably be written as:

  if (DwarfVersion == 0)
DwarfVersion = ParseDebugDefaultVersion(...);

  if (DwarfVersion == 0)
DwarfVersion = getToolChain().GetDefaultDwarfVersion();

To make these more symmetric?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69822/new/

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice created this revision.
cmtice added a reviewer: dblaikie.
Herald added subscribers: cfe-commits, fedor.sergeev, aprantl.
Herald added a project: clang.

Want to decouple setting the DWARF version from enabling/disabling generation 
of debug info.  This adds a new flag  '-fdebug-default-version=N' that sets the 
version of DWARF to be generated, but does not turn on/off debug info 
generation.


Repository:
  rC Clang

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/CodeGen/debug-default-version.c

Index: clang/test/CodeGen/debug-default-version.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,VER4
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target i386-pc-solaris -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// VER2: !{i32 2, !"Dwarf Version", i32 2}
+// VER3: !{i32 2, !"Dwarf Version", i32 3}
+// VER4: !{i32 2, !"Dwarf Version", i32 4}
+// VER5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+