[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

Thanks for the reply!

FWIW, this warning is triggered by code which uses glib, which contains a 
number of function(some_type*) to function(void*) casts, for example here: 
https://github.com/GNOME/glib/blob/339aaa3719757614af61f427b66a46893e6dc760/glib/glib-autocleanups.h#L47,
 which expands to function pointer casts here: 
https://github.com/GNOME/glib/blob/339aaa3719757614af61f427b66a46893e6dc760/glib/gmacros.h#L1360

I imagine glib is reasonably commonly used, so this might cause some build 
failures (we're seeing some in ChromiumOS). Not sure if that's enough to affect 
the choice of having this on or off by default when -Wcast-function-type is in 
effect, but I thought I would provide the data point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134831

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

[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

Is this intended to warn on code that casts a function taking a pointer to some 
non-void type to a function that takes a void*?

  void set(void (*g)(int*)) {
f = (void(*)(void*)) g;
  }

gives me

  warning: cast from 'void (*)(int *)' to 'void (*)(void *)' converts to 
incompatible function type [-Wcast-function-type-strict]

I didn't see this mentioned in the diff description, comments, or test. Is the 
behavior intentional? Are these types actually incompatible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134831

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

[PATCH] D108603: [clang][codegen] Set CurLinkModule in CodeGenAction::ExecuteAction

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c829ce1e362: [clang][codegen] Set CurLinkModule in 
CodeGenAction::ExecuteAction (authored by inglorion).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108603

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/linker-diagnostic1.ll
  clang/test/CodeGen/linker-diagnostic.ll
  clang/test/Misc/serialized-diags-driver.c

Index: clang/test/Misc/serialized-diags-driver.c
===
--- clang/test/Misc/serialized-diags-driver.c
+++ clang/test/Misc/serialized-diags-driver.c
@@ -8,7 +8,8 @@
 // RUN: %clang -Wx-typoed-warning -Wall -fsyntax-only --serialize-diagnostics %t.diag %s
 // RUN: c-index-test -read-diagnostics %t.diag 2>&1 | FileCheck %s
 
-// CHECK: warning: unknown warning option '-Wx-typoed-warning' [-Wunknown-warning-option] []
+// CHECK: warning: unknown warning option '-Wx-typoed-warning'
+// CHECK-SAME: [-Wunknown-warning-option] []
 
 // CHECK: warning: variable 'voodoo' is uninitialized when used here [-Wuninitialized]
 // CHECK: note: initialize the variable 'voodoo' to silence this warning []
Index: clang/test/CodeGen/linker-diagnostic.ll
===
--- /dev/null
+++ clang/test/CodeGen/linker-diagnostic.ll
@@ -0,0 +1,17 @@
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r %t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv4-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o -Wno-override-module 2>&1 | FileCheck %s
+
+; CHECK: linking module '{{.*}}/bar.o': Linking two modules of different target triples: '{{.*}}/foo.o' is 'thumbv6-unknown-linux-gnueabihf' whereas '{{.*}}/bar.o' is 'armv4-none-unknown-eabi'
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}
Index: clang/test/CodeGen/Inputs/linker-diagnostic1.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/linker-diagnostic1.ll
@@ -0,0 +1,9 @@
+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -160,7 +160,7 @@
 const PreprocessorOptions ,
 const CodeGenOptions ,
 const TargetOptions ,
-const LangOptions ,
+const LangOptions , llvm::Module *Module,
 SmallVector LinkModules, LLVMContext ,
 CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
@@ -170,7 +170,7 @@
   LLVMIRGenerationRefCount(0),
   Gen(CreateLLVMCodeGen(Diags, "", HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
-  LinkModules(std::move(LinkModules)) {
+  LinkModules(std::move(LinkModules)), CurLinkModule(Module) {
   TimerIsEnabled = CodeGenOpts.TimePasses;
   llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
   llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
@@ -779,11 +779,7 @@
 ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
 break;
   case DK_Linker:
-assert(CurLinkModule);
-// FIXME: stop eating the warnings and notes.
-if (Severity != DS_Error)
-  return;
-DiagID = diag::err_fe_cannot_link_module;
+ComputeDiagID(Severity, linking_module, DiagID);
 break;
   case llvm::DK_OptimizationRemark:
 // Optimization remarks are always handled completely by this
@@ -845,9 +841,9 @@
 DI.print(DP);
   }
 
-  if (DiagID == diag::err_fe_cannot_link_module) {
-Diags.Report(diag::err_fe_cannot_link_module)
-<< CurLinkModule->getModuleIdentifier() << MsgStorage;
+  if (DI.getKind() == DK_Linker) {
+assert(CurLinkModule && "CurLinkModule must be set for linker diagnostics");
+Diags.Report(DiagID) << CurLinkModule->getModuleIdentifier() << MsgStorage;
 return;
   }
 
@@ -1088,7 +1084,7 @@
   // BackendConsumer.
   BackendConsumer Result(BA, CI.getDiagnostics(), 

[PATCH] D108603: [clang][codegen] Set CurLinkModule in CodeGenAction::ExecuteAction

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
inglorion requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

CodeGenAction::ExecuteAction creates a BackendConsumer for the
purpose of handling diagnostics. The BackendConsumer's
DiagnosticHandlerImpl method expects CurLinkModule to be set,
but this did not happen on the code path that goes through
ExecuteAction. This change makes it so that the BackendConsumer
constructor used by ExecuteAction requires the Module to be
specified and passes the appropriate module in ExecuteAction.

The change also adds a test that fails without this change
and passes with it. To make the test work, the FIXME in the
handling of DK_Linker diagnostics was addressed so that warnings
and notes are no longer silently discarded. Since this introduces
a new warning diagnostic, a flag to control it (-Wlinker-warnings)
has also been added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108603

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/linker-diagnostic1.ll
  clang/test/CodeGen/linker-diagnostic.ll
  clang/test/Misc/serialized-diags-driver.c

Index: clang/test/Misc/serialized-diags-driver.c
===
--- clang/test/Misc/serialized-diags-driver.c
+++ clang/test/Misc/serialized-diags-driver.c
@@ -8,7 +8,8 @@
 // RUN: %clang -Wx-typoed-warning -Wall -fsyntax-only --serialize-diagnostics %t.diag %s
 // RUN: c-index-test -read-diagnostics %t.diag 2>&1 | FileCheck %s
 
-// CHECK: warning: unknown warning option '-Wx-typoed-warning' [-Wunknown-warning-option] []
+// CHECK: warning: unknown warning option '-Wx-typoed-warning'
+// CHECK-SAME: [-Wunknown-warning-option] []
 
 // CHECK: warning: variable 'voodoo' is uninitialized when used here [-Wuninitialized]
 // CHECK: note: initialize the variable 'voodoo' to silence this warning []
Index: clang/test/CodeGen/linker-diagnostic.ll
===
--- /dev/null
+++ clang/test/CodeGen/linker-diagnostic.ll
@@ -0,0 +1,17 @@
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r %t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv4-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o -Wno-override-module 2>&1 | FileCheck %s
+
+; CHECK: linking module '{{.*}}/bar.o': Linking two modules of different target triples: '{{.*}}/foo.o' is 'thumbv6-unknown-linux-gnueabihf' whereas '{{.*}}/bar.o' is 'armv4-none-unknown-eabi'
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}
Index: clang/test/CodeGen/Inputs/linker-diagnostic1.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/linker-diagnostic1.ll
@@ -0,0 +1,9 @@
+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -160,7 +160,7 @@
 const PreprocessorOptions ,
 const CodeGenOptions ,
 const TargetOptions ,
-const LangOptions ,
+const LangOptions , llvm::Module *Module,
 SmallVector LinkModules, LLVMContext ,
 CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
@@ -170,7 +170,7 @@
   LLVMIRGenerationRefCount(0),
   Gen(CreateLLVMCodeGen(Diags, "", HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
-  LinkModules(std::move(LinkModules)) {
+  LinkModules(std::move(LinkModules)), CurLinkModule(Module) {
   TimerIsEnabled = CodeGenOpts.TimePasses;
   llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
   llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
@@ -779,11 +779,7 @@
 ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
 break;
   case DK_Linker:
-assert(CurLinkModule);
-// FIXME: stop eating the warnings and notes.
-if (Severity != DS_Error)
-  return;
-DiagID = diag::err_fe_cannot_link_module;
+ComputeDiagID(Severity, linking_module, DiagID);
 break;
   

[PATCH] D108499: [clang][codegen] Don't assert on CurLinkModule for silenced diagnostic

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

There are some possible follow-ups here:

1. Making sure CurLinkModule is set in all (or at least more) cases.
2. Addressing the FIXME about warnings and notes.

I have some partial implementations of those ideas, but I figure we can take 
this change first while I figure out the other bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108499

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

[PATCH] D108499: [clang][codegen] Don't assert on CurLinkModule for silenced diagnostic

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
inglorion requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR51564.

Under certain circumstances, it is possible to enter Clang's
BackendConsumer::DiagnosticHandlerImpl without CurLinkModule set.
For diagnostics of kind DK_Linker, this results in an assert.
However, these diagnostics are never actually surfaced, unless
they are errors. Crashing the compiler for diagnostics we don't
actually surface seems not worth it, so this change moves the
assert below the check so that it only triggers for diagnostics
we actually surface.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108499

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/linker-diagnostic1.ll
  clang/test/CodeGen/linker-diagnostic.ll


Index: clang/test/CodeGen/linker-diagnostic.ll
===
--- /dev/null
+++ clang/test/CodeGen/linker-diagnostic.ll
@@ -0,0 +1,15 @@
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r 
%t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv6-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}
Index: clang/test/CodeGen/Inputs/linker-diagnostic1.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/linker-diagnostic1.ll
@@ -0,0 +1,9 @@
+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -779,10 +779,10 @@
 ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
 break;
   case DK_Linker:
-assert(CurLinkModule);
 // FIXME: stop eating the warnings and notes.
 if (Severity != DS_Error)
   return;
+assert(CurLinkModule);
 DiagID = diag::err_fe_cannot_link_module;
 break;
   case llvm::DK_OptimizationRemark:


Index: clang/test/CodeGen/linker-diagnostic.ll
===
--- /dev/null
+++ clang/test/CodeGen/linker-diagnostic.ll
@@ -0,0 +1,15 @@
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r %t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv6-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}
Index: clang/test/CodeGen/Inputs/linker-diagnostic1.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/linker-diagnostic1.ll
@@ -0,0 +1,9 @@
+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -779,10 +779,10 @@
 ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
 break;
   case DK_Linker:
-assert(CurLinkModule);
 // FIXME: stop eating the warnings and notes.
 if (Severity != DS_Error)
   return;
+assert(CurLinkModule);
 DiagID = diag::err_fe_cannot_link_module;
 break;
   case llvm::DK_OptimizationRemark:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3758
+  // Normally -gsplit-dwarf is only useful with -gN. For -gsplit-dwarf in the
+  // backend phase of a distributed ThinLTO which does object file generation
+  // and no IR generation, -gN should not be needed. So allow -gsplit-dwarf 
with

tejohnson wrote:
> MaskRay wrote:
> > dblaikie wrote:
> > > tejohnson wrote:
> > > > Better to simply check if the input type is IR/BC via 
> > > > types::isLLVMIR(Input.getType()). Technically you don't need ThinLTO to 
> > > > do a clang compile with bitcode input. Looks like Input can be passed 
> > > > down from the caller.
> > > Good point! +1 to that.
> > That currently does not work:
> > `clang -g -c -flto a.c; fclang -gsplit-dwarf -g a.o -o a`
> > there is no .dwo
> I assume you mean clang and not "fclang"?
> 
> Try with "-x ir" like "clang -gsplit-dwarf -g -x ir a.o -o a". I thought that 
> the -x ir was needed when passing a .o file that was actually bitcode?
> I thought that the -x ir was needed when passing a .o file that was actually 
> bitcode?

I think that's a linker invocation, not a codegen invocation.

I would have spelled it

  clang -fuse-ld=lld -flto -gsplit-dwarf -g a.o -o a

And, for me at least, it does use debug fission:

  objdump -g a | grep dwo
  DW_AT_GNU_dwo_name DW_FORM_strp
  DW_AT_GNU_dwo_id   DW_FORM_data8
  <14>   DW_AT_GNU_dwo_name: (indirect string, offset: 0x29): a_dwo/0.dwo
  <18>   DW_AT_GNU_dwo_id  : 0x24fab96c73f98143
0x0020 2f746d70 2f747374 00615f64 776f2f30 /tmp/tst.a_dwo/0
0x0030 2e64776f 00 .dwo.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94647

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

[PATCH] D94647: [Driver] -gsplit-dwarf: Produce .dwo regardless of -gN for -fthinlto-index=

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

Can we not just make -gsplit-dwarf set the DwarfFissionKind without also 
looking at other things? That way, we don't have to worry about detecting the 
right mix of -g, -g2, -fthinlto-index, -x ir, bitcode input, etc. If there is 
some reason why this doesn't work, I would also like it to be expressed in a 
comment in the code so that someone doesn't come along and simplify/break the 
code later on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94647

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

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

For Chrome on Chrome OS, this is https://crbug.com/1158215

Here, we saw our links fail with "output file too large". Investigation 
revealed that debug info was being included in the binary, instead of in .dwo 
files as expected. That turned out to be caused by this change.

It seems that there are cases where -gsplit-dwarf is passed, but it does not 
take effect because there isn't also a -g (or -g) option:

  clang -c -fthinlto-index=foo.o.thinlto.bc -gsplit-dwarf foo.o -o foo.bin.o

Shows no .dwo file and debug info in foo.bin.o, whereas

  clang -c -fthinlto-index=foo.o.thinlto.bc -gsplit-dwarf -g foo.o -o foo.bin.o

shows a .dwo file which is referenced from foo.bin.o

The only thing that is different here is that the latter command has "-g" 
whereas the former doesn't.

In other words, the new implementation still doesn't make -g and -gsplit-dwarf 
fully orthogonal; instead of -gsplit-dwarf always turning on debug fission, it 
*only* turns it on when there is also a -g or -g option - at least in 
distributed ThinLTO code generation.

It seems that orthogonality was the motivation for this change, and that 
-gsplit-dwarf without -g is supposed to work in the linker (per MaskRay's 
comment about -g being ignored by the linker).

Can we make it so that -gsplit-dwarf turns on debug fission and we then use 
that even if no -g options were passed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80391

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

[PATCH] D81203: clang-cl: accept -f[no-]data-sections and -f[no-]function-sections

[Bob Haarman via Phabricator via cfe-commits]

inglorion abandoned this revision.
inglorion added a comment.

Ok, thanks for clarifying.

The benefit this would get us is not having to spell the flags differently 
depending on whether the frontend is clang or clang-cl. I figured this would 
fit in with all the -f options we already take and not really increase the risk 
of conflicting with future cl.exe options. But as you said, there are clang-cl 
options that will enable the exact same functionality, so I'll abandon this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81203



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

[PATCH] D81203: clang-cl: accept -f[no-]data-sections and -f[no-]function-sections

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
inglorion added reviewers: hans, thakis.
Herald added a project: clang.

This adds -fdata-sections, -ffunction-sections, and their negations to
the list of -f options accepted by clang-cl. As a result, these
options can be used with the same spelling for both clang and
clang-cl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81203

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -102,9 +102,11 @@
 // GS_-NOT: -stack-protector
 
 // RUN: %clang_cl /Gy -### -- %s 2>&1 | FileCheck -check-prefix=Gy %s
+// RUN: %clang_cl -ffunction-sections -### -- %s 2>&1 | FileCheck 
-check-prefix=Gy %s
 // Gy: -ffunction-sections
 
 // RUN: %clang_cl /Gy /Gy- -### -- %s 2>&1 | FileCheck -check-prefix=Gy_ %s
+// RUN: %clang_cl -fno-function-sections -### -- %s 2>&1 | FileCheck 
-check-prefix=Gy_ %s
 // Gy_-NOT: -ffunction-sections
 
 // RUN: %clang_cl /Gs -### -- %s 2>&1 | FileCheck -check-prefix=Gs %s
@@ -115,9 +117,11 @@
 // Gs4096: "-mstack-probe-size=4096"
 
 // RUN: %clang_cl /Gw -### -- %s 2>&1 | FileCheck -check-prefix=Gw %s
+// RUN: %clang_cl -fdata-sections -### -- %s 2>&1 | FileCheck -check-prefix=Gw 
%s
 // Gw: -fdata-sections
 
 // RUN: %clang_cl /Gw /Gw- -### -- %s 2>&1 | FileCheck -check-prefix=Gw_ %s
+// RUN: %clang_cl -fno-data-sections -### -- %s 2>&1 | FileCheck 
-check-prefix=Gw_ %s
 // Gw_-NOT: -fdata-sections
 
 // RUN: %clang_cl /Imyincludedir -### -- %s 2>&1 | FileCheck 
-check-prefix=SLASH_I %s
@@ -635,11 +639,15 @@
 // RUN: -fno-coverage-mapping \
 // RUN: -fdiagnostics-color \
 // RUN: -fno-diagnostics-color \
+// RUN: -fdata-sections \
+// RUN: -fno-data-sections \
 // RUN: -fdebug-compilation-dir . \
 // RUN: -fdebug-compilation-dir=. \
 // RUN: -fdiagnostics-parseable-fixits \
 // RUN: -fdiagnostics-absolute-paths \
 // RUN: -ferror-limit=10 \
+// RUN: -ffunction-sections \
+// RUN: -fno-function-sections \
 // RUN: -fmsc-version=1800 \
 // RUN: -fno-strict-aliasing \
 // RUN: -fstrict-aliasing \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1998,17 +1998,17 @@
   HelpText<"Store string literals as writable data">;
 def fzero_initialized_in_bss : Flag<["-"], "fzero-initialized-in-bss">, 
Group;
 def ffunction_sections : Flag<["-"], "ffunction-sections">, Group,
-  Flags<[CC1Option]>,
+  Flags<[CoreOption, CC1Option]>,
   HelpText<"Place each function in its own section">;
-def fno_function_sections : Flag<["-"], "fno-function-sections">, 
Group;
+def fno_function_sections : Flag<["-"], "fno-function-sections">, 
Group, Flags<[CoreOption, CC1Option]>;
 def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, 
Group,
   Flags<[CC1Option, CC1AsOption]>,
   HelpText<"Place each function's basic blocks in unique sections (ELF Only) : 
all | labels | none | list=">,
   DocBrief<[{Generate labels for each basic block or place each basic block or 
a subset of basic blocks in its own section.}]>,
   Values<"all,labels,none,list=">;
 def fdata_sections : Flag <["-"], "fdata-sections">, Group,
- Flags<[CC1Option]>, HelpText<"Place each data in its own section">;
-def fno_data_sections : Flag <["-"], "fno-data-sections">, Group;
+ Flags<[CoreOption, CC1Option]>, HelpText<"Place each data in its own 
section">;
+def fno_data_sections : Flag <["-"], "fno-data-sections">, Group, 
Flags<[CoreOption, CC1Option]>;
 def fstack_size_section : Flag<["-"], "fstack-size-section">, Group, 
Flags<[CC1Option]>,
   HelpText<"Emit section containing metadata on function stack sizes">;
 def fno_stack_size_section : Flag<["-"], "fno-stack-size-section">, 
Group,


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -102,9 +102,11 @@
 // GS_-NOT: -stack-protector
 
 // RUN: %clang_cl /Gy -### -- %s 2>&1 | FileCheck -check-prefix=Gy %s
+// RUN: %clang_cl -ffunction-sections -### -- %s 2>&1 | FileCheck -check-prefix=Gy %s
 // Gy: -ffunction-sections
 
 // RUN: %clang_cl /Gy /Gy- -### -- %s 2>&1 | FileCheck -check-prefix=Gy_ %s
+// RUN: %clang_cl -fno-function-sections -### -- %s 2>&1 | FileCheck -check-prefix=Gy_ %s
 // Gy_-NOT: -ffunction-sections
 
 // RUN: %clang_cl /Gs -### -- %s 2>&1 | FileCheck -check-prefix=Gs %s
@@ -115,9 +117,11 @@
 // Gs4096: "-mstack-probe-size=4096"
 
 // RUN: %clang_cl /Gw -### -- %s 2>&1 | FileCheck -check-prefix=Gw %s
+// RUN: %clang_cl -fdata-sections -### -- %s 2>&1 | FileCheck -check-prefix=Gw %s
 // Gw: -fdata-sections
 
 // RUN: %clang_cl /Gw /Gw- -### -- %s 2>&1 | FileCheck 

[PATCH] D64458: add -fthinlto-index= option to clang-cl

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366146: add -fthinlto-index= option to clang-cl (authored by 
inglorion, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64458?vs=209954=209980#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64458

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/test/Driver/cl-thinlto-backend.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1270,7 +1270,7 @@
"of 0 means the number of threads will be derived from "
"the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group,
+  Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, 
CoreOption]>;
Index: cfe/trunk/test/Driver/cl-thinlto-backend.c
===
--- cfe/trunk/test/Driver/cl-thinlto-backend.c
+++ cfe/trunk/test/Driver/cl-thinlto-backend.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cl -c -flto=thin -Fo%t.obj %s
+// RUN: llvm-lto2 run -thinlto-distributed-indexes -o %t.exe %t.obj
+
+// -fthinlto_index should be passed to cc1
+// RUN: %clang_cl -### -c -fthinlto-index=%t.thinlto.bc -Fo%t1.obj \
+// RUN: %t.obj 2>&1 | FileCheck %s
+
+// CHECK: -fthinlto-index=
+// CHECK: "-x" "ir"


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1270,7 +1270,7 @@
"of 0 means the number of threads will be derived from "
"the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group,
+  Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, CoreOption]>;
Index: cfe/trunk/test/Driver/cl-thinlto-backend.c
===
--- cfe/trunk/test/Driver/cl-thinlto-backend.c
+++ cfe/trunk/test/Driver/cl-thinlto-backend.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cl -c -flto=thin -Fo%t.obj %s
+// RUN: llvm-lto2 run -thinlto-distributed-indexes -o %t.exe %t.obj
+
+// -fthinlto_index should be passed to cc1
+// RUN: %clang_cl -### -c -fthinlto-index=%t.thinlto.bc -Fo%t1.obj \
+// RUN: %t.obj 2>&1 | FileCheck %s
+
+// CHECK: -fthinlto-index=
+// CHECK: "-x" "ir"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64458: add -fthinlto-index= option to clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 209954.
inglorion added a comment.

Simplified after rebasing on top of r366127.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64458

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-thinlto-backend.c


Index: clang/test/Driver/cl-thinlto-backend.c
===
--- /dev/null
+++ clang/test/Driver/cl-thinlto-backend.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cl -c -flto=thin -Fo%t.obj %s
+// RUN: llvm-lto2 run -thinlto-distributed-indexes -o %t.exe %t.obj
+
+// -fthinlto_index should be passed to cc1
+// RUN: %clang_cl -### -c -fthinlto-index=%t.thinlto.bc -Fo%t1.obj \
+// RUN: %t.obj 2>&1 | FileCheck %s
+
+// CHECK: -fthinlto-index=
+// CHECK: "-x" "ir"
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1270,7 +1270,7 @@
"of 0 means the number of threads will be derived from "
"the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group,
+  Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, 
CoreOption]>;


Index: clang/test/Driver/cl-thinlto-backend.c
===
--- /dev/null
+++ clang/test/Driver/cl-thinlto-backend.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cl -c -flto=thin -Fo%t.obj %s
+// RUN: llvm-lto2 run -thinlto-distributed-indexes -o %t.exe %t.obj
+
+// -fthinlto_index should be passed to cc1
+// RUN: %clang_cl -### -c -fthinlto-index=%t.thinlto.bc -Fo%t1.obj \
+// RUN: %t.obj 2>&1 | FileCheck %s
+
+// CHECK: -fthinlto-index=
+// CHECK: "-x" "ir"
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1270,7 +1270,7 @@
"of 0 means the number of threads will be derived from "
"the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group,
+  Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, CoreOption]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64610: [clang] allow -fthinlto-index= without -x ir

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
inglorion marked an inline comment as done.
Closed by commit rL366127: [clang] allow -fthinlto-index= without -x ir 
(authored by inglorion, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64610?vs=209948=209952#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64610

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/thinlto_backend.c


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2119,6 +2119,12 @@
   Diag(clang::diag::warn_drv_treating_input_as_cxx)
   << getTypeName(OldTy) << getTypeName(Ty);
   }
+
+  // If running with -fthinlto-index=, extensions that normally 
identify
+  // native object files actually identify LLVM bitcode files.
+  if (Args.hasArgNoClaim(options::OPT_fthinlto_index_EQ) &&
+  Ty == types::TY_Object)
+Ty = types::TY_LLVM_BC;
 }
 
 // -ObjC and -ObjC++ override the default language, but only for 
"source
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3647,8 +3647,7 @@
 
   if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) {
 if (!types::isLLVMIR(Input.getType()))
-  D.Diag(diag::err_drv_argument_only_allowed_with) << A->getAsString(Args)
-   << "-x ir";
+  D.Diag(diag::err_drv_arg_requires_bitcode_input) << A->getAsString(Args);
 Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ);
   }
 
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -159,6 +159,8 @@
   "cannot read configuration file '%0'">;
 def err_drv_nested_config_file: Error<
   "option '--config' is not allowed inside configuration file">;
+def err_drv_arg_requires_bitcode_input: Error<
+  "option '%0' requires input to be LLVM bitcode">;
 
 def err_target_unsupported_arch
   : Error<"the target architecture '%0' is not supported by the target '%1'">;
Index: cfe/trunk/test/Driver/thinlto_backend.c
===
--- cfe/trunk/test/Driver/thinlto_backend.c
+++ cfe/trunk/test/Driver/thinlto_backend.c
@@ -2,8 +2,14 @@
 // RUN: llvm-lto -thinlto -o %t %t.o
 
 // -fthinlto_index should be passed to cc1
-// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 
2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
+// CHECK-THINLTOBE-ACTION-SAME: {{"?-x"? "?ir"?}}
+
+// Check that this also works without -x ir.
+// RUN: %clang -O2 -o %t1.o %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 
 // -save-temps should be passed to cc1
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc 
-save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS 
-check-prefix=CHECK-SAVE-TEMPS-CWD
@@ -15,5 +21,6 @@
 // CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
 
 // Ensure clang driver gives the expected error for incorrect input type
-// RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | 
FileCheck %s -check-prefix=CHECK-WARNING
-// CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only 
allowed with '-x ir'
+// RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-WARNING
+// CHECK-WARNING: error: option '-fthinlto-index={{.*}}' requires input to be 
LLVM bitcode


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2119,6 +2119,12 @@
   Diag(clang::diag::warn_drv_treating_input_as_cxx)
   << getTypeName(OldTy) << getTypeName(Ty);
   }
+
+  // If running with -fthinlto-index=, extensions that normally identify
+  // native object files actually identify LLVM bitcode files.
+  if (Args.hasArgNoClaim(options::OPT_fthinlto_index_EQ) &&
+  Ty == types::TY_Object)
+

[PATCH] D64610: [clang] allow -fthinlto-index= without -x ir

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 209948.
inglorion added a comment.

Fix typo pointed out by MaskRay (thanks!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64610

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/thinlto_backend.c


Index: clang/test/Driver/thinlto_backend.c
===
--- clang/test/Driver/thinlto_backend.c
+++ clang/test/Driver/thinlto_backend.c
@@ -2,8 +2,14 @@
 // RUN: llvm-lto -thinlto -o %t %t.o
 
 // -fthinlto_index should be passed to cc1
-// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 
2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
+// CHECK-THINLTOBE-ACTION-SAME: {{"?-x"? "?ir"?}}
+
+// Check that this also works without -x ir.
+// RUN: %clang -O2 -o %t1.o %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 
 // -save-temps should be passed to cc1
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc 
-save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS 
-check-prefix=CHECK-SAVE-TEMPS-CWD
@@ -15,5 +21,6 @@
 // CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
 
 // Ensure clang driver gives the expected error for incorrect input type
-// RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | 
FileCheck %s -check-prefix=CHECK-WARNING
-// CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only 
allowed with '-x ir'
+// RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-WARNING
+// CHECK-WARNING: error: option '-fthinlto-index={{.*}}' requires input to be 
LLVM bitcode
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3647,8 +3647,7 @@
 
   if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) {
 if (!types::isLLVMIR(Input.getType()))
-  D.Diag(diag::err_drv_argument_only_allowed_with) << A->getAsString(Args)
-   << "-x ir";
+  D.Diag(diag::err_drv_arg_requires_bitcode_input) << A->getAsString(Args);
 Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ);
   }
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2119,6 +2119,12 @@
   Diag(clang::diag::warn_drv_treating_input_as_cxx)
   << getTypeName(OldTy) << getTypeName(Ty);
   }
+
+  // If running with -fthinlto-index=, extensions that normally 
identify
+  // native object files actually identify LLVM bitcode files.
+  if (Args.hasArgNoClaim(options::OPT_fthinlto_index_EQ) &&
+  Ty == types::TY_Object)
+Ty = types::TY_LLVM_BC;
 }
 
 // -ObjC and -ObjC++ override the default language, but only for 
"source
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -159,6 +159,8 @@
   "cannot read configuration file '%0'">;
 def err_drv_nested_config_file: Error<
   "option '--config' is not allowed inside configuration file">;
+def err_drv_arg_requires_bitcode_input: Error<
+  "option '%0' requires input to be LLVM bitcode">;
 
 def err_target_unsupported_arch
   : Error<"the target architecture '%0' is not supported by the target '%1'">;


Index: clang/test/Driver/thinlto_backend.c
===
--- clang/test/Driver/thinlto_backend.c
+++ clang/test/Driver/thinlto_backend.c
@@ -2,8 +2,14 @@
 // RUN: llvm-lto -thinlto -o %t %t.o
 
 // -fthinlto_index should be passed to cc1
-// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
+// CHECK-THINLTOBE-ACTION-SAME: {{"?-x"? "?ir"?}}
+
+// Check that this also works without -x ir.
+// RUN: %clang -O2 -o %t1.o %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 
 // -save-temps should be passed to cc1
 // RUN: %clang 

[PATCH] D64610: [clang] allow -fthinlto-index= without -x ir

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

This was suggested on D64458 .

If we take this change, D64458  can be 
simplified to just accepting -fthinlto-index= as a CoreOption without needing 
any further changes to Driver.cpp.

I also changed the error message in this change to reflect the fact that we no 
longer require -x ir; we just require the input to be a bitcode file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64610



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

[PATCH] D64610: [clang] allow -fthinlto-index= without -x ir

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
inglorion added reviewers: tejohnson, rnk, pcc.
Herald added subscribers: arphaman, dexonsmith, steven_wu, mehdi_amini.
Herald added a project: clang.

Previously, passing -fthinlto-index= to clang required that bitcode
files be explicitly marked by -x ir. This change makes us detect files
with object file extensions as bitcode files when -fthinlto-index= is
present, so that explicitly marking them is no longer necessary.
Explicitly specifying -x ir is still accepted and continues to be part
of the test case to ensure we continue to support it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64610

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/thinlto_backend.c


Index: clang/test/Driver/thinlto_backend.c
===
--- clang/test/Driver/thinlto_backend.c
+++ clang/test/Driver/thinlto_backend.c
@@ -2,8 +2,14 @@
 // RUN: llvm-lto -thinlto -o %t %t.o
 
 // -fthinlto_index should be passed to cc1
-// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 
2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### \
+// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
+// CHECK-THINLTOBE-ACTION-SAME: {{"?-x"? "?ir"?}}
+
+// Check that this also works without -x ir.
+// RUN: %clang -O2 -o %t1.o %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 
 // -save-temps should be passed to cc1
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc 
-save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS 
-check-prefix=CHECK-SAVE-TEMPS-CWD
@@ -15,5 +21,6 @@
 // CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
 
 // Ensure clang driver gives the expected error for incorrect input type
-// RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | 
FileCheck %s -check-prefix=CHECK-WARNING
-// CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only 
allowed with '-x ir'
+// RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-WARNING
+// CHECK-WARNING: error: option '-fthinlto-index={{.*}}' requires input to be 
LLVM bitcode
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3660,8 +3660,7 @@
 
   if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) {
 if (!types::isLLVMIR(Input.getType()))
-  D.Diag(diag::err_drv_argument_only_allowed_with) << A->getAsString(Args)
-   << "-x ir";
+  D.Diag(diag::err_drv_arg_requires_bitcode_input) << A->getAsString(Args);
 Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ);
   }
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2119,6 +2119,12 @@
   Diag(clang::diag::warn_drv_treating_input_as_cxx)
   << getTypeName(OldTy) << getTypeName(Ty);
   }
+
+  // If running with -thinlto-index=, extensions that normally identify
+  // native object files actually identify LLVM bitcode files.
+  if (Args.hasArgNoClaim(options::OPT_fthinlto_index_EQ) &&
+  Ty == types::TY_Object)
+Ty = types::TY_LLVM_BC;
 }
 
 // -ObjC and -ObjC++ override the default language, but only for 
"source
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -159,6 +159,8 @@
   "cannot read configuration file '%0'">;
 def err_drv_nested_config_file: Error<
   "option '--config' is not allowed inside configuration file">;
+def err_drv_arg_requires_bitcode_input: Error<
+  "option '%0' requires input to be LLVM bitcode">;
 
 def err_target_unsupported_arch
   : Error<"the target architecture '%0' is not supported by the target '%1'">;


Index: clang/test/Driver/thinlto_backend.c
===
--- clang/test/Driver/thinlto_backend.c
+++ clang/test/Driver/thinlto_backend.c
@@ -2,8 +2,14 @@
 // RUN: llvm-lto -thinlto -o %t %t.o
 
 // -fthinlto_index should be passed to cc1
-// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### \
+// RUN: 2>&1 | 

[PATCH] D64458: add -fthinlto-index= option to clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

> Do we really need to support clang-cl syntax here? Can't the user of 
> -fthinlto-index= use the regular clang driver?

We could, of course, require using the clang driver for this feature. But I 
would argue that clang-cl exists to make it easy for projects that use 
MSVC-style command lines to switch to Clang and get access to the benefits it 
provides, including features for which there is no equivalent MSVC flag. So I 
think this is reasonable to add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64458



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

[PATCH] D64458: add -fthinlto-index= option to clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

This is similar to r254927 for the clang driver, and reuses the tests from that 
commit for the backend part. On the frontend, it differs in that clang-cl does 
not support -x (per the comment in clang/lib/Driver/Driver.cpp line 2068: "// 
No driver mode exposes -x and /TC or /TP; we don't support mixing them.") 
Instead of requiring -x ir, the behavior added to clang-cl here is to expect 
that what would be detected as a native object file based on file extension is 
really a bitcode file. If it isn't, the backend will cause us to say "error: 
Invalid bitcode signature", which I think is adequate as a diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64458



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

[PATCH] D64458: add -fthinlto-index= option to clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
inglorion added reviewers: tejohnson, pcc, rnk.
Herald added subscribers: arphaman, dexonsmith, steven_wu, mehdi_amini.
Herald added a project: clang.

This adds a -fthinlto-index= option to clang-cl, which allows it to
be used to drive ThinLTO backend passes. This allows clang-cl to be
used for distributed ThinLTO.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64458

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cl-thinlto-backend.c


Index: clang/test/Driver/cl-thinlto-backend.c
===
--- /dev/null
+++ clang/test/Driver/cl-thinlto-backend.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cl -c -flto=thin -Fo%t.obj %s
+// RUN: llvm-lto2 run -thinlto-distributed-indexes -o %t.exe %t.obj
+
+// -fthinlto_index should be passed to cc1
+// RUN: %clang_cl -### -c -fthinlto-index=%t.thinlto.bc -Fo%t1.obj \
+// RUN: %t.obj 2>&1 | FileCheck %s
+
+// CHECK: -fthinlto-index=
+// CHECK: "-x" "ir"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2119,6 +2119,13 @@
   Diag(clang::diag::warn_drv_treating_input_as_cxx)
   << getTypeName(OldTy) << getTypeName(Ty);
   }
+
+  // If the driver is invoked as clang-cl with -thinlto-index=,
+  // extensions that normally identify native object files actually
+  // identify LLVM bitcode files.
+  if (IsCLMode() && Ty == types::TY_Object &&
+  Args.hasArgNoClaim(options::OPT_fthinlto_index_EQ))
+Ty = types::TY_LLVM_BC;
 }
 
 // -ObjC and -ObjC++ override the default language, but only for 
"source
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1270,7 +1270,7 @@
"of 0 means the number of threads will be derived from "
"the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group,
+  Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, 
CoreOption]>;


Index: clang/test/Driver/cl-thinlto-backend.c
===
--- /dev/null
+++ clang/test/Driver/cl-thinlto-backend.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cl -c -flto=thin -Fo%t.obj %s
+// RUN: llvm-lto2 run -thinlto-distributed-indexes -o %t.exe %t.obj
+
+// -fthinlto_index should be passed to cc1
+// RUN: %clang_cl -### -c -fthinlto-index=%t.thinlto.bc -Fo%t1.obj \
+// RUN: %t.obj 2>&1 | FileCheck %s
+
+// CHECK: -fthinlto-index=
+// CHECK: "-x" "ir"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2119,6 +2119,13 @@
   Diag(clang::diag::warn_drv_treating_input_as_cxx)
   << getTypeName(OldTy) << getTypeName(Ty);
   }
+
+  // If the driver is invoked as clang-cl with -thinlto-index=,
+  // extensions that normally identify native object files actually
+  // identify LLVM bitcode files.
+  if (IsCLMode() && Ty == types::TY_Object &&
+  Args.hasArgNoClaim(options::OPT_fthinlto_index_EQ))
+Ty = types::TY_LLVM_BC;
 }
 
 // -ObjC and -ObjC++ override the default language, but only for "source
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1270,7 +1270,7 @@
"of 0 means the number of threads will be derived from "
"the number of CPUs detected)">;
 def fthinlto_index_EQ : Joined<["-"], "fthinlto-index=">,
-  Flags<[CC1Option]>, Group,
+  Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Perform ThinLTO importing using provided function summary index">;
 def fmacro_backtrace_limit_EQ : Joined<["-"], "fmacro-backtrace-limit=">,
 Group, Flags<[DriverOption, CoreOption]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

[Bob Haarman via Phabricator via cfe-commits]

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

Thank you. Given that proceeding with the wrong value will result in either an 
undefined reference or a reference to what might well be the wrong function at 
link time, I think making this an error (as you've done) is strictly better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62975



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

[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

Can you clarify "which will usually result in a linker error"? E.g. an example 
of link.exe rejecting the object file or the wrong function being called. The 
reason I ask is that if we can be sure at compile time that the resulting code 
will not work or do the wrong thing, I think giving an error is appropriate. 
But if that isn't the case, we would be rejecting code that cl.exe accepts and 
we might want to make it a Warning instead.




Comment at: clang/lib/Sema/SemaExpr.cpp:14801
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+TT.getArch() != llvm::Triple::x86_64))

Do we need those checks or would it be enough to just check the calling 
convention?

Also, I think s/Do nothing/return false/



Comment at: clang/lib/Sema/SemaExpr.cpp:14813
+  default:
+break;
+  }

You can just return false here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62975



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

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



Comment at: clang/lib/AST/MicrosoftMangle.cpp:389
+  if (FE) {
+llvm::MD5 Hasher;
+llvm::MD5::MD5Result Hash;

Instead of MD5, can we use xxhash (or whatever the fastest hash algorithm in 
LLVM is these days)? I don't think we need a particular hash algorithm, so we 
might as well go with the fastest one we have (as long as it does a good job 
avoiding collisions, of course).


https://reviews.llvm.org/D50877



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

[PATCH] D48601: Added -fcrash-diagnostics-dir flag

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336604: Added -fcrash-diagnostics-dir flag (authored by 
inglorion, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48601?vs=153773=154695#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48601

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/crash-diagnostics-dir.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -798,6 +798,7 @@
 Group;
 def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, 
Group, Flags<[NoArgumentUnused]>,
   HelpText<"Disable auto-generation of preprocessed source files and a script 
for reproduction during a clang crash">;
+def fcrash_diagnostics_dir : Joined<["-"], "fcrash-diagnostics-dir=">, 
Group, Flags<[NoArgumentUnused]>;
 def fcreate_profile : Flag<["-"], "fcreate-profile">, Group;
 def fcxx_exceptions: Flag<["-"], "fcxx-exceptions">, Group,
   HelpText<"Enable C++ exceptions">, Flags<[CC1Option]>;
Index: cfe/trunk/test/Driver/crash-diagnostics-dir.c
===
--- cfe/trunk/test/Driver/crash-diagnostics-dir.c
+++ cfe/trunk/test/Driver/crash-diagnostics-dir.c
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: not %clang -fcrash-diagnostics-dir=%t -c %s 2>&1 | FileCheck %s
+#pragma clang __debug parser_crash
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK: diagnostic msg: 
{{.*}}Output{{/|\\}}crash-diagnostics-dir.c.tmp{{(/|\\).*}}.c
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1291,12 +1291,13 @@
   // Assume associated files are based off of the first temporary file.
   CrashReportInfo CrashInfo(TempFiles[0], VFS);
 
-  std::string Script = CrashInfo.Filename.rsplit('.').first.str() + ".sh";
+  llvm::SmallString<128> Script(CrashInfo.Filename);
+  llvm::sys::path::replace_extension(Script, "sh");
   std::error_code EC;
   llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew);
   if (EC) {
 Diag(clang::diag::note_drv_command_failed_diag_msg)
-<< "Error generating run script: " + Script + " " + EC.message();
+<< "Error generating run script: " << Script << " " << EC.message();
   } else {
 ScriptOS << "# Crash reproducer for " << getClangFullVersion() << "\n"
  << "# Driver args: ";
@@ -1308,7 +1309,7 @@
   ScriptOS << "\n# Additional information: " << AdditionalInformation
<< "\n";
 if (Report)
-  Report->TemporaryFiles.push_back(Script);
+  Report->TemporaryFiles.push_back(Script.str());
 Diag(clang::diag::note_drv_command_failed_diag_msg) << Script;
   }
 
@@ -4018,8 +4019,22 @@
   CCGenDiagnostics) {
 StringRef Name = llvm::sys::path::filename(BaseInput);
 std::pair Split = Name.split('.');
-std::string TmpName = GetTemporaryPath(
-Split.first, types::getTypeTempSuffix(JA.getType(), IsCLMode()));
+SmallString<128> TmpName;
+const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
+Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
+if (CCGenDiagnostics && A) {
+  SmallString<128> CrashDirectory(A->getValue());
+  llvm::sys::path::append(CrashDirectory, Split.first);
+  const char *Middle = Suffix ? "-%%." : "-%%";
+  std::error_code EC =
+  llvm::sys::fs::createUniqueFile(CrashDirectory + Middle + Suffix, 
TmpName);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+} else {
+  TmpName = GetTemporaryPath(Split.first, Suffix);
+}
 return C.addTempFile(C.getArgs().MakeArgString(TmpName));
   }
 


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -798,6 +798,7 @@
 Group;
 def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, Group, Flags<[NoArgumentUnused]>,
   HelpText<"Disable auto-generation of preprocessed source files and a script for reproduction during a clang crash">;
+def fcrash_diagnostics_dir : Joined<["-"], "fcrash-diagnostics-dir=">, Group, Flags<[NoArgumentUnused]>;
 def fcreate_profile : Flag<["-"], "fcreate-profile">, Group;
 def fcxx_exceptions: Flag<["-"], "fcxx-exceptions">, Group,
   HelpText<"Enable C++ exceptions">, Flags<[CC1Option]>;
Index: cfe/trunk/test/Driver/crash-diagnostics-dir.c

[PATCH] D37943: [docs] add Windows examples to ThinLTO.rst

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313425: [docs] add Windows examples to ThinLTO.rst (authored 
by inglorion).

Changed prior to commit:
  https://reviews.llvm.org/D37943?vs=115522=115524#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37943

Files:
  cfe/trunk/docs/ThinLTO.rst


Index: cfe/trunk/docs/ThinLTO.rst
===
--- cfe/trunk/docs/ThinLTO.rst
+++ cfe/trunk/docs/ThinLTO.rst
@@ -63,7 +63,7 @@
 - **ld64**:
   Starting with `Xcode 8 `_.
 - **lld**:
-  Starting with r284050 (ELF only).
+  Starting with r284050 for ELF, r298942 for COFF.
 
 Usage
 =
@@ -78,6 +78,13 @@
   % clang -flto=thin -O2 file1.c file2.c -c
   % clang -flto=thin -O2 file1.o file2.o -o a.out
 
+When using lld-link, the -flto option need only be added to the compile step:
+
+.. code-block:: console
+
+  % clang-cl -flto=thin -O2 -c file1.c file2.c
+  % lld-link /out:a.exe file1.obj file2.obj
+
 As mentioned earlier, by default the linkers will launch the ThinLTO backend
 threads in parallel, passing the resulting native object files back to the
 linker for the final native link.  As such, the usage model the same as
@@ -111,6 +118,8 @@
   ``-Wl,-mllvm,-threads=N``
 - lld:
   ``-Wl,--thinlto-jobs=N``
+- lld-link:
+  ``/opt:lldltojobs=N``
 
 Incremental
 ---
@@ -125,7 +134,7 @@
   ``-Wl,-cache_path_lto,/path/to/cache``
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocache:/path/to/cache``
 
 Cache Pruning
@@ -138,7 +147,7 @@
 
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-policy,POLICY``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocachepolicy:POLICY``
 
 A policy string is a series of key-value pairs separated by ``:`` characters.
@@ -187,13 +196,20 @@
when configuring the bootstrap compiler build:
 
   * ``-DLLVM_ENABLE_LTO=Thin``
-  * ``-DLLVM_PARALLEL_LINK_JOBS=1``
-(since the ThinLTO link invokes parallel backend jobs)
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang++``
   * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib``
   * ``-DCMAKE_AR=/path/to/host/llvm-ar``
 
+  Or, on Windows:
+
+  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DCMAKE_C_COMPILER=/path/to/host/clang-cl.exe``
+  * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang-cl.exe``
+  * ``-DCMAKE_LINKER=/path/to/host/lld-link.exe``
+  * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib.exe``
+  * ``-DCMAKE_AR=/path/to/host/llvm-ar.exe``
+
 #. To use additional linker arguments for controlling the backend
parallelism_ or enabling incremental_ builds of the bootstrap compiler,
after configuring the build, modify the resulting CMakeCache.txt file in the


Index: cfe/trunk/docs/ThinLTO.rst
===
--- cfe/trunk/docs/ThinLTO.rst
+++ cfe/trunk/docs/ThinLTO.rst
@@ -63,7 +63,7 @@
 - **ld64**:
   Starting with `Xcode 8 `_.
 - **lld**:
-  Starting with r284050 (ELF only).
+  Starting with r284050 for ELF, r298942 for COFF.
 
 Usage
 =
@@ -78,6 +78,13 @@
   % clang -flto=thin -O2 file1.c file2.c -c
   % clang -flto=thin -O2 file1.o file2.o -o a.out
 
+When using lld-link, the -flto option need only be added to the compile step:
+
+.. code-block:: console
+
+  % clang-cl -flto=thin -O2 -c file1.c file2.c
+  % lld-link /out:a.exe file1.obj file2.obj
+
 As mentioned earlier, by default the linkers will launch the ThinLTO backend
 threads in parallel, passing the resulting native object files back to the
 linker for the final native link.  As such, the usage model the same as
@@ -111,6 +118,8 @@
   ``-Wl,-mllvm,-threads=N``
 - lld:
   ``-Wl,--thinlto-jobs=N``
+- lld-link:
+  ``/opt:lldltojobs=N``
 
 Incremental
 ---
@@ -125,7 +134,7 @@
   ``-Wl,-cache_path_lto,/path/to/cache``
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocache:/path/to/cache``
 
 Cache Pruning
@@ -138,7 +147,7 @@
 
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-policy,POLICY``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocachepolicy:POLICY``
 
 A policy string is a series of key-value pairs separated by ``:`` characters.
@@ -187,13 +196,20 @@
when configuring the bootstrap compiler build:
 
   * ``-DLLVM_ENABLE_LTO=Thin``
-  * ``-DLLVM_PARALLEL_LINK_JOBS=1``
-(since the ThinLTO link invokes parallel backend jobs)
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang++``
   * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib``
   * ``-DCMAKE_AR=/path/to/host/llvm-ar``
 
+  Or, on Windows:
+
+  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DCMAKE_C_COMPILER=/path/to/host/clang-cl.exe``
+  

[PATCH] D37943: [docs] add Windows examples to ThinLTO.rst

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
Herald added subscribers: eraman, mehdi_amini.

https://reviews.llvm.org/D37943

Files:
  clang/docs/ThinLTO.rst


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -63,7 +63,7 @@
 - **ld64**:
   Starting with `Xcode 8 `_.
 - **lld**:
-  Starting with r284050 (ELF only).
+  Starting with r284050 for ELF, r298942 for COFF.
 
 Usage
 =
@@ -78,6 +78,13 @@
   % clang -flto=thin -O2 file1.c file2.c -c
   % clang -flto=thin -O2 file1.o file2.o -o a.out
 
+When using lld-link, the -flto option need only be added to the compile step:
+
+.. code-block:: console
+
+  % clang-cl -flto=thin -O2 -c file1.c file2.c
+  % lld-link /out:a.exe file1.obj file2.obj
+
 As mentioned earlier, by default the linkers will launch the ThinLTO backend
 threads in parallel, passing the resulting native object files back to the
 linker for the final native link.  As such, the usage model the same as
@@ -111,6 +118,8 @@
   ``-Wl,-mllvm,-threads=N``
 - lld:
   ``-Wl,--thinlto-jobs=N``
+- lld-link:
+  ``/opt:lldltojobs=N``
 
 Incremental
 ---
@@ -125,7 +134,7 @@
   ``-Wl,-cache_path_lto,/path/to/cache``
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocache:/path/to/cache``
 
 Cache Pruning
@@ -138,7 +147,7 @@
 
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-policy,POLICY``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocachepolicy:POLICY``
 
 A policy string is a series of key-value pairs separated by ``:`` characters.
@@ -187,13 +196,20 @@
when configuring the bootstrap compiler build:
 
   * ``-DLLVM_ENABLE_LTO=Thin``
-  * ``-DLLVM_PARALLEL_LINK_JOBS=1``
-(since the ThinLTO link invokes parallel backend jobs)
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang++``
   * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib``
   * ``-DCMAKE_AR=/path/to/host/llvm-ar``
 
+  Or, on Windows:
+
+  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DCMAKE_C_COMPILER=/path/to/host/clang-cl.exe``
+  * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang-cl.exe``
+  * ``-DCMAKE_LINKER=/path/to/host/lld-link.exe``
+  * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib.exe``
+  * ``-DCMAKE_AR=/path/to/host/llvm-ar.exe``
+
 #. To use additional linker arguments for controlling the backend
parallelism_ or enabling incremental_ builds of the bootstrap compiler,
after configuring the build, modify the resulting CMakeCache.txt file in the


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -63,7 +63,7 @@
 - **ld64**:
   Starting with `Xcode 8 `_.
 - **lld**:
-  Starting with r284050 (ELF only).
+  Starting with r284050 for ELF, r298942 for COFF.
 
 Usage
 =
@@ -78,6 +78,13 @@
   % clang -flto=thin -O2 file1.c file2.c -c
   % clang -flto=thin -O2 file1.o file2.o -o a.out
 
+When using lld-link, the -flto option need only be added to the compile step:
+
+.. code-block:: console
+
+  % clang-cl -flto=thin -O2 -c file1.c file2.c
+  % lld-link /out:a.exe file1.obj file2.obj
+
 As mentioned earlier, by default the linkers will launch the ThinLTO backend
 threads in parallel, passing the resulting native object files back to the
 linker for the final native link.  As such, the usage model the same as
@@ -111,6 +118,8 @@
   ``-Wl,-mllvm,-threads=N``
 - lld:
   ``-Wl,--thinlto-jobs=N``
+- lld-link:
+  ``/opt:lldltojobs=N``
 
 Incremental
 ---
@@ -125,7 +134,7 @@
   ``-Wl,-cache_path_lto,/path/to/cache``
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-dir=/path/to/cache``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocache:/path/to/cache``
 
 Cache Pruning
@@ -138,7 +147,7 @@
 
 - ELF lld (as of LLVM 5.0):
   ``-Wl,--thinlto-cache-policy,POLICY``
-- COFF lld (as of LLVM 6.0):
+- COFF lld-link (as of LLVM 6.0):
   ``/lldltocachepolicy:POLICY``
 
 A policy string is a series of key-value pairs separated by ``:`` characters.
@@ -187,13 +196,20 @@
when configuring the bootstrap compiler build:
 
   * ``-DLLVM_ENABLE_LTO=Thin``
-  * ``-DLLVM_PARALLEL_LINK_JOBS=1``
-(since the ThinLTO link invokes parallel backend jobs)
   * ``-DCMAKE_C_COMPILER=/path/to/host/clang``
   * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang++``
   * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib``
   * ``-DCMAKE_AR=/path/to/host/llvm-ar``
 
+  Or, on Windows:
+
+  * ``-DLLVM_ENABLE_LTO=Thin``
+  * ``-DCMAKE_C_COMPILER=/path/to/host/clang-cl.exe``
+  * ``-DCMAKE_CXX_COMPILER=/path/to/host/clang-cl.exe``
+  * ``-DCMAKE_LINKER=/path/to/host/lld-link.exe``
+  * ``-DCMAKE_RANLIB=/path/to/host/llvm-ranlib.exe``
+  * ``-DCMAKE_AR=/path/to/host/llvm-ar.exe``
+
 #. To use 

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

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312965: [codeview] omit debug locations for nested exprs 
unless column info enabled (authored by inglorion).

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

Repository:
  rL LLVM

https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114715.
inglorion added a comment.

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


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114465.
inglorion added a comment.

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


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114463.
inglorion added a comment.

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


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114299.
inglorion added a comment.

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


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

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

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

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


https://reviews.llvm.org/D37529



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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114292.
inglorion added a comment.

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


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion planned changes to this revision.
inglorion added a comment.

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


https://reviews.llvm.org/D37529



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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114097.
inglorion marked 5 inline comments as done.
inglorion added a comment.

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


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



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

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


https://reviews.llvm.org/D37529



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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
Herald added a subscriber: aprantl.

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

Fixes PR34312.


https://reviews.llvm.org/D37529

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

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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



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

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


https://reviews.llvm.org/D37529



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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



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

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



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

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



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

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



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

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


https://reviews.llvm.org/D37529



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

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

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 114060.
inglorion added a comment.

removed accidentally left in include and reformatted mangled comment


https://reviews.llvm.org/D37529

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

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

[PATCH] D31633: test for thinlto handling of internal linkage

[Bob Haarman via Phabricator via cfe-commits]

inglorion abandoned this revision.
inglorion added a comment.

We will not be needing this.


https://reviews.llvm.org/D31633



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

[PATCH] D31633: test for thinlto handling of internal linkage

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

This is the test for https://reviews.llvm.org/D31632.


https://reviews.llvm.org/D31633



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

[PATCH] D31633: test for thinlto handling of internal linkage

[Bob Haarman via Phabricator via cfe-commits]

inglorion created this revision.
Herald added a subscriber: Prazek.

https://reviews.llvm.org/D31633

Files:
  test/CodeGenCXX/thinlto-promote-internals.cpp


Index: test/CodeGenCXX/thinlto-promote-internals.cpp
===
--- /dev/null
+++ test/CodeGenCXX/thinlto-promote-internals.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.0.24215 -emit-llvm \
+// RUN: -flto=thin -flto-unit -o - %s | FileCheck %s
+
+// CHECK: @"\01??_7B@?A@@6B@" = internal unnamed_addr alias
+// CHECK: define internal %"class.(anonymous namespace)::B"
+
+class A {
+public:
+  virtual int foo() { return 0; }
+};
+
+namespace {
+  class B : public A { };
+}
+
+A *makeA() { return new B(); }
+


Index: test/CodeGenCXX/thinlto-promote-internals.cpp
===
--- /dev/null
+++ test/CodeGenCXX/thinlto-promote-internals.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.0.24215 -emit-llvm \
+// RUN: -flto=thin -flto-unit -o - %s | FileCheck %s
+
+// CHECK: @"\01??_7B@?A@@6B@" = internal unnamed_addr alias
+// CHECK: define internal %"class.(anonymous namespace)::B"
+
+class A {
+public:
+  virtual int foo() { return 0; }
+};
+
+namespace {
+  class B : public A { };
+}
+
+A *makeA() { return new B(); }
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30663: Use filename in linemarker when compiling preprocessed source (Revised)

[Bob Haarman via Phabricator via cfe-commits]

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

Fixing the other issues in a follow-up seems fine. This lgtm.


https://reviews.llvm.org/D30663



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

[PATCH] D30663: Use filename in linemarker when compiling preprocessed source (Revised)

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



Comment at: test/Frontend/preprocessed-input.c:3
+// RUN: %clang -emit-llvm -S -o - %t.i | FileCheck %s
+// CHECK: source_filename = {{.*}}preprocessed-input.c"{{$}}

Actually, I think you don't even have to run the preprocessor - you can just 
put the file with the linemarker here and check that the name from the 
linemarker is propagated, right?

Also, it seems that there is a similar issue to the one you're trying to fix 
when going from .ll to .o (or .ll to .s, for that matter) - the name is taken 
from the file you're reading from, not from the source_filename directive in 
that file. Of course, that doesn't differ from gcc (given that gcc doesn't 
handle .ll files), but you may want to address that, too, for consistency.


https://reviews.llvm.org/D30663



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

[PATCH] D30591: Introduce the feature "linux" for tests only for linux

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

Checking for linux when really you want to check for ELF doesn't seem right. In 
this case, I think there is an better way to do it; instead of relying on 
llvm-objdump, could you emit an LLVM assembly file and check that for presence 
of the string you want? I think if you compile with clang -g -S -emit-llvm, it 
will give you LLVM assembly with metadata for the records you need and you 
won't need to generate an object file.


https://reviews.llvm.org/D30591



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

[PATCH] D30239: enable -flto=thin in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296373: enable -flto=thin in clang-cl (authored by 
inglorion).

Changed prior to commit:
  https://reviews.llvm.org/D30239?vs=89598=89913#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30239

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/cl-options.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, 
Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, 
Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -133,6 +133,7 @@
 "invalid output type '%0' for use with gcc tool">;
 def err_drv_cc_print_options_failure : Error<
 "unable to open CC_PRINT_OPTIONS file: %0">;
+def err_drv_lto_without_lld : Error<"LTO requires -fuse-ld=lld">;
 def err_drv_preamble_format : Error<
 "incorrect format for -preamble-bytes=N,END">;
 def err_drv_conflicting_deployment_targets : Error<
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -522,6 +522,15 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck 
-check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
+// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck 
-check-prefix=LTO-WITHOUT-LLD %s
+// LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2353,8 +2353,12 @@
   Arg *FinalPhaseArg;
   phases::ID FinalPhase = getFinalPhase(Args, );
 
-  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
-Diag(clang::diag::err_drv_emit_llvm_link);
+  if (FinalPhase == phases::Link) {
+if (Args.hasArg(options::OPT_emit_llvm))
+  Diag(clang::diag::err_drv_emit_llvm_link);
+if (IsCLMode() && LTOMode != LTOK_None &&
+!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+  Diag(clang::diag::err_drv_lto_without_lld);
   }
 
   // Reject -Z* at the top level, these options should never have been exposed


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -133,6 +133,7 @@
 "invalid output type '%0' for use with gcc tool">;
 def err_drv_cc_print_options_failure : Error<
 "unable to open CC_PRINT_OPTIONS file: %0">;
+def err_drv_lto_without_lld : Error<"LTO requires -fuse-ld=lld">;
 def err_drv_preamble_format : Error<
 "incorrect format for -preamble-bytes=N,END">;
 def err_drv_conflicting_deployment_targets : Error<
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ 

[PATCH] D30239: enable -flto=thin in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 89598.
inglorion added a comment.

changed error message


https://reviews.llvm.org/D30239

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/cl-options.c


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,15 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck 
-check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
+// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck 
-check-prefix=LTO-WITHOUT-LLD %s
+// LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2352,8 +2352,12 @@
   Arg *FinalPhaseArg;
   phases::ID FinalPhase = getFinalPhase(Args, );
 
-  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
-Diag(clang::diag::err_drv_emit_llvm_link);
+  if (FinalPhase == phases::Link) {
+if (Args.hasArg(options::OPT_emit_llvm))
+  Diag(clang::diag::err_drv_emit_llvm_link);
+if (IsCLMode() && LTOMode != LTOK_None &&
+!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+  Diag(clang::diag::err_drv_lto_without_lld);
   }
 
   // Reject -Z* at the top level, these options should never have been exposed
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, 
Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, 
Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -133,6 +133,7 @@
 "invalid output type '%0' for use with gcc tool">;
 def err_drv_cc_print_options_failure : Error<
 "unable to open CC_PRINT_OPTIONS file: %0">;
+def err_drv_lto_without_lld : Error<"LTO requires -fuse-ld=lld">;
 def err_drv_preamble_format : Error<
 "incorrect format for -preamble-bytes=N,END">;
 def err_drv_conflicting_deployment_targets : Error<


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,15 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck -check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
+// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2352,8 +2352,12 @@
   Arg *FinalPhaseArg;
   phases::ID FinalPhase = getFinalPhase(Args, );
 
-  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
-Diag(clang::diag::err_drv_emit_llvm_link);
+  if (FinalPhase == phases::Link) {
+if (Args.hasArg(options::OPT_emit_llvm))
+  Diag(clang::diag::err_drv_emit_llvm_link);
+if (IsCLMode() && LTOMode != LTOK_None &&
+!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+  Diag(clang::diag::err_drv_lto_without_lld);
   }
 
   // Reject -Z* at the top level, these options should never have been exposed
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group;
 def flimited_precision_EQ : Joined<["-"], 

[PATCH] D30239: enable -flto=thin in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 89590.
inglorion added a comment.

fail early with a friendlier message when using -flto without -fuse-ld=lld


https://reviews.llvm.org/D30239

Files:
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  test/Driver/cl-options.c


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,15 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck 
-check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
+// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck 
-check-prefix=LTO-WITHOUT-LLD %s
+// LTO-WITHOUT-LLD: invalid argument '-flto' only allowed with '-fuse-ld=lld'
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2352,8 +2352,13 @@
   Arg *FinalPhaseArg;
   phases::ID FinalPhase = getFinalPhase(Args, );
 
-  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
-Diag(clang::diag::err_drv_emit_llvm_link);
+  if (FinalPhase == phases::Link) {
+if (Args.hasArg(options::OPT_emit_llvm))
+  Diag(clang::diag::err_drv_emit_llvm_link);
+if (IsCLMode() && LTOMode != LTOK_None &&
+!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+  Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< "-flto" << "-fuse-ld=lld";
   }
 
   // Reject -Z* at the top level, these options should never have been exposed
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, 
Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, 
Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,15 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck -check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
+// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
+// LTO-WITHOUT-LLD: invalid argument '-flto' only allowed with '-fuse-ld=lld'
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2352,8 +2352,13 @@
   Arg *FinalPhaseArg;
   phases::ID FinalPhase = getFinalPhase(Args, );
 
-  if (FinalPhase == phases::Link && Args.hasArg(options::OPT_emit_llvm)) {
-Diag(clang::diag::err_drv_emit_llvm_link);
+  if (FinalPhase == phases::Link) {
+if (Args.hasArg(options::OPT_emit_llvm))
+  Diag(clang::diag::err_drv_emit_llvm_link);
+if (IsCLMode() && LTOMode != LTOK_None &&
+!Args.getLastArgValue(options::OPT_fuse_ld_EQ).equals_lower("lld"))
+  Diag(clang::diag::err_drv_argument_only_allowed_with)
+<< "-flto" << "-fuse-ld=lld";
   }
 
   // Reject -Z* at the top level, these options should never have been exposed
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
___
cfe-commits mailing list

[PATCH] D30239: enable -flto=thin in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 89583.
inglorion added a comment.

added missing --


https://reviews.llvm.org/D30239

Files:
  include/clang/Driver/Options.td
  test/Driver/cl-options.c


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,12 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck 
-check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, 
Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, 
Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,12 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto -- %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin -- %s 2>&1 | FileCheck -check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30239: enable -flto=thin in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion added inline comments.



Comment at: test/Driver/cl-options.c:525
 
+// RUN: %clang_cl -### /c -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto

hans wrote:
> This needs `--` before `%s`, otherwise if `%s` expands to e.g. `/Users/foo` 
> it will be interpreted as the `/U` option :-)
> 
> Same thing below.
Good catch. That's what I get for not copy-pasting it. ;-)


https://reviews.llvm.org/D30239



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

[PATCH] D30239: enable -flto=thin in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion updated this revision to Diff 89577.
inglorion retitled this revision from "enable -flto=thin, -flto-jobs=, and 
-fthinlto-index= in clang-cl" to "enable -flto=thin in clang-cl".
inglorion added a comment.

Implemented @hans's suggestion of moving the tests into cl-options.c.

Also restricted the implementation to only implement -flto=. -flto-index= 
doesn't seem useful without further changes (it just complains that it requires 
-x ir, which is not supported by clang-cl). -flto-jobs would require a fair bit 
of extra work compared to what is here now, and I'm not sure it's worth it. The 
number of jobs lld-link uses can be controlled with /link:/opt:lldltojobs=N, 
anyway. We can implement support for this later if we want it - for now I 
really just want -flto=thin.


https://reviews.llvm.org/D30239

Files:
  include/clang/Driver/Options.td
  test/Driver/cl-options.c


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,12 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin %s 2>&1 | FileCheck 
-check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, 
Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, 
Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, 
Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;


Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -522,6 +522,12 @@
 
 // RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
 
+// RUN: %clang_cl -### /c -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
+// LTO: -flto
+
+// RUN: %clang_cl -### /c -flto=thin %s 2>&1 | FileCheck -check-prefix=LTO-THIN %s
+// LTO-THIN: -flto=thin
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -957,7 +957,7 @@
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group;
-def flto_EQ : Joined<["-"], "flto=">, Flags<[CC1Option]>, Group,
+def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Set LTO mode to either 'full' or 'thin'">;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group,
   HelpText<"Enable LTO in 'full' mode">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30239: enable -flto=thin, -flto-jobs=, and -fthinlto-index= in clang-cl

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

@mehdi_amini:

> Is clang-cl using lld as default? How is the switch done? Ideally we should 
> have a nice error message from the driver if -flto is used without lld.

I believe we use link.exe by default. You can use lld by passing -fuse-ld=lld 
to the compiler.

I can add an error message when -flto is used without -fuse-ld=lld at least for 
the case when linking is actually performed. Of course, it's possible to invoke 
clang-cl without it doing any linking. If you're only compiling, it's perfectly 
valid to use -flto without -fuse-ld=lld.


https://reviews.llvm.org/D30239



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

[PATCH] D25216: Improve error message when referencing a non-tag type with a tag

[Bob Haarman via Phabricator via cfe-commits]

inglorion added a comment.

I like it. I'll give others a little time to respond, but if no objections are 
raised and nobody else accepts first, I'll accept it tomorrow.


https://reviews.llvm.org/D25216



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