[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Clement Courbet via Phabricator via cfe-commits
courbet abandoned this revision.
courbet marked an inline comment as done.
courbet added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+std::vector&& obj = ...;
+return std::move(obj);  // calls StatusOr::StatusOr(std::vector&&)
+  }

lebedev.ri wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > courbet wrote:
> > > > JonasToth wrote:
> > > > > While checking this example it seems clang already has a warning for 
> > > > > that case?
> > > > > 
> > > > > https://godbolt.org/z/q5zzh-
> > > > > 
> > > > > What parts of this check will be more then the warnings already do?
> > > > I was not aware of that, thanks for pointing that out. I don't think 
> > > > the check does more than the warning in that case. TBH I have not seen 
> > > > instances of this while running the check on our codebase (I'm only 
> > > > looking at a sample of the mistakes though, there are too many hits to 
> > > > look at all of them). All mistakes I have seen are of the `const` kind. 
> > > > 
> > > > The value we get from having this in the form of a check is more 
> > > > control over which types are allowed through the clang-tidy options.
> > > The `const std::vector f` isn't diagnosed by the existing diag: 
> > > https://godbolt.org/z/ZTQ3H6
> > so should we split this up in a diagnostic and a clang-tidy check? maybe it 
> > makes more sense to improve the warning further?
> I'd +1 just enhancing the clang diagnostic.
> The downside is that it does not allow blacklist, sure.
OK I'll work on improving the warning.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Thank you, looks good to me.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69145



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


[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+std::vector&& obj = ...;
+return std::move(obj);  // calls StatusOr::StatusOr(std::vector&&)
+  }

JonasToth wrote:
> lebedev.ri wrote:
> > courbet wrote:
> > > JonasToth wrote:
> > > > While checking this example it seems clang already has a warning for 
> > > > that case?
> > > > 
> > > > https://godbolt.org/z/q5zzh-
> > > > 
> > > > What parts of this check will be more then the warnings already do?
> > > I was not aware of that, thanks for pointing that out. I don't think the 
> > > check does more than the warning in that case. TBH I have not seen 
> > > instances of this while running the check on our codebase (I'm only 
> > > looking at a sample of the mistakes though, there are too many hits to 
> > > look at all of them). All mistakes I have seen are of the `const` kind. 
> > > 
> > > The value we get from having this in the form of a check is more control 
> > > over which types are allowed through the clang-tidy options.
> > The `const std::vector f` isn't diagnosed by the existing diag: 
> > https://godbolt.org/z/ZTQ3H6
> so should we split this up in a diagnostic and a clang-tidy check? maybe it 
> makes more sense to improve the warning further?
I'd +1 just enhancing the clang diagnostic.
The downside is that it does not allow blacklist, sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D70165#1750606 , @mitchell-stellar 
wrote:

> Yes, there was a failing unit test that had to be fixed. I reverted the first 
> commit and then committed a version that fixed the failing test.


I mean, the commit message (including differential link) was wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[clang] d044dcc - Revert "[clang][IFS] Driver pipeline: generate interface stubs after standard pipeline."

2019-11-18 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T02:08:22-05:00
New Revision: d044dcc5e492181b1517347013a780d9eda6c3c3

URL: 
https://github.com/llvm/llvm-project/commit/d044dcc5e492181b1517347013a780d9eda6c3c3
DIFF: 
https://github.com/llvm/llvm-project/commit/d044dcc5e492181b1517347013a780d9eda6c3c3.diff

LOG: Revert "[clang][IFS] Driver pipeline: generate interface stubs after 
standard pipeline."

This reverts commit 58ea00b51fe9b011301484957556872fced7dd08.

Test for .o + .ifs sidecar files is brittle and failing on bots.
Reverting to unblock.

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/InterfaceStubs.cpp
clang/lib/Driver/Types.cpp
clang/test/InterfaceStubs/driver-test.c
clang/test/InterfaceStubs/windows.cpp

Removed: 
clang/test/InterfaceStubs/driver-test2.c



diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 40cada744d05..cdf4a579f431 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -292,6 +292,10 @@ phases::ID Driver::getFinalPhase(const DerivedArgList ,
  (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
 FinalPhase = phases::Compile;
 
+  // clang interface stubs
+  } else if ((PhaseArg = DAL.getLastArg(options::OPT_emit_interface_stubs))) {
+FinalPhase = phases::IfsMerge;
+
   // -S only runs up to the backend.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
 FinalPhase = phases::Backend;
@@ -3498,68 +3502,6 @@ void Driver::BuildActions(Compilation , DerivedArgList 
,
 Actions.push_back(
 C.MakeAction(MergerInputs, types::TY_Image));
 
-  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector PhaseList;
-if (Args.hasArg(options::OPT_c)) {
-  llvm::SmallVector 
CompilePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
-  llvm::copy_if(PhaseList, std::back_inserter(CompilePhaseList),
-[&](phases::ID Phase) { return Phase <= phases::Compile; 
});
-} else {
-  types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
-}
-
-ActionList MergerInputs;
-
-for (auto  : Inputs) {
-  types::ID InputType = I.first;
-  const Arg *InputArg = I.second;
-
-  // Currently clang and the llvm assembler do not support generating 
symbol
-  // stubs from assembly, so we skip the input on asm files. For ifs files
-  // we rely on the normal pipeline setup in the pipeline setup code above.
-  if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
-  InputType == types::TY_Asm)
-continue;
-
-  Action *Current = C.MakeAction(*InputArg, InputType);
-
-  for (auto Phase : PhaseList) {
-switch (Phase) {
-default:
-  llvm_unreachable(
-  "IFS Pipeline can only consist of Compile followed by 
IfsMerge.");
-case phases::Compile: {
-  // Only IfsMerge (llvm-ifs) can handle .o files by looking for ifs
-  // files where the .o file is located. The compile action can not
-  // handle this.
-  if (InputType == types::TY_Object)
-break;
-
-  Current = C.MakeAction(Current, types::TY_IFS_CPP);
-  break;
-}
-case phases::IfsMerge: {
-  assert(Phase == PhaseList.back() &&
- "merging must be final compilation step.");
-  MergerInputs.push_back(Current);
-  Current = nullptr;
-  break;
-}
-}
-  }
-
-  // If we ended with something, add to the output list.
-  if (Current)
-Actions.push_back(Current);
-}
-
-// Add an interface stubs merge action if necessary.
-if (!MergerInputs.empty())
-  Actions.push_back(
-  C.MakeAction(MergerInputs, types::TY_Image));
-  }
-
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a 
custom
   // Compile phase that prints out supported cpu models and quits.
   if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {
@@ -3661,6 +3603,8 @@ Action *Driver::ConstructPhaseAction(
   return C.MakeAction(Input, types::TY_ModuleFile);
 if (Args.hasArg(options::OPT_verify_pch))
   return C.MakeAction(Input, types::TY_Nothing);
+if (Args.hasArg(options::OPT_emit_interface_stubs))
+  return C.MakeAction(Input, types::TY_IFS_CPP);
 return C.MakeAction(Input, types::TY_LLVM_BC);
   }
   case phases::Backend: {
@@ -3689,16 +3633,11 @@ void Driver::BuildJobs(Compilation ) const {
   Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o);
 
   // It is an error to provide a -o option if we are making multiple output
-  // files. There is one exception, IfsMergeJob: when generating interface 
stubs
-  // enabled we want to be able to generate the stub file at the same time that
-  // we generate the real library/a.out. So when 

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

2019-11-18 Thread Puyan Lotfi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG58ea00b51fe9: [clang][IFS] Driver pipeline: generate 
interface stubs after standard pipeline. (authored by plotfi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test2.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test2.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test2.c
@@ -0,0 +1,14 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   driver-test2.o object.o weak.o -S -o - | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1 2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -9,6 +9,7 @@
 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {
@@ -21,13 +22,36 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto  : Inputs)
-CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write 

[clang] 58ea00b - [clang][IFS] Driver pipeline: generate interface stubs after standard pipeline.

2019-11-18 Thread Puyan Lotfi via cfe-commits

Author: Puyan Lotfi
Date: 2019-11-19T01:18:02-05:00
New Revision: 58ea00b51fe9b011301484957556872fced7dd08

URL: 
https://github.com/llvm/llvm-project/commit/58ea00b51fe9b011301484957556872fced7dd08
DIFF: 
https://github.com/llvm/llvm-project/commit/58ea00b51fe9b011301484957556872fced7dd08.diff

LOG: [clang][IFS] Driver pipeline: generate interface stubs after standard 
pipeline.

Up until now, clang interface stubs has replaced the standard
PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
conjunction with it. So what when you build your code you will get an
a.out or lib.so as well as an interface stub file.

Example:

clang -shared -o libfoo.so -emit-interface-stubs ...

will generate both a libfoo.so and a libfoo.ifso. The .so file will
contain the code from the standard compilation pipeline and the .ifso
file will contain the ELF stub library.

Differential Revision: https://reviews.llvm.org/D70274

Added: 
clang/test/InterfaceStubs/driver-test2.c

Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/InterfaceStubs.cpp
clang/lib/Driver/Types.cpp
clang/test/InterfaceStubs/driver-test.c
clang/test/InterfaceStubs/windows.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index cdf4a579f431..40cada744d05 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList ,
  (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
 FinalPhase = phases::Compile;
 
-  // clang interface stubs
-  } else if ((PhaseArg = DAL.getLastArg(options::OPT_emit_interface_stubs))) {
-FinalPhase = phases::IfsMerge;
-
   // -S only runs up to the backend.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
 FinalPhase = phases::Backend;
@@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation , DerivedArgList 
,
 Actions.push_back(
 C.MakeAction(MergerInputs, types::TY_Image));
 
+  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
+llvm::SmallVector PhaseList;
+if (Args.hasArg(options::OPT_c)) {
+  llvm::SmallVector 
CompilePhaseList;
+  types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
+  llvm::copy_if(PhaseList, std::back_inserter(CompilePhaseList),
+[&](phases::ID Phase) { return Phase <= phases::Compile; 
});
+} else {
+  types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
+}
+
+ActionList MergerInputs;
+
+for (auto  : Inputs) {
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+
+  // Currently clang and the llvm assembler do not support generating 
symbol
+  // stubs from assembly, so we skip the input on asm files. For ifs files
+  // we rely on the normal pipeline setup in the pipeline setup code above.
+  if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
+  InputType == types::TY_Asm)
+continue;
+
+  Action *Current = C.MakeAction(*InputArg, InputType);
+
+  for (auto Phase : PhaseList) {
+switch (Phase) {
+default:
+  llvm_unreachable(
+  "IFS Pipeline can only consist of Compile followed by 
IfsMerge.");
+case phases::Compile: {
+  // Only IfsMerge (llvm-ifs) can handle .o files by looking for ifs
+  // files where the .o file is located. The compile action can not
+  // handle this.
+  if (InputType == types::TY_Object)
+break;
+
+  Current = C.MakeAction(Current, types::TY_IFS_CPP);
+  break;
+}
+case phases::IfsMerge: {
+  assert(Phase == PhaseList.back() &&
+ "merging must be final compilation step.");
+  MergerInputs.push_back(Current);
+  Current = nullptr;
+  break;
+}
+}
+  }
+
+  // If we ended with something, add to the output list.
+  if (Current)
+Actions.push_back(Current);
+}
+
+// Add an interface stubs merge action if necessary.
+if (!MergerInputs.empty())
+  Actions.push_back(
+  C.MakeAction(MergerInputs, types::TY_Image));
+  }
+
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a 
custom
   // Compile phase that prints out supported cpu models and quits.
   if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {
@@ -3603,8 +3661,6 @@ Action *Driver::ConstructPhaseAction(
   return C.MakeAction(Input, types::TY_ModuleFile);
 if (Args.hasArg(options::OPT_verify_pch))
   return C.MakeAction(Input, types::TY_Nothing);
-if (Args.hasArg(options::OPT_emit_interface_stubs))
-  return C.MakeAction(Input, types::TY_IFS_CPP);
 return C.MakeAction(Input, types::TY_LLVM_BC);
   }
   case phases::Backend: {
@@ -3633,11 +3689,16 @@ 

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

2019-11-18 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 229973.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test2.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test2.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test2.c
@@ -0,0 +1,14 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   driver-test2.o object.o weak.o -S -o - | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1 2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -9,6 +9,7 @@
 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {
@@ -21,13 +22,36 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto  : Inputs)
-CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
+  // invoked then we would like to get 

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

2019-11-18 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment.

This is normally done by using `-Bstatic`/`-Bdynamic` around the library. See 
`tools::addOpenMPRuntime`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70416



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Michele Scandale via Phabricator via cfe-commits
michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2408
+case options::OPT_frounding_math:
+  // The default setting for frounding-math is True and ffast-math
+  // sets fno-rounding-math, but we only want to use constrained

Isn't the default `-fno-rounding-math`?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2416
+  RoundingFPMath = false;
+  RoundingMathPresent = false;
+  break;

Shouldn't this be set to `true` similarly to what you do for 
`TrappingMathPresent ` to track whether there is an explicit option related to 
rounding math?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2574
+// FP Exception Behavior is also set to strict
+assert(FPExceptionBehavior.equals("strict"));
+CmdArgs.push_back("-ftrapping-math");

Running `clang -### -ftrapping-math -ffp-exception-behavior=ignore` lead to 
this assertion to fail. As far as I can see `TrappingMath` is not changed in 
the case FPExceptionBehavior is "ignore" or "maytrap".
Clearly in the "ignore" case it should be safe to just set `TrappingMath` to 
false, but I'm not sure about the "maytrap" case.
It seems that `-ffp-exception-behavior` is more general than 
`-f{,no-}trapping-math`, so it seems natural to me to see `ftrapping-math` and 
`foo-trapping-math` as aliases for `ffp-exception-behavior=strict` and 
`ffp-exception-behavior=ignore` respectively. If we agree on this, then I would 
expect the reasoning inside the compiler only in terms of `FPExceptionBehavior`.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");

With this change if I run `clang -### -ffast-math test.c` I don't see 
`-fno-trapping-math` passed to the CC1. 
This is changing the changes the value of the function level attribute 
"no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
Is this an intended change?

Moreover since with this patch the default value for trapping math changed, the 
"no-trapping-math" function level attribute is incorrect also for default case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D64305: [clangd] Add path mappings functionality

2019-11-18 Thread William Wagner via Phabricator via cfe-commits
wwagner19 marked 2 inline comments as done.
wwagner19 added inline comments.



Comment at: clang-tools-extra/clangd/PathMapping.cpp:153
+// Converts \pPath to a posix-style absolute, i.e. if it's a windows path
+// then the backward-slash notation will be converted to forward slash
+llvm::Expected parsePath(llvm::StringRef Path) {

sammccall wrote:
> wwagner19 wrote:
> > sammccall wrote:
> > > "posix-style" doesn't really describe representing `c:\foo` as `/c:/foo`. 
> > > That's really *just* a URI thing AFAIK.
> > > 
> > > Something like "Converts a unix/windows path to the path part of a file 
> > > URI".
> > > But in that case, I think the implementation is just 
> > > `URI::createFile(Path).body()`. Does that pass tests?
> > Oh I did not realize `createFile` was a thing, however looking at the way 
> > it's implemented now, won't that throw an assert if a non-absolute path is 
> > passed in? If so, is that desirable at all?
> > 
> > IIUC, if I were to use that API, then wouldn't it make more sense for it to 
> > return an `llvm::Expected`? If we want to consolidate the logic to one 
> > place, I'd be happy to try and refactor the signature.
> I think allowing relative paths to be specified in path mappings is 
> needlessly confusing. Clangd flags are typically specified in a config file 
> somewhere, and that config often doesn't know about the working directory.
> 
> We don't want to hit the assert, so I'd suggest testing if it's absolute 
> first, and returning an error if not.
So even with checking for absolute paths before calling `createFile`, it still 
won't work quite right. Currently, `createFile`, and consequently 
`FileSystemScheme().uriFromAbsolutePath(AbsolutePath)`, converts paths 
differently depending on the environment Clangd is running on (via WIN32 or 
some other means).

e.g. if we had mapping `C:\home=/workarea` and Clangd built/running on linux, 
then the `replace_path_prefix` by default would use posix style, which won't 
replace the `\`. This may not be **too** useful in practice, but it's a small 
price to pay for windows-unix-interop, I feel.


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

https://reviews.llvm.org/D64305



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


[PATCH] D64305: [clangd] Add path mappings functionality

2019-11-18 Thread William Wagner via Phabricator via cfe-commits
wwagner19 updated this revision to Diff 229969.
wwagner19 marked 2 inline comments as done.
wwagner19 added a comment.

Awesome! I am not an LLVM committer, so if you could commit on my behalf that'd 
be great- although I'm not sure how LLVM handles squashing/merging, continuous 
integration, etc., so please let me know if I need to do anything else (aside 
from the code of course).

Once again, thanks for all the help - I learned a lot!


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

https://reviews.llvm.org/D64305

Files:
  clang-tools-extra/clangd/PathMapping.cpp


Index: clang-tools-extra/clangd/PathMapping.cpp
===
--- clang-tools-extra/clangd/PathMapping.cpp
+++ clang-tools-extra/clangd/PathMapping.cpp
@@ -128,21 +128,6 @@
  llvm::inconvertibleErrorCode());
 }
 
-// Returns whether a path mapping should take place for OrigPath
-// i.e. MappingPath is a proper sub-path of  OrigPath
-bool mappingMatches(llvm::StringRef OrigPath, llvm::StringRef MappingPath) {
-  namespace path = llvm::sys::path;
-  auto OrigPathItr = path::begin(OrigPath, path::Style::posix);
-  auto OrigPathEnd = path::end(OrigPath);
-  auto MappingPathItr = path::begin(MappingPath, path::Style::posix);
-  auto MappingPathEnd = path::end(MappingPath);
-  if (std::distance(OrigPathItr, OrigPathEnd) <
-  std::distance(MappingPathItr, MappingPathEnd)) {
-return false;
-  }
-  return std::equal(MappingPathItr, MappingPathEnd, OrigPathItr);
-}
-
 // Converts a unix/windows path to the path portion of a file URI
 // e.g. "C:\foo" -> "/C:/foo"
 llvm::Expected parsePath(llvm::StringRef Path) {
@@ -207,11 +192,11 @@
 const std::string  = Dir == PathMapping::Direction::ClientToServer
 ? Mapping.ServerPath
 : Mapping.ClientPath;
-if (mappingMatches(Uri->body(), From)) {
-  std::string MappedBody = std::move(Uri->body());
-  MappedBody.replace(MappedBody.find(From), From.length(), To);
-  auto MappedUri = URI(Uri->scheme(), Uri->authority(), 
MappedBody.c_str());
-  return MappedUri.toString();
+llvm::StringRef Body = Uri->body();
+if (Body.consume_front(From) && (Body.empty() || Body.front() == '/')) {
+  std::string MappedBody = (To + Body).str();
+  return URI(Uri->scheme(), Uri->authority(), MappedBody.c_str())
+  .toString();
 }
   }
   return llvm::None;


Index: clang-tools-extra/clangd/PathMapping.cpp
===
--- clang-tools-extra/clangd/PathMapping.cpp
+++ clang-tools-extra/clangd/PathMapping.cpp
@@ -128,21 +128,6 @@
  llvm::inconvertibleErrorCode());
 }
 
-// Returns whether a path mapping should take place for OrigPath
-// i.e. MappingPath is a proper sub-path of  OrigPath
-bool mappingMatches(llvm::StringRef OrigPath, llvm::StringRef MappingPath) {
-  namespace path = llvm::sys::path;
-  auto OrigPathItr = path::begin(OrigPath, path::Style::posix);
-  auto OrigPathEnd = path::end(OrigPath);
-  auto MappingPathItr = path::begin(MappingPath, path::Style::posix);
-  auto MappingPathEnd = path::end(MappingPath);
-  if (std::distance(OrigPathItr, OrigPathEnd) <
-  std::distance(MappingPathItr, MappingPathEnd)) {
-return false;
-  }
-  return std::equal(MappingPathItr, MappingPathEnd, OrigPathItr);
-}
-
 // Converts a unix/windows path to the path portion of a file URI
 // e.g. "C:\foo" -> "/C:/foo"
 llvm::Expected parsePath(llvm::StringRef Path) {
@@ -207,11 +192,11 @@
 const std::string  = Dir == PathMapping::Direction::ClientToServer
 ? Mapping.ServerPath
 : Mapping.ClientPath;
-if (mappingMatches(Uri->body(), From)) {
-  std::string MappedBody = std::move(Uri->body());
-  MappedBody.replace(MappedBody.find(From), From.length(), To);
-  auto MappedUri = URI(Uri->scheme(), Uri->authority(), MappedBody.c_str());
-  return MappedUri.toString();
+llvm::StringRef Body = Uri->body();
+if (Body.consume_front(From) && (Body.empty() || Body.front() == '/')) {
+  std::string MappedBody = (To + Body).str();
+  return URI(Uri->scheme(), Uri->authority(), MappedBody.c_str())
+  .toString();
 }
   }
   return llvm::None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D67508: [RISCV] support mutilib in baremetal environment

2019-11-18 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits
khchen updated this revision to Diff 229966.
khchen added a comment.

rebase and fix failed testcases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67508

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/bin/ld
  clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32i/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32im/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk/riscv64-unknown-elf/lib/rv64imafdc/lp64d/crt0.o
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -14,8 +14,8 @@
 // C-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
-// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
 // C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
 // C-RV64-BAREMETAL-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
@@ -29,8 +29,8 @@
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib{{/|}}crt0.o"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf{{/|}}lib"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf{{/|}}lib"
 // C-RV64-BAREMETAL-NOSYSROOT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" 

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

2019-11-18 Thread Josh Kunz via Phabricator via cfe-commits
jkz created this revision.
jkz added reviewers: saugustine, sivachandra.
Herald added a project: clang.

In the GNU toolchain, `-static-libgcc` implies that the unwindlib will be 
linked statically. However, when `--unwindlib=libunwind`, this flag is ignored, 
and a bare `-lunwind` is added to the linker args.  Unfortunately, this means 
that if both `libunwind.so`, and `libunwind.a` are present in the library path, 
`libunwind.so` will be chosen in all cases where `-static` is not set.

This change makes `-static-libgcc` affect the `-l` flag produced by 
`--unwindlib=libunwind`. After this patch, providing `-static-libgcc 
--unwindlib=libunwind` will cause the driver to explicitly emit 
`-l:libunwind.a` to statically link libunwind. For all other cases it will emit 
`-l:libunwind.so` matching current behavior with a more explicit link line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70416

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/compiler-rt-unwind.c


Index: clang/test/Driver/compiler-rt-unwind.c
===
--- clang/test/Driver/compiler-rt-unwind.c
+++ clang/test/Driver/compiler-rt-unwind.c
@@ -13,7 +13,15 @@
 // RUN: --gcc-toolchain="" \
 // RUN:   | FileCheck --check-prefix=RTLIB-GCC-UNWINDLIB-COMPILER-RT %s
 // RTLIB-GCC-UNWINDLIB-COMPILER-RT: "{{.*}}lgcc"
-// RTLIB-GCC-UNWINDLIB-COMPILER-RT: "{{.*}}lunwind"
+// RTLIB-GCC-UNWINDLIB-COMPILER-RT: "{{.*}}l:libunwind.so"
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=libgcc --unwindlib=libunwind \
+// RUN: -static-libgcc \
+// RUN: --gcc-toolchain="" \
+// RUN:   | FileCheck --check-prefix=RTLIB-GCC-STATIC-UNWINDLIB-COMPILER-RT %s
+// RTLIB-GCC-STATIC-UNWINDLIB-COMPILER-RT: "{{.*}}lgcc"
+// RTLIB-GCC-STATIC-UNWINDLIB-COMPILER-RT: "{{.*}}l:libunwind.a"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1   \
 // RUN: --target=x86_64-unknown-linux -rtlib=compiler-rt \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1209,7 +1209,10 @@
 break;
   }
   case ToolChain::UNW_CompilerRT:
-CmdArgs.push_back("-lunwind");
+if (LGT == LibGccType::StaticLibGcc)
+  CmdArgs.push_back("-l:libunwind.a");
+else
+  CmdArgs.push_back("-l:libunwind.so");
 break;
   }
 


Index: clang/test/Driver/compiler-rt-unwind.c
===
--- clang/test/Driver/compiler-rt-unwind.c
+++ clang/test/Driver/compiler-rt-unwind.c
@@ -13,7 +13,15 @@
 // RUN: --gcc-toolchain="" \
 // RUN:   | FileCheck --check-prefix=RTLIB-GCC-UNWINDLIB-COMPILER-RT %s
 // RTLIB-GCC-UNWINDLIB-COMPILER-RT: "{{.*}}lgcc"
-// RTLIB-GCC-UNWINDLIB-COMPILER-RT: "{{.*}}lunwind"
+// RTLIB-GCC-UNWINDLIB-COMPILER-RT: "{{.*}}l:libunwind.so"
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=libgcc --unwindlib=libunwind \
+// RUN: -static-libgcc \
+// RUN: --gcc-toolchain="" \
+// RUN:   | FileCheck --check-prefix=RTLIB-GCC-STATIC-UNWINDLIB-COMPILER-RT %s
+// RTLIB-GCC-STATIC-UNWINDLIB-COMPILER-RT: "{{.*}}lgcc"
+// RTLIB-GCC-STATIC-UNWINDLIB-COMPILER-RT: "{{.*}}l:libunwind.a"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1   \
 // RUN: --target=x86_64-unknown-linux -rtlib=compiler-rt \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1209,7 +1209,10 @@
 break;
   }
   case ToolChain::UNW_CompilerRT:
-CmdArgs.push_back("-lunwind");
+if (LGT == LibGccType::StaticLibGcc)
+  CmdArgs.push_back("-l:libunwind.a");
+else
+  CmdArgs.push_back("-l:libunwind.so");
 break;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69618: NeonEmitter: clean up prototype modifiers

2019-11-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

LGTM with a couple nits.




Comment at: clang/include/clang/Basic/arm_neon_incl.td:203
 //
 // The modifier 'd' means "default" and does not modify the base type in any
 // way. The available modifiers are given below.

'd' is gone.



Comment at: clang/utils/convert_arm_neon.py:1
+#!/usr/bin/env python3
+

Are you going to commit this script?  If you are, probably makes sense to 
include some sort of date, so it's clear which change you're talking about, and 
when it makes sense to remove it from the tree.


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

https://reviews.llvm.org/D69618



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


[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

2019-11-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Approval is not ever a time-based process; someone appropriate actually has to 
do the work of reviewing the patch.  If a patch doesn't get reviewed, you 
"ping" it a couple times, to note that you're waiting for a review.  If it 
still isn't reviewed at that point, and you're not sure what to do, send an 
email to cfe-dev.

In this case, it was on me to review; I apologize for not looking at this 
earlier.




Comment at: llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp:438
+  if (STI.isGPRegisterReserved(BasePtr - ARM::R0))
+return false;
   // We may also need a base pointer if there are dynamic allocas or stack

I'm a little concerned about this... what happens if r6 is reserved, and the 
code uses a construct which LLVM implements using a base pointer?  For example:

```
void f(void a(char*, char*), int n) {
  char r[n];
  char r2[16] __attribute((aligned(16)));
  a(r, r2);
}
```

(There are potentially other ways to implement this construct.  But I'm pretty 
sure using a base pointer is the only method which is currently implemented.)


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

https://reviews.llvm.org/D68862



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


Re: [clang] dd870f6 - Fix warning about unused std::unique result, erase shifted elements

2019-11-18 Thread David Blaikie via cfe-commits
OK - thanks for the context!

On Mon, Nov 18, 2019 at 5:07 PM Ben Langmuir  wrote:

> Nice find.
>
> The change LGTM, and is clearly what I always intended it to do.  I was
> confused how this ever worked at all, but I see that (at least in libc++)
> we move the values into place rather than swap them, so the values at the
> end are still the largest values for types with a trivial move constructor.
>
> I am not working in this area these days and am not setup to provide a
> test case in any reasonable amount of time.  I think it would need a gtest,
> as I'm not sure this change is obvservable otherwise.
>
> Ben
>
> On Nov 18, 2019, at 4:44 PM, David Blaikie  wrote:
>
> Hey Ben - if you're still involved with this part of the project - could
> you check that this change (to code you committed back in 2014, r215810) is
> correct & add a test case if possible?
>
> On Thu, Nov 7, 2019 at 10:39 AM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: Reid Kleckner
>> Date: 2019-11-07T10:39:29-08:00
>> New Revision: dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571.diff
>>
>> LOG: Fix warning about unused std::unique result, erase shifted elements
>>
>> This is actually a functional change. I haven't added any new test
>> coverage. I'm just trying to fix the warning and hoping for the best.
>>
>> Added:
>>
>>
>> Modified:
>> clang/include/clang/Serialization/ContinuousRangeMap.h
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/include/clang/Serialization/ContinuousRangeMap.h
>> b/clang/include/clang/Serialization/ContinuousRangeMap.h
>> index 0c05537dd108..c2665c097416 100644
>> --- a/clang/include/clang/Serialization/ContinuousRangeMap.h
>> +++ b/clang/include/clang/Serialization/ContinuousRangeMap.h
>> @@ -118,14 +118,17 @@ class ContinuousRangeMap {
>>
>>  ~Builder() {
>>llvm::sort(Self.Rep, Compare());
>> -  std::unique(Self.Rep.begin(), Self.Rep.end(),
>> -  [](const_reference A, const_reference B) {
>> -// FIXME: we should not allow any duplicate keys, but there are
>> a lot of
>> -// duplicate 0 -> 0 mappings to remove first.
>> -assert((A == B || A.first != B.first) &&
>> -   "ContinuousRangeMap::Builder given non-unique keys");
>> -return A == B;
>> -  });
>> +  Self.Rep.erase(
>> +  std::unique(
>> +  Self.Rep.begin(), Self.Rep.end(),
>> +  [](const_reference A, const_reference B) {
>> +// FIXME: we should not allow any duplicate keys, but
>> there are
>> +// a lot of duplicate 0 -> 0 mappings to remove first.
>> +assert((A == B || A.first != B.first) &&
>> +   "ContinuousRangeMap::Builder given non-unique
>> keys");
>> +return A == B;
>> +  }),
>> +  Self.Rep.end());
>>  }
>>
>>  void insert(const value_type ) {
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang] dd870f6 - Fix warning about unused std::unique result, erase shifted elements

2019-11-18 Thread Ben Langmuir via cfe-commits
Nice find.

The change LGTM, and is clearly what I always intended it to do.  I was 
confused how this ever worked at all, but I see that (at least in libc++) we 
move the values into place rather than swap them, so the values at the end are 
still the largest values for types with a trivial move constructor.

I am not working in this area these days and am not setup to provide a test 
case in any reasonable amount of time.  I think it would need a gtest, as I'm 
not sure this change is obvservable otherwise.

Ben

> On Nov 18, 2019, at 4:44 PM, David Blaikie  wrote:
> 
> Hey Ben - if you're still involved with this part of the project - could you 
> check that this change (to code you committed back in 2014, r215810) is 
> correct & add a test case if possible?
> 
> On Thu, Nov 7, 2019 at 10:39 AM Reid Kleckner via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> 
> Author: Reid Kleckner
> Date: 2019-11-07T10:39:29-08:00
> New Revision: dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
>  
> 
> DIFF: 
> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571.diff
>  
> 
> 
> LOG: Fix warning about unused std::unique result, erase shifted elements
> 
> This is actually a functional change. I haven't added any new test
> coverage. I'm just trying to fix the warning and hoping for the best.
> 
> Added: 
> 
> 
> Modified: 
> clang/include/clang/Serialization/ContinuousRangeMap.h
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/clang/include/clang/Serialization/ContinuousRangeMap.h 
> b/clang/include/clang/Serialization/ContinuousRangeMap.h
> index 0c05537dd108..c2665c097416 100644
> --- a/clang/include/clang/Serialization/ContinuousRangeMap.h
> +++ b/clang/include/clang/Serialization/ContinuousRangeMap.h
> @@ -118,14 +118,17 @@ class ContinuousRangeMap {
> 
>  ~Builder() {
>llvm::sort(Self.Rep, Compare());
> -  std::unique(Self.Rep.begin(), Self.Rep.end(),
> -  [](const_reference A, const_reference B) {
> -// FIXME: we should not allow any duplicate keys, but there are a 
> lot of
> -// duplicate 0 -> 0 mappings to remove first.
> -assert((A == B || A.first != B.first) &&
> -   "ContinuousRangeMap::Builder given non-unique keys");
> -return A == B;
> -  });
> +  Self.Rep.erase(
> +  std::unique(
> +  Self.Rep.begin(), Self.Rep.end(),
> +  [](const_reference A, const_reference B) {
> +// FIXME: we should not allow any duplicate keys, but there 
> are
> +// a lot of duplicate 0 -> 0 mappings to remove first.
> +assert((A == B || A.first != B.first) &&
> +   "ContinuousRangeMap::Builder given non-unique keys");
> +return A == B;
> +  }),
> +  Self.Rep.end());
>  }
> 
>  void insert(const value_type ) {
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> 

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


Re: [clang] dd870f6 - Fix warning about unused std::unique result, erase shifted elements

2019-11-18 Thread David Blaikie via cfe-commits
Hey Ben - if you're still involved with this part of the project - could
you check that this change (to code you committed back in 2014, r215810) is
correct & add a test case if possible?

On Thu, Nov 7, 2019 at 10:39 AM Reid Kleckner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Reid Kleckner
> Date: 2019-11-07T10:39:29-08:00
> New Revision: dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
>
> URL:
> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
> DIFF:
> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571.diff
>
> LOG: Fix warning about unused std::unique result, erase shifted elements
>
> This is actually a functional change. I haven't added any new test
> coverage. I'm just trying to fix the warning and hoping for the best.
>
> Added:
>
>
> Modified:
> clang/include/clang/Serialization/ContinuousRangeMap.h
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/include/clang/Serialization/ContinuousRangeMap.h
> b/clang/include/clang/Serialization/ContinuousRangeMap.h
> index 0c05537dd108..c2665c097416 100644
> --- a/clang/include/clang/Serialization/ContinuousRangeMap.h
> +++ b/clang/include/clang/Serialization/ContinuousRangeMap.h
> @@ -118,14 +118,17 @@ class ContinuousRangeMap {
>
>  ~Builder() {
>llvm::sort(Self.Rep, Compare());
> -  std::unique(Self.Rep.begin(), Self.Rep.end(),
> -  [](const_reference A, const_reference B) {
> -// FIXME: we should not allow any duplicate keys, but there are a
> lot of
> -// duplicate 0 -> 0 mappings to remove first.
> -assert((A == B || A.first != B.first) &&
> -   "ContinuousRangeMap::Builder given non-unique keys");
> -return A == B;
> -  });
> +  Self.Rep.erase(
> +  std::unique(
> +  Self.Rep.begin(), Self.Rep.end(),
> +  [](const_reference A, const_reference B) {
> +// FIXME: we should not allow any duplicate keys, but
> there are
> +// a lot of duplicate 0 -> 0 mappings to remove first.
> +assert((A == B || A.first != B.first) &&
> +   "ContinuousRangeMap::Builder given non-unique
> keys");
> +return A == B;
> +  }),
> +  Self.Rep.end());
>  }
>
>  void insert(const value_type ) {
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang] d0f3c82 - Fix uninitialized variable warnings. NFCI.

2019-11-18 Thread David Blaikie via cfe-commits
Which compiler/what sort of warning was this addressing? (it can be
beneficial to leave variables uninitialized if their value isn't intended
to be used - so things like asan can catch bugs where the read of
uninitialized is unintended)

On Sat, Nov 2, 2019 at 11:27 AM Simon Pilgrim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Simon Pilgrim
> Date: 2019-11-02T18:03:21Z
> New Revision: d0f3c822160e36e10588bc86dabde6ab8d63cf10
>
> URL:
> https://github.com/llvm/llvm-project/commit/d0f3c822160e36e10588bc86dabde6ab8d63cf10
> DIFF:
> https://github.com/llvm/llvm-project/commit/d0f3c822160e36e10588bc86dabde6ab8d63cf10.diff
>
> LOG: Fix uninitialized variable warnings. NFCI.
>
> Added:
>
>
> Modified:
> clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
> b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
> index 778375010041..f694c3e4380a 100644
> --- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
> +++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
> @@ -134,7 +134,7 @@ namespace {
>
>  const Record *ExplicitDef;
>
> -GroupInfo() : ExplicitDef(nullptr) {}
> +GroupInfo() : IDNo(0), ExplicitDef(nullptr) {}
>};
>  } // end anonymous namespace.
>
> @@ -554,7 +554,7 @@ struct SelectPiece : Piece {
>
>ModifierType ModKind;
>std::vector Options;
> -  int Index;
> +  int Index = 0;
>
>static bool classof(const Piece *P) {
>  return P->getPieceClass() == SelectPieceClass ||
> @@ -566,7 +566,7 @@ struct PluralPiece : SelectPiece {
>PluralPiece() : SelectPiece(PluralPieceClass, MT_Plural) {}
>
>std::vector OptionPrefixes;
> -  int Index;
> +  int Index = 0;
>
>static bool classof(const Piece *P) {
>  return P->getPieceClass() == PluralPieceClass;
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 229943.
rnk added a comment.

- comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -332,6 +332,9 @@
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = (Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -332,6 +332,9 @@
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70340: Add a key method to Sema to optimize debug info size

2019-11-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D70340#1749073 , @thakis wrote:

> With the overhead being the cost of a single vtable with one entry? Or is 
> there more?


I guess I worry about the extra dead vtable pointer in Sema. But, I don't think 
it matters. I think we should do this. I'll re-upload with comments and update 
the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



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


[PATCH] D69934: [clangd] Implement rename by using SelectionTree and findExplicitReferences.

2019-11-18 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

The test fails on Windows:

http://45.33.8.238/win/2544/step_7.txt

  Failing Tests (2):
  Clangd Unit Tests :: ./ClangdTests.exe/RenameTest.Renameable
  Clangd Unit Tests :: ./ClangdTests.exe/RenameTest.WithinFileRename

I'm guessing this needs the -fno-delayed-template-parsing treatment (look for 
that in other files in clangd/unittests)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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


[clang-tools-extra] f805c60 - Revert "[clangd] Implement rename by using SelectionTree and findExplicitReferences."

2019-11-18 Thread Wolfgang Pieb via cfe-commits

Author: Wolfgang Pieb
Date: 2019-11-18T15:39:05-08:00
New Revision: f805c60a093325c16ce4200d2615ef48555d9cb8

URL: 
https://github.com/llvm/llvm-project/commit/f805c60a093325c16ce4200d2615ef48555d9cb8
DIFF: 
https://github.com/llvm/llvm-project/commit/f805c60a093325c16ce4200d2615ef48555d9cb8.diff

LOG: Revert "[clangd] Implement rename by using SelectionTree and 
findExplicitReferences."

This reverts commit 4f80fc2491cc35730a9a84b86975278b7daa8522.

Caused buildbot failure at
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/58251

Added: 


Modified: 
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index fb83083384f9..3969f3e2e4e2 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -8,16 +8,14 @@
 
 #include "refactor/Rename.h"
 #include "AST.h"
-#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
-#include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
-#include "clang/AST/DeclCXX.h"
-#include "clang/AST/DeclTemplate.h"
-#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
+#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
+#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 
 namespace clang {
 namespace clangd {
@@ -36,17 +34,6 @@ llvm::Optional filePath(const SymbolLocation 
,
   return *Path;
 }
 
-// Returns true if the given location is expanded from any macro body.
-bool isInMacroBody(const SourceManager , SourceLocation Loc) {
-  while (Loc.isMacroID()) {
-if (SM.isMacroBodyExpansion(Loc))
-  return true;
-Loc = SM.getImmediateMacroCallerLoc(Loc);
-  }
-
-  return false;
-}
-
 // Query the index to find some other files where the Decl is referenced.
 llvm::Optional getOtherRefFile(const Decl , StringRef MainFile,
 const SymbolIndex ) {
@@ -69,41 +56,12 @@ llvm::Optional getOtherRefFile(const Decl , 
StringRef MainFile,
   return OtherFile;
 }
 
-llvm::DenseSet locateDeclAt(ParsedAST ,
-  SourceLocation TokenStartLoc) {
-  unsigned Offset =
-  AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
-
-  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
-  const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
-  if (!SelectedNode)
-return {};
-
-  // If the location points to a Decl, we check it is actually on the name
-  // range of the Decl. This would avoid allowing rename on unrelated tokens.
-  //   ^class Foo {} // SelectionTree returns CXXRecordDecl,
-  // // we don't attempt to trigger rename on this position.
-  // FIXME: make this work on destructors, e.g. "~F^oo()".
-  if (const auto *D = SelectedNode->ASTNode.get()) {
-if (D->getLocation() != TokenStartLoc)
-  return {};
-  }
-
-  llvm::DenseSet Result;
-  for (const auto *D :
-   targetDecl(SelectedNode->ASTNode,
-  DeclRelation::Alias | DeclRelation::TemplatePattern))
-Result.insert(D);
-  return Result;
-}
-
 enum ReasonToReject {
   NoSymbolFound,
   NoIndexProvided,
   NonIndexable,
   UsedOutsideFile,
   UnsupportedSymbol,
-  AmbiguousSymbol,
 };
 
 // Check the symbol Decl is renameable (per the index) within the file.
@@ -167,8 +125,6 @@ llvm::Error makeError(ReasonToReject Reason) {
   return "symbol may be used in other files (not eligible for indexing)";
 case UnsupportedSymbol:
   return "symbol is not a supported kind (e.g. namespace, macro)";
-case AmbiguousSymbol:
-  return "there are multiple symbols at the given location";
 }
 llvm_unreachable("unhandled reason kind");
   };
@@ -178,38 +134,22 @@ llvm::Error makeError(ReasonToReject Reason) {
 }
 
 // Return all rename occurrences in the main file.
-std::vector findOccurrencesWithinFile(ParsedAST ,
-  const NamedDecl ) {
-  // In theory, locateDeclAt should return the primary template. However, if 
the
-  // cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
-  // will be the CXXRecordDecl, for this case, we need to get the primary
-  // template maunally.
-  const auto  =
-  ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
-  // getUSRsForDeclaration will find other related symbols, e.g. virtual and 
its
-  // overriddens, primary template and all explicit specializations.
-  // FIXME: get rid of the remaining tooling APIs.
-  std::vector RenameUSRs = tooling::getUSRsForDeclaration(
-  

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso abandoned this revision.
Charusso added a comment.

This patch moved to D70411 .


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

https://reviews.llvm.org/D69813



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


[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a comment.

Thanks @lebedev.ri for taking the time to think this through and reply. All 
that makes sense, so I've changed the default to `0`.

In addition to adding it to the ReleaseNotes, once clang-tidy 10 is released 
I'll reply to a StackOverflow question about this error with a link to the new 
option. That should further increase the odds of anyone who searches on the 
`gcc` warning/error easily finding the clang-tidy option.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69145



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


[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc updated this revision to Diff 229934.
poelmanc edited the summary of this revision.
poelmanc added a comment.

Switch default to `0`. Add Release Note with some detail to increase the 
chances of someone finding this with  an Internet search on the error message.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy %s readability-redundant-member-init %t
+// RUN: %check_clang_tidy %s readability-redundant-member-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-member-init.IgnoreBaseInCopyConstructors, \
+// RUN:   value: 1}] \
+// RUN: }"
 
 struct S {
   S() = default;
@@ -116,6 +120,35 @@
   };
 };
 
+// struct whose inline copy constructor default-initializes its base class
+struct WithCopyConstructor1 : public T {
+  WithCopyConstructor1(const WithCopyConstructor1& other) : T(),
+f(),
+g()
+  {}
+  S f, g;
+};
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'f' is redundant
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'g' is redundant
+// CHECK-FIXES: WithCopyConstructor1(const WithCopyConstructor1& other) : T()
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: {}
+
+// struct whose copy constructor default-initializes its base class
+struct WithCopyConstructor2 : public T {
+  WithCopyConstructor2(const WithCopyConstructor2& other);
+  S a;
+};
+WithCopyConstructor2::WithCopyConstructor2(const WithCopyConstructor2& other)
+  : T(), a()
+{}
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: initializer for member 'a' is redundant
+// CHECK-FIXES: {{^}}  : T() {{$}}
+// CHECK-NEXT: {}
+
 // Initializer not written
 struct NF1 {
   NF1() {}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
@@ -6,7 +6,8 @@
 Finds member initializations that are unnecessary because the same default
 constructor would be called if they were not present.
 
-Example:
+Example
+---
 
 .. code-block:: c++
 
@@ -18,3 +19,27 @@
   private:
 std::string s;
   };
+
+Options
+---
+
+.. option:: IgnoreBaseInCopyConstructors
+
+Default is ``0``.
+
+When non-zero, the check will ignore unnecessary base class initializations
+within copy constructors, since some compilers issue warnings/errors when
+base classes are not explicitly intialized in copy constructors. For example,
+``gcc`` with ``-Wextra`` or ``-Werror=extra`` issues warning or error
+``base class ‘Bar’ should be explicitly initialized in the copy constructor``
+if ``Bar()`` were removed in the following example:
+
+.. code-block:: c++
+
+  // Explicitly initializing member s and base class Bar is unnecessary.
+  struct Foo : public Bar {
+// Remove s() below. If IgnoreBaseInCopyConstructors!=0, keep Bar().
+Foo(const Foo& foo) : Bar(), s() {}
+std::string s;
+  };
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,13 @@
   The check now supports the ``AllowOverrideAndFinal`` option to eliminate
   conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.
 
+- Improved :doc:`readability-redundant-member-init
+  ` check.
+
+  The check  now supports the ``IgnoreBaseInCopyConstructors`` option to avoid
+  `"base class \‘Foo\’ should be explicitly initialized in the copy constructor"`
+  warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
+
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h

[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked an inline comment as done.
Charusso added a comment.

Somehow the `MallocBugVisitor` is broken in the wild, and we need to support a 
lot more to call it non-alpha (see the `FIXME`s). I do not want to spend time 
on fixing the visitor, but rather I would make this checker alpha, and move on. 
After a while when we have enough alarms we could see what could go wrong. The 
reports are not bad, I tried to make it super false-positive free.

@aaron.ballman the name became `security.cert.str.31.c` and we lack the support 
to invoke every C checker with `str.*.c` (as I know), but for now it should be 
fine.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h:18
+namespace ento {
+
 /// Helper class to store information about the dynamic size.

That change is totally unrelated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70411



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


[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-18 Thread Zachary Turner via Phabricator via cfe-commits
zturner marked 2 inline comments as done.
zturner added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:116
+static const Expr *ignoreTemporariesAndImplicitCasts(const Expr *E) {
+  if (const auto *T = dyn_cast(E))
+return ignoreTemporariesAndImplicitCasts(T->GetTemporaryExpr());

aaron.ballman wrote:
> What about `CXXBindTemporaryExpr`?
> 
> Would `Expr::IgnoreImplicit()` do too much stripping because it removes 
> `FullExpr` as well?
I don't actually know.  This patch is literally my first exposure to clang's 
frontend and AST manipulations, so I was kind of just trying different things 
until something worked here.  I didn't even know about `IgnoreImplicit()` or 
`FullExpr`.  I'll play around with them and report back.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:153
+
+  return HNM.matchesNode(*dyn_cast(D));
+}

aaron.ballman wrote:
> This should probably use `cast<>` if it's going to assume the returned value 
> is never null.
Good point.  Since we're talking about this code anyway, it felt super hacky to 
instantiate an AST matcher just to check for the qualified name of a Decl.  Is 
there a better way to do this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70368



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


[clang] ea1db31 - [CodeGen] Assign locations to calls to special struct helpers

2019-11-18 Thread Vedant Kumar via cfe-commits

Author: Vedant Kumar
Date: 2019-11-18T15:07:59-08:00
New Revision: ea1db31d20a74e5025a86b0df49163d5bfecb67b

URL: 
https://github.com/llvm/llvm-project/commit/ea1db31d20a74e5025a86b0df49163d5bfecb67b
DIFF: 
https://github.com/llvm/llvm-project/commit/ea1db31d20a74e5025a86b0df49163d5bfecb67b.diff

LOG: [CodeGen] Assign locations to calls to special struct helpers

Assign artificial locations to calls to special struct-related helper
functions.

Such calls may not inherit a location if emitted within FinishFunction,
at which point the lexical scope stack may be empty, causing CGDebugInfo
to report the current DebugLoc as empty.

Fixes an IR verifier complaint about a call to '__destructor_8_s0' not
having a !dbg location attached.

rdar://57293361

Added: 


Modified: 
clang/lib/CodeGen/CGNonTrivialStruct.cpp
clang/test/CodeGenObjC/nontrivial-c-struct-exception.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGNonTrivialStruct.cpp 
b/clang/lib/CodeGen/CGNonTrivialStruct.cpp
index 05615aa12881..332e51e57ded 100644
--- a/clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ b/clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -817,6 +817,7 @@ template 
 static void callSpecialFunction(G &, StringRef FuncName, QualType QT,
 bool IsVolatile, CodeGenFunction ,
 std::array Addrs) {
+  auto SetArtificialLoc = ApplyDebugLocation::CreateArtificial(CGF);
   for (unsigned I = 0; I < N; ++I)
 Addrs[I] = CGF.Builder.CreateBitCast(Addrs[I], CGF.CGM.Int8PtrPtrTy);
   QT = IsVolatile ? QT.withVolatile() : QT;

diff  --git a/clang/test/CodeGenObjC/nontrivial-c-struct-exception.m 
b/clang/test/CodeGenObjC/nontrivial-c-struct-exception.m
index 7db53bb74249..1733a019026c 100644
--- a/clang/test/CodeGenObjC/nontrivial-c-struct-exception.m
+++ b/clang/test/CodeGenObjC/nontrivial-c-struct-exception.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks 
-fobjc-runtime=ios-11.0 -fobjc-exceptions -fexceptions -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks 
-fobjc-runtime=ios-11.0 -fobjc-exceptions -fexceptions 
-debug-info-kind=line-tables-only -emit-llvm -o - %s | FileCheck %s
 
 // CHECK: %[[STRUCT_STRONG:.*]] = type { i32, i8* }
 // CHECK: %[[STRUCT_WEAK:.*]] = type { i32, i8* }
@@ -25,8 +25,8 @@
 // CHECK-NEXT: ret void
 
 // CHECK: landingpad { i8*, i32 }
-// CHECK: %[[V9:.*]] = bitcast %[[STRUCT_STRONG]]* %[[AGG_TMP]] to i8**
-// CHECK: call void @__destructor_8_s8(i8** %[[V9]])
+// CHECK: %[[V9:.*]] = bitcast %[[STRUCT_STRONG]]* %[[AGG_TMP]] to i8**{{.*}}, 
!dbg [[ARTIFICIAL_LOC_1:![0-9]+]]
+// CHECK: call void @__destructor_8_s8(i8** %[[V9]]){{.*}}, !dbg 
[[ARTIFICIAL_LOC_1]]
 // CHECK: br label
 
 // CHECK: resume
@@ -48,8 +48,8 @@ void testStrongException(void) {
 // CHECK: ret void
 
 // CHECK: landingpad { i8*, i32 }
-// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_WEAK]]* %[[AGG_TMP]] to i8**
-// CHECK: call void @__destructor_8_w8(i8** %[[V3]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_WEAK]]* %[[AGG_TMP]] to i8**{{.*}}, 
!dbg [[ARTIFICIAL_LOC_2:![0-9]+]]
+// CHECK: call void @__destructor_8_w8(i8** %[[V3]]){{.*}}, !dbg 
[[ARTIFICIAL_LOC_2]]
 // CHECK: br label
 
 // CHECK: resume
@@ -60,3 +60,6 @@ void testStrongException(void) {
 void testWeakException(void) {
   calleeWeak(genWeak(), genWeak());
 }
+
+// CHECK-DAG: [[ARTIFICIAL_LOC_1]] = !DILocation(line: 0
+// CHECK-DAG: [[ARTIFICIAL_LOC_2]] = !DILocation(line: 0



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


[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny.

This checker warns on most of the following rules:
https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator


Repository:
  rC Clang

https://reviews.llvm.org/D70411

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Checkers/AllocationState.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/cert/str31-c-fp-suppression.cpp
  clang/test/Analysis/cert/str31-c-notes.cpp
  clang/test/Analysis/cert/str31-c.cpp

Index: clang/test/Analysis/cert/str31-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-c.cpp
@@ -0,0 +1,181 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,security.cert.str.31.c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR31:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+namespace test_gets_bad {
+#define BUFFER_SIZE 1024
+
+void func(void) {
+  char buf[BUFFER_SIZE];
+  if (gets(buf)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf'}}
+}
+} // namespace test_gets_bad
+
+namespace test_gets_good {
+enum { BUFFERSIZE = 32 };
+
+void func(void) {
+  char buff[BUFFERSIZE];
+
+  if (fgets(buff, sizeof(buff), stdin)) {}
+}
+} // namespace test_gets_good
+
+namespace test_sprintf_bad {
+void func(const char *name) {
+  char buf[128];
+  sprintf(buf, "%s.txt", name);
+  // expected-warning@-1 {{'sprintf' could write outside of 'buf'}}
+}
+} // namespace test_sprintf_bad
+
+namespace test_sprintf_good {
+void func(const char *name) {
+  char buff[128];
+  snprintf(buff, sizeof(buff), "%s.txt", name);
+}
+} // namespace test_sprintf_good
+
+namespace test_fscanf_bad {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buf[BUF_LENGTH];
+  if (fscanf(stdin, "%s", buf)) {}
+  // expected-warning@-1 {{'fscanf' could write outside of 'buf'}}
+}
+} // namespace test_fscanf_bad
+
+namespace test_fscanf_good {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buff[BUF_LENGTH];
+  if (fscanf(stdin, "%1023s", buff)) {}
+}
+} // namespace test_fscanf_good
+
+namespace test_strcpy_bad {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char prog_name[128];
+  strcpy(prog_name, name);
+  // expected-warning@-1 {{'strcpy' could write outside of 'prog_name'}}
+  return 0;
+}
+
+void func(void) {
+  char buff[256];
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+strcpy(buff, editor);
+// expected-warning@-1 {{'strcpy' could write outside of 'buff'}}
+  }
+}
+} // namespace test_strcpy_bad
+
+namespace test_strcpy_good {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char *prog_name2 = (char *)malloc(strlen(name) + 1);
+  if (prog_name2 != NULL) {
+strcpy(prog_name2, name);
+  }
+  free(prog_name2);
+  return 0;
+}
+
+void func(void) {
+  char *buff2;
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+size_t len = strlen(editor) + 1;
+buff2 = (char *)malloc(len);
+if (buff2 != NULL) {
+  strcpy(buff2, editor);
+}
+free(buff2);
+  }
+}
+} // namespace test_strcpy_good
+
+//===--===//
+// The following is from the rule's page which we do not handle yet.
+//===--===//
+
+namespace test_loop_index_bad {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // namespace test_loop_index_bad
+
+namespace test_loop_index_good {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n - 1); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // namespace test_loop_index_good
+
+namespace 

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

2019-11-18 Thread Gabor Bencze via Phabricator via cfe-commits
gbencze added a comment.

In D70052#1749730 , @JonasToth wrote:

> There is a `ExprMutAnalyzer` that is able to find mutation of expressions in 
> general (even though it is kinda experimental still). Maybe that should be 
> utilized somehow? I think the current implementation does not cover when the 
> address is taken and mutation happens through pointers/references in free 
> standing functions, does it?
>
> On the other hand it makes the check more complicated, slower. Additionally 
> the most cases are catched with this version, i guess.


You're right, the current version does not cover mutations through pointers and 
references. I'm not sure how common these would be, but `ExprMutAnalyzer` seems 
like a great option if we wanted to add support for those cases as well.


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

https://reviews.llvm.org/D70052



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


[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

2019-11-18 Thread Gabor Bencze via Phabricator via cfe-commits
gbencze updated this revision to Diff 229925.
gbencze added a comment.

Formatting changes (curly braces, newline at EOF)
Remove incorrect flag from test


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

https://reviews.llvm.org/D70052

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
  clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop58-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp
@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s cert-oop58-cpp %t
+
+// Example test cases from CERT rule
+// https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP58-CPP.+Copy+operations+must+not+mutate+the+source+object
+namespace test_mutating_noncompliant_example {
+class A {
+  mutable int m;
+
+public:
+  A() : m(0) {}
+  explicit A(int m) : m(m) {}
+
+  A(const A ) : m(other.m) {
+other.m = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+  }
+
+  A =(const A ) {
+if ( != this) {
+  m = other.m;
+  other.m = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: mutating copied object
+}
+return *this;
+  }
+
+  int get_m() const { return m; }
+};
+} // namespace test_mutating_noncompliant_example
+
+namespace test_mutating_compliant_example {
+class B {
+  int m;
+
+public:
+  B() : m(0) {}
+  explicit B(int m) : m(m) {}
+
+  B(const B ) : m(other.m) {}
+  B(B &) : m(other.m) {
+other.m = 0; //no-warning: mutation allowed in move constructor
+  }
+
+  B =(const B ) {
+if ( != this) {
+  m = other.m;
+}
+return *this;
+  }
+
+  B =(B &) {
+m = other.m;
+other.m = 0; //no-warning: mutation allowed in move assignment operator
+return *this;
+  }
+
+  int get_m() const { return m; }
+};
+} // namespace test_mutating_compliant_example
+
+namespace test_mutating_pointer {
+class C {
+  C *ptr;
+  int value;
+
+  C();
+  C(C ) {
+other = {};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+other.ptr = nullptr;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+other.value = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+
+// no-warning: mutating a pointee is allowed
+other.ptr->value = 0;
+*other.ptr = {};
+  }
+};
+} // namespace test_mutating_pointer
+
+namespace test_mutating_indirect_member {
+struct S {
+  int x;
+};
+
+class D {
+  S s;
+  D(D ) {
+other.s = {};
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+other.s.x = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: mutating copied object
+  }
+};
+} // namespace test_mutating_indirect_member
+
+namespace test_mutating_other_object {
+class E {
+  E();
+  E(E ) {
+E tmp;
+// no-warning: mutating an object that is not the source is allowed
+tmp = {};
+  }
+};
+} // namespace test_mutating_other_object
+
+namespace test_mutating_member_function {
+class F {
+  int a;
+
+public:
+  void bad_func() { a = 12; }
+  void fine_func() const;
+  void fine_func_2(int x) { x = 5; }
+  void questionable_func();
+
+  F(F ) : a(other.a) {
+this->bad_func(); // no-warning: mutating this is allowed
+
+other.bad_func();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+
+other.fine_func();
+other.fine_func_2(42);
+other.questionable_func();
+  }
+};
+} // namespace test_mutating_member_function
+
+namespace test_mutating_function_on_nested_object {
+struct S {
+  int x;
+  void mutate(int y) {
+x = y;
+  }
+};
+
+class G {
+  S s;
+  G(G ) {
+s.mutate(0); // no-warning: mutating this is allowed
+
+other.s.mutate(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: call mutates copied object
+  }
+};
+} // namespace test_mutating_function_on_nested_object
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -105,6 +105,7 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
+   cert-oop58-cpp
clang-analyzer-core.CallAndMessage (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DivideZero (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DynamicTypePropagation
Index: 

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

2019-11-18 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+std::vector&& obj = ...;
+return std::move(obj);  // calls StatusOr::StatusOr(std::vector&&)
+  }

lebedev.ri wrote:
> courbet wrote:
> > JonasToth wrote:
> > > While checking this example it seems clang already has a warning for that 
> > > case?
> > > 
> > > https://godbolt.org/z/q5zzh-
> > > 
> > > What parts of this check will be more then the warnings already do?
> > I was not aware of that, thanks for pointing that out. I don't think the 
> > check does more than the warning in that case. TBH I have not seen 
> > instances of this while running the check on our codebase (I'm only looking 
> > at a sample of the mistakes though, there are too many hits to look at all 
> > of them). All mistakes I have seen are of the `const` kind. 
> > 
> > The value we get from having this in the form of a check is more control 
> > over which types are allowed through the clang-tidy options.
> The `const std::vector f` isn't diagnosed by the existing diag: 
> https://godbolt.org/z/ZTQ3H6
so should we split this up in a diagnostic and a clang-tidy check? maybe it 
makes more sense to improve the warning further?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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


[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

2019-11-18 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd accepted this revision.
compnerd added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/Driver.cpp:3491
+  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
+
+llvm::SmallVector PhaseList;

Extraneous whitespace



Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:37
+  if (OutputFilename != "-") {
+if (llvm::sys::path::extension(OutputFilename) == ".so")
+  llvm::sys::path::replace_extension(OutputFilename,

Is there a way to check if the extension is a shared library extension, or 
better yet, check for the output type



Comment at: clang/test/InterfaceStubs/driver-test.c:4
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 
-emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1   2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s

Unnecessary whitespace change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274



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


[PATCH] D70306: clang: Exherbo multiarch ajustments

2019-11-18 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd requested changes to this revision.
compnerd added a comment.
This revision now requires changes to proceed.

The test adjustments are incorrect.  They should optionally accept the triple 
in the path.




Comment at: clang/lib/Driver/ToolChains/Linux.cpp:360
+  addPathIfExists(D,
+  LibPath + "/../../" + GCCTriple.str() + "/lib/../" +
+  OSLibDir + SelectedMultilib.osSuffix(),

Could you use `llvm::sys::path::stem` please?



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:889
+  break;
+}
+  }

Why not just always construct the path?  `"/usr/" + getTriple.str() + 
"/include"` is always going to be the path that we have for exherbo



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:984
+  if (Distro == Distro::Exherbo &&
+  addLibStdCXXIncludePaths(
+  LibDir.str() + "/../../" + TripleStr + "/include",

Why is this part of the `if`?  This should be part of the code executed 
conditionally.


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

https://reviews.llvm.org/D70306



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


[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

2019-11-18 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen updated this revision to Diff 229922.
cchen added a comment.

Add and fix test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69316

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -292,4 +292,39 @@
   {++arg;}
 }
 #endif
+///==///
+// RUN: %clang_cc1 -DCK5 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK5 --check-prefix CK5-64
+// RUN: %clang_cc1 -DCK5 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK5 --check-prefix CK5-64
+// RUN: %clang_cc1 -DCK5 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK5 --check-prefix CK5-32
+// RUN: %clang_cc1 -DCK5 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK5 --check-prefix CK5-32
+
+// RUN: %clang_cc1 -DCK5 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK5 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK5 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK5 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+#ifdef CK5
+
+// CK5: [[SIZE:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 80]
+// CK5: [[MTYPE:@.+]] = {{.+}}constant [1 x i64] [i64 33]
+
+void target_update_contiguous() {
+  int marr[10][10][10];
+
+  // CK5-DAG: call void @__tgt_target_data_update(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE]]{{.+}})
+  // CK5-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
+  // CK5-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
+  // CK5-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
+  // CK5-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
+  // CK5-DAG: [[BPC0:%.+]] = bitcast i8** [[BP0]] to [10 x [10 x [10 x i32]]]**
+  // CK5-DAG: [[PC0:%.+]] = bitcast i8** [[P0]] to [10 x i32]**
+  // CK5-DAG: store [10 x [10 x [10 x i32]]]* %marr, [10 x [10 x [10 x i32]]]** [[BPC0]]
+  // CK5-DAG: store [10 x i32]* {{.+}}, [10 x i32]** [[PC0]]
+  #pragma omp target update to(marr[2][0:2][0:2])
+}
+#endif
 #endif
Index: clang/test/OpenMP/target_update_ast_print.cpp
===
--- clang/test/OpenMP/target_update_ast_print.cpp
+++ clang/test/OpenMP/target_update_ast_print.cpp
@@ -5,6 +5,14 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -DOMP5 -verify -fopenmp -fopenmp-version=50 -ast-print %s | FileCheck %s --check-prefix=OMP5
+// RUN: %clang_cc1 -DOMP5 -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: 

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment.

Yes, there was a failing unit test that had to be fixed. I reverted the first 
commit and then committed a version that fixed the failing test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Bill Wendling via Phabricator via cfe-commits
void marked 2 inline comments as done.
void added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:831
+  for (auto i = as->begin_outputs(), e = as->end_outputs(); i != e; ++i)
+if (const VarDecl *VD = findVar(*i).getDecl())
+  vals[VD] = Initialized;

nickdesaulniers wrote:
> this is still not a range based for loop.
> 
> Does:
> 
> ```
> for (const auto  : as->outputs())
>   if (const VarDecl *VD = findVar(O).getDecl())
> vals[VD] = Initialized;
> ```
> not work?
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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


[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

This is a great re-write, thank you for working on it!




Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:105
+  CallableInfo Callable;
+
+  SmallVector BindArguments;

You can remove some newlines here.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:116
+static const Expr *ignoreTemporariesAndImplicitCasts(const Expr *E) {
+  if (const auto *T = dyn_cast(E))
+return ignoreTemporariesAndImplicitCasts(T->GetTemporaryExpr());

What about `CXXBindTemporaryExpr`?

Would `Expr::IgnoreImplicit()` do too much stripping because it removes 
`FullExpr` as well?



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:124
+static const Expr *ignoreTemporariesAndPointers(const Expr *E) {
+
+  if (const auto *T = dyn_cast(E))

Spurious newline



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:130
+
+  if (const auto *T = dyn_cast(E))
+return ignoreTemporariesAndPointers(T->GetTemporaryExpr());

Similar question here about bound temporaries as above.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:143-147
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+  OS << "capture" << CaptureIndex++;
+  OS.flush();
+  return S;

`llvm::utostr()` instead of using a streaming interface?



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:144
+  std::string S;
+  llvm::raw_string_ostream OS(S);
+  OS << "capture" << CaptureIndex++;

I think you can drop the `llvm::` here and elsewhere in the file.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:153
+
+  return HNM.matchesNode(*dyn_cast(D));
+}

This should probably use `cast<>` if it's going to assume the returned value is 
never null.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:196-199
+  for (const auto *Child : Statement->children()) {
+if (anyDescendantIsLocal(Child))
+  return true;
+  }

`return llvm::any_of(Statement->children(), anyDescendantIsLocal);`?



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:431-434
+  if (!Candidates.empty())
+return Candidates;
+
+  return Candidates;

It seems like `return Candidates;` will suffice. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70368



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


[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 229921.
void added a comment.

Adjust loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/Analysis/uninit-asm-goto.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/Parser/asm-goto.c
  clang/test/Parser/asm-goto.cpp
  clang/test/Sema/asm-goto.cpp

Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -1,53 +1,51 @@
 // RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
 // RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -verify -fsyntax-only
 
-struct NonTrivial {
-  ~NonTrivial();
+struct S {
+  ~S();
   int f(int);
 private:
   int k;
 };
-void JumpDiagnostics(int n) {
-// expected-error@+1 {{cannot jump from this goto statement to its label}}
+void test1(int n) {
+  // expected-error@+1 {{cannot jump from this goto statement to its label}}
   goto DirectJump;
-// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp1;
+  // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  S s1;
 
 DirectJump:
-// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm goto("jmp %l0;" Later);
-// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp2;
-// expected-note@+1 {{possible target of asm goto statement}}
+  // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  S s2;
+  // expected-note@+1 {{possible target of asm goto statement}}
 Later:
   return;
 }
 
-struct S { ~S(); };
-void foo(int a) {
+struct T { ~T(); };
+void test2(int a) {
   if (a) {
 FOO:
-// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
-// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
-S s;
-void *p = &
-// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
-  asm goto("jmp %l0;" BAR);
-// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+T t;
+void *p = &
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" bar);
+// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
 goto *p;
 p = &
 goto *p;
 return;
   }
-// expected-note@+2 {{possible target of asm goto statement}}
-// expected-note@+1 {{possible target of indirect goto statement}}
-BAR:
+  // expected-note@+2 {{possible target of asm goto statement}}
+  // expected-note@+1 {{possible target of indirect goto statement}}
+bar:
   return;
 }
 
-
-//Asm goto:
-int test16(int n)
+int test3(int n)
 {
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
@@ -57,7 +55,7 @@
   return ({int a[n];label_true: 2;});
   // expected-note@+1 {{jump bypasses initialization of variable length array}}
   int b[n];
-// expected-note@+1 {{possible target of asm goto statement}}
+  // expected-note@+1 {{possible target of asm goto statement}}
 loop:
   return 0;
 }
Index: clang/test/Parser/asm-goto.cpp
===
--- clang/test/Parser/asm-goto.cpp
+++ clang/test/Parser/asm-goto.cpp
@@ -1,14 +1,54 @@
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -fsyntax-only -verify -std=c++11 %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify -std=c++11 %s
 
-int zoo ()
-{
+int a, b, c, d, e, f, g, h, i, j, k, l;
+
+void test1(void) {
+  __asm__ volatile goto (""
+:: [a] "r" (a), [b] "r" (b), [c] "r" (c), [d] "r" (d),
+   [e] "r" (e), [f] "r" (f), [g] "r" (g), [h] "r" (h),
+   [i] "r" (i), [j] "r" (j), [k] "r" (k), [l] "r" (l)
+::lab1,lab2);
+lab1: return;
+lab2: return;
+}
+
+void test2(void) {
+  __asm__ volatile goto (""
+:: [a] "r,m" (a), [b] "r,m" (b), [c] "r,m" (c), [d] "r,m" (d),
+   [e] "r,m" (e), [f] "r,m" (f), [g] "r,m" (g), [h] "r,m" (h),
+   [i] "r,m" (i), [j] "r,m" (j), [k] "r,m" (k), [l] "r,m" (l)
+:: lab);
+  lab: return;
+}
+
+int 

[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:831
+  for (auto i = as->begin_outputs(), e = as->end_outputs(); i != e; ++i)
+if (const VarDecl *VD = findVar(*i).getDecl())
+  vals[VD] = Initialized;

this is still not a range based for loop.

Does:

```
for (const auto  : as->outputs())
  if (const VarDecl *VD = findVar(O).getDecl())
vals[VD] = Initialized;
```
not work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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


[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: mitchell-stellar, lebedev.ri.
lebedev.ri added a comment.

@poelmanc @mitchell-stellar
I'm confused by the diff - did this land? Was the correct patch committed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Ah, so it *really* landed in rG06f3dabe4a2e85a32ade27c0769b6084c828a206 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70165



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


[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

2019-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp:73-79
+  if (const auto *MemberCall =
+  Result.Nodes.getNodeAs(MutatingCallName)) {
+diag(MemberCall->getBeginLoc(), "call mutates copied object");
+  } else if (const auto *Assignment =
+ Result.Nodes.getNodeAs(MutatingOperatorName)) {
+diag(Assignment->getBeginLoc(), "mutating copied object");
+  }

You can elide the curly braces here per our usual coding conventions.



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-oop58-cpp.rst:12
+`_.
\ No newline at end of file


Please add the newline to the end of the file.



Comment at: clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp:1
+// RUN: %check_clang_tidy %s cert-oop58-cpp %t -std=c++11-or-later
+

This `-std` looks incorrect to me -- you can remove it entirely (we already 
default to later than C++11 anyway).



Comment at: clang-tools-extra/test/clang-tidy/checkers/cert-oop58-cpp.cpp:150
+} // namespace test_mutating_function_on_nested_object
\ No newline at end of file


Please add the newline to the end of the file.


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

https://reviews.llvm.org/D70052



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


[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D69145#1750529 , @aaron.ballman 
wrote:

> In D69145#1748733 , @lebedev.ri 
> wrote:
>
> > In D69145#1748716 , @poelmanc 
> > wrote:
> >
> > > In D69145#1715611 , @mgehre 
> > > wrote:
> > >
> > > > Exactly due to the issue you are fixing here, we ended up disabling the 
> > > > complete check because we didn't want to live with the warnings it 
> > > > produced on -Wextra.
> > > >  Therefore, I'm actually strongly in favor to enable the option by 
> > > > default.
> > > >
> > > > When others see that clang-tidy fixits introduce warnings (with 
> > > > -Wextra) or even break their build (with -Werror), they might not look 
> > > > into check options, but just disable the check directly.
> > >
> > >
> > > Just pinging to see if anyone has any thoughts on moving forward with 
> > > this. Thanks in advance for any feedback!
> >
> >
> > If this would default to off i'd signoff.
>
>
> It sounds like you and @mgehre would like a different default for the new 
> option?


I suppose. It's not unlike "clang -Wall is not supposed to match gcc's -Wall" 
issue.
Everyone may expect different behavior. Defaulting to on (don't warn) isn't 
misguided,
there's merit in not changing the code if it would break some other compiler 
the code supports.

But the other way around is also true, what if that gcc's diagnostic is not 
enabled,
or gcc is just not used? One then would need to actually know that there is an 
option
that should be flipped. **Here** i would prefer to have to disable something
if it's unwanted rather than somehow finding out that something can be enabled..

As a more meaningful data point, a similar relaxation to another check was 
added in D70165 ,
and there the default is to still warn - so not following the precedent seems 
inconsistent to me.

> Personally, I don't have a strong opinion on the default, but



> I do think we should have the option so that users can control the behavior 
> so it doesn't conflict between clang-tidy and gcc.

Yes, i don't disagree that having *an ability* to control this is good.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69145



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


[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

2019-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a subscriber: lebedev.ri.
aaron.ballman added a comment.

In D67545#1749579 , @lebedev.ri wrote:

> In D67545#1749561 , @balazske wrote:
>
> > Ping. It should be accepted before I can land it.
>
>
> @aaron.ballman already accepted.


And I'm happy to re-affirm my acceptance. :-D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545



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


[PATCH] D70359: [clangd] Show values of more expressions on hover

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:255
+  // Show enums symbolically, not numerically like APValue::printPretty().
+  if (E->getType()->isEnumeralType() &&
+  Constant.Val.getInt().getMinSignedBits() <= 64) {

kadircet wrote:
> both `Constant.Val.getInt` and `ECD->getInitVal` are `APSInt` and it has an 
> `operator==`, why cast to `int64_t` in between?
operator== asserts(!) that the widths are equal. (And they're not always)
added a comment



Comment at: clang-tools-extra/clangd/Hover.cpp:423
+  //  - certain expressions (sizeof etc)
+  //  - built-in types
 }

lh123 wrote:
> lh123 wrote:
> > sammccall wrote:
> > > lh123 wrote:
> > > > I think we should also support hover on literal.
> > > sure - can you explain what you'd like to see/when you'd expect it to be 
> > > useful?
> > such as:
> > 1. Hovering on "abc" shows char[4], we can get the length information of 
> > the string.
> > 2. Get actual type for user-defined literal types.
> > ```c++
> > struct foo {};
> > struct foo {};
> > 
> > foo operator""_foo(unsigned long long v) { return {}; }
> > 
> > int main() {
> > 1_foo;
> > }
> > 
> > ```
> > 
> ```
> struct foo {};
> 
> foo operator""_foo(unsigned long long v) { return {}; }
> 
> int main() {
> 1_foo;
> }
> ```
Added a comment about literals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70359



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


[clang] 0213add - [NFC] Fix 'target' condition in checkTargetFeatures

2019-11-18 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2019-11-18T13:43:52-08:00
New Revision: 0213adde218530bc31e5c4e50b49704c6bb2f2e9

URL: 
https://github.com/llvm/llvm-project/commit/0213adde218530bc31e5c4e50b49704c6bb2f2e9
DIFF: 
https://github.com/llvm/llvm-project/commit/0213adde218530bc31e5c4e50b49704c6bb2f2e9.diff

LOG: [NFC] Fix 'target' condition in checkTargetFeatures

checkTargetFeatures was incorrectly checking for cpu_specific instead of
just 'target'. While this function was never called in that situation,
it seemed correct to fix the condition.  Additionally, multiversion
functions can never be always_inline, but if any function accidentially
ended up here we shouldn't diagnose.

Note that the adding of target-features to the list is unnecessary since
the getFunctionFeatureMap actually considers attribute target,
however adding it results in significantly better error messages by
putting the 'target' features first (and thus first to fail).
Otherwise, the error message would be the first feature 'implied' by the
target attribute, and not necessarily the feature listed in the
attribute itself.

Added: 


Modified: 
clang/lib/CodeGen/CodeGenFunction.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 6f2bd8c16b3b..68b599e88bc3 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2223,8 +2223,8 @@ void CodeGenFunction::checkTargetFeatures(SourceLocation 
Loc,
   << TargetDecl->getDeclName()
   << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
 
-  } else if (TargetDecl->hasAttr() ||
- TargetDecl->hasAttr()) {
+  } else if (!TargetDecl->isMultiVersion() &&
+ TargetDecl->hasAttr()) {
 // Get the required features for the callee.
 
 const TargetAttr *TD = TargetDecl->getAttr();



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


[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

2019-11-18 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added inline comments.



Comment at: clang/lib/CodeGen/CGLoopInfo.cpp:302-306
+// Imply vectorize.enable when it is not already disabled/enabled.
+Args.push_back(
+MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
+  ConstantAsMetadata::get(ConstantInt::get(
+  llvm::Type::getInt1Ty(Ctx), 1))}));

SjoerdMeijer wrote:
> Meinersbur wrote:
> > [serious] Why not reusing the `Args.push_back` code from above? I think it 
> > is important `vectorize_predicate` and `vectorize_width` (and ever 
> > additional property we introduce in the future) the same way. IMHO 
> > everything else becomes more and more confusing.
> > I have the following in mind:
> > 
> > ```
> > if (Attrs.VectorizeEnable != LoopAttributes::Unspecified ||
> >   IsVectorPredicateEnabled || Attrs.VectorizeWidth > 1) {
> > auto AttrVal = Attrs.VectorizeEnable != LoopAttributes::Disable;
> > Args.push_back(..., ConstantInt::get(AttrVal));
> > }
> > ```
> > 
> No worries, and thanks for looking again! I was a bit reluctant to touch that 
> piece of logic (and actually thought this if-elseif was not too bad and 
> explicit in identifying the different cases), but yeah, what you suggest make 
> sense, so will address this soon.
Sounds like a typical case of "[[ https://en.wikipedia.org/wiki/Technical_debt 
| technical debt ]]".


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

https://reviews.llvm.org/D69628



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


[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D69145#1748733 , @lebedev.ri wrote:

> In D69145#1748716 , @poelmanc wrote:
>
> > In D69145#1715611 , @mgehre wrote:
> >
> > > Exactly due to the issue you are fixing here, we ended up disabling the 
> > > complete check because we didn't want to live with the warnings it 
> > > produced on -Wextra.
> > >  Therefore, I'm actually strongly in favor to enable the option by 
> > > default.
> > >
> > > When others see that clang-tidy fixits introduce warnings (with -Wextra) 
> > > or even break their build (with -Werror), they might not look into check 
> > > options, but just disable the check directly.
> >
> >
> > Just pinging to see if anyone has any thoughts on moving forward with this. 
> > Thanks in advance for any feedback!
>
>
> If this would default to off i'd signoff.


It sounds like you and @mgehre would like a different default for the new 
option? Personally, I don't have a strong opinion on the default, but I do 
think we should have the option so that users can control the behavior so it 
doesn't conflict between clang-tidy and gcc.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69145



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


[PATCH] D70256: [FPEnv] clang support for constrained FP builtins

2019-11-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:357
+
+  if (ConstrainedIntrinsicID != Intrinsic::not_intrinsic &&
+  CGF.Builder.getIsFPConstrained()) {

kpn wrote:
> rjmccall wrote:
> > I don't see any situation where `not_intrinsic` is passed in here; why all 
> > these checks?
> I tend to be conservative, and I'm not good at predicting the future. I can 
> remove the checks easily in the next round.
I just can't imagine why you would ever call this function without a 
constrained intrinsic ID when its only purpose is to choose between a 
constrained and unconstrained intrinsic.  It's always better to start with 
stronger preconditions and then generalize later if necessary.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2302
+  return RValue::get(Builder.CreateCall(F, {Base, Exponent}));
+}
   }

kpn wrote:
> rjmccall wrote:
> > What can this not use the standard path?
> I'll take another look. It wasn't using the standard path before so I made my 
> added code match. If it can use the standard path in both cases then maybe it 
> should?
Yeah.  It's possible the type-heterogeneity (the power always being an `int`) 
stops that from working for some reason, but if not, it's always nicer to 
re-use more code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70256



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


[PATCH] D70317: [clangd] More sensible output for constructors/destructors in hover.

2019-11-18 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

Build result: pass - 60168 tests passed, 0 failed and 731 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70317



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


[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D70325#1749432 , @kadircet wrote:

> LGTM, with a question. What about default template params? I believe we would 
> also like to print them, could you add a test case for that?


They're not printed, as the type is "as written". Added a testcase and a FIXME.
(I don't think this case is terribly important - either behavior seems to have 
its advantages, the combination of default parameters and partial 
specialization is fairly rare, and not much confusion seems likely in practice)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70325



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


[PATCH] D70256: [FPEnv] clang support for constrained FP builtins

2019-11-18 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn marked 2 inline comments as done.
kpn added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:357
+
+  if (ConstrainedIntrinsicID != Intrinsic::not_intrinsic &&
+  CGF.Builder.getIsFPConstrained()) {

rjmccall wrote:
> I don't see any situation where `not_intrinsic` is passed in here; why all 
> these checks?
I tend to be conservative, and I'm not good at predicting the future. I can 
remove the checks easily in the next round.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2302
+  return RValue::get(Builder.CreateCall(F, {Base, Exponent}));
+}
   }

rjmccall wrote:
> What can this not use the standard path?
I'll take another look. It wasn't using the standard path before so I made my 
added code match. If it can use the standard path in both cases then maybe it 
should?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70256



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added a comment.

In D62731#1750427 , @mibintc wrote:

> In D62731#1750412 , @kpn wrote:
>
> > Does anyone think a warning is appropriate because the new flags are 
> > exercising experimental, incomplete code in both clang and llvm? The 
> > warning would be removed when we believe the feature is complete and ready 
> > to use.
>
>
> @kpn Can you say more about "incomplete code in ... clang".  I don't know 
> what's missing from clang.


See D70256 . Calls to clang's math builtins 
that are llvm intrinsics need to be changed to be calls to the constrained 
intrinsics. I've been waiting to submit the patches because they weren't 
testable without these command line options.

There may be other issues as well. I'm not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70325: [clangd] Fix hover 'local scope' to include class template params

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 229910.
sammccall added a comment.

add fixme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70325

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -910,16 +910,15 @@
}},
   // Constructor of partially-specialized class template
   {R"cpp(
-  template struct X;
+  template struct X;
   template struct X{ [[^X]](); };
   )cpp",
[](HoverInfo ) {
  HI.NamespaceScope = "";
  HI.Name = "X";
- HI.LocalScope = "X::";// FIXME: Should be X::
+ HI.LocalScope = "X::"; // FIXME: X::
  HI.Kind = SymbolKind::Constructor;
- HI.Type = "void ()";  // FIXME: Should be None
- HI.ReturnType = "void";   // FIXME: Should be None or X
+ HI.ReturnType = "X";
  HI.Definition = "X()";
  HI.Parameters.emplace();
}},
@@ -1020,8 +1019,8 @@
  HI.Type = "enum Color";
  HI.Value = "1";
}},
-  // FIXME: We should use the Decl referenced, even if it comes from an
-  // implicit instantiation.
+  // FIXME: We should use the Decl referenced, even if from an implicit
+  // instantiation. Then the scope would be Add<1, 2> and the value 3.
   {R"cpp(
 template struct Add {
   static constexpr int result = a + b;
@@ -1034,7 +1033,7 @@
  HI.Kind = SymbolKind::Property;
  HI.Type = "const int";
  HI.NamespaceScope = "";
- HI.LocalScope = "Add::";
+ HI.LocalScope = "Add::";
}},
   {R"cpp(
 const char *[[ba^r]] = "1234";
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -414,12 +414,12 @@
 static std::string getLocalScope(const Decl *D) {
   std::vector Scopes;
   const DeclContext *DC = D->getDeclContext();
-  auto GetName = [](const Decl *D) {
-const NamedDecl *ND = dyn_cast(D);
-std::string Name = ND->getNameAsString();
-// FIXME(sammccall): include template params/specialization args?.
-if (!Name.empty())
-  return Name;
+  auto GetName = [](const TypeDecl *D) {
+if (!D->getDeclName().isEmpty()) {
+  PrintingPolicy Policy = D->getASTContext().getPrintingPolicy();
+  Policy.SuppressScope = true;
+  return declaredType(D).getAsString(Policy);
+}
 if (auto RD = dyn_cast(D))
   return ("(anonymous " + RD->getKindName() + ")").str();
 return std::string("");
@@ -566,6 +566,51 @@
   Req, [&](const Symbol ) { Hover.Documentation = S.Documentation; });
 }
 
+// Populates Type, ReturnType, and Parameters for function-like decls.
+static void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
+  const FunctionDecl *FD,
+  const PrintingPolicy ) {
+  HI.Parameters.emplace();
+  for (const ParmVarDecl *PVD : FD->parameters()) {
+HI.Parameters->emplace_back();
+auto  = HI.Parameters->back();
+if (!PVD->getType().isNull()) {
+  P.Type.emplace();
+  llvm::raw_string_ostream OS(*P.Type);
+  PVD->getType().print(OS, Policy);
+} else {
+  std::string Param;
+  llvm::raw_string_ostream OS(Param);
+  PVD->dump(OS);
+  OS.flush();
+  elog("Got param with null type: {0}", Param);
+}
+if (!PVD->getName().empty())
+  P.Name = PVD->getNameAsString();
+if (PVD->hasDefaultArg()) {
+  P.Default.emplace();
+  llvm::raw_string_ostream Out(*P.Default);
+  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+}
+  }
+
+  if (const auto* CCD = llvm::dyn_cast(FD)) {
+// Constructor's "return type" is the class type.
+HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+// Don't provide any type for the constructor itself.
+  } else if (const auto* CDD = llvm::dyn_cast(FD)){
+HI.ReturnType = "void";
+  } else {
+HI.ReturnType = FD->getReturnType().getAsString(Policy);
+
+QualType FunctionType = FD->getType();
+if (const VarDecl *VD = llvm::dyn_cast(D)) // Lambdas
+  FunctionType = VD->getType().getDesugaredType(D->getASTContext());
+HI.Type = FunctionType.getAsString(Policy);
+  }
+  // FIXME: handle variadics.
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 static HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -601,45 +646,7 @@
 
   // Fill in types and params.
   if 

[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment.

Changes made. PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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


[PATCH] D69586: [profile] Support online merging with continuous sync mode

2019-11-18 Thread Vedant Kumar via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2492b5a12550: [profile] Support online merging with 
continuous sync mode (authored by vsk).
Herald added projects: clang, Sanitizers.
Herald added subscribers: Sanitizers, cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69586

Files:
  clang/docs/SourceBasedCodeCoverage.rst
  compiler-rt/lib/profile/InstrProfilingBuffer.c
  compiler-rt/lib/profile/InstrProfilingFile.c
  compiler-rt/lib/profile/InstrProfilingPort.h
  compiler-rt/test/profile/ContinuousSyncMode/online-merging.c

Index: compiler-rt/test/profile/ContinuousSyncMode/online-merging.c
===
--- /dev/null
+++ compiler-rt/test/profile/ContinuousSyncMode/online-merging.c
@@ -0,0 +1,138 @@
+// Test the online merging mode (%m) along with continuous mode (%c).
+//
+// Create & cd into a temporary directory.
+// RUN: rm -rf %t.dir && mkdir -p %t.dir && cd %t.dir
+//
+// Create two DSOs and a driver program that uses them.
+// RUN: echo "void dso1(void) {}" > dso1.c
+// RUN: echo "void dso2(void) {}" > dso2.c
+// RUN: %clang_pgogen -dynamiclib -o %t.dir/dso1.dylib dso1.c
+// RUN: %clang_pgogen -dynamiclib -o %t.dir/dso2.dylib dso2.c
+// RUN: %clang_pgogen -o main.exe %s %t.dir/dso1.dylib %t.dir/dso2.dylib
+//
+// === Round 1 ===
+// Test merging+continuous mode without any file contention.
+//
+// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%m%c.profraw" %run %t.dir/main.exe nospawn
+// RUN: llvm-profdata merge -o %t.profdata %t.dir/profdir
+// RUN: llvm-profdata show --counts --all-functions %t.profdata | FileCheck %s -check-prefix=ROUND1
+
+// ROUND1-LABEL: Counters:
+// ROUND1-DAG:   dso1:
+// ROUND1-DAG: Hash: 0x{{.*}}
+// ROUND1-DAG: Counters: 1
+// ROUND1-DAG: Block counts: [1]
+// ROUND1-DAG:   dso2:
+// ROUND1-DAG: Hash: 0x{{.*}}
+// ROUND1-DAG: Counters: 1
+// ROUND1-DAG: Block counts: [1]
+// ROUND1-DAG:   main:
+// ROUND1-DAG: Hash: 0x{{.*}}
+// ROUND1-LABEL: Instrumentation level: IR
+// ROUND1-NEXT: Functions shown: 3
+// ROUND1-NEXT: Total functions: 3
+// ROUND1-NEXT: Maximum function count: 1
+// ROUND1-NEXT: Maximum internal block count: 1
+//
+// === Round 2 ===
+// Test merging+continuous mode with some file contention.
+//
+// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%m%c.profraw" %run %t.dir/main.exe spawn 'LLVM_PROFILE_FILE=%t.dir/profdir/%m%c.profraw'
+// RUN: llvm-profdata merge -o %t.profdata %t.dir/profdir
+// RUN: llvm-profdata show --counts --all-functions %t.profdata | FileCheck %s -check-prefix=ROUND2
+
+// ROUND2-LABEL: Counters:
+// ROUND2-DAG:   dso1:
+// ROUND2-DAG: Hash: 0x{{.*}}
+// ROUND2-DAG: Counters: 1
+// ROUND2-DAG: Block counts: [97]
+// ROUND2-DAG:   dso2:
+// ROUND2-DAG: Hash: 0x{{.*}}
+// ROUND2-DAG: Counters: 1
+// ROUND2-DAG: Block counts: [97]
+// ROUND2-DAG:   main:
+// ROUND2-DAG: Hash: 0x{{.*}}
+// ROUND2-LABEL: Instrumentation level: IR
+// ROUND2-NEXT: Functions shown: 3
+// ROUND2-NEXT: Total functions: 3
+// ROUND2-NEXT: Maximum function count: 97
+// ROUND2-NEXT: Maximum internal block count: 33
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+const int num_child_procs_to_spawn = 32;
+
+extern int __llvm_profile_is_continuous_mode_enabled(void);
+extern char *__llvm_profile_get_filename(void);
+
+void dso1(void);
+void dso2(void);
+
+// Change to "#define" for debug output.
+#undef DEBUG_TEST
+
+#ifdef DEBUG_TEST
+#define DEBUG(...) fprintf(stderr, __VA_ARGS__);
+#else
+#define DEBUG(...)
+#endif
+
+int main(int argc, char *const argv[]) {
+  if (strcmp(argv[1], "nospawn") == 0) {
+DEBUG("Hello from child (pid = %d, cont-mode-enabled = %d, profile = %s).\n",
+getpid(), __llvm_profile_is_continuous_mode_enabled(), __llvm_profile_get_filename());
+
+dso1();
+dso2();
+return 0;
+  } else if (strcmp(argv[1], "spawn") == 0) {
+// This is the start of Round 2.
+// Expect Counts[dsoX] = 1, as this was the state at the end of Round 1.
+
+int I;
+pid_t child_pids[num_child_procs_to_spawn];
+char *const child_argv[] = {argv[0], "nospawn", NULL};
+char *const child_envp[] = {argv[2], NULL};
+for (I = 0; I < num_child_procs_to_spawn; ++I) {
+  dso1(); // Counts[dsoX] += 2 * num_child_procs_to_spawn
+  dso2();
+
+  DEBUG("Spawning child with argv = {%s, %s, NULL} and envp = {%s, NULL}\n",
+  child_argv[0], child_argv[1], child_envp[0]);
+
+  int ret = posix_spawn(_pids[I], argv[0], NULL, NULL, child_argv,
+  child_envp);
+  if (ret != 0) {
+fprintf(stderr, "Child %d could not be spawned: ret = %d, msg = %s\n",
+I, ret, strerror(ret));
+return 1;
+  }
+
+  DEBUG("Spawned child %d (pid = %d).\n", I, child_pids[I]);
+}
+for (I = 0; I < 

[PATCH] D70357: [clangd] Untangle Hover from XRefs, move into own file.

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 4 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:374
+  } else {
+auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
+if (!Offset) {

kadircet wrote:
> kadircet wrote:
> > nit: `SM.getFileOffset(SourceLocationBeg)` ?
> why not just expose getDeclAtPosition in `AST.h` ?
> nit: SM.getFileOffset(SourceLocationBeg) ?
I was deliberately trying to avoid using the "rewind token" logic where it's 
not needed. We've had bugs with it before, as well as with selectiontree, and 
composing them unneccesarily is harder to debug.

> why not just expose getDeclAtPosition in AST.h ?
I don't think it's better than the expanded form - it's just plugging a couple 
of functions together, and the choices it makes (flattening SourceLocation into 
an offset, the DeclRelationSet, only looking at the common ancestor and nothing 
higher up) aren't obvious.

It also privileges decls over other types of things (e.g. the followup patch 
looks for Exprs in the selection tree to show their values, and that couldn't 
happen if it was hidden behind getDeclAtPosition)



Comment at: clang-tools-extra/clangd/unittests/ASTTests.cpp:13
+#include "TestTU.h"
+
+#include "clang/Basic/SourceManager.h"

kadircet wrote:
> unintended newline ?
intended (subproject vs clang) but dosen't seem to match local style. removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70357



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


[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGee31adb7fa42: Populate CUDA flags on FreeBSD too, as many 
other toolchains do. (authored by dim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/test/Driver/cuda-options-freebsd.cu

Index: clang/test/Driver/cuda-options-freebsd.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-options-freebsd.cu
@@ -0,0 +1,289 @@
+// Tests CUDA compilation pipeline construction in Driver.
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// Simple compilation case. Compile device-side to PTX assembly and make sure
+// we use it on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix NOLINK %s
+
+// Typical compilation + link case.
+// RUN: %clang -### -target x86_64-unknown-freebsd %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-host-only disables device-side compilation, but doesn't
+// disable host-side compilation/linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// Verify that --cuda-device-only disables host-side compilation and linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// Check that the last of --cuda-compile-host-device, --cuda-host-only, and
+// --cuda-device-only wins.
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:   --cuda-compile-host-device %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:   --cuda-compile-host-device %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-gpu-arch option passes the correct GPU architecture to
+// device compilation.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-gpu-arch=sm_30 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix DEVICE-SM30 -check-prefix HOST \
+// RUN:-check-prefix INCLUDES-DEVICE -check-prefix NOLINK %s
+
+// Verify that there is one device-side compilation per --cuda-gpu-arch args
+// and that all results are included on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd \
+// RUN:   --cuda-gpu-arch=sm_35 --cuda-gpu-arch=sm_30 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefixes DEVICE,DEVICE-NOSAVE,DEVICE2 \
+// RUN: -check-prefixes DEVICE-SM30,DEVICE2-SM35 \
+// RUN: -check-prefixes INCLUDES-DEVICE,INCLUDES-DEVICE2 \
+// RUN: -check-prefixes HOST,HOST-NOSAVE,NOLINK %s
+
+// Verify that device-side results are passed to the correct tool when
+// -save-temps is used.
+// RUN: %clang -### -target x86_64-unknown-freebsd -save-temps -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-SAVE \
+// RUN:-check-prefix HOST -check-prefix HOST-SAVE -check-prefix NOLINK %s
+
+// 

[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via Phabricator via cfe-commits
dim added a comment.

In D69990#1750348 , @tra wrote:

> LGTM, though I'm curious if it's particularly useful. Last time I checked 
> NVIDIA didn't ship libcudart for FreeBSD and without it it's rather 
> cumbersome to use CUDA in practice.


After extracting the necessary CUDA stuff and enabling Linux emulation (for 
`ptxas`), at least a "hello world" sample program compiles to an object file:

  $ 
~/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/bin/clang
 --cuda-path=/share/dim/src/freebsd/cuda/cuda-10.1 --cuda-gpu-arch=sm_60 -c 
hello.cu -v
  clang version 10.0.0 (https://github.com/llvm/llvm-project.git 
014799db369c8e30c222c0e9d3ea143f349c3db9)
  Target: x86_64-unknown-freebsd13.0
  Thread model: posix
  InstalledDir: 
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/bin
  Found CUDA installation: /share/dim/src/freebsd/cuda/cuda-10.1, version 10.1
   
"/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/bin/clang"
 -cc1 -triple nvptx64-nvidia-cuda -aux-triple x86_64-unknown-freebsd13.0 -S 
-disable-free -main-file-name hello.cu -mrelocation-model static -mthread-model 
posix -mframe-pointer=all -fno-rounding-math -no-integrated-as -fuse-init-array 
-fcuda-is-device -mlink-builtin-bitcode 
/share/dim/src/freebsd/cuda/cuda-10.1/nvvm/libdevice/libdevice.10.bc 
-target-feature +ptx64 -target-sdk-version=10.1 -target-cpu sm_60 
-dwarf-column-info -debugger-tuning=gdb -v -resource-dir 
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/lib/clang/10.0.0
 -internal-isystem 
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/lib/clang/10.0.0/include/cuda_wrappers
 -internal-isystem /share/dim/src/freebsd/cuda/cuda-10.1/include -include 
__clang_cuda_runtime_wrapper.h -internal-isystem /usr/include/c++/v1 
-internal-isystem /usr/include/c++/v1 -fdeprecated-macro 
-fno-dwarf-directory-asm -fno-autolink -fdebug-compilation-dir /tmp 
-ferror-limit 19 -fmessage-length 160 -fgnuc-version=4.2.1 -fobjc-runtime=gcc 
-fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o 
/home/dim/tmp/hello-f032c8.s -x cuda hello.cu
  clang -cc1 version 10.0.0 based upon LLVM 10.0.0git default target 
x86_64-unknown-freebsd13.0
  ignoring duplicate directory "/usr/include/c++/v1"
  #include "..." search starts here:
  #include <...> search starts here:
   
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/lib/clang/10.0.0/include/cuda_wrappers
   /share/dim/src/freebsd/cuda/cuda-10.1/include
   /usr/include/c++/v1
   
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/lib/clang/10.0.0/include
   /usr/include
  End of search list.
   "/share/dim/src/freebsd/cuda/cuda-10.1/bin/ptxas" -m64 -O0 -v --gpu-name 
sm_60 --output-file /home/dim/tmp/hello-54422a.o /home/dim/tmp/hello-f032c8.s
  ptxas info: 23 bytes gmem
  ptxas info: Compiling entry function '_Z10cuda_hellov' for 'sm_60'
  ptxas info: Function properties for _Z10cuda_hellov
  0 bytes stack frame, 0 bytes spill stores, 0 bytes spill loads
  ptxas info: Used 8 registers, 320 bytes cmem[0]
   "/share/dim/src/freebsd/cuda/cuda-10.1/bin/fatbinary" -64 --create 
/home/dim/tmp/hello-9cd109.fatbin 
--image=profile=sm_60,file=/home/dim/tmp/hello-54422a.o 
--image=profile=compute_60,file=/home/dim/tmp/hello-f032c8.s
   
"/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/bin/clang"
 -cc1 -triple x86_64-unknown-freebsd13.0 -target-sdk-version=10.1 -aux-triple 
nvptx64-nvidia-cuda -emit-obj -mrelax-all -disable-free -main-file-name 
hello.cu -mrelocation-model static -mthread-model posix -mframe-pointer=all 
-fno-rounding-math -masm-verbose -mconstructor-aliases -munwind-tables 
-fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -v 
-resource-dir 
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/lib/clang/10.0.0
 -internal-isystem 
/home/dim/obj/llvm/llvmorg-10-init-10100-g9a5b7b785bf-freebsd13-amd64-ninja-rel-1/lib/clang/10.0.0/include/cuda_wrappers
 -internal-isystem /share/dim/src/freebsd/cuda/cuda-10.1/include -include 
__clang_cuda_runtime_wrapper.h -internal-isystem /usr/include/c++/v1 
-internal-isystem /usr/include/c++/v1 -fdeprecated-macro 
-fdebug-compilation-dir /tmp -ferror-limit 19 -fmessage-length 160 
-fgnuc-version=4.2.1 -fobjc-runtime=gnustep -fcxx-exceptions -fexceptions 
-fdiagnostics-show-option -fcolor-diagnostics -fcuda-include-gpubinary 
/home/dim/tmp/hello-9cd109.fatbin -faddrsig -o hello.o -x cuda hello.cu
  clang -cc1 version 10.0.0 based upon LLVM 10.0.0git default target 
x86_64-unknown-freebsd13.0
  ignoring duplicate directory "/usr/include/c++/v1"
  #include "..." search starts here:
  #include <...> search starts here:
   

[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 229904.
void marked 2 inline comments as done.
void added a comment.

Remove a "const_cast" and use iterators for output constraints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/Analysis/uninit-asm-goto.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/Parser/asm-goto.c
  clang/test/Parser/asm-goto.cpp
  clang/test/Sema/asm-goto.cpp

Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -1,53 +1,51 @@
 // RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
 // RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -verify -fsyntax-only
 
-struct NonTrivial {
-  ~NonTrivial();
+struct S {
+  ~S();
   int f(int);
 private:
   int k;
 };
-void JumpDiagnostics(int n) {
-// expected-error@+1 {{cannot jump from this goto statement to its label}}
+void test1(int n) {
+  // expected-error@+1 {{cannot jump from this goto statement to its label}}
   goto DirectJump;
-// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp1;
+  // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  S s1;
 
 DirectJump:
-// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm goto("jmp %l0;" Later);
-// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp2;
-// expected-note@+1 {{possible target of asm goto statement}}
+  // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  S s2;
+  // expected-note@+1 {{possible target of asm goto statement}}
 Later:
   return;
 }
 
-struct S { ~S(); };
-void foo(int a) {
+struct T { ~T(); };
+void test2(int a) {
   if (a) {
 FOO:
-// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
-// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
-S s;
-void *p = &
-// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
-  asm goto("jmp %l0;" BAR);
-// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+T t;
+void *p = &
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" bar);
+// expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
 goto *p;
 p = &
 goto *p;
 return;
   }
-// expected-note@+2 {{possible target of asm goto statement}}
-// expected-note@+1 {{possible target of indirect goto statement}}
-BAR:
+  // expected-note@+2 {{possible target of asm goto statement}}
+  // expected-note@+1 {{possible target of indirect goto statement}}
+bar:
   return;
 }
 
-
-//Asm goto:
-int test16(int n)
+int test3(int n)
 {
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
@@ -57,7 +55,7 @@
   return ({int a[n];label_true: 2;});
   // expected-note@+1 {{jump bypasses initialization of variable length array}}
   int b[n];
-// expected-note@+1 {{possible target of asm goto statement}}
+  // expected-note@+1 {{possible target of asm goto statement}}
 loop:
   return 0;
 }
Index: clang/test/Parser/asm-goto.cpp
===
--- clang/test/Parser/asm-goto.cpp
+++ clang/test/Parser/asm-goto.cpp
@@ -1,14 +1,54 @@
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -fsyntax-only -verify -std=c++11 %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify -std=c++11 %s
 
-int zoo ()
-{
+int a, b, c, d, e, f, g, h, i, j, k, l;
+
+void test1(void) {
+  __asm__ volatile goto (""
+:: [a] "r" (a), [b] "r" (b), [c] "r" (c), [d] "r" (d),
+   [e] "r" (e), [f] "r" (f), [g] "r" (g), [h] "r" (h),
+   [i] "r" (i), [j] "r" (j), [k] "r" (k), [l] "r" (l)
+::lab1,lab2);
+lab1: return;
+lab2: return;
+}
+
+void test2(void) {
+  __asm__ volatile goto (""
+:: [a] "r,m" (a), [b] "r,m" (b), [c] "r,m" (c), [d] "r,m" (d),
+   [e] "r,m" (e), [f] "r,m" (f), [g] "r,m" (g), [h] "r,m" (h),
+   [i] "r,m" (i), [j] 

[clang] 2492b5a - [profile] Support online merging with continuous sync mode

2019-11-18 Thread Vedant Kumar via cfe-commits

Author: Vedant Kumar
Date: 2019-11-18T12:56:58-08:00
New Revision: 2492b5a12550f7c4bb428c3761392f2ce47fa269

URL: 
https://github.com/llvm/llvm-project/commit/2492b5a12550f7c4bb428c3761392f2ce47fa269
DIFF: 
https://github.com/llvm/llvm-project/commit/2492b5a12550f7c4bb428c3761392f2ce47fa269.diff

LOG: [profile] Support online merging with continuous sync mode

Make it possible to use online profile merging ("%m" mode) with
continuous sync ("%c" mode).

To implement this, the merged profile is locked in the runtime
initialization step and either a) filled out for the first time or b)
checked for compatibility. Then, the profile can simply be mmap()'d with
MAP_SHARED set. With the mmap() in place, counter updates from every
process which uses an image are mapped onto the same set of physical
pages assigned by the filesystem cache. After the mmap() is set up, the
profile is unlocked.

Differential Revision: https://reviews.llvm.org/D69586

Added: 
compiler-rt/test/profile/ContinuousSyncMode/online-merging.c

Modified: 
clang/docs/SourceBasedCodeCoverage.rst
compiler-rt/lib/profile/InstrProfilingBuffer.c
compiler-rt/lib/profile/InstrProfilingFile.c
compiler-rt/lib/profile/InstrProfilingPort.h

Removed: 




diff  --git a/clang/docs/SourceBasedCodeCoverage.rst 
b/clang/docs/SourceBasedCodeCoverage.rst
index 0beb284e475e..73197a57713f 100644
--- a/clang/docs/SourceBasedCodeCoverage.rst
+++ b/clang/docs/SourceBasedCodeCoverage.rst
@@ -90,12 +90,11 @@ directory structure will be created.  Additionally, the 
following special
 * "%c" expands out to nothing, but enables a mode in which profile counter
   updates are continuously synced to a file. This means that if the
   instrumented program crashes, or is killed by a signal, perfect coverage
-  information can still be recovered. Continuous mode is not yet compatible 
with
-  the "%Nm" merging mode described above, does not support value profiling for
-  PGO, and is only supported on Darwin. Support for Linux may be mostly
-  complete but requires testing, and support for Fuchsia/Windows may require
-  more extensive changes: please get involved if you are interested in porting
-  this feature.
+  information can still be recovered. Continuous mode does not support value
+  profiling for PGO, and is only supported on Darwin at the moment. Support for
+  Linux may be mostly complete but requires testing, and support for
+  Fuchsia/Windows may require more extensive changes: please get involved if
+  you are interested in porting this feature.
 
 .. code-block:: console
 

diff  --git a/compiler-rt/lib/profile/InstrProfilingBuffer.c 
b/compiler-rt/lib/profile/InstrProfilingBuffer.c
index 089ff5a0153d..174280fd4b52 100644
--- a/compiler-rt/lib/profile/InstrProfilingBuffer.c
+++ b/compiler-rt/lib/profile/InstrProfilingBuffer.c
@@ -10,11 +10,7 @@
 #include "InstrProfilingInternal.h"
 #include "InstrProfilingPort.h"
 
-/* When continuous mode is enabled (%c), this parameter is set to 1. This is
- * incompatible with the in-process merging mode. Lifting this restriction
- * may be complicated, as merging mode requires a lock on the profile, and
- * mmap() mode would require that lock to be held for the entire process
- * lifetime.
+/* When continuous mode is enabled (%c), this parameter is set to 1.
  *
  * This parameter is defined here in InstrProfilingBuffer.o, instead of in
  * InstrProfilingFile.o, to sequester all libc-dependent code in

diff  --git a/compiler-rt/lib/profile/InstrProfilingFile.c 
b/compiler-rt/lib/profile/InstrProfilingFile.c
index 6594fa991d59..208ae587bb3e 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -380,11 +380,6 @@ static void truncateCurrentFile(void) {
   if (!Filename)
 return;
 
-  /* By pass file truncation to allow online raw profile
-   * merging. */
-  if (lprofCurFilename.MergePoolSize)
-return;
-
   /* Only create the profile directory and truncate an existing profile once.
* In continuous mode, this is necessary, as the profile is written-to by the
* runtime initializer. */
@@ -397,8 +392,14 @@ static void truncateCurrentFile(void) {
   setenv(LPROF_INIT_ONCE_ENV, LPROF_INIT_ONCE_ENV, 1);
 #endif
 
+  /* Create the profile dir (even if online merging is enabled), so that
+   * the profile file can be set up if continuous mode is enabled. */
   createProfileDir(Filename);
 
+  /* By pass file truncation to allow online raw profile merging. */
+  if (lprofCurFilename.MergePoolSize)
+return;
+
   /* Truncate the file.  Later we'll reopen and append. */
   File = fopen(Filename, "w");
   if (!File)
@@ -406,6 +407,33 @@ static void truncateCurrentFile(void) {
   fclose(File);
 }
 
+#ifndef _MSC_VER
+static void assertIsZero(int *i) {
+  if (*i)
+PROF_WARN("Expected flag to be 0, but got: %d\n", *i);
+}
+#endif
+
+/* Write a partial profile to 

[clang] ee31adb - Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via cfe-commits

Author: Dimitry Andric
Date: 2019-11-18T21:54:25+01:00
New Revision: ee31adb7fa42f5b601d9612f23755b4604f83cac

URL: 
https://github.com/llvm/llvm-project/commit/ee31adb7fa42f5b601d9612f23755b4604f83cac
DIFF: 
https://github.com/llvm/llvm-project/commit/ee31adb7fa42f5b601d9612f23755b4604f83cac.diff

LOG: Populate CUDA flags on FreeBSD too, as many other toolchains do.

Summary:
This allows `clang` to be used to compile CUDA programs. Compiled
simple helloworld.cu with this.

Reviewers: dim, emaste, tra, yaxunl, ABataev

Reviewed By: tra

Subscribers: dim, emaste, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69990

Added: 
clang/test/Driver/cuda-options-freebsd.cu

Modified: 
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/lib/Driver/ToolChains/FreeBSD.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp 
b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index 5e2c4883290f..3e5e8a00652d 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -397,6 +397,11 @@ void FreeBSD::AddCXXStdlibLibArgs(const ArgList ,
   }
 }
 
+void FreeBSD::AddCudaIncludeArgs(const ArgList ,
+ ArgStringList ) const {
+  CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
+}
+
 Tool *FreeBSD::buildAssembler() const {
   return new tools::freebsd::Assembler(*this);
 }

diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.h 
b/clang/lib/Driver/ToolChains/FreeBSD.h
index 1d503a621d0e..53bc5265c9e2 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.h
+++ b/clang/lib/Driver/ToolChains/FreeBSD.h
@@ -64,6 +64,8 @@ class LLVM_LIBRARY_VISIBILITY FreeBSD : public Generic_ELF {
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
+  void AddCudaIncludeArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const override;
 
   llvm::ExceptionHandling GetExceptionModel(
   const llvm::opt::ArgList ) const override;

diff  --git a/clang/test/Driver/cuda-options-freebsd.cu 
b/clang/test/Driver/cuda-options-freebsd.cu
new file mode 100644
index ..cee38e48b272
--- /dev/null
+++ b/clang/test/Driver/cuda-options-freebsd.cu
@@ -0,0 +1,289 @@
+// Tests CUDA compilation pipeline construction in Driver.
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// Simple compilation case. Compile device-side to PTX assembly and make sure
+// we use it on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix NOLINK %s
+
+// Typical compilation + link case.
+// RUN: %clang -### -target x86_64-unknown-freebsd %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-host-only disables device-side compilation, but doesn't
+// disable host-side compilation/linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// Verify that --cuda-device-only disables host-side compilation and linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// Check that the last of --cuda-compile-host-device, --cuda-host-only, and
+// --cuda-device-only wins.
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// 

[PATCH] D70317: [clangd] More sensible output for constructors/destructors in hover.

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:598
+// Constructor's "return type" is the class type.
+HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+// Don't provide any type for the constructor itself.

hokein wrote:
> this looks reasonable though I'd prefer not setting the return type for 
> ctor/dtor. If we decide to go down this path, we'd better make other 
> "returnType" places (`Symbol::ReturnType`, CodeCompletion) align with this.
So code completion shows ~foo() as type void, and has no return type for 
constructors (including when it would matter for ranking!). Indexing code 
doesn't have a return type for either.
Per offline discussion, I'd rather fix those to include the type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70317



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


[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

2019-11-18 Thread dmajor via Phabricator via cfe-commits
dmajor added a comment.

Hi, I filed https://bugs.llvm.org/show_bug.cgi?id=44049 for some strange 
crashes that we're seeing because the CFG code overwrites the lower byte of 
function pointers before jumping to them. (Commenting separately here because I 
was unable to CC @ajpaverd in Bugzilla)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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


[PATCH] D70317: [clangd] More sensible output for constructors/destructors in hover.

2019-11-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 229900.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Update tests and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70317

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -918,11 +918,20 @@
  HI.Name = "X";
  HI.LocalScope = "X::";// FIXME: Should be X::
  HI.Kind = SymbolKind::Constructor;
- HI.Type = "void ()";  // FIXME: Should be None
- HI.ReturnType = "void";   // FIXME: Should be None or X
+ HI.ReturnType = "X";
  HI.Definition = "X()";
  HI.Parameters.emplace();
}},
+  {"class X { [[^~]]X(); };", // FIXME: Should be [[~X]]()
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.Name = "~X";
+ HI.LocalScope = "X::";
+ HI.Kind = SymbolKind::Constructor;
+ HI.ReturnType = "void";
+ HI.Definition = "~X()";
+ HI.Parameters.emplace();
+   }},
 
   // auto on lambda
   {R"cpp(
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -566,6 +566,51 @@
   Req, [&](const Symbol ) { Hover.Documentation = S.Documentation; });
 }
 
+// Populates Type, ReturnType, and Parameters for function-like decls.
+static void fillFunctionTypeAndParams(HoverInfo , const Decl *D,
+  const FunctionDecl *FD,
+  const PrintingPolicy ) {
+  HI.Parameters.emplace();
+  for (const ParmVarDecl *PVD : FD->parameters()) {
+HI.Parameters->emplace_back();
+auto  = HI.Parameters->back();
+if (!PVD->getType().isNull()) {
+  P.Type.emplace();
+  llvm::raw_string_ostream OS(*P.Type);
+  PVD->getType().print(OS, Policy);
+} else {
+  std::string Param;
+  llvm::raw_string_ostream OS(Param);
+  PVD->dump(OS);
+  OS.flush();
+  elog("Got param with null type: {0}", Param);
+}
+if (!PVD->getName().empty())
+  P.Name = PVD->getNameAsString();
+if (PVD->hasDefaultArg()) {
+  P.Default.emplace();
+  llvm::raw_string_ostream Out(*P.Default);
+  PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
+}
+  }
+
+  if (const auto* CCD = llvm::dyn_cast(FD)) {
+// Constructor's "return type" is the class type.
+HI.ReturnType = declaredType(CCD->getParent()).getAsString(Policy);
+// Don't provide any type for the constructor itself.
+  } else if (const auto* CDD = llvm::dyn_cast(FD)){
+HI.ReturnType = "void";
+  } else {
+HI.ReturnType = FD->getReturnType().getAsString(Policy);
+
+QualType FunctionType = FD->getType();
+if (const VarDecl *VD = llvm::dyn_cast(D)) // Lambdas
+  FunctionType = VD->getType().getDesugaredType(D->getASTContext());
+HI.Type = FunctionType.getAsString(Policy);
+  }
+  // FIXME: handle variadics.
+}
+
 /// Generate a \p Hover object given the declaration \p D.
 static HoverInfo getHoverContents(const Decl *D, const SymbolIndex *Index) {
   HoverInfo HI;
@@ -601,45 +646,7 @@
 
   // Fill in types and params.
   if (const FunctionDecl *FD = getUnderlyingFunction(D)) {
-HI.ReturnType.emplace();
-{
-  llvm::raw_string_ostream OS(*HI.ReturnType);
-  FD->getReturnType().print(OS, Policy);
-}
-
-HI.Parameters.emplace();
-for (const ParmVarDecl *PVD : FD->parameters()) {
-  HI.Parameters->emplace_back();
-  auto  = HI.Parameters->back();
-  if (!PVD->getType().isNull()) {
-P.Type.emplace();
-llvm::raw_string_ostream OS(*P.Type);
-PVD->getType().print(OS, Policy);
-  } else {
-std::string Param;
-llvm::raw_string_ostream OS(Param);
-PVD->dump(OS);
-OS.flush();
-elog("Got param with null type: {0}", Param);
-  }
-  if (!PVD->getName().empty())
-P.Name = PVD->getNameAsString();
-  if (PVD->hasDefaultArg()) {
-P.Default.emplace();
-llvm::raw_string_ostream Out(*P.Default);
-PVD->getDefaultArg()->printPretty(Out, nullptr, Policy);
-  }
-}
-
-HI.Type.emplace();
-llvm::raw_string_ostream TypeOS(*HI.Type);
-// Lambdas
-if (const VarDecl *VD = llvm::dyn_cast(D))
-  VD->getType().getDesugaredType(D->getASTContext()).print(TypeOS, Policy);
-// Functions
-else
-  FD->getType().print(TypeOS, Policy);
-// FIXME: handle variadics.
+fillFunctionTypeAndParams(HI, D, 

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: llvm/include/llvm/IR/IRBuilder.h:262
 Function *F = BB->getParent();
-if (!F->hasFnAttribute(Attribute::StrictFP)) {
+if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
   F->addFnAttr(Attribute::StrictFP);

kpn wrote:
> This looks reasonable to me. 
> 
> It smells like there's a larger strictfp IRBuilder issue, but that's not an 
> issue for this patch here. The larger issue won't be hit since the new 
> options affect the entire compilation. It therefore shouldn't block this 
> patch.
Does IRBuilder actually support inserting into an unparented basic block?  I 
feel like this is exposing a much more serious mis-use of IRBuilder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

In D62731#1750412 , @kpn wrote:

> Does anyone think a warning is appropriate because the new flags are 
> exercising experimental, incomplete code in both clang and llvm? The warning 
> would be removed when we believe the feature is complete and ready to use.


@kpn Can you say more about "incomplete code in ... clang".  I don't know 
what's missing from clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D70256: [FPEnv] clang support for constrained FP builtins

2019-11-18 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added a comment.

D62731  is required for testing this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70256



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added a comment.

Does anyone think a warning is appropriate because the new flags are exercising 
experimental, incomplete code in both clang and llvm? The warning would be 
removed when we believe the feature is complete and ready to use.




Comment at: llvm/include/llvm/IR/IRBuilder.h:262
 Function *F = BB->getParent();
-if (!F->hasFnAttribute(Attribute::StrictFP)) {
+if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
   F->addFnAttr(Attribute::StrictFP);

This looks reasonable to me. 

It smells like there's a larger strictfp IRBuilder issue, but that's not an 
issue for this patch here. The larger issue won't be hit since the new options 
affect the entire compilation. It therefore shouldn't block this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[clang] 8bfb353 - [Sema] Fix a -Wobjc-signed-char-bool false-positive

2019-11-18 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2019-11-18T12:15:20-08:00
New Revision: 8bfb353bb33cd2bcd2ef28e36eb8b90123b153c4

URL: 
https://github.com/llvm/llvm-project/commit/8bfb353bb33cd2bcd2ef28e36eb8b90123b153c4
DIFF: 
https://github.com/llvm/llvm-project/commit/8bfb353bb33cd2bcd2ef28e36eb8b90123b153c4.diff

LOG: [Sema] Fix a -Wobjc-signed-char-bool false-positive

Unsigned bit-field flags can only have boolean values, so handle that case in
Expr::isKnownToHaveBooleanValue.

rdar://56256999

Added: 


Modified: 
clang/lib/AST/Expr.cpp
clang/test/SemaObjC/signed-char-bool-conversion.m

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 3438c3aadc6b..4fd5fed5beef 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -191,6 +191,12 @@ bool Expr::isKnownToHaveBooleanValue() const {
   if (const auto *OVE = dyn_cast(E))
 return OVE->getSourceExpr()->isKnownToHaveBooleanValue();
 
+  if (const FieldDecl *FD = E->getSourceBitField())
+if (FD->getType()->isUnsignedIntegerType() &&
+!FD->getBitWidth()->isValueDependent() &&
+FD->getBitWidthValue(FD->getASTContext()) == 1)
+  return true;
+
   return false;
 }
 

diff  --git a/clang/test/SemaObjC/signed-char-bool-conversion.m 
b/clang/test/SemaObjC/signed-char-bool-conversion.m
index 476ecc6a0603..6945d86fc26d 100644
--- a/clang/test/SemaObjC/signed-char-bool-conversion.m
+++ b/clang/test/SemaObjC/signed-char-bool-conversion.m
@@ -47,3 +47,59 @@ void t2(BoolProp *bp) {
   bp.p = 1;
   bp.p = 2; // expected-warning {{implicit conversion from constant value 2 to 
'BOOL'; the only well defined values for 'BOOL' are YES and NO}}
 }
+
+struct has_bf {
+  int signed_bf1 : 1;
+  int signed_bf2 : 2;
+  unsigned unsigned_bf1 : 1;
+  unsigned unsigned_bf2 : 2;
+
+  struct has_bf *nested;
+};
+
+void t3(struct has_bf *bf) {
+  b = bf->signed_bf1; // expected-warning{{implicit conversion from integral 
type 'int' to 'BOOL'}}
+  b = bf->signed_bf2; // expected-warning{{implicit conversion from integral 
type 'int' to 'BOOL'}}
+  b = bf->unsigned_bf1; // no warning
+  b = bf->unsigned_bf2; // expected-warning{{implicit conversion from integral 
type 'unsigned int' to 'BOOL'}}
+  struct has_bf local;
+  b = local.unsigned_bf1;
+  b = local.unsigned_bf2; // expected-warning{{implicit conversion from 
integral type 'unsigned int' to 'BOOL'}}
+  b = local.nested->unsigned_bf1;
+  b = local.nested->unsigned_bf2; // expected-warning{{implicit conversion 
from integral type 'unsigned int' to 'BOOL'}}
+}
+
+__attribute__((objc_root_class))
+@interface BFIvar {
+  struct has_bf bf;
+  unsigned unsigned_bf1 : 1;
+  unsigned unsigned_bf2 : 2;
+}
+@end
+
+@implementation BFIvar
+-(void)m {
+  b = bf.unsigned_bf1;
+  b = bf.unsigned_bf2; // expected-warning{{implicit conversion from integral 
type 'unsigned int' to 'BOOL'}}
+  b = unsigned_bf1;
+  b = unsigned_bf2; // expected-warning{{implicit conversion from integral 
type 'unsigned int' to 'BOOL'}}
+}
+@end
+
+#ifdef __cplusplus
+template 
+struct S {
+  unsigned i : sizeof(T);
+};
+
+template 
+void f() {
+  S i;
+  BOOL b = i.i; // expected-warning{{implicit conversion from integral type 
'unsigned int' to 'BOOL'}}
+}
+
+int main() {
+  f();
+  f(); // expected-note {{in instantiation of function template 
specialization 'f' requested here}}
+}
+#endif



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

I added a nullptr check in IRBuilder.h and a test case to cover the segfault 
reported in https://bugs.llvm.org/show_bug.cgi?id=44048


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 229893.
mibintc added a reviewer: kpn.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -259,7 +259,7 @@
 assert(BB && "Must have a basic block to set any function attributes!");
 
 Function *F = BB->getParent();
-if (!F->hasFnAttribute(Attribute::StrictFP)) {
+if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
   F->addFnAttr(Attribute::StrictFP);
 }
   }
Index: clang/test/Driver/fp-model.c
===
--- /dev/null
+++ clang/test/Driver/fp-model.c
@@ -0,0 +1,130 @@
+// Test that incompatible combinations of -ffp-model= options
+// and other floating point options get a warning diagnostic.
+//
+// REQUIRES: clang-driver
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN %s
+// WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN1 %s
+// WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN2 %s
+// WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN3 %s
+// WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN4 %s
+// WARN4: warning: overriding '-ffp-model=strict' option with '-ffinite-math-only' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN5 %s
+// WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN6 %s
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN7 %s
+// WARN7: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN8 %s
+// WARN8: warning: overriding '-ffp-model=strict' option with '-fno-honor-infinities' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN9 %s
+// WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNa %s
+// WARNa: warning: overriding '-ffp-model=strict' option with '-fno-rounding-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-signed-zeros -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNb %s
+// WARNb: warning: overriding '-ffp-model=strict' option with '-fno-signed-zeros' [-Woverriding-t-option]
+
+// RUN: %clang -### 

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc reopened this revision.
mibintc added a comment.
This revision is now accepted and ready to land.

Reopening since patch was reverted


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

2019-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision.
dexonsmith added a comment.

Pushed as d4e1ba3fa9dfec2613bdcc7db0b58dea490c56b1 
.


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

https://reviews.llvm.org/D69991



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


[clang] d4e1ba3 - Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

2019-11-18 Thread Duncan P. N. Exon Smith via cfe-commits

Author: Pierre Habouzit
Date: 2019-11-18T11:48:40-08:00
New Revision: d4e1ba3fa9dfec2613bdcc7db0b58dea490c56b1

URL: 
https://github.com/llvm/llvm-project/commit/d4e1ba3fa9dfec2613bdcc7db0b58dea490c56b1
DIFF: 
https://github.com/llvm/llvm-project/commit/d4e1ba3fa9dfec2613bdcc7db0b58dea490c56b1.diff

LOG: Implement __attribute__((objc_direct)), 
__attribute__((objc_direct_members))

__attribute__((objc_direct)) is an attribute on methods declaration, and
__attribute__((objc_direct_members)) on implementation, categories or
extensions.

A `direct` property specifier is added (@property(direct) type name)

These attributes / specifiers cause the method to have no associated
Objective-C metadata (for the property or the method itself), and the
calling convention to be a direct C function call.

The symbol for the method has enforced hidden visibility and such direct
calls are hence unreachable cross image. An explicit C function must be
made if so desired to wrap them.

The implicit `self` and `_cmd` arguments are preserved, however to
maintain compatibility with the usual `objc_msgSend` semantics,
3 fundamental precautions are taken:

1) for instance methods, `self` is nil-checked. On arm64 backends this
   typically adds a single instruction (cbz x0, ) to the
   codegen, for the vast majority of the cases when the return type is a
   scalar.

2) for class methods, because the class may not be realized/initialized
   yet, a call to `[self self]` is emitted. When the proper deployment
   target is used, this is optimized to `objc_opt_self(self)`.

   However, long term we might want to emit something better that the
   optimizer can reason about. When inlining kicks in, these calls
   aren't optimized away as the optimizer has no idea that a single call
   is really necessary.

3) the calling convention for the `_cmd` argument is changed: the caller
   leaves the second argument to the call undefined, and the selector is
   loaded inside the body when it's referenced only.

As far as error reporting goes, the compiler refuses:
- making any overloads direct,
- making an overload of a direct method,
- implementations marked as direct when the declaration in the
  interface isn't (the other way around is allowed, as the direct
  attribute is inherited from the declaration),
- marking methods required for protocol conformance as direct,
- messaging an unqualified `id` with a direct method,
- forming any @selector() expression with only direct selectors.

As warnings:
- any inconsistency of direct-related calling convention when
  @selector() or messaging is used,
- forming any @selector() expression with a possibly direct selector.

Lastly an `objc_direct_members` attribute is added that can decorate
`@implementation` blocks and causes methods only declared there (and in
no `@interface`) to be automatically direct. When decorating an
`@interface` then all methods and properties declared in this block are
marked direct.

Radar-ID: rdar://problem/2684889
Differential Revision: https://reviews.llvm.org/D69991
Reviewed-By: John McCall

Added: 
clang/test/CodeGenObjC/direct-method.m
clang/test/SemaObjC/method-direct-properties.m
clang/test/SemaObjC/method-direct.m

Modified: 
clang/include/clang/AST/DeclObjC.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/ObjCRuntime.h
clang/include/clang/Sema/DeclSpec.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclObjC.cpp
clang/lib/AST/DeclPrinter.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGObjCGNU.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CGObjCRuntime.h
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaObjCProperty.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test

Removed: 




diff  --git a/clang/include/clang/AST/DeclObjC.h 
b/clang/include/clang/AST/DeclObjC.h
index e5d3ebfadc06..b98aef6b499d 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -410,7 +410,7 @@ class ObjCMethodDecl : public NamedDecl, public DeclContext 
{
   /// \return the type for \c self and set \arg selfIsPseudoStrong and
   /// \arg selfIsConsumed accordingly.
   QualType getSelfType(ASTContext , const ObjCInterfaceDecl *OID,
-   bool , bool );
+   bool , bool ) const;
 
   ImplicitParamDecl * getSelfDecl() const { return SelfDecl; }
   void setSelfDecl(ImplicitParamDecl *SD) { SelfDecl = SD; }
@@ -476,6 +476,9 @@ class ObjCMethodDecl : public NamedDecl, public DeclContext 
{
 ObjCMethodDeclBits.HasSkippedBody = Skipped;
   }
 
+  /// True if 

[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM, though I'm curious if it's particularly useful. Last time I checked 
NVIDIA didn't ship libcudart for FreeBSD and without it it's rather cumbersome 
to use CUDA in practice.
You can compile a kernel, but kernel loading, launching, and related data 
transfers will all need to be done via driver API. It should be possible to 
implement a functional replacement, but I'm not aware of any existing 
open-source implementations. I'm also not sure if clang will be able to deal 
with CUDA headers correctly on FreeBSD as CUDA headers do sometimes seem to 
rely on implementation specifics of Linux headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990



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


Re: [clang] b4e2b11 - [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Francis Visoiu Mistrih via cfe-commits
Thanks. I restricted this to darwin and moved the test to a separate file.

Re-landed here: e15b26fbbd90 Reland: [Remarks][Driver] Use different remark 
files when targeting multiple architectures

> On Nov 18, 2019, at 10:53 AM, Reid Kleckner  wrote:
> 
> I reverted this, the test fails for me locally. Does -arch work on non-Mac 
> targets?
> 
> On Mon, Nov 18, 2019 at 10:39 AM Francis Visoiu Mistrih via cfe-commits 
>  wrote:
> 
> Author: Francis Visoiu Mistrih
> Date: 2019-11-18T10:38:10-08:00
> New Revision: b4e2b112b58154a89171df39dae80044865ff4ff
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff
> DIFF: 
> https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff.diff
> 
> LOG: [Remarks][Driver] Use different remark files when targeting multiple 
> architectures
> 
> When the driver is targeting multiple architectures at once, for things
> like Universal Mach-Os, we need to emit different remark files for each
> cc1 invocation to avoid overwriting the files from a different
> invocation.
> 
> For example:
> 
> $ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h
> 
> will create two remark files:
> 
> * foo-x86_64.opt.yaml
> * foo-x86_64h.opt.yaml
> 
> Added: 
> 
> 
> Modified: 
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/test/Driver/opt-record.c
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
> b/clang/lib/Driver/ToolChains/Clang.cpp
> index 2b1c24275e3d..f5591c48d4e0 100644
> --- a/clang/lib/Driver/ToolChains/Clang.cpp
> +++ b/clang/lib/Driver/ToolChains/Clang.cpp
> @@ -5403,6 +5403,8 @@ void Clang::ConstructJob(Compilation , const 
> JobAction ,
>  if (A) {
>CmdArgs.push_back(A->getValue());
>  } else {
> +  bool hasMultipleArchs =
> +  Args.getAllArgValues(options::OPT_arch).size() > 1;
>SmallString<128> F;
> 
>if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
> @@ -5427,6 +5429,22 @@ void Clang::ConstructJob(Compilation , const 
> JobAction ,
>  }
>}
> 
> +  // If we're having more than one "-arch", we should name the files
> +  // 
> diff erently so that every cc1 invocation writes to a 
> diff erent file.
> +  // We're doing that by appending "-" with "" being the arch
> +  // name from the triple.
> +  if (hasMultipleArchs) {
> +// First, remember the extension.
> +SmallString<64> OldExtension = llvm::sys::path::extension(F);
> +// then, remove it.
> +llvm::sys::path::replace_extension(F, "");
> +// attach - to it.
> +F += "-";
> +F += Triple.getArchName();
> +// put back the extension.
> +llvm::sys::path::replace_extension(F, OldExtension);
> +  }
> +
>std::string Extension = "opt.";
>if (const Arg *A =
>Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
> 
> diff  --git a/clang/test/Driver/opt-record.c b/clang/test/Driver/opt-record.c
> index 062d0acc17da..d8d2aa53ed40 100644
> --- a/clang/test/Driver/opt-record.c
> +++ b/clang/test/Driver/opt-record.c
> @@ -18,6 +18,7 @@
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record 
> -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s 
> -check-prefix=CHECK-EQ-FORMAT
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 
> | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format 
> -fno-save-optimization-record %s 2>&1 | FileCheck %s 
> --check-prefix=CHECK-FOPT-DISABLE-FORMAT
> +// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
> x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
>  //
>  // CHECK: "-cc1"
>  // CHECK: "-opt-record-file" "FOO.opt.yaml"
> @@ -41,3 +42,8 @@
>  // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
> 
>  // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
> +
> +// CHECK-MULTIPLE-ARCH: "-cc1"
> +// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
> +// CHECK-MULTIPLE-ARCH: "-cc1"
> +// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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


[PATCH] D69876: Allow output constraints on "asm goto"

2019-11-18 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:830
+
+  for (unsigned i = 0, e = as->getNumOutputs(); i != e; ++i) {
+FindVarResult Var = findVar(as->getOutputExpr(i));

`GCCAsmStmt` inherits from `AsmStmt` which has an `outputs()` method for 
returning an iterator. Please use that an a range based for loop.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:877
+if (as->isAsmGoto())
+  tf.Visit(const_cast(as));
   return vals.updateValueVectorWithScratch(block);

Don't declare `as` as `const`, then you won't need this `const_cast`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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


[clang] e15b26f - Reland: [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Francis Visoiu Mistrih via cfe-commits

Author: Francis Visoiu Mistrih
Date: 2019-11-18T11:17:38-08:00
New Revision: e15b26fbbd901315a1402f97e83abf29bdce9a9f

URL: 
https://github.com/llvm/llvm-project/commit/e15b26fbbd901315a1402f97e83abf29bdce9a9f
DIFF: 
https://github.com/llvm/llvm-project/commit/e15b26fbbd901315a1402f97e83abf29bdce9a9f.diff

LOG: Reland: [Remarks][Driver] Use different remark files when targeting 
multiple architectures

When the driver is targeting multiple architectures at once, for things
like Universal Mach-Os, we need to emit different remark files for each
cc1 invocation to avoid overwriting the files from a different
invocation.

For example:

$ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h

will create two remark files:

* foo-x86_64.opt.yaml
* foo-x86_64h.opt.yaml

Added: 
clang/test/Driver/darwin-opt-record.c

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 6f25eea243ad..3e00c323fc65 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5218,6 +5218,9 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 if (A) {
   CmdArgs.push_back(A->getValue());
 } else {
+  bool hasMultipleArchs =
+  Triple.isOSDarwin() && // Only supported on Darwin platforms.
+  Args.getAllArgValues(options::OPT_arch).size() > 1;
   SmallString<128> F;
 
   if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
@@ -5242,6 +5245,22 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 }
   }
 
+  // If we're having more than one "-arch", we should name the files
+  // 
diff erently so that every cc1 invocation writes to a 
diff erent file.
+  // We're doing that by appending "-" with "" being the arch
+  // name from the triple.
+  if (hasMultipleArchs) {
+// First, remember the extension.
+SmallString<64> OldExtension = llvm::sys::path::extension(F);
+// then, remove it.
+llvm::sys::path::replace_extension(F, "");
+// attach - to it.
+F += "-";
+F += Triple.getArchName();
+// put back the extension.
+llvm::sys::path::replace_extension(F, OldExtension);
+  }
+
   std::string Extension = "opt.";
   if (const Arg *A =
   Args.getLastArg(options::OPT_fsave_optimization_record_EQ))

diff  --git a/clang/test/Driver/darwin-opt-record.c 
b/clang/test/Driver/darwin-opt-record.c
new file mode 100644
index ..ca0fad7ee16d
--- /dev/null
+++ b/clang/test/Driver/darwin-opt-record.c
@@ -0,0 +1,8 @@
+// REQUIRES: system-darwin
+
+// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
+//
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"



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


Re: [PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via cfe-commits
No, it's just the bug that Jorge found. I understand it might have
highlighted an existing bug, but reverting back to clean is the best
bet here and then we can recommit when we get the latent bug fixed :)

On Mon, Nov 18, 2019 at 11:08 AM Melanie Blower via Phabricator
 wrote:
>
> mibintc added a comment.
>
> In D62731#1750312 , @echristo wrote:
>
> > In D62731#1749916 , @mibintc wrote:
> >
> > > Thanks I see it, I'm working on a patch. Previously there was no support 
> > > for frounding-math (unimplemented). This patch enables the option. In the 
> > > IR builder, there's a call to a runtime function in the exception handler 
> > > which is unexpectedly null.  I start by adding a null pointer check.
> >
> >
> > Had a crash on valid here for days, let's revert and then get a fix when 
> > recommitting. I'll respond to the thread when reverting. Thanks :)
>
>
> @echristo I just saw the bug was reported today, is the "crash on valid" 
> visible on the bots?  Can you provide url showing same? Thanks
> I opened https://bugs.llvm.org/show_bug.cgi?id=44048 for @jgorbe
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62731/new/
>
> https://reviews.llvm.org/D62731
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via Phabricator via cfe-commits
dim added reviewers: tra, yaxunl, ABataev.
dim added a comment.

Adding a few people who might know a bit more about CUDA specific things. 
Please take a look if this review makes any sense, thanks. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990



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


[PATCH] D69990: Populate CUDA flags on FreeBSD too, as many other toolchains do.

2019-11-18 Thread Dimitry Andric via Phabricator via cfe-commits
dim updated this revision to Diff 229883.
dim added a comment.

- Add cuda options test, copied from cuda-options.cu.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/test/Driver/cuda-options-freebsd.cu

Index: clang/test/Driver/cuda-options-freebsd.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-options-freebsd.cu
@@ -0,0 +1,289 @@
+// Tests CUDA compilation pipeline construction in Driver.
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// Simple compilation case. Compile device-side to PTX assembly and make sure
+// we use it on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix NOLINK %s
+
+// Typical compilation + link case.
+// RUN: %clang -### -target x86_64-unknown-freebsd %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-host-only disables device-side compilation, but doesn't
+// disable host-side compilation/linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// Verify that --cuda-device-only disables host-side compilation and linking.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// Check that the last of --cuda-compile-host-device, --cuda-host-only, and
+// --cuda-device-only wins.
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-host-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix NODEVICE -check-prefix HOST \
+// RUN:-check-prefix NOINCLUDES-DEVICE -check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-compile-host-device \
+// RUN:--cuda-device-only %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix NOHOST -check-prefix NOLINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-host-only \
+// RUN:   --cuda-compile-host-device %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-device-only \
+// RUN:   --cuda-compile-host-device %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix HOST -check-prefix INCLUDES-DEVICE \
+// RUN:-check-prefix LINK %s
+
+// Verify that --cuda-gpu-arch option passes the correct GPU architecture to
+// device compilation.
+// RUN: %clang -### -target x86_64-unknown-freebsd --cuda-gpu-arch=sm_30 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-NOSAVE \
+// RUN:-check-prefix DEVICE-SM30 -check-prefix HOST \
+// RUN:-check-prefix INCLUDES-DEVICE -check-prefix NOLINK %s
+
+// Verify that there is one device-side compilation per --cuda-gpu-arch args
+// and that all results are included on the host side.
+// RUN: %clang -### -target x86_64-unknown-freebsd \
+// RUN:   --cuda-gpu-arch=sm_35 --cuda-gpu-arch=sm_30 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefixes DEVICE,DEVICE-NOSAVE,DEVICE2 \
+// RUN: -check-prefixes DEVICE-SM30,DEVICE2-SM35 \
+// RUN: -check-prefixes INCLUDES-DEVICE,INCLUDES-DEVICE2 \
+// RUN: -check-prefixes HOST,HOST-NOSAVE,NOLINK %s
+
+// Verify that device-side results are passed to the correct tool when
+// -save-temps is used.
+// RUN: %clang -### -target x86_64-unknown-freebsd -save-temps -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix DEVICE -check-prefix DEVICE-SAVE \
+// RUN:-check-prefix HOST -check-prefix HOST-SAVE -check-prefix NOLINK %s
+
+// Verify that device-side results are passed to the correct tool when
+// 

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

In D62731#1750312 , @echristo wrote:

> In D62731#1749916 , @mibintc wrote:
>
> > Thanks I see it, I'm working on a patch. Previously there was no support 
> > for frounding-math (unimplemented). This patch enables the option. In the 
> > IR builder, there's a call to a runtime function in the exception handler 
> > which is unexpectedly null.  I start by adding a null pointer check.
>
>
> Had a crash on valid here for days, let's revert and then get a fix when 
> recommitting. I'll respond to the thread when reverting. Thanks :)


@echristo I just saw the bug was reported today, is the "crash on valid" 
visible on the bots?  Can you provide url showing same? Thanks
I opened https://bugs.llvm.org/show_bug.cgi?id=44048 for @jgorbe


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

The incorrect code is actually in the IRBuilder which is part of a different 
patch...

In D62731#1750312 , @echristo wrote:

> In D62731#1749916 , @mibintc wrote:
>
> > Thanks I see it, I'm working on a patch. Previously there was no support 
> > for frounding-math (unimplemented). This patch enables the option. In the 
> > IR builder, there's a call to a runtime function in the exception handler 
> > which is unexpectedly null.  I start by adding a null pointer check.
>
>
> Had a crash on valid here for days, let's revert and then get a fix when 
> recommitting. I'll respond to the thread when reverting. Thanks :)





Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


Re: [clang] b4e2b11 - [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Reid Kleckner via cfe-commits
I reverted this, the test fails for me locally. Does -arch work on non-Mac
targets?

On Mon, Nov 18, 2019 at 10:39 AM Francis Visoiu Mistrih via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Francis Visoiu Mistrih
> Date: 2019-11-18T10:38:10-08:00
> New Revision: b4e2b112b58154a89171df39dae80044865ff4ff
>
> URL:
> https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff
> DIFF:
> https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff.diff
>
> LOG: [Remarks][Driver] Use different remark files when targeting multiple
> architectures
>
> When the driver is targeting multiple architectures at once, for things
> like Universal Mach-Os, we need to emit different remark files for each
> cc1 invocation to avoid overwriting the files from a different
> invocation.
>
> For example:
>
> $ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h
>
> will create two remark files:
>
> * foo-x86_64.opt.yaml
> * foo-x86_64h.opt.yaml
>
> Added:
>
>
> Modified:
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/test/Driver/opt-record.c
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp
> b/clang/lib/Driver/ToolChains/Clang.cpp
> index 2b1c24275e3d..f5591c48d4e0 100644
> --- a/clang/lib/Driver/ToolChains/Clang.cpp
> +++ b/clang/lib/Driver/ToolChains/Clang.cpp
> @@ -5403,6 +5403,8 @@ void Clang::ConstructJob(Compilation , const
> JobAction ,
>  if (A) {
>CmdArgs.push_back(A->getValue());
>  } else {
> +  bool hasMultipleArchs =
> +  Args.getAllArgValues(options::OPT_arch).size() > 1;
>SmallString<128> F;
>
>if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
> @@ -5427,6 +5429,22 @@ void Clang::ConstructJob(Compilation , const
> JobAction ,
>  }
>}
>
> +  // If we're having more than one "-arch", we should name the files
> +  //
> diff erently so that every cc1 invocation writes to a
> diff erent file.
> +  // We're doing that by appending "-" with "" being the
> arch
> +  // name from the triple.
> +  if (hasMultipleArchs) {
> +// First, remember the extension.
> +SmallString<64> OldExtension = llvm::sys::path::extension(F);
> +// then, remove it.
> +llvm::sys::path::replace_extension(F, "");
> +// attach - to it.
> +F += "-";
> +F += Triple.getArchName();
> +// put back the extension.
> +llvm::sys::path::replace_extension(F, OldExtension);
> +  }
> +
>std::string Extension = "opt.";
>if (const Arg *A =
>Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
>
> diff  --git a/clang/test/Driver/opt-record.c
> b/clang/test/Driver/opt-record.c
> index 062d0acc17da..d8d2aa53ed40 100644
> --- a/clang/test/Driver/opt-record.c
> +++ b/clang/test/Driver/opt-record.c
> @@ -18,6 +18,7 @@
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record
> -fsave-optimization-record=some-format %s 2>&1 | FileCheck %s
> -check-prefix=CHECK-EQ-FORMAT
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s
> 2>&1 | FileCheck %s -check-prefix=CHECK-EQ-FORMAT
>  // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format
> -fno-save-optimization-record %s 2>&1 | FileCheck %s
> --check-prefix=CHECK-FOPT-DISABLE-FORMAT
> +// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64
> -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
>  //
>  // CHECK: "-cc1"
>  // CHECK: "-opt-record-file" "FOO.opt.yaml"
> @@ -41,3 +42,8 @@
>  // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
>
>  // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
> +
> +// CHECK-MULTIPLE-ARCH: "-cc1"
> +// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
> +// CHECK-MULTIPLE-ARCH: "-cc1"
> +// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1ff5f0c - Revert "[Remarks][Driver] Use different remark files when targeting multiple architectures"

2019-11-18 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2019-11-18T10:52:41-08:00
New Revision: 1ff5f0ced3163e457c799bd3bd838153806d6320

URL: 
https://github.com/llvm/llvm-project/commit/1ff5f0ced3163e457c799bd3bd838153806d6320
DIFF: 
https://github.com/llvm/llvm-project/commit/1ff5f0ced3163e457c799bd3bd838153806d6320.diff

LOG: Revert "[Remarks][Driver] Use different remark files when targeting 
multiple architectures"

This reverts commit b4e2b112b58154a89171df39dae80044865ff4ff.

Test doesn't appear to pass on Windows, maybe all non-Mac.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/opt-record.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 2e3624e6cd24..6f25eea243ad 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5218,8 +5218,6 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 if (A) {
   CmdArgs.push_back(A->getValue());
 } else {
-  bool hasMultipleArchs =
-  Args.getAllArgValues(options::OPT_arch).size() > 1;
   SmallString<128> F;
 
   if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
@@ -5244,22 +5242,6 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 }
   }
 
-  // If we're having more than one "-arch", we should name the files
-  // 
diff erently so that every cc1 invocation writes to a 
diff erent file.
-  // We're doing that by appending "-" with "" being the arch
-  // name from the triple.
-  if (hasMultipleArchs) {
-// First, remember the extension.
-SmallString<64> OldExtension = llvm::sys::path::extension(F);
-// then, remove it.
-llvm::sys::path::replace_extension(F, "");
-// attach - to it.
-F += "-";
-F += Triple.getArchName();
-// put back the extension.
-llvm::sys::path::replace_extension(F, OldExtension);
-  }
-
   std::string Extension = "opt.";
   if (const Arg *A =
   Args.getLastArg(options::OPT_fsave_optimization_record_EQ))

diff  --git a/clang/test/Driver/opt-record.c b/clang/test/Driver/opt-record.c
index d8d2aa53ed40..062d0acc17da 100644
--- a/clang/test/Driver/opt-record.c
+++ b/clang/test/Driver/opt-record.c
@@ -18,7 +18,6 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-fsave-optimization-record=some-format %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format 
-fno-save-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-FOPT-DISABLE-FORMAT
-// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
 //
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
@@ -42,8 +41,3 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
-
-// CHECK-MULTIPLE-ARCH: "-cc1"
-// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
-// CHECK-MULTIPLE-ARCH: "-cc1"
-// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"



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


[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In D62731#1749916 , @mibintc wrote:

> Thanks I see it, I'm working on a patch. Previously there was no support for 
> frounding-math (unimplemented). This patch enables the option. In the IR 
> builder, there's a call to a runtime function in the exception handler which 
> is unexpectedly null.  I start by adding a null pointer check.


Had a crash on valid here for days, let's revert and then get a fix when 
recommitting. I'll respond to the thread when reverting. Thanks :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


Re: [PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via cfe-commits
Hi All,

I've gone ahead and temporarily reverted here:

echristo@jhereg ~/s/llvm-project> git push
To github.com:llvm/llvm-project.git
   a77b66a0562..30e7ee3c4ba  master -> master

and we can just reapply when the issues are fixed. Thanks :)

-eric

On Mon, Nov 18, 2019 at 6:58 AM Melanie Blower via Phabricator via
cfe-commits  wrote:
>
> mibintc added a comment.
>
> Thanks I see it, I'm working on a patch. Previously there was no support for 
> frounding-math (unimplemented). This patch enables the option. In the IR 
> builder, there's a call to a runtime function in the exception handler which 
> is unexpectedly null.  I start by adding a null pointer check.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62731/new/
>
> https://reviews.llvm.org/D62731
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 30e7ee3 - Temporarily Revert "Add support for options -frounding-math, ftrapping-math, -ffp-model=, and -ffp-exception-behavior="

2019-11-18 Thread Eric Christopher via cfe-commits

Author: Eric Christopher
Date: 2019-11-18T10:46:48-08:00
New Revision: 30e7ee3c4bac4a12ea584a879aa320bd4e035cc2

URL: 
https://github.com/llvm/llvm-project/commit/30e7ee3c4bac4a12ea584a879aa320bd4e035cc2
DIFF: 
https://github.com/llvm/llvm-project/commit/30e7ee3c4bac4a12ea584a879aa320bd4e035cc2.diff

LOG: Temporarily Revert "Add support for options -frounding-math, 
ftrapping-math, -ffp-model=, and -ffp-exception-behavior="
and a follow-up NFC rearrangement as it's causing a crash on valid. Testcase is 
on the original review thread.

This reverts commits af57dbf12e54f3a8ff48534bf1078f4de104c1cd and 
e6584b2b7b2de06f1e59aac41971760cac1e1b79

Added: 


Modified: 
clang/docs/UsersManual.rst
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/clang_f_opts.c
clang/test/Driver/fast-math.c
llvm/include/llvm/IR/IRBuilder.h
llvm/include/llvm/IR/IntrinsicInst.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/CMakeLists.txt
llvm/lib/IR/IntrinsicInst.cpp
llvm/unittests/IR/IRBuilderTest.cpp

Removed: 
clang/test/CodeGen/fpconstrained.c
clang/test/Driver/fp-model.c
llvm/include/llvm/IR/FPEnv.h
llvm/lib/IR/FPEnv.cpp



diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 62e2575c6b26..714681d7f4ce 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1231,10 +1231,10 @@ are listed below.
 
 **-f[no-]trapping-math**
 
-   Control floating point exception behavior. ``-fno-trapping-math`` allows 
optimizations that assume that floating point operations cannot generate traps 
such as divide-by-zero, overflow and underflow.
-
-- The option ``-ftrapping-math`` behaves identically to 
``-ffp-exception-behavior=strict``.
-- The option ``-fno-trapping-math`` behaves identically to 
``-ffp-exception-behavior=ignore``.   This is the default.
+   ``-fno-trapping-math`` allows optimizations that assume that
+   floating point operations cannot generate traps such as divide-by-zero,
+   overflow and underflow. Defaults to ``-ftrapping-math``.
+   Currently this option has no effect.
 
 .. option:: -ffp-contract=
 
@@ -1319,52 +1319,6 @@ are listed below.
 
Defaults to ``-fno-finite-math``.
 
-.. _opt_frounding-math:
-
-**-f[no-]rounding-math**
-
-Force floating-point operations to honor the dynamically-set rounding mode by 
default.
-
-The result of a floating-point operation often cannot be exactly represented 
in the result type and therefore must be rounded.  IEEE 754 describes 
diff erent rounding modes that control how to perform this rounding, not all of 
which are supported by all implementations.  C provides interfaces 
(``fesetround`` and ``fesetenv``) for dynamically controlling the rounding 
mode, and while it also recommends certain conventions for changing the 
rounding mode, these conventions are not typically enforced in the ABI.  Since 
the rounding mode changes the numerical result of operations, the compiler must 
understand something about it in order to optimize floating point operations.
-
-Note that floating-point operations performed as part of constant 
initialization are formally performed prior to the start of the program and are 
therefore not subject to the current rounding mode.  This includes the 
initialization of global variables and local ``static`` variables.  
Floating-point operations in these contexts will be rounded using 
``FE_TONEAREST``.
-
-- The option ``-fno-rounding-math`` allows the compiler to assume that the 
rounding mode is set to ``FE_TONEAREST``.  This is the default.
-- The option ``-frounding-math`` forces the compiler to honor the 
dynamically-set rounding mode.  This prevents optimizations which might affect 
results if the rounding mode changes or is 
diff erent from the default; for example, it prevents floating-point operations 
from being reordered across most calls and prevents constant-folding when the 
result is not exactly representable.
-
-.. option:: -ffp-model=
-
-   Specify floating point behavior. ``-ffp-model`` is an umbrella
-   option that encompasses functionality provided by other, single
-   purpose, floating point options.  Valid values are: ``precise``, ``strict``,
-   and ``fast``.
-   Details:
-
-   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
-   * ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are 

[clang] a77b66a - Allocate builtins table earlier to fix bug found by ubsan

2019-11-18 Thread Reid Kleckner via cfe-commits

Author: Reid Kleckner
Date: 2019-11-18T10:41:30-08:00
New Revision: a77b66a05625ea4271c2d76f65428cce02e4699e

URL: 
https://github.com/llvm/llvm-project/commit/a77b66a05625ea4271c2d76f65428cce02e4699e
DIFF: 
https://github.com/llvm/llvm-project/commit/a77b66a05625ea4271c2d76f65428cce02e4699e.diff

LOG: Allocate builtins table earlier to fix bug found by ubsan

Follow up to 979da9a4c3ba

Added: 


Modified: 
clang/lib/Lex/Preprocessor.cpp

Removed: 




diff  --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 496eae980c00..0e9be3923630 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -113,6 +113,8 @@ 
Preprocessor::Preprocessor(std::shared_ptr PPOpts,
   // We haven't read anything from the external source.
   ReadMacrosFromExternalSource = false;
 
+  BuiltinInfo = std::make_unique();
+
   // "Poison" __VA_ARGS__, __VA_OPT__ which can only appear in the expansion of
   // a macro. They get unpoisoned where it is allowed.
   (Ident__VA_ARGS__ = getIdentifierInfo("__VA_ARGS__"))->setIsPoisoned();
@@ -203,7 +205,6 @@ void Preprocessor::Initialize(const TargetInfo ,
   this->AuxTarget = AuxTarget;
 
   // Initialize information about built-ins.
-  BuiltinInfo = std::make_unique();
   BuiltinInfo->InitializeTarget(Target, AuxTarget);
   HeaderInfo.setTarget(Target);
 



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


[clang] b4e2b11 - [Remarks][Driver] Use different remark files when targeting multiple architectures

2019-11-18 Thread Francis Visoiu Mistrih via cfe-commits

Author: Francis Visoiu Mistrih
Date: 2019-11-18T10:38:10-08:00
New Revision: b4e2b112b58154a89171df39dae80044865ff4ff

URL: 
https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff
DIFF: 
https://github.com/llvm/llvm-project/commit/b4e2b112b58154a89171df39dae80044865ff4ff.diff

LOG: [Remarks][Driver] Use different remark files when targeting multiple 
architectures

When the driver is targeting multiple architectures at once, for things
like Universal Mach-Os, we need to emit different remark files for each
cc1 invocation to avoid overwriting the files from a different
invocation.

For example:

$ clang -c -o foo.o -fsave-optimization-record -arch x86_64 -arch x86_64h

will create two remark files:

* foo-x86_64.opt.yaml
* foo-x86_64h.opt.yaml

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/opt-record.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 2b1c24275e3d..f5591c48d4e0 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5403,6 +5403,8 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 if (A) {
   CmdArgs.push_back(A->getValue());
 } else {
+  bool hasMultipleArchs =
+  Args.getAllArgValues(options::OPT_arch).size() > 1;
   SmallString<128> F;
 
   if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
@@ -5427,6 +5429,22 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 }
   }
 
+  // If we're having more than one "-arch", we should name the files
+  // 
diff erently so that every cc1 invocation writes to a 
diff erent file.
+  // We're doing that by appending "-" with "" being the arch
+  // name from the triple.
+  if (hasMultipleArchs) {
+// First, remember the extension.
+SmallString<64> OldExtension = llvm::sys::path::extension(F);
+// then, remove it.
+llvm::sys::path::replace_extension(F, "");
+// attach - to it.
+F += "-";
+F += Triple.getArchName();
+// put back the extension.
+llvm::sys::path::replace_extension(F, OldExtension);
+  }
+
   std::string Extension = "opt.";
   if (const Arg *A =
   Args.getLastArg(options::OPT_fsave_optimization_record_EQ))

diff  --git a/clang/test/Driver/opt-record.c b/clang/test/Driver/opt-record.c
index 062d0acc17da..d8d2aa53ed40 100644
--- a/clang/test/Driver/opt-record.c
+++ b/clang/test/Driver/opt-record.c
@@ -18,6 +18,7 @@
 // RUN: %clang -### -S -o FOO -fsave-optimization-record 
-fsave-optimization-record=some-format %s 2>&1 | FileCheck %s 
-check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format %s 2>&1 | 
FileCheck %s -check-prefix=CHECK-EQ-FORMAT
 // RUN: %clang -### -S -o FOO -fsave-optimization-record=some-format 
-fno-save-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-FOPT-DISABLE-FORMAT
+// RUN: %clang -### -S -o FOO -fsave-optimization-record -arch x86_64 -arch 
x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
 //
 // CHECK: "-cc1"
 // CHECK: "-opt-record-file" "FOO.opt.yaml"
@@ -41,3 +42,8 @@
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"
+
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
+// CHECK-MULTIPLE-ARCH: "-cc1"
+// CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"



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


[PATCH] D70355: [clang-format] [NFC] add recent changes to release notes

2019-11-18 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D70355



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


[PATCH] D68155: [clang][NFC] Make various uses of Regex const

2019-11-18 Thread Thomas Preud'homme via Phabricator via cfe-commits
thopre accepted this revision.
thopre added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68155



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


[PATCH] D70349: [Attr] Fix `-ast-print` for `asm` attribute

2019-11-18 Thread Joel E. Denny via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc85fa79d3663: [Attr] Fix `-ast-print` for `asm` attribute 
(authored by jdenny).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70349

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-print-attr.c


Index: clang/test/AST/ast-print-attr.c
===
--- clang/test/AST/ast-print-attr.c
+++ clang/test/AST/ast-print-attr.c
@@ -10,3 +10,8 @@
 // FIXME: Too many parens here!
 // CHECK: using C = int ((*))() __attribute__((cdecl));
 using C = int (*)() [[gnu::cdecl]];
+
+// CHECK: int fun_asm() asm("");
+int fun_asm() asm("");
+// CHECK: int var_asm asm("");
+int var_asm asm("");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7019,8 +7019,9 @@
   }
 }
 
-NewVD->addAttr(::new (Context) AsmLabelAttr(
-Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true));
+NewVD->addAttr(AsmLabelAttr::Create(Context, Label,
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8923,9 +8924,9 @@
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString(), /*IsLiteralLabel=*/true));
+NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(),
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());


Index: clang/test/AST/ast-print-attr.c
===
--- clang/test/AST/ast-print-attr.c
+++ clang/test/AST/ast-print-attr.c
@@ -10,3 +10,8 @@
 // FIXME: Too many parens here!
 // CHECK: using C = int ((*))() __attribute__((cdecl));
 using C = int (*)() [[gnu::cdecl]];
+
+// CHECK: int fun_asm() asm("");
+int fun_asm() asm("");
+// CHECK: int var_asm asm("");
+int var_asm asm("");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7019,8 +7019,9 @@
   }
 }
 
-NewVD->addAttr(::new (Context) AsmLabelAttr(
-Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true));
+NewVD->addAttr(AsmLabelAttr::Create(Context, Label,
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8923,9 +8924,9 @@
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString(), /*IsLiteralLabel=*/true));
+NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(),
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c85fa79 - [Attr] Fix `-ast-print` for `asm` attribute

2019-11-18 Thread Joel E. Denny via cfe-commits

Author: Joel E. Denny
Date: 2019-11-18T11:55:25-05:00
New Revision: c85fa79d3663ecb3117e178b2a79ffa721d18e32

URL: 
https://github.com/llvm/llvm-project/commit/c85fa79d3663ecb3117e178b2a79ffa721d18e32
DIFF: 
https://github.com/llvm/llvm-project/commit/c85fa79d3663ecb3117e178b2a79ffa721d18e32.diff

LOG: [Attr] Fix `-ast-print` for `asm` attribute

Without this fix, the tests introduced here produce the following
assert fail:

```
clang: /home/jdenny/llvm/clang/include/clang/Basic/AttributeCommonInfo.h:163: 
unsigned int clang::AttributeCommonInfo::getAttributeSpellingListIndex() const: 
Assertion `(isAttributeSpellingListCalculated() || AttrName) && "Spelling 
cannot be found"' failed.
```

The bug was introduced by D67368, which caused `AsmLabelAttr`'s
spelling index to be set to `SpellingNotCalculated`.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D70349

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/AST/ast-print-attr.c

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b469217108ce..6d857e832c4b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7019,8 +7019,9 @@ NamedDecl *Sema::ActOnVariableDeclarator(
   }
 }
 
-NewVD->addAttr(::new (Context) AsmLabelAttr(
-Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true));
+NewVD->addAttr(AsmLabelAttr::Create(Context, Label,
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8923,9 +8924,9 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator , 
DeclContext *DC,
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString(), /*IsLiteralLabel=*/true));
+NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(),
+/*IsLiteralLabel=*/true,
+SE->getStrTokenLoc(0)));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());

diff  --git a/clang/test/AST/ast-print-attr.c b/clang/test/AST/ast-print-attr.c
index 223e27b39790..6448437c5eab 100644
--- a/clang/test/AST/ast-print-attr.c
+++ b/clang/test/AST/ast-print-attr.c
@@ -10,3 +10,8 @@ using B = int ** __ptr32 *[3];
 // FIXME: Too many parens here!
 // CHECK: using C = int ((*))() __attribute__((cdecl));
 using C = int (*)() [[gnu::cdecl]];
+
+// CHECK: int fun_asm() asm("");
+int fun_asm() asm("");
+// CHECK: int var_asm asm("");
+int var_asm asm("");



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


[clang] c3eded0 - [OPENMP50]Fix PR44024: runtime assert in distribute construct.

2019-11-18 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2019-11-18T11:14:27-05:00
New Revision: c3eded068c64bfe614d25359927a2917ff8e4a35

URL: 
https://github.com/llvm/llvm-project/commit/c3eded068c64bfe614d25359927a2917ff8e4a35
DIFF: 
https://github.com/llvm/llvm-project/commit/c3eded068c64bfe614d25359927a2917ff8e4a35.diff

LOG: [OPENMP50]Fix PR44024: runtime assert in distribute construct.

If the code is emitted for distribute construct, the nonmonotonic
modifier should not be added.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/distribute_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index cd20cb83afb9..a38007001258 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3616,7 +3616,9 @@ static int addMonoNonMonoModifier(CodeGenModule , 
OpenMPSchedType Schedule,
   if (CGM.getLangOpts().OpenMP >= 50 && Modifier == 0) {
 if (!(Schedule == OMP_sch_static_chunked || Schedule == OMP_sch_static ||
   Schedule == OMP_sch_static_balanced_chunked ||
-  Schedule == OMP_ord_static_chunked || Schedule == OMP_ord_static))
+  Schedule == OMP_ord_static_chunked || Schedule == OMP_ord_static ||
+  Schedule == OMP_dist_sch_static_chunked ||
+  Schedule == OMP_dist_sch_static))
   Modifier = OMP_sch_modifier_nonmonotonic;
   }
   return Schedule | Modifier;

diff  --git a/clang/test/OpenMP/distribute_codegen.cpp 
b/clang/test/OpenMP/distribute_codegen.cpp
index 9641b8a1359b..e3b65ca85603 100644
--- a/clang/test/OpenMP/distribute_codegen.cpp
+++ b/clang/test/OpenMP/distribute_codegen.cpp
@@ -5,6 +5,13 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix CHECK --check-prefix CHECK-32  --check-prefix HCHECK
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s 
-emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32 
--check-prefix HCHECK
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s 
--check-prefix CHECK --check-prefix CHECK-64  --check-prefix HCHECK
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -x c++ -triple 
i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | 
FileCheck %s --check-prefix CHECK --check-prefix CHECK-32  --check-prefix HCHECK
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -triple 
i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch 
%t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK 
--check-prefix CHECK-32 --check-prefix HCHECK
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
@@ -14,6 +21,14 @@
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s 
-emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck 
--check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -x c++ -triple 

[clang-tools-extra] 2054ed0 - [clangd] Store xref for Macros in ParsedAST.

2019-11-18 Thread Utkarsh Saxena via cfe-commits

Author: Utkarsh Saxena
Date: 2019-11-18T16:47:18+01:00
New Revision: 2054ed052f15b584e1bce57c8f765991eab2da7d

URL: 
https://github.com/llvm/llvm-project/commit/2054ed052f15b584e1bce57c8f765991eab2da7d
DIFF: 
https://github.com/llvm/llvm-project/commit/2054ed052f15b584e1bce57c8f765991eab2da7d.diff

LOG: [clangd] Store xref for Macros in ParsedAST.

This patch adds the cross references for Macros in the MainFile.
We add references for the main file to the ParsedAST. We query the
references from it using the SymbolID.
Xref outside main file will be added to the index in a separate patch.

Added: 
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp

Modified: 
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CollectMacros.h 
b/clang-tools-extra/clangd/CollectMacros.h
index 619c9f54b58a..17e9917542bd 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -9,10 +9,13 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
 
+#include "AST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "index/SymbolID.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Lex/PPCallbacks.h"
+#include "llvm/ADT/DenseMap.h"
 #include 
 
 namespace clang {
@@ -22,7 +25,11 @@ struct MainFileMacros {
   llvm::StringSet<> Names;
   // Instead of storing SourceLocation, we have to store the token range 
because
   // SourceManager from preamble is not available when we build the AST.
-  std::vector Ranges;
+  llvm::DenseMap> MacroRefs;
+  // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
+  // reference to an undefined macro. Store them separately, e.g. for semantic
+  // highlighting.
+  std::vector UnknownMacros;
 };
 
 /// Collects macro references (e.g. definitions, expansions) in the main file.
@@ -80,8 +87,12 @@ class CollectMainFileMacros : public PPCallbacks {
   return;
 
 if (auto Range = getTokenRange(SM, LangOpts, Loc)) {
-  Out.Names.insert(MacroNameTok.getIdentifierInfo()->getName());
-  Out.Ranges.push_back(*Range);
+  auto Name = MacroNameTok.getIdentifierInfo()->getName();
+  Out.Names.insert(Name);
+  if (auto SID = getSymbolID(Name, MI, SM))
+Out.MacroRefs[*SID].push_back(*Range);
+  else
+Out.UnknownMacros.push_back(*Range);
 }
   }
   const SourceManager 

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 9838600584c6..4e47a83d8da0 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -311,9 +311,14 @@ std::vector 
getSemanticHighlightings(ParsedAST ) {
 if (auto Kind = kindForReference(R))
   Builder.addToken(R.NameLoc, *Kind);
   });
-  // Add highlightings for macro expansions.
-  for (const auto  : AST.getMacros().Ranges)
+  // Add highlightings for macro references.
+  for (const auto  : AST.getMacros().MacroRefs) {
+for (const auto  : SIDToRefs.second)
+  Builder.addToken({HighlightingKind::Macro, M});
+  }
+  for (const auto  : AST.getMacros().UnknownMacros)
 Builder.addToken({HighlightingKind::Macro, M});
+
   return std::move(Builder).collect();
 }
 

diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index d604379b75fc..9697b8eec19a 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -917,8 +917,7 @@ ReferencesResult findReferences(ParsedAST , Position 
Pos, uint32_t Limit,
  MainFileRefs.end());
   for (const auto  : MainFileRefs) {
 if (auto Range =
-getTokenRange(AST.getASTContext().getSourceManager(),
-  AST.getASTContext().getLangOpts(), Ref.Loc)) {
+getTokenRange(SM, AST.getASTContext().getLangOpts(), Ref.Loc)) {
   Location Result;
   Result.range = *Range;
   Result.uri = URIForFile::canonicalize(*MainFilePath, *MainFilePath);

diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt 
b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index 7e298b6ad053..21c29196034f 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -30,6 +30,7 @@ add_unittest(ClangdUnitTests ClangdTests
   ClangdTests.cpp
   CodeCompleteTests.cpp
   CodeCompletionStringsTests.cpp
+  CollectMacrosTests.cpp
   ContextTests.cpp
   DexTests.cpp
   DiagnosticsTests.cpp

diff  --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp 

  1   2   >