date:20160707

Re: [PATCH] D22069: clang-tidy modernize-loop-convert: preserve type of alias declaration (bug 28341)

2016-07-07 Thread Matthias Gehre via cfe-commits

mgehre added inline comments.


Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:525
@@ +524,3 @@
+  DeclarationType = DeclarationType.getNonReferenceType();
+if (Descriptor.ElemType.isNull() || DeclarationType.isNull() ||
+!Context->hasSameUnqualifiedType(DeclarationType, Descriptor.ElemType))

alexfh wrote:
> 1. Can the `AliasVar->getType().isNull()` condition be true?
> 2. If it can, consider `!Descriptor.ElemType.isNull().isNull()` and 
> `AliasVar->getType().isNull()`. In this case setting `Descriptor.ElemType` to 
> `AliasVar->getType()` (which is null) doesn't make much sense.
> 
> I'd probably just wrap the added code in `if (!AliasVar->getType().isNull())`.
Thanks for you fast review.

I copied the block from isAliasDecl(). I don't see any reason why the types can 
be Null, but I'm also not an expert in llvm.

When would a VarDecl have no type? Maybe I should put an assert instead?


http://reviews.llvm.org/D22069



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

Re: [PATCH] D21329: Rename and rework `_LIBCPP_TRIVIAL_PAIR_COPY_CTOR`. Move FreeBSD configuration in-tree.

2016-07-07 Thread Dimitry Andric via cfe-commits

dim added a comment.

In http://reviews.llvm.org/D21329#475105, @theraven wrote:

> Looks fine to me, though I wonder if we want to move to the new ABI for 
> FreeBSD11 and use the old one for <=10.

For 11 it is certainly too late now, since it is ABI/API frozen now, and we 
would still need to find some way of providing backwards compat for 
applications that were linked against the earlier libc++, which still had the 
non-trivial pair copy constructor.

http://reviews.llvm.org/D21329

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

Re: [PATCH] D21991: [libunwind][ARM] Improve unwinder stack usage - Make WMMX support optional

2016-07-07 Thread Asiri Rathnayake via cfe-commits

rmaprath added inline comments.


Comment at: src/Registers.hpp:1497
@@ -1479,3 +1496,3 @@
 _LIBUNWIND_ABORT("unsupported arm register");
 }
 

compnerd wrote:
> Early returns would be nicer imo.
Not sure if I follow, did you mean to check the bounds of `regNum` the first 
thing and `_LIBUNWIND_ABORT` sooner than later? Might convolute the code given 
the conditional on `__ARM_WMMX` and I'm not sure what benefit it brings? Or 
perhaps I misunderstood you?

I'll commit the current patch as it is for the moment and then do a clean-up 
(once I understand what you mean).

Thanks.


http://reviews.llvm.org/D21991



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

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Dean Michael Berris via cfe-commits

dberris updated this revision to Diff 63038.
dberris added a comment.

- Check first whether `D` is actually not nullptr


http://reviews.llvm.org/D20352

Files:
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3193,12 +3193,17 @@
   return false;
 }
 
-static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
   CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
   CmdArgs.push_back("-latomic");
+  if (TC.GetCXXStdlibType(Args) == ToolChain::CST_Libcxx)
+CmdArgs.push_back("-lc++");
+  else
+CmdArgs.push_back("-lstdc++");
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
 CmdArgs.push_back("-ldl");
 }
@@ -9441,7 +9446,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -691,7 +691,7 @@
 Fn->addFnAttr(llvm::Attribute::SafeStack);
 
   // Apply xray attributes to the function (as a string, for now)
-  if (ShouldXRayInstrumentFunction()) {
+  if (D && ShouldXRayInstrumentFunction()) {
 if (const auto *XRayAttr = D->getAttr()) {
   if (XRayAttr->alwaysXRayInstrument())
 Fn->addFnAttr("function-instrument", "xray-always");


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3193,12 +3193,17 @@
   return false;
 }
 
-static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
   CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
   CmdArgs.push_back("-latomic");
+  if (TC.GetCXXStdlibType(Args) == ToolChain::CST_Libcxx)
+CmdArgs.push_back("-lc++");
+  else
+CmdArgs.push_back("-lstdc++");
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
 CmdArgs.push_back("-ldl");
 }
@@ -9441,7 +9446,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -691,7 +691,7 @@
 Fn->addFnAttr(llvm::Attribute::SafeStack);
 
   // Apply xray attributes to the function (as a string, for now)
-  if (ShouldXRayInstrumentFunction()) {
+  if (D && ShouldXRayInstrumentFunction()) {
 if (const auto *XRayAttr = D->getAttr()) {
   if (XRayAttr->alwaysXRayInstrument())
 Fn->addFnAttr("function-instrument", "xray-always");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Dean Michael Berris via cfe-commits

dberris updated this revision to Diff 63039.
dberris added a comment.

Undo previous change -- updated the wrong patch. :(


http://reviews.llvm.org/D20352

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/xray-attributes-supported.cpp
  test/Sema/xray-always-instrument-attr.c
  test/Sema/xray-always-instrument-attr.cpp

Index: test/Sema/xray-always-instrument-attr.cpp
===
--- /dev/null
+++ test/Sema/xray-always-instrument-attr.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+void foo [[clang::xray_always_instrument]] ();
+
+struct [[clang::xray_always_instrument]] a { int x; }; // expected-warning {{'xray_always_instrument' attribute only applies to functions and methods}}
+
+class b {
+ void c [[clang::xray_always_instrument]] ();
+};
+
+void baz [[clang::xray_always_instrument("not-supported")]] (); // expected-error {{'xray_always_instrument' attribute takes no arguments}}
Index: test/Sema/xray-always-instrument-attr.c
===
--- /dev/null
+++ test/Sema/xray-always-instrument-attr.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c11
+void foo() __attribute__((xray_always_instrument));
+
+struct __attribute__((xray_always_instrument)) a { int x; }; // expected-warning {{'xray_always_instrument' attribute only applies to functions and methods}}
+
+void bar() __attribute__((xray_always_instrument("not-supported"))); // expected-error {{'xray_always_instrument' attribute takes no arguments}}
Index: test/CodeGen/xray-attributes-supported.cpp
===
--- /dev/null
+++ test/CodeGen/xray-attributes-supported.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5909,10 +5909,13 @@
   case AttributeList::AT_TypeTagForDatatype:
 handleTypeTagForDatatypeAttr(S, D, Attr);
 break;
-
   case AttributeList::AT_RenderScriptKernel:
 handleSimpleAttribute(S, D, Attr);
 break;
+  // XRay attributes.
+  case AttributeList::AT_XRayInstrument:
+handleSimpleAttribute(S, D, Attr);
+break;
   }
 }
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -686,6 +686,9 @@
   }
 
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
+  Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+  Opts.XRayInstructionThreshold =
+  getLastArgIntValue(Args, OPT_fxray_instruction_threshold_, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
   Opts.CompressDebugSections = Args.hasArg(OPT_compress_debug_sections);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3181,6 +3181,28 @@
   return !StaticRuntimes.empty();
 }
 
+static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_fxray_instrument,
+  options::OPT_fnoxray_instrument, false)) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+  return false;
+}
+
+static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lpthread");
+  CmdArgs.push_back("-lrt");
+  CmdArgs.push_back("-lm");
+  CmdArgs.push_back("-latomic");
+  if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
+CmdArgs.push_back("-ldl");
+}
+
 static bool areOptimizationsEnabled(const ArgList &Args) {
   // Find the last -O arg and see if it is non-zero.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group))
@@ -4582,6 +4604,16 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
 
+

Re: [PATCH] D21983: Add C++ dependencies to xray runtime

2016-07-07 Thread Dean Michael Berris via cfe-commits

dberris updated this revision to Diff 63040.
dberris added a comment.

- Check first whether `D` is actually not nullptr


http://reviews.llvm.org/D21983

Files:
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3193,12 +3193,17 @@
   return false;
 }
 
-static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
   CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
   CmdArgs.push_back("-latomic");
+  if (TC.GetCXXStdlibType(Args) == ToolChain::CST_Libcxx)
+CmdArgs.push_back("-lc++");
+  else
+CmdArgs.push_back("-lstdc++");
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
 CmdArgs.push_back("-ldl");
 }
@@ -9441,7 +9446,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -691,7 +691,7 @@
 Fn->addFnAttr(llvm::Attribute::SafeStack);
 
   // Apply xray attributes to the function (as a string, for now)
-  if (ShouldXRayInstrumentFunction()) {
+  if (D && ShouldXRayInstrumentFunction()) {
 if (const auto *XRayAttr = D->getAttr()) {
   if (XRayAttr->alwaysXRayInstrument())
 Fn->addFnAttr("function-instrument", "xray-always");


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3193,12 +3193,17 @@
   return false;
 }
 
-static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
   CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
   CmdArgs.push_back("-latomic");
+  if (TC.GetCXXStdlibType(Args) == ToolChain::CST_Libcxx)
+CmdArgs.push_back("-lc++");
+  else
+CmdArgs.push_back("-lstdc++");
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
 CmdArgs.push_back("-ldl");
 }
@@ -9441,7 +9446,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -691,7 +691,7 @@
 Fn->addFnAttr(llvm::Attribute::SafeStack);
 
   // Apply xray attributes to the function (as a string, for now)
-  if (ShouldXRayInstrumentFunction()) {
+  if (D && ShouldXRayInstrumentFunction()) {
 if (const auto *XRayAttr = D->getAttr()) {
   if (XRayAttr->alwaysXRayInstrument())
 Fn->addFnAttr("function-instrument", "xray-always");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Dean Michael Berris via cfe-commits

dberris updated this revision to Diff 63041.
dberris added a comment.

- Check D is valid before using it


http://reviews.llvm.org/D20352

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/xray-attributes-supported.cpp
  test/Sema/xray-always-instrument-attr.c
  test/Sema/xray-always-instrument-attr.cpp

Index: test/Sema/xray-always-instrument-attr.cpp
===
--- /dev/null
+++ test/Sema/xray-always-instrument-attr.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+void foo [[clang::xray_always_instrument]] ();
+
+struct [[clang::xray_always_instrument]] a { int x; }; // expected-warning {{'xray_always_instrument' attribute only applies to functions and methods}}
+
+class b {
+ void c [[clang::xray_always_instrument]] ();
+};
+
+void baz [[clang::xray_always_instrument("not-supported")]] (); // expected-error {{'xray_always_instrument' attribute takes no arguments}}
Index: test/Sema/xray-always-instrument-attr.c
===
--- /dev/null
+++ test/Sema/xray-always-instrument-attr.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c11
+void foo() __attribute__((xray_always_instrument));
+
+struct __attribute__((xray_always_instrument)) a { int x; }; // expected-warning {{'xray_always_instrument' attribute only applies to functions and methods}}
+
+void bar() __attribute__((xray_always_instrument("not-supported"))); // expected-error {{'xray_always_instrument' attribute takes no arguments}}
Index: test/CodeGen/xray-attributes-supported.cpp
===
--- /dev/null
+++ test/CodeGen/xray-attributes-supported.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5909,10 +5909,13 @@
   case AttributeList::AT_TypeTagForDatatype:
 handleTypeTagForDatatypeAttr(S, D, Attr);
 break;
-
   case AttributeList::AT_RenderScriptKernel:
 handleSimpleAttribute(S, D, Attr);
 break;
+  // XRay attributes.
+  case AttributeList::AT_XRayInstrument:
+handleSimpleAttribute(S, D, Attr);
+break;
   }
 }
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -686,6 +686,9 @@
   }
 
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
+  Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+  Opts.XRayInstructionThreshold =
+  getLastArgIntValue(Args, OPT_fxray_instruction_threshold_, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
   Opts.CompressDebugSections = Args.hasArg(OPT_compress_debug_sections);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3181,6 +3181,28 @@
   return !StaticRuntimes.empty();
 }
 
+static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_fxray_instrument,
+  options::OPT_fnoxray_instrument, false)) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+  return false;
+}
+
+static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lpthread");
+  CmdArgs.push_back("-lrt");
+  CmdArgs.push_back("-lm");
+  CmdArgs.push_back("-latomic");
+  if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
+CmdArgs.push_back("-ldl");
+}
+
 static bool areOptimizationsEnabled(const ArgList &Args) {
   // Find the last -O arg and see if it is non-zero.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group))
@@ -4582,6 +4604,16 @@
 
   Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions);
 
+  if (Args.hasArg(

Re: [PATCH] D21983: Add C++ dependencies to xray runtime

2016-07-07 Thread Dean Michael Berris via cfe-commits

dberris updated this revision to Diff 63042.
dberris added a comment.

Rebase


http://reviews.llvm.org/D21983

Files:
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3193,12 +3193,17 @@
   return false;
 }
 
-static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
   CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
   CmdArgs.push_back("-latomic");
+  if (TC.GetCXXStdlibType(Args) == ToolChain::CST_Libcxx)
+CmdArgs.push_back("-lc++");
+  else
+CmdArgs.push_back("-lstdc++");
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
 CmdArgs.push_back("-ldl");
 }
@@ -9441,7 +9446,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3193,12 +3193,17 @@
   return false;
 }
 
-static void linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
   CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
   CmdArgs.push_back("-latomic");
+  if (TC.GetCXXStdlibType(Args) == ToolChain::CST_Libcxx)
+CmdArgs.push_back("-lc++");
+  else
+CmdArgs.push_back("-lstdc++");
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD)
 CmdArgs.push_back("-ldl");
 }
@@ -9441,7 +9446,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20795: Added basic capabilities to detect source code clones.

2016-07-07 Thread Vassil Vassilev via cfe-commits

v.g.vassilev requested changes to this revision.
This revision now requires changes to proceed.


Comment at: include/clang/AST/CloneDetection.h:148
@@ +147,3 @@
+/// This class only searches for clones in exectuable source code
+/// (e.g. function bodies). Other clones (e.g. cloned comments or declarations)
+/// are not supported.

It might be a good idea to detect cloned comments, too.


Comment at: include/clang/AST/CloneDetection.h:153
@@ +152,3 @@
+/// in the given statements.
+/// This is done by hashing all statements using a locality-sensitive hash
+/// function that generates identical hash values for similar statement

Can you move this on the previous line?


Comment at: include/clang/AST/CloneDetection.h:166
@@ +165,3 @@
+/// that are too small to be reported.
+/// A greater value indicates that the related StmtSequence is probably
+/// more interesting to the user.

This should go on prev line too. Please check all comments for this pattern.


Comment at: include/clang/AST/CloneDetection.h:215
@@ +214,3 @@
+  /// StmtSequences that were identified to be clones of each other.
+  std::vector findClones(unsigned MinGroupComplexity = 10);
+

We shouldn't copy (or hope this would be std::moved). Can you pass it as a 
reference to the argument list?


Comment at: lib/AST/CloneDetection.cpp:22
@@ +21,3 @@
+
+StmtSequence::StmtSequence(CompoundStmt *Stmt, ASTContext *Context,
+   unsigned StartIndex, unsigned EndIndex)

It is a very common pattern in clang the ASTContext to be passed as 
ASTContext&, this should simplify the body of the constructors.


Comment at: lib/AST/CloneDetection.cpp:45
@@ +44,3 @@
+  // surround the other sequence.
+  bool StartIsInBounds = Context->getSourceManager().isBeforeInTranslationUnit(
+ getStartLoc(), Other.getStartLoc()) ||

I'd factor out the Context->getSourceManager() in a local variable.


Comment at: lib/AST/CloneDetection.cpp:54
@@ +53,3 @@
+
+Stmt *const *StmtSequence::begin() const {
+  if (holdsSequence()) {

I am not sure what is the intent but shouldn't that be `Stmt const* const*`?


Comment at: lib/AST/CloneDetection.cpp:56
@@ +55,3 @@
+  if (holdsSequence()) {
+auto CS = static_cast(S);
+return CS->body_begin() + StartIndex;

Please use `auto CS = cast(S);`

This logic will crash on `void f() try {} catch(...){}` In this case we do not 
generate a CompoundStmt.


Comment at: lib/AST/CloneDetection.cpp:64
@@ +63,3 @@
+  if (holdsSequence()) {
+auto CS = static_cast(S);
+return CS->body_begin() + EndIndex;

Same as above.


Comment at: lib/AST/CloneDetection.cpp:84
@@ +83,3 @@
+  void addData(unsigned Data) {
+Value *= 53;
+Value += Data;

Still a floating magic constant. Could you factor it out in a variable with a 
meaningful name?


Comment at: lib/AST/CloneDetection.cpp:112
@@ +111,3 @@
+
+  // We need to traverse postorder over the AST for our algorithm
+  // to work as each parent expects that its children were already hashed.

Doxygen style /// ?


Comment at: lib/AST/CloneDetection.cpp:139
@@ +138,3 @@
+// Iterate over each child in the sub-sequence.
+for (auto I = StartIterator; I != EndIterator; ++I) {
+  Stmt *Child = *I;

Could we use a range-based for loop. This looks odd.


Comment at: lib/AST/CloneDetection.cpp:161
@@ +160,3 @@
+  void SaveData(StmtSequence CurrentStmt, HashValue Hash,
+   unsigned Complexity);
+

Indent.


Comment at: lib/AST/CloneDetection.cpp:164
@@ +163,3 @@
+  CloneDetector &CD;
+  ASTContext &Context;
+};

Could you move these members above. I don't think this is common in the 
codebase.


Comment at: lib/AST/CloneDetection.cpp:293
@@ +292,3 @@
+  // contains another group, we only need to return the bigger group.
+  for (unsigned I = 0; I < Result.size(); ++I) {
+for (unsigned J = 0; J < Result.size(); ++J) {

Capital letters are more common when naming iterators. What about small `i` and 
`j` here. And down you can use `I` and you won't need to break the line.


http://reviews.llvm.org/D20795



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

Re: [PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

2016-07-07 Thread Haojian Wu via cfe-commits

hokein updated this revision to Diff 63043.
hokein marked 3 inline comments as done.
hokein added a comment.

- Address Daniel's comments.
- Add tests for nested classes.


http://reviews.llvm.org/D21603

Files:
  include-fixer/IncludeFixer.cpp
  include-fixer/IncludeFixerContext.h
  include-fixer/SymbolIndexManager.cpp
  include-fixer/SymbolIndexManager.h
  include-fixer/find-all-symbols/SymbolInfo.cpp
  include-fixer/find-all-symbols/SymbolInfo.h
  include-fixer/tool/ClangIncludeFixer.cpp
  unittests/include-fixer/IncludeFixerTest.cpp

Index: unittests/include-fixer/IncludeFixerTest.cpp
===
--- unittests/include-fixer/IncludeFixerTest.cpp
+++ unittests/include-fixer/IncludeFixerTest.cpp
@@ -82,14 +82,17 @@
   IncludeFixerContext FixerContext;
   IncludeFixerActionFactory Factory(*SymbolIndexMgr, FixerContext, "llvm");
 
-  runOnCode(&Factory, Code, "input.cc", ExtraArgs);
-  if (FixerContext.Headers.empty())
+  std::string FakeFileName = "input.cc";
+  runOnCode(&Factory, Code, FakeFileName, ExtraArgs);
+  if (FixerContext.getMatchedSymbols().empty())
 return Code;
   tooling::Replacements Replacements =
   clang::include_fixer::createInsertHeaderReplacements(
-  Code, "input.cc", FixerContext.Headers.front());
+  Code, FakeFileName, FixerContext.getHeaders().front());
   clang::RewriterTestContext Context;
-  clang::FileID ID = Context.createInMemoryFile("input.cc", Code);
+  clang::FileID ID = Context.createInMemoryFile(FakeFileName, Code);
+  if (FixerContext.getSymbolRange().getLength() > 0)
+Replacements.insert(FixerContext.createSymbolReplacement(FakeFileName, 0));
   clang::tooling::applyAllReplacements(Replacements, Context.Rewrite);
   return Context.getRewrittenText(ID);
 }
@@ -113,23 +116,17 @@
   "#include \"foo.h\"\n#include \nstd::string::size_type foo;\n",
   runIncludeFixer("#include \"foo.h\"\nstd::string::size_type foo;\n"));
 
-  // string without "std::" can also be fixed since fixed db results go through
-  // SymbolIndexManager, and SymbolIndexManager matches unqualified identifiers
-  // too.
-  EXPECT_EQ("#include \nstring foo;\n",
+  EXPECT_EQ("#include \nstd::string foo;\n",
 runIncludeFixer("string foo;\n"));
 
-  // Fully qualified name.
-  EXPECT_EQ("#include \n::std::string foo;\n",
-runIncludeFixer("::std::string foo;\n"));
   // Should not match std::string.
   EXPECT_EQ("::string foo;\n", runIncludeFixer("::string foo;\n"));
 }
 
 TEST(IncludeFixer, IncompleteType) {
   EXPECT_EQ(
   "#include \"foo.h\"\n#include \n"
-  "namespace std {\nclass string;\n}\nstring foo;\n",
+  "namespace std {\nclass string;\n}\nstd::string foo;\n",
   runIncludeFixer("#include \"foo.h\"\n"
   "namespace std {\nclass string;\n}\nstring foo;\n"));
 }
@@ -186,7 +183,7 @@
 runIncludeFixer("namespace A { c::b::bar b; }\n"));
   // FIXME: The header should not be added here. Remove this after we support
   // full match.
-  EXPECT_EQ("#include \"bar.h\"\nnamespace A {\nb::bar b;\n}",
+  EXPECT_EQ("#include \"bar.h\"\nnamespace A {\na::b::bar b;\n}",
 runIncludeFixer("namespace A {\nb::bar b;\n}"));
 }
 
@@ -221,6 +218,44 @@
 runIncludeFixer("a::Vector v;"));
 }
 
+TEST(IncludeFixer, FixNamespaceQualifiers) {
+  EXPECT_EQ("#include \"bar.h\"\na::b::bar b;\n",
+runIncludeFixer("b::bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\na::b::bar b;\n",
+runIncludeFixer("a::b::bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\na::b::bar b;\n",
+runIncludeFixer("bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar b;\n}\n",
+runIncludeFixer("namespace a {\nb::bar b;\n}\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar b;\n}\n",
+runIncludeFixer("namespace a {\nbar b;\n}\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nnamespace b{\nbar b;\n}\n}\n",
+runIncludeFixer("namespace a {\nnamespace b{\nbar b;\n}\n}\n"));
+  EXPECT_EQ("c::b::bar b;\n",
+runIncludeFixer("c::b::bar b;\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace c {\na::b::bar b;\n}\n",
+runIncludeFixer("namespace c {\nbar b;\n}\n"));
+
+  // Test nested classes.
+  EXPECT_EQ("#include \"bar.h\"\nnamespace c {\na::b::bar::t b;\n}\n",
+runIncludeFixer("namespace c {\nbar::t b;\n}\n"));
+  EXPECT_EQ("#include \"bar.h\"\nnamespace a {\nb::bar::t b;\n}\n",
+runIncludeFixer("namespace a {\nbar::t b;\n}\n"));
+
+  EXPECT_EQ(
+  "#include \"color.h\"\nint test = a::b::Green;\n",
+  runIncludeFixer("int test = Green;\n"));
+  EXPECT_EQ("#include \"color.h\"\nnamespace d {\nint test = a::b::Green;\n}\n",
+runIncludeFixer("namespace d {\nint test = Green;\n}\n"));
+  EXPECT_EQ("#include \"color.h\"\nnamespace a {\nint test = b::Green;\n}\n",
+runIncludeFixer("namespace a {\nint test = Gr

Re: [PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

2016-07-07 Thread Haojian Wu via cfe-commits

hokein added inline comments.


Comment at: unittests/include-fixer/IncludeFixerTest.cpp:144
@@ -141,1 +143,3 @@
+runIncludeFixer("a::b::foo bar;\n",
+/*FixNamespaceQualifiers=*/true, IncludePath));
 

djasper wrote:
> I think we should set this to true everywhere or more precisely completely 
> remove this parameter. Now that the flag is gone, we are starting to test 
> implementation details IMO.
Yeah, this parameter is not needed. Removed it.


http://reviews.llvm.org/D21603



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

Re: [PATCH] D21851: [Driver][OpenMP][CUDA] Add capability to bundle object files in sections of the host binary format.

2016-07-07 Thread Jonas Hahnfeld via cfe-commits

Hahnfeld added inline comments.


Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:477-490
@@ +476,16 @@
+
+// Do the incremental linking. We write to the output file directly. So, we
+// close it and use the name to pass down to clang.
+OS.close();
+SmallString<128> TargetName = getTriple(TargetNames.front());
+const char *ClangArgs[] = {"clang",
+   "-r",
+   "-target",
+   TargetName.c_str(),
+   "-o",
+   OutputFileNames.front().c_str(),
+   InputFileNames.front().c_str(),
+   BitcodeFileName.c_str(),
+   "-nostdlib",
+   nullptr};
+

`test/Driver/clang-offload-bundler.c` gives me
```
/..//bin/ld: unrecognised emulation mode: elf64lppc
Supported emulations: elf_x86_64 elf32_x86_64 elf_i386 i386linux elf_l1om 
elf_k1om
```
and therefore fails.

I'm on an x86_64 Linux and obviously my `GNU ld version 2.23.52.0.1-55.el7 
20130226` doesn't support Power :-(


http://reviews.llvm.org/D21851



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

Re: [PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

2016-07-07 Thread Daniel Jasper via cfe-commits

djasper accepted this revision.
djasper added a comment.

Looks good.


http://reviews.llvm.org/D21603



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

Re: [PATCH] D21978: [analyzer] Add LocationContext to SymbolMetadata

2016-07-07 Thread Aleksei Sidorin via cfe-commits

a.sidorin added a comment.

In http://reviews.llvm.org/D21978#476200, @NoQ wrote:

> I also never noticed that the block count gets reset on every stack frame, 
> that's pretty unobvious.

Yes, that was surprising for me too.

> Uhm, and it's already there in SymbolConjured. Anyway, fixing the block count 
> itself seems to be pretty complicated.

> 

> Also, maybe once the ScopeContext thing is ready, we'd be able to omit the 
> block count completely.

Not sure. According to review comments, we may want for ScopeContext to be 
created for scopes with variables declared only (it seems reasonable). So 
ScopeContext may cover multiple blocks.

http://reviews.llvm.org/D21978

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

Re: [PATCH] D21857: [Driver][OpenMP] Add support to create jobs for unbundling actions.

2016-07-07 Thread Jonas Hahnfeld via cfe-commits

Hahnfeld added a comment.

Hi Samuel,

this looks pretty good overall!

I've only encountered one issue which I don't really know is caused by which 
patch: I can't use the (exact) same triple for offloading and the host when 
compiling separately. I found out that the device object file is taken as input 
for the final link step and `ld` therefore complains

  <...>/lib64/crt1.o: In function `_start':
  (.text+0x20): undefined reference to `main'

I've not yet found the reason - are there some `unordered_maps` that don't keep 
the order? I'm also seeing that the first argument to `clang-offload-bundler` 
is not always for the host...

Thanks,
Jonas


http://reviews.llvm.org/D21857



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

[libunwind] r274744 - [libunwind][ARM] Improve unwinder stack usage - Make WMMX support optional

2016-07-07 Thread Asiri Rathnayake via cfe-commits

Author: asiri
Date: Thu Jul  7 05:55:39 2016
New Revision: 274744

URL: http://llvm.org/viewvc/llvm-project?rev=274744&view=rev
Log:
[libunwind][ARM] Improve unwinder stack usage - Make WMMX support optional

These registers are only available on a limited set of ARM targets (those
based on XScale). Other targets should not have to pay the cost of these.

This patch shaves off about ~300 bytes of stack usage and ~1KB of code-size.

Differential revision: http://reviews.llvm.org/D21991
Reviewers: bcraig, compnerd

Change-Id: I2d7a1911a193bd70b123e78747e1a7d1482463c7

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/src/Registers.hpp
libunwind/trunk/src/Unwind-EHABI.cpp
libunwind/trunk/src/UnwindRegistersRestore.S
libunwind/trunk/src/UnwindRegistersSave.S

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=274744&r1=274743&r2=274744&view=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Thu Jul  7 05:55:39 2016
@@ -105,6 +105,7 @@ option(LIBUNWIND_ENABLE_PEDANTIC "Compil
 option(LIBUNWIND_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF)
 option(LIBUNWIND_ENABLE_SHARED "Build libunwind as a shared library." ON)
 option(LIBUNWIND_ENABLE_CROSS_UNWINDING "Enable cross-platform unwinding 
support." OFF)
+option(LIBUNWIND_ENABLE_ARM_WMMX "Enable unwinding support for ARM WMMX 
registers." OFF)
 
 set(LIBUNWIND_TARGET_TRIPLE "" CACHE STRING "Target triple for cross 
compiling.")
 set(LIBUNWIND_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.")
@@ -237,6 +238,15 @@ if (NOT LIBUNWIND_ENABLE_CROSS_UNWINDING
   list(APPEND LIBUNWIND_COMPILE_FLAGS -D_LIBUNWIND_IS_NATIVE_ONLY)
 endif ()
 
+# ARM WMMX register support
+if (LIBUNWIND_ENABLE_ARM_WMMX)
+  # __ARM_WMMX is a compiler pre-define (as per the ACLE 2.0). Clang does not
+  # define this macro for any supported target at present. Therefore, here we
+  # provide the option to explicitly enable support for WMMX registers in the
+  # unwinder.
+  list(APPEND LIBUNWIND_COMPILE_FLAGS -D__ARM_WMMX)
+endif ()
+
 # This is the _ONLY_ place where add_definitions is called.
 if (MSVC)
   add_definitions(-D_CRT_SECURE_NO_WARNINGS)

Modified: libunwind/trunk/include/__libunwind_config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/__libunwind_config.h?rev=274744&r1=274743&r2=274744&view=diff
==
--- libunwind/trunk/include/__libunwind_config.h (original)
+++ libunwind/trunk/include/__libunwind_config.h Thu Jul  7 05:55:39 2016
@@ -36,8 +36,13 @@
 #  define _LIBUNWIND_CURSOR_SIZE 78
 # elif defined(__arm__)
 #  define _LIBUNWIND_TARGET_ARM 1
-#  define _LIBUNWIND_CONTEXT_SIZE 60
-#  define _LIBUNWIND_CURSOR_SIZE 67
+#  if defined(__ARM_WMMX)
+#define _LIBUNWIND_CONTEXT_SIZE 60
+#define _LIBUNWIND_CURSOR_SIZE 67
+#  else
+#define _LIBUNWIND_CONTEXT_SIZE 42
+#define _LIBUNWIND_CURSOR_SIZE 49
+#  endif
 # elif defined(__or1k__)
 #  define _LIBUNWIND_TARGET_OR1K 1
 #  define _LIBUNWIND_CONTEXT_SIZE 16

Modified: libunwind/trunk/src/Registers.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Registers.hpp?rev=274744&r1=274743&r2=274744&view=diff
==
--- libunwind/trunk/src/Registers.hpp (original)
+++ libunwind/trunk/src/Registers.hpp Thu Jul  7 05:55:39 2016
@@ -1343,10 +1343,12 @@ public:
 }
 if (_saved_vfp_d16_d31)
   restoreVFPv3(_vfp_d16_d31);
+#if defined(__ARM_WMMX)
 if (_saved_iwmmx)
   restoreiWMMX(_iwmmx);
 if (_saved_iwmmx_control)
   restoreiWMMXControl(_iwmmx_control);
+#endif
   }
 
 private:
@@ -1360,13 +1362,15 @@ private:
   static void saveVFPWithFSTMD(unw_fpreg_t*);
   static void saveVFPWithFSTMX(unw_fpreg_t*);
   static void saveVFPv3(unw_fpreg_t*);
-  static void saveiWMMX(unw_fpreg_t*);
-  static void saveiWMMXControl(uint32_t*);
   static void restoreVFPWithFLDMD(unw_fpreg_t*);
   static void restoreVFPWithFLDMX(unw_fpreg_t*);
   static void restoreVFPv3(unw_fpreg_t*);
+#if defined(__ARM_WMMX)
+  static void saveiWMMX(unw_fpreg_t*);
+  static void saveiWMMXControl(uint32_t*);
   static void restoreiWMMX(unw_fpreg_t*);
   static void restoreiWMMXControl(uint32_t*);
+#endif
   void restoreCoreAndJumpTo();
 
   // ARM registers
@@ -1384,47 +1388,53 @@ private:
   bool _saved_vfp_d0_d15;
   // Whether VFPv3 D16-D31 are saved.
   bool _saved_vfp_d16_d31;
-  // Whether iWMMX data registers are saved.
-  bool _saved_iwmmx;
-  // Whether iWMMX control registers are saved.
-  bool _saved_iwmmx_control;
   // VFP registers D0-D15, + padding if saved using FSTMX
   unw_fpreg_t _vfp_d0_d15_pad[17];
   // VFPv3 registers D16-D31, always saved using FSTMD
   unw_fpr

Re: [PATCH] D21991: [libunwind][ARM] Improve unwinder stack usage - Make WMMX support optional

2016-07-07 Thread Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL274744: [libunwind][ARM] Improve unwinder stack usage - Make 
WMMX support optional (authored by asiri).

Changed prior to commit:
  http://reviews.llvm.org/D21991?vs=62890&id=63049#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D21991

Files:
  libunwind/trunk/CMakeLists.txt
  libunwind/trunk/include/__libunwind_config.h
  libunwind/trunk/src/Registers.hpp
  libunwind/trunk/src/Unwind-EHABI.cpp
  libunwind/trunk/src/UnwindRegistersRestore.S
  libunwind/trunk/src/UnwindRegistersSave.S

Index: libunwind/trunk/src/UnwindRegistersSave.S
===
--- libunwind/trunk/src/UnwindRegistersSave.S
+++ libunwind/trunk/src/UnwindRegistersSave.S
@@ -378,15 +378,16 @@
   vstmia r0, {d16-d31}
   JMP(lr)
 
+#if defined(_LIBUNWIND_ARM_WMMX)
+
 @
 @ static void libunwind::Registers_arm::saveiWMMX(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm9saveiWMMXEPy)
-#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || defined(__ARM_WMMX)
   stcl p1, cr0, [r0], #8  @ wstrd wR0, [r0], #8
   stcl p1, cr1, [r0], #8  @ wstrd wR1, [r0], #8
   stcl p1, cr2, [r0], #8  @ wstrd wR2, [r0], #8
@@ -403,7 +404,6 @@
   stcl p1, cr13, [r0], #8  @ wstrd wR13, [r0], #8
   stcl p1, cr14, [r0], #8  @ wstrd wR14, [r0], #8
   stcl p1, cr15, [r0], #8  @ wstrd wR15, [r0], #8
-#endif
   JMP(lr)
 
 @
@@ -414,14 +414,14 @@
 @
   .p2align 2
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm16saveiWMMXControlEPj)
-#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || defined(__ARM_WMMX)
   stc2 p1, cr8, [r0], #4  @ wstrw wCGR0, [r0], #4
   stc2 p1, cr9, [r0], #4  @ wstrw wCGR1, [r0], #4
   stc2 p1, cr10, [r0], #4  @ wstrw wCGR2, [r0], #4
   stc2 p1, cr11, [r0], #4  @ wstrw wCGR3, [r0], #4
-#endif
   JMP(lr)
 
+#endif
+
 #elif defined(__or1k__)
 
 #
Index: libunwind/trunk/src/UnwindRegistersRestore.S
===
--- libunwind/trunk/src/UnwindRegistersRestore.S
+++ libunwind/trunk/src/UnwindRegistersRestore.S
@@ -383,15 +383,16 @@
   vldmia r0, {d16-d31}
   JMP(lr)
 
+#if defined(__ARM_WMMX)
+
 @
 @ static void libunwind::Registers_arm::restoreiWMMX(unw_fpreg_t* values)
 @
 @ On entry:
 @  values pointer is in r0
 @
   .p2align 2
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm12restoreiWMMXEPy)
-#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || defined(__ARM_WMMX)
   ldcl p1, cr0, [r0], #8  @ wldrd wR0, [r0], #8
   ldcl p1, cr1, [r0], #8  @ wldrd wR1, [r0], #8
   ldcl p1, cr2, [r0], #8  @ wldrd wR2, [r0], #8
@@ -408,7 +409,6 @@
   ldcl p1, cr13, [r0], #8  @ wldrd wR13, [r0], #8
   ldcl p1, cr14, [r0], #8  @ wldrd wR14, [r0], #8
   ldcl p1, cr15, [r0], #8  @ wldrd wR15, [r0], #8
-#endif
   JMP(lr)
 
 @
@@ -419,14 +419,14 @@
 @
   .p2align 2
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_arm19restoreiWMMXControlEPj)
-#if (!defined(__ARM_ARCH_6M__) && !defined(__ARM_ARCH_6SM__)) || defined(__ARM_WMMX)
   ldc2 p1, cr8, [r0], #4  @ wldrw wCGR0, [r0], #4
   ldc2 p1, cr9, [r0], #4  @ wldrw wCGR1, [r0], #4
   ldc2 p1, cr10, [r0], #4  @ wldrw wCGR2, [r0], #4
   ldc2 p1, cr11, [r0], #4  @ wldrw wCGR3, [r0], #4
-#endif
   JMP(lr)
 
+#endif
+
 #elif defined(__or1k__)
 
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind14Registers_or1k6jumptoEv)
Index: libunwind/trunk/src/Registers.hpp
===
--- libunwind/trunk/src/Registers.hpp
+++ libunwind/trunk/src/Registers.hpp
@@ -1343,10 +1343,12 @@
 }
 if (_saved_vfp_d16_d31)
   restoreVFPv3(_vfp_d16_d31);
+#if defined(__ARM_WMMX)
 if (_saved_iwmmx)
   restoreiWMMX(_iwmmx);
 if (_saved_iwmmx_control)
   restoreiWMMXControl(_iwmmx_control);
+#endif
   }
 
 private:
@@ -1360,13 +1362,15 @@
   static void saveVFPWithFSTMD(unw_fpreg_t*);
   static void saveVFPWithFSTMX(unw_fpreg_t*);
   static void saveVFPv3(unw_fpreg_t*);
-  static void saveiWMMX(unw_fpreg_t*);
-  static void saveiWMMXControl(uint32_t*);
   static void restoreVFPWithFLDMD(unw_fpreg_t*);
   static void restoreVFPWithFLDMX(unw_fpreg_t*);
   static void restoreVFPv3(unw_fpreg_t*);
+#if defined(__ARM_WMMX)
+  static void saveiWMMX(unw_fpreg_t*);
+  static void saveiWMMXControl(uint32_t*);
   static void restoreiWMMX(unw_fpreg_t*);
   static void restoreiWMMXControl(uint32_t*);
+#endif
   void restoreCoreAndJumpTo();
 
   // ARM registers
@@ -1384,47 +1388,53 @@
   bool _saved_vfp_d0_d15;
   // Whether VFPv3 D16-D31 are saved.
   bool _saved_vfp_d16_d31;
-  // Whether iWMMX data registers are saved.
-  bool _saved_iwmmx;
-  // Whether iWMMX control registers are saved.
-  bool _saved_iwmmx_control;
   // VFP registers D0-D15, + padding if saved using FSTMX
   unw_fpreg_t _vfp_d0_d15_pad[17];
   // VFPv3 r

r274745 - [OPENMP] Do not create helper expressions in dependent contexts, NFC.

2016-07-07 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Thu Jul  7 06:04:06 2016
New Revision: 274745

URL: http://llvm.org/viewvc/llvm-project?rev=274745&view=rev
Log:
[OPENMP] Do not create helper expressions in dependent contexts, NFC.

OpenMP relies on some helper expressions generated during semantic
analysis. But they are required only for codegen and not required in
dependent contexts. Patch removes generation of some of helper
expressions.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=274745&r1=274744&r2=274745&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Thu Jul  7 06:04:06 2016
@@ -4380,6 +4380,8 @@ bool OpenMPIterationSpaceChecker::CheckI
 static ExprResult
 tryBuildCapture(Sema &SemaRef, Expr *Capture,
 llvm::MapVector &Captures) {
+  if (SemaRef.CurContext->isDependentContext())
+return ExprResult(Capture);
   if (Capture->isEvaluatable(SemaRef.Context, Expr::SE_AllowSideEffects))
 return SemaRef.PerformImplicitConversion(
 Capture->IgnoreImpCasts(), Capture->getType(), Sema::AA_Converting,
@@ -8224,10 +8226,12 @@ OMPClause *Sema::ActOnOpenMPPrivateClaus
 *this, VDPrivate, RefExpr->getType().getUnqualifiedType(), ELoc);
 
 DeclRefExpr *Ref = nullptr;
-if (!VD)
+if (!VD && !CurContext->isDependentContext())
   Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/false);
 DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_private, Ref);
-Vars.push_back(VD ? RefExpr->IgnoreParens() : Ref);
+Vars.push_back((VD || CurContext->isDependentContext())
+   ? RefExpr->IgnoreParens()
+   : Ref);
 PrivateCopies.push_back(VDPrivateRefExpr);
   }
 
@@ -8522,7 +8526,7 @@ OMPClause *Sema::ActOnOpenMPFirstprivate
 *this, VDPrivate, RefExpr->getType().getUnqualifiedType(),
 RefExpr->getExprLoc());
 DeclRefExpr *Ref = nullptr;
-if (!VD) {
+if (!VD && !CurContext->isDependentContext()) {
   if (TopDVar.CKind == OMPC_lastprivate)
 Ref = TopDVar.PrivateCopy;
   else {
@@ -8532,7 +8536,9 @@ OMPClause *Sema::ActOnOpenMPFirstprivate
   }
 }
 DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_firstprivate, Ref);
-Vars.push_back(VD ? RefExpr->IgnoreParens() : Ref);
+Vars.push_back((VD || CurContext->isDependentContext())
+   ? RefExpr->IgnoreParens()
+   : Ref);
 PrivateCopies.push_back(VDPrivateRefExpr);
 Inits.push_back(VDInitRefExpr);
   }
@@ -8661,7 +8667,7 @@ OMPClause *Sema::ActOnOpenMPLastprivateC
   continue;
 
 DeclRefExpr *Ref = nullptr;
-if (!VD) {
+if (!VD && !CurContext->isDependentContext()) {
   if (TopDVar.CKind == OMPC_firstprivate)
 Ref = TopDVar.PrivateCopy;
   else {
@@ -8685,7 +8691,9 @@ OMPClause *Sema::ActOnOpenMPLastprivateC
   }
 }
 DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_lastprivate, Ref);
-Vars.push_back(VD ? RefExpr->IgnoreParens() : Ref);
+Vars.push_back((VD || CurContext->isDependentContext())
+   ? RefExpr->IgnoreParens()
+   : Ref);
 SrcExprs.push_back(PseudoSrcExpr);
 DstExprs.push_back(PseudoDstExpr);
 AssignmentOps.push_back(AssignmentOp.get());
@@ -8737,10 +8745,12 @@ OMPClause *Sema::ActOnOpenMPSharedClause
 }
 
 DeclRefExpr *Ref = nullptr;
-if (!VD && IsOpenMPCapturedDecl(D))
+if (!VD && IsOpenMPCapturedDecl(D) && !CurContext->isDependentContext())
   Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/true);
 DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_shared, Ref);
-Vars.push_back((VD || !Ref) ? RefExpr->IgnoreParens() : Ref);
+Vars.push_back((VD || !Ref || CurContext->isDependentContext())
+   ? RefExpr->IgnoreParens()
+   : Ref);
   }
 
   if (Vars.empty())
@@ -9421,7 +9431,7 @@ OMPClause *Sema::ActOnOpenMPReductionCla
 
 DeclRefExpr *Ref = nullptr;
 Expr *VarsExpr = RefExpr->IgnoreParens();
-if (!VD) {
+if (!VD && !CurContext->isDependentContext()) {
   if (ASE || OASE) {
 TransformExprToCaptures RebuildToCapture(*this, D);
 VarsExpr =
@@ -9579,7 +9589,7 @@ OMPClause *Sema::ActOnOpenMPLinearClause
 VarDecl *Init = buildVarDecl(*this, ELoc, Type, ".linear.start");
 Expr *InitExpr;
 DeclRefExpr *Ref = nullptr;
-if (!VD) {
+if (!VD && !CurContext->isDependentContext()) {
   Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/false);
   if (!IsOpenMPCapturedDecl(D)) {
 ExprCaptures.push_back(Ref->getDecl());
@@ -9606,7 +9616,9 @@ OMPClause *Sema::ActOnOpenMPLinearClause
 auto InitRef = buildDeclRefExpr(*this, Init, Type, ELoc);
 
 DSAStack->addDSA(D, RefExpr->Igno

r274746 - Fix "not all control paths return a value" warning on MSVC

2016-07-07 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Thu Jul  7 06:12:02 2016
New Revision: 274746

URL: http://llvm.org/viewvc/llvm-project?rev=274746&view=rev
Log:
Fix "not all control paths return a value" warning on MSVC

Modified:
cfe/trunk/lib/Basic/Targets.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=274746&r1=274745&r2=274746&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Jul  7 06:12:02 2016
@@ -1804,6 +1804,8 @@ public:
   return "610";
 case CudaArch::SM_62:
   return "620";
+ default:
+   llvm_unreachable("unhandled CudaArch");
 }
   }();
   Builder.defineMacro("__CUDA_ARCH__", CUDAArchCode);


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

r274748 - Fix "not all control paths return a value" warning on MSVC

2016-07-07 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Thu Jul  7 06:24:38 2016
New Revision: 274748

URL: http://llvm.org/viewvc/llvm-project?rev=274748&view=rev
Log:
Fix "not all control paths return a value" warning on MSVC

This time without causing a 'all enums handled' warning on other compilers.

Modified:
cfe/trunk/lib/Basic/Targets.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=274748&r1=274747&r2=274748&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Jul  7 06:24:38 2016
@@ -1777,7 +1777,7 @@ public:
   // Set __CUDA_ARCH__ for the GPU specified.
   std::string CUDAArchCode = [this] {
 switch (GPU) {
-case CudaArch::UNKNOWN:
+default:
   assert(false && "No GPU arch when compiling CUDA device code.");
   return "";
 case CudaArch::SM_20:
@@ -1804,8 +1804,6 @@ public:
   return "610";
 case CudaArch::SM_62:
   return "620";
- default:
-   llvm_unreachable("unhandled CudaArch");
 }
   }();
   Builder.defineMacro("__CUDA_ARCH__", CUDAArchCode);


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

Re: [PATCH] D22034: [MSVC][DLL] use local vftable mangling only exported classes with virtual destructor

2016-07-07 Thread Dmitry Polukhin via cfe-commits

DmitryPolukhin added a comment.

In http://reviews.llvm.org/D22034#475603, @majnemer wrote:

> Thinking about this some more, it is possible for clang to emit code that 
> will make everybody happy:
>
> If a class is being constructed in a constexpr context and all the vftable 
> entries it references are marked import, emit local vftables and reference 
> them in the object.  If a vftable entry it references is not marked import, 
> report an error.
>
> If a class is constructed via operator new and all the vftable entries it 
> references are marked import, emit local vftables and store it in the object 
> after the constructors run.  If a vftable entry it references is not marked 
> import, report an error.
>
> If a class is constructed via a local or global variable and all the vftable 
> entries it references are marked import, create a `dllimport 
> available_externally` vftable.  Otherwise create a normal `dllimport 
> external` vftable.
>
> I believe this behavior captures the best behavior across the spectrum of 
> functionality we all care about: it supports devirtualization, constexpr and 
> importing classes which don't have all their vftable methods exported.

I'll try to implement this approach except for special handling for 
constriction via new that seems to be out of scope for this issue and can be 
implemented independent later. Small addition, I think there is no sense to 
check if vftable references to entry that is not marked as dllimport. Linker 
will do this work for us and in some cases function may come from libraries 
(static or dynamic) but in the source may not be properly marked as dllimport 
(it is the behavior that MSVC does check it in compiler and relays on linker).

http://reviews.llvm.org/D22034

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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Benjamin Kramer via cfe-commits

bkramer added inline comments.


Comment at: clang-rename/tool/ClangRename.cpp:39
@@ -38,3 +38,2 @@
 #include "llvm/Support/Host.h"
-#include 
 #include 

This looks unrelated.


Comment at: clang-rename/tool/clang-rename.py:14
@@ +13,3 @@
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the

The comment should say that you have to save the file before running the tool.


Comment at: clang-rename/tool/clang-rename.py:40
@@ +39,3 @@
+command = [binary,
+   filename,
+   '-i',

Maybe add a FIXME to input the unsaved file via stdin like we do for 
clang-format?


Comment at: clang-rename/tool/clang-rename.py:47
@@ +46,3 @@
+ stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+stdout, stderr = p.communicate()

stdin is pipe but you never write to it? Is that intentional?


Comment at: clang-rename/tool/clang-rename.py:48
@@ +47,3 @@
+ stdin=subprocess.PIPE)
+stdout, stderr = p.communicate()
+

Should we output stderr in case the tool fails?


http://reviews.llvm.org/D22087



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

Re: [PATCH] D21814: clang-rename: support multiple renames with one invocation

2016-07-07 Thread Benjamin Kramer via cfe-commits

bkramer added a comment.

In http://reviews.llvm.org/D21814#475322, @klimek wrote:

> I think we really want 2 tools:
>  a) one that is optimized for oldname->newname renames, and supports the 
> multi-TU case really well
>  b) one that is meant to be integrated with editors and works mainly off of a 
> location in a file
>
> I'm a bit torn whether putting those 2 into the same executable is a good 
> idea. Looping in Benjamin for additional ideas.

I'm fine with both things living in the same binary. The location thing is just 
a different way of specifying the symbol name. I fail to see how that's related 
to this review though.

http://reviews.llvm.org/D21814

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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 updated this revision to Diff 63054.
omtcyf0 marked 5 inline comments as done.

http://reviews.llvm.org/D22087

Files:
  clang-rename/tool/clang-rename.py

Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,60 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cf :pyf /clang-include-fixer.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print 'Couldn\'t determine cursor position. Is your file empty?'
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if sterr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()


Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,60 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cf :pyf /clang-include-fixer.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print 'Couldn\'t determine cursor position. Is your file empty?'
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if sterr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Benjamin Kramer via cfe-commits

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

looks good. Still no commit access? ;)


http://reviews.llvm.org/D22087



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

[PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 created this revision.
omtcyf0 added reviewers: alexfh, klimek.
omtcyf0 added a subscriber: cfe-commits.

This patch introduces basic Vim integration for clang-rename tool.

For setup reference see clang-rename/tool/clang-rename.py

http://reviews.llvm.org/D22087

Files:
  clang-rename/tool/ClangRename.cpp
  clang-rename/tool/clang-rename.py

Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,55 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cf :pyf /clang-include-fixer.py
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print 'Couldn\'t determine cursor position. Is your file empty?'
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()
Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -36,7 +36,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/Host.h"
-#include 
 #include 
 
 using namespace llvm;


Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,55 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cf :pyf /clang-include-fixer.py
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print 'Couldn\'t determine cursor position. Is your file empty?'
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ stdin=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()
Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -36,7 +36,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/Host.h"
-#include 
 #include 
 
 using namespace llvm;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Eric Liu via cfe-commits

ioeric added inline comments.


Comment at: clang-rename/tool/clang-rename.py:12
@@ +11,3 @@
+
+map ,cf :pyf /clang-include-fixer.py
+

Maybe a different key binding so that it doesn't conflict with include-fixer's 
suggested key binding? `,cr` for clang-rename maybe? 


Comment at: clang-rename/tool/clang-rename.py:33
@@ +32,3 @@
+if offset < 0:
+print 'Couldn\'t determine cursor position. Is your file empty?'
+return

Redirect error messages to stderr so that they are highlighted in vim?


http://reviews.llvm.org/D22087



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 added a comment.

Yup. FeelsBadMan.

One note about that header removal: how do I do it then? I thought attaching 
such changes to a random patch is not a problem. Otherwise there will be some 
one-line patches for such things, won't they?



Comment at: clang-rename/tool/ClangRename.cpp:39
@@ -38,3 +38,2 @@
 #include "llvm/Support/Host.h"
-#include 
 #include 

bkramer wrote:
> This looks unrelated.
Right, but I'm not sure one-line patches are welcome.

Is it not alright to put such into random patches? Otherwise creating 
one-liner...


http://reviews.llvm.org/D22087



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

Re: [PATCH] D21834: Implementing 'If statement with Initializer'

2016-07-07 Thread Anton Bikineev via cfe-commits

AntonBikineev updated this revision to Diff 63056.
AntonBikineev added a comment.

@rsmith,
I've added some tests for c++1z init statement. Please let me know if there is 
anything else I should add or change.


http://reviews.llvm.org/D21834

Files:
  include/clang/AST/Stmt.h
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/Stmt.cpp
  lib/Analysis/BodyFarm.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CodeGenCXX/cxx1z-init-statement.cpp
  test/PCH/cxx1z-init-statement.cpp
  test/PCH/cxx1z-init-statement.h
  test/Parser/cxx1z-init-statement.cpp
  test/SemaCXX/cxx1z-init-statement-warn-unused.cpp
  test/SemaCXX/cxx1z-init-statement.cpp

Index: test/SemaCXX/cxx1z-init-statement.cpp
===
--- test/SemaCXX/cxx1z-init-statement.cpp
+++ test/SemaCXX/cxx1z-init-statement.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 -std=c++1z -verify %s
+
+void testIf() {
+  int x = 0;
+  if (x; x) ++x;
+  if (int t = 0; t) ++t; else --t;
+
+  if (int x, y = 0; y) // expected-note 2 {{previous definition is here}}
+int x = 0; // expected-error {{redefinition of 'x'}}
+  else
+int x = 0; // expected-error {{redefinition of 'x'}}
+
+  if (x; int a = 0) ++a;
+  if (x, +x; int a = 0) // expected-note 2 {{previous definition is here}} expected-warning {{unused}}
+int a = 0; // expected-error {{redefinition of 'a'}}
+  else
+int a = 0; // expected-error {{redefinition of 'a'}}
+
+  if (int b = 0; b)
+;
+  b = 2; // expected-error {{use of undeclared identifier}}
+}
+
+void testSwitch() {
+  int x = 0;
+  switch (x; x) {
+case 1:
+  ++x;
+  }
+
+  switch (int x, y = 0; y) {
+case 1:
+  ++x;
+default:
+  ++y;
+  }
+
+  switch (int x, y = 0; y) { // expected-note 2 {{previous definition is here}}
+case 0:
+  int x = 0; // expected-error {{redefinition of 'x'}}
+case 1:
+  int y = 0; // expected-error {{redefinition of 'y'}}
+  };
+
+  switch (x; int a = 0) {
+case 0:
+  ++a;
+  }
+
+  switch (x, +x; int a = 0) { // expected-note {{previous definition is here}} expected-warning {{unused}}
+case 0:
+  int a = 0; // expected-error {{redefinition of 'a'}} // expected-note {{previous definition is here}}
+case 1:
+  int a = 0; // expected-error {{redefinition of 'a'}}
+  }
+
+  switch (int b = 0; b) {
+case 0:
+  break;
+  }
+  b = 2; // expected-error {{use of undeclared identifier}}
+}
+
+constexpr bool constexpr_if_init(int n) {
+  if (int a = n; ++a > 0)
+return true;
+  else
+return false;
+}
+
+constexpr int constexpr_switch_init(int n) {
+  switch (int p = n + 2; p) {
+case 0:
+  return 0;
+case 1:
+  return 1;
+default:
+  return -1;
+  }
+}
+
+void test_constexpr_init_stmt() {
+  constexpr bool a = constexpr_if_init(-2);
+  static_assert(!a, "");
+  static_assert(constexpr_if_init(1), "");
+
+  constexpr int b = constexpr_switch_init(-1);
+  static_assert(b == 1, "");
+  static_assert(constexpr_switch_init(-2) == 0, "");
+  static_assert(constexpr_switch_init(-5) == -1, "");
+}
Index: test/SemaCXX/cxx1z-init-statement-warn-unused.cpp
===
--- test/SemaCXX/cxx1z-init-statement-warn-unused.cpp
+++ test/SemaCXX/cxx1z-init-statement-warn-unused.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++1z -verify -Wuninitialized %s
+
+void testIf() {
+  if (bool b; b) // expected-warning {{uninitialized}} expected-note {{to silence}}
+;
+  if (int a, b = 2; a) // expected-warning {{uninitialized}} expected-note {{to silence}}
+;
+}
+
+void testSwitch() {
+  switch (bool b; b) { // expected-warning {{uninitialized}} expected-warning {{boolean value}} expected-note {{to silence}}
+case 0:
+  break;
+  }
+  switch (int a, b = 7; a) { // expected-warning {{uninitialized}} expected-note {{to silence}}
+case 0:
+  break;
+  }
+}
Index: test/Parser/cxx1z-init-statement.cpp
===
--- test/Parser/cxx1z-init-statement.cpp
+++ test/Parser/cxx1z-init-statement.cpp
@@ -4,17 +4,17 @@
 typedef int T;
 int f() {
   // init-statement declarations
-  if (T n = 0; n != 0) {} // expected-error {{not yet supported}}
-  if (T f(); f()) {} // expected-error {{not yet supported}}
-  if (T(f()); f()) {} // expected-error {{not yet supported}}
-  if (T(f()), g, h; f()) {} // expected-error {{not yet supported}}
-  if (T f(); f()) {} // expected-error {{not yet supported}}
-  if (T f(), g, h; f()) {} // expected-error {{not yet supported}}
+  if (T n = 0; n != 0) {}
+  if (T f(); f()) {}
+  if (T(f()); f()) {}
+  if (T(f()), g, h; f()) {}
+  if (T f(); f()) {}
+  if (T f(), g, h; f()) {}
 
   // init-statement expressions
-  if (T{f()}; f())

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Benjamin Kramer via cfe-commits

bkramer added inline comments.


Comment at: clang-rename/tool/ClangRename.cpp:39
@@ -38,3 +38,2 @@
 #include "llvm/Support/Host.h"
-#include 
 #include 

omtcyf0 wrote:
> bkramer wrote:
> > This looks unrelated.
> Right, but I'm not sure one-line patches are welcome.
> 
> Is it not alright to put such into random patches? Otherwise creating 
> one-liner...
I'd do it on the next change to that file, but I can also include it in the 
patch this time. I generally dislike having completely unrelated changes in a 
patch and the unnecessary include here doesn't really hurt anyone.


http://reviews.llvm.org/D22087



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

Re: [PATCH] D21983: Add C++ dependencies to xray runtime

2016-07-07 Thread Aaron Ballman via cfe-commits

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

LGTM (once other deps land).


http://reviews.llvm.org/D21983



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-rename/tool/clang-rename.py:12
@@ +11,3 @@
+
+map ,cf :pyf /clang-include-fixer.py
+

ioeric wrote:
> Maybe a different key binding so that it doesn't conflict with 
> include-fixer's suggested key binding? `,cr` for clang-rename maybe? 
Also, "clang-include-fixer.py" should be updated.


http://reviews.llvm.org/D22087



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-rename/tool/ClangRename.cpp:39
@@ -38,3 +38,2 @@
 #include "llvm/Support/Host.h"
-#include 
 #include 

bkramer wrote:
> omtcyf0 wrote:
> > bkramer wrote:
> > > This looks unrelated.
> > Right, but I'm not sure one-line patches are welcome.
> > 
> > Is it not alright to put such into random patches? Otherwise creating 
> > one-liner...
> I'd do it on the next change to that file, but I can also include it in the 
> patch this time. I generally dislike having completely unrelated changes in a 
> patch and the unnecessary include here doesn't really hurt anyone.
 > I'm not sure one-line patches are welcome.

Cleanup-only patches are fine.


http://reviews.llvm.org/D22087



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh accepted this revision.
alexfh added a comment.

LG


http://reviews.llvm.org/D22087



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh added inline comments.


Comment at: clang-rename/tool/clang-rename.py:17
@@ +16,3 @@
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.

s/,cf/,cr/


http://reviews.llvm.org/D22087



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

Re: [PATCH] D22069: clang-tidy modernize-loop-convert: preserve type of alias declaration (bug 28341)

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
This revision now requires changes to proceed.


Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:525
@@ +524,3 @@
+  DeclarationType = DeclarationType.getNonReferenceType();
+if (Descriptor.ElemType.isNull() || DeclarationType.isNull() ||
+!Context->hasSameUnqualifiedType(DeclarationType, Descriptor.ElemType))

mgehre wrote:
> alexfh wrote:
> > 1. Can the `AliasVar->getType().isNull()` condition be true?
> > 2. If it can, consider `!Descriptor.ElemType.isNull().isNull()` and 
> > `AliasVar->getType().isNull()`. In this case setting `Descriptor.ElemType` 
> > to `AliasVar->getType()` (which is null) doesn't make much sense.
> > 
> > I'd probably just wrap the added code in `if 
> > (!AliasVar->getType().isNull())`.
> Thanks for you fast review.
> 
> I copied the block from isAliasDecl(). I don't see any reason why the types 
> can be Null, but I'm also not an expert in llvm.
> 
> When would a VarDecl have no type? Maybe I should put an assert instead?
You can try to add an assert and run the check on llvm, for example.

Alternatively, wrap the added code in `if (!AliasVar->getType().isNull())`


http://reviews.llvm.org/D22069



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

Re: [PATCH] D21992: [clang-tidy] new cppcoreguidelines-slicing

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh added a comment.

Thanks for the new awesome check!

Please run the check on LLVM and include your analysis of the results in the 
patch description. Another couple of comments below.



Comment at: clang-tidy/cppcoreguidelines/SlicingCheck.cpp:93
@@ +92,3 @@
+  continue;
+CXXRecordDecl *BaseRecord =
+
cast_or_null(BaseRecordType->getDecl()->getDefinition());

Please use `auto *`


Comment at: docs/clang-tidy/checks/cppcoreguidelines-slicing.rst:7
@@ +6,3 @@
+Flags slicing of member variables or vtable. See the relevant CppCoreGuidelines
+sections for details:
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es63-dont-slice

Links to the C++ Core Guidelines are good, but a few words of explanation and 
an example here wouldn't hurt either.


http://reviews.llvm.org/D21992



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

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

http://reviews.llvm.org/D22087



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 updated this revision to Diff 63058.

http://reviews.llvm.org/D22087

Files:
  clang-rename/tool/clang-rename.py

Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cr'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()


Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cr'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 updated this revision to Diff 63057.
omtcyf0 marked 2 inline comments as done.

http://reviews.llvm.org/D22087

Files:
  clang-rename/tool/clang-rename.py

Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()


Index: clang-rename/tool/clang-rename.py
===
--- /dev/null
+++ clang-rename/tool/clang-rename.py
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cf'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 marked an inline comment as done.


Comment at: clang-rename/tool/clang-rename.py:13
@@ +12,3 @@
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.

Aw, sure; sorry for that. That's an artifact from first version of the script, 
so this line was simply copy-pasted...


http://reviews.llvm.org/D22087



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

Re: [PATCH] D21814: clang-rename: support multiple renames with one invocation

2016-07-07 Thread Manuel Klimek via cfe-commits

klimek added a comment.

In http://reviews.llvm.org/D21814#476572, @bkramer wrote:

> In http://reviews.llvm.org/D21814#475322, @klimek wrote:
>
> > I think we really want 2 tools:
> >  a) one that is optimized for oldname->newname renames, and supports the 
> > multi-TU case really well
> >  b) one that is meant to be integrated with editors and works mainly off of 
> > a location in a file
> >
> > I'm a bit torn whether putting those 2 into the same executable is a good 
> > idea. Looping in Benjamin for additional ideas.
>
>
> I'm fine with both things living in the same binary. The location thing is 
> just a different way of specifying the symbol name. I fail to see how that's 
> related to this review though.


Well, it's about how much features we want to pack into the same binary; once 
we basically have multiple tools in one tool, adding more different use cases 
makes sense, I think.


http://reviews.llvm.org/D21814



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

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
This revision now requires changes to proceed.

Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:147
@@ +146,3 @@
+const Type *MpiTypeMismatchCheck::argumentType(const CallExpr *const CE,
+   const size_t idx) {
+  const QualType QT = CE->getArg(idx)->IgnoreImpCasts()->getType();

What about this comment?

Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:152
@@ +151,3 @@
+  } else if (QT.getTypePtr()->isPointerType()) {
+return QT.getTypePtr()->getPointeeType()->getBaseElementTypeUnsafe();
+  }

What about this comment? I find it convenient to mark comments "Done" when I 
address them, so it's easier to track the action items.

Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:259
@@ +258,3 @@
+  auto Loc = ArgumentExpression->getSourceRange().getBegin();
+  diag(Loc, (llvm::Twine("Buffer type '") + BufferTypeName +
+ "' does not match the MPI datatype '" + MPIDatatype + "'.")

Diagnostics are not complete sentences, so they should not start with a capital 
letter and should not end with a period.

Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:259
@@ +258,3 @@
+  auto Loc = ArgumentExpression->getSourceRange().getBegin();
+  diag(Loc, (llvm::Twine("Buffer type '") + BufferTypeName +
+ "' does not match the MPI datatype '" + MPIDatatype + "'.")

alexfh wrote:
> Diagnostics are not complete sentences, so they should not start with a 
> capital letter and should not end with a period.
Please use formatting placeholders instead of concatenation: `diag("buffer type 
'%0' does not match ...") << BufferTypeName << ;`. 

Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:271
@@ +270,3 @@
+bool Error = false;
+std::string BufferTypeName;
+

It looks like you're trying to avoid unnecessary allocations by passing this 
buffer into the calls below. It would probably be even more efficient, if you 
moved this variable out of the loop.

Comment at: clang-tidy/misc/MpiTypeMismatchCheck.h:128
@@ +127,3 @@
+
+  static llvm::SmallVector MPITypes;
+  static std::multimap BuiltinMatches;

These static members and corresponding methods don't have to be in the class. 
My comment might have been ambiguous, but I was actually suggesting to change 
the methods that use these maps to free-standing static functions in the .cpp 
file and make these maps static local variables in the corresponding functions.

http://reviews.llvm.org/D21962

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

[PATCH] D22090: [analyzer] Add more FileIDs to PlistDiagnostic map

2016-07-07 Thread Aleksei Sidorin via cfe-commits

a.sidorin created this revision.
a.sidorin added reviewers: zaks.anna, dcoughlin.
a.sidorin added a subscriber: cfe-commits.

Some FileIDs that may be used by PlistDiagnostics are not added while building 
a list of pieces. This leads to assertion violation in `GetFID()` function. 
This patch tries to add such missing FileIDs. It also contains small 
refactoring of this piece of code.

Authors: Aleksei Sidorin, Ilya Palachev.

http://reviews.llvm.org/D22090

Files:
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  test/Analysis/diagnostics/Inputs/include/SomeMacro.def
  test/Analysis/diagnostics/Inputs/include/Something.h
  test/Analysis/diagnostics/plist-diagnostics-include-check.cpp

Index: test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
===
--- /dev/null
+++ test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.ExprInspection -analyzer-output=plist-multi-file -o plist.xml %s
+
+#include "Inputs/include/Something.h"
+
+void foo() {
+  Something()
+#define SOME_MACRO .run();
+#include "Inputs/include/SomeMacro.def"
+}
Index: test/Analysis/diagnostics/Inputs/include/Something.h
===
--- /dev/null
+++ test/Analysis/diagnostics/Inputs/include/Something.h
@@ -0,0 +1,9 @@
+void clang_analyzer_warnIfReached();
+
+class Something {
+public:
+  Something () { }
+  void run() {
+clang_analyzer_warnIfReached();
+  }
+};
Index: test/Analysis/diagnostics/Inputs/include/SomeMacro.def
===
--- /dev/null
+++ test/Analysis/diagnostics/Inputs/include/SomeMacro.def
@@ -0,0 +1 @@
+SOME_MACRO
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -372,40 +372,41 @@
   if (!Diags.empty())
 SM = &(*(*Diags.begin())->path.begin())->getLocation().getManager();
 
-  
-  for (std::vector::iterator DI = Diags.begin(),
-   DE = Diags.end(); DI != DE; ++DI) {
+  auto AddPieceFID = [&FM, &Fids, SM](const PathDiagnosticPiece *Piece)->void {
+AddFID(FM, Fids, SM, Piece->getLocation().asLocation());
+ArrayRef Ranges = Piece->getRanges();
+for (const SourceRange &Range : Ranges) {
+  AddFID(FM, Fids, SM, Range.getBegin());
+  AddFID(FM, Fids, SM, Range.getEnd());
+}
+  };
 
-const PathDiagnostic *D = *DI;
+  for (const PathDiagnostic *D : Diags) {
 
 SmallVector WorkList;
 WorkList.push_back(&D->path);
 
 while (!WorkList.empty()) {
-  const PathPieces &path = *WorkList.pop_back_val();
-
-  for (PathPieces::const_iterator I = path.begin(), E = path.end(); I != E;
-   ++I) {
-const PathDiagnosticPiece *piece = I->getPtr();
-AddFID(FM, Fids, SM, piece->getLocation().asLocation());
-ArrayRef Ranges = piece->getRanges();
-for (ArrayRef::iterator I = Ranges.begin(),
- E = Ranges.end(); I != E; ++I) {
-  AddFID(FM, Fids, SM, I->getBegin());
-  AddFID(FM, Fids, SM, I->getEnd());
-}
+  const PathPieces &Path = *WorkList.pop_back_val();
+
+  for (const auto &Iter : Path) {
+const PathDiagnosticPiece *Piece = Iter.getPtr();
+AddPieceFID(Piece);
+
+if (const PathDiagnosticCallPiece *Call =
+dyn_cast(Piece)) {
+  if (IntrusiveRefCntPtr
+CallEnterWithin = Call->getCallEnterWithinCallerEvent())
+AddPieceFID(CallEnterWithin.getPtr());
 
-if (const PathDiagnosticCallPiece *call =
-dyn_cast(piece)) {
-  IntrusiveRefCntPtr
-callEnterWithin = call->getCallEnterWithinCallerEvent();
-  if (callEnterWithin)
-AddFID(FM, Fids, SM, callEnterWithin->getLocation().asLocation());
+ if (IntrusiveRefCntPtr
+CallEnterEvent = Call->getCallEnterEvent())
+   AddPieceFID(CallEnterEvent.getPtr());
 
-  WorkList.push_back(&call->path);
+  WorkList.push_back(&Call->path);
 }
 else if (const PathDiagnosticMacroPiece *macro =
- dyn_cast(piece)) {
+ dyn_cast(Piece)) {
   WorkList.push_back(¯o->subPieces);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh added a comment.

Sorry for the delay. Your patch was lost in my inbox. Feel free to ping me 
earlier.



Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but 
having int as return type.
+If check finds case like this then it function return type to bool.

Prazek wrote:
> alexfh wrote:
> > It may well not be a bug. It's definitely not a bug for `extern "C"` 
> > functions and functions in C code. We definitely don't need to change the 
> > function return type, since it's a rather involved change (and clang-tidy 
> > checks currently are simply not able to make such change correctly in case 
> > of a function with external linkage). So please remove this automated fix.
> This check runs only on C++ functions. Maybe checking that the function 
> isExternC() would be enough?
As I've said, extern "C" is not the only way to make a function a part of an 
API exposed to external users.


Comment at: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp:192
@@ +191,3 @@
+  // CHECK-FIXES: bool yat2() {
+
+  auto l = []() {

Prazek wrote:
> Why it is dangerous? What I see after runnign on llvm code base, that it is 
> one of the most frequent case.
> The only bug is the extern "C" thing.
Well, `extern "C"` is not the only way to expose functions to some external 
code. This fix potentially changes ABI, which is generally not a safe thing to 
do. I think, we should make the fix optional.


http://reviews.llvm.org/D18821



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

Re: [PATCH] D22046: [clang-tidy] Add dependency on clang-headers

2016-07-07 Thread Nico Weber via cfe-commits

thakis added a comment.

r274751, thanks!


http://reviews.llvm.org/D22046



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

[clang-tools-extra] r274751 - [clang-tidy] Add dependency on clang-headers

2016-07-07 Thread Nico Weber via cfe-commits

Author: nico
Date: Thu Jul  7 08:19:45 2016
New Revision: 274751

URL: http://llvm.org/viewvc/llvm-project?rev=274751&view=rev
Log:
[clang-tidy] Add dependency on clang-headers

Currently, to be able to process a source file including e.g. stddef.h with
clang-tidy, one has to build both clang-tidy and the clang-headers target.
Since stddef.h is needed for virtually any source file, let clang-tidy depend
on clang-headers, so that it Just Works after it has been built.

http://reviews.llvm.org/D22046

Modified:
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt?rev=274751&r1=274750&r2=274751&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt Thu Jul  7 08:19:45 
2016
@@ -5,6 +5,9 @@ set(LLVM_LINK_COMPONENTS
 add_clang_executable(clang-tidy
   ClangTidyMain.cpp
   )
+add_dependencies(clang-tidy
+  clang-headers
+  )
 target_link_libraries(clang-tidy
   clangAST
   clangASTMatchers


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

Re: [PATCH] D20857: [clang-tidy] Add modernize-explicit-operator-bool check.

2016-07-07 Thread Alexander Kornienko via cfe-commits

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Sorry for the delay. Feel free to ping me earlier.



Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:38
@@ +37,3 @@
+  Finder->addMatcher(
+  cxxConversionDecl(returns(booleanType()), unless(isExplicit()))
+  .bind("operator-bool"),

Please merge these two matchers to avoid repeated work. Something along the 
lines of:

  cxxConversionDecl(unless(isExplicit()),
anyOf(cxxConversionDecl(returns(booleanType())).bind("operator-bool"),
  cxxConversionDecl(returns(pointerType(pointee(isConstQualified(), 
voidType(.bind("operator-void-pointer";


Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:55
@@ +54,3 @@
+diag(MatchedDecl->getLocation(),
+ "operator bool declaration is not explicit")
+<< FixItHint::CreateInsertion(MatchedDecl->getLocation(), "explicit ");

The message should say a bit more on why it is bad, e.g.: "implicit operator 
bool allows erroneous comparisons; consider marking operator bool 'explicit'". 
Or in a more prescriptive way: "operator bool should be marked 'explicit' to 
avoid erroneous comparisons". Feel free to suggest a better option.


Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:64
@@ +63,3 @@
+
+// FIXME: This tries to change the type and add explicit, but
+// MatchedDecl->getTypeSpecStartLoc() gets the start of void, not the start

Looks like you'll need to re-lex the range of the declaration and find the 
location of tokens `const`, `void`, `*` between `operator` and `(`. See 
clang-tidy/modernize/UseOverrideCheck.cpp for an example.


http://reviews.llvm.org/D20857



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

[PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 created this revision.
omtcyf0 added reviewers: alexfh, klimek, bkramer, ioeric.
omtcyf0 added a subscriber: cfe-commits.

This patch does the following:

* enforces proper formatting for few files (i.e. deals with 80 linewidth 
violations and few other things)
* ensures '\n' chars are passed to the output streams instead of "\n" strings
* fixes a bug caused by calling cl::PrintHelpMessage(), which occasionally 
calls exit(0), so that exit(1) (which is right after cl::PrintHelpMessage line) 
becomes dead code


http://reviews.llvm.org/D22091

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/USRLocFinder.cpp
  clang-rename/USRLocFinder.h
  clang-rename/tool/ClangRename.cpp

Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -36,7 +36,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/Host.h"
-#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +82,7 @@
 #define CLANG_RENAME_VERSION "0.0.1"
 
 static void PrintVersion() {
-  outs() << "clang-rename version " << CLANG_RENAME_VERSION << "\n";
+  outs() << "clang-rename version " << CLANG_RENAME_VERSION << '\n';
 }
 
 using namespace clang;
@@ -101,7 +100,6 @@
 
   if (NewName.empty()) {
 errs() << "clang-rename: no new name provided.\n\n";
-cl::PrintHelpMessage();
 exit(1);
   }
 
@@ -115,12 +113,14 @@
   const auto &USRs = USRAction.getUSRs();
   const auto &PrevName = USRAction.getUSRSpelling();
 
-  if (PrevName.empty())
+  if (PrevName.empty()) {
 // An error should have already been printed.
 exit(1);
+  }
 
-  if (PrintName)
-errs() << "clang-rename: found name: " << PrevName << "\n";
+  if (PrintName) {
+errs() << "clang-rename: found name: " << PrevName << '\n';
+  }
 
   // Perform the renaming.
   rename::RenamingAction RenameAction(NewName, PrevName, USRs,
Index: clang-rename/USRLocFinder.h
===
--- clang-rename/USRLocFinder.h
+++ clang-rename/USRLocFinder.h
@@ -35,4 +35,4 @@
 }
 }
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_LOC_FINDER_H
+#endif  // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_LOC_FINDER_H
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -33,9 +33,9 @@
 // translation unit and stores them for later usage.
 class USRLocFindingASTVisitor
 : public clang::RecursiveASTVisitor {
-public:
-  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName) : USR(USR), PrevName(PrevName) {
-  }
+ public:
+  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName)
+  : USR(USR), PrevName(PrevName) {}
 
   // Declaration visitors:
 
@@ -60,8 +60,10 @@
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 const ASTContext &Context = ConstructorDecl->getASTContext();
-for (clang::CXXConstructorDecl::init_const_iterator it = ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
-  const clang::CXXCtorInitializer* Initializer = *it;
+for (clang::CXXConstructorDecl::init_const_iterator it =
+ ConstructorDecl->init_begin();
+ it != ConstructorDecl->init_end(); ++it) {
+  const clang::CXXCtorInitializer *Initializer = *it;
   if (Initializer->getSourceOrder() == -1) {
 // Ignore implicit initializers.
 continue;
@@ -71,9 +73,12 @@
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.
   SourceLocation Location = Initializer->getSourceLocation();
-  StringRef TokenName = Lexer::getSourceText(CharSourceRange::getTokenRange(Location), Context.getSourceManager(), Context.getLangOpts());
+  StringRef TokenName = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Location),
+  Context.getSourceManager(), Context.getLangOpts());
   if (TokenName == PrevName) {
-// The token of the source location we find actually has the old name.
+// The token of the source location we find actually has the old
+// name.
 LocationsFound.push_back(Initializer->getSourceLocation());
   }
 }
@@ -169,7 +174,7 @@
 return LocationsFound;
   }
 
-private:
+ private:
   // Namespace traversal:
   void checkNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
 while (NameLoc) {
@@ -183,14 +188,15 @@
   bool handleCXXNamedCastExpr(clang::CXXNamedCastExpr *Expr) {
 clang::QualType Type = Expr->getType();
 // See if this a cast of a pointer.
-const RecordDecl* Decl = Type->getPointeeCXXRecordDecl();
+const RecordDecl *Decl = Type->getPointeeCXXRecordDecl();
 if (!Decl) {
   // See if this is a cast of a referen

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 added a comment.

@bkramer can you please land the patch?


http://reviews.llvm.org/D22087



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

[clang-tools-extra] r274759 - [clang-rename] add basic vim integration

2016-07-07 Thread Benjamin Kramer via cfe-commits

Author: d0k
Date: Thu Jul  7 09:35:32 2016
New Revision: 274759

URL: http://llvm.org/viewvc/llvm-project?rev=274759&view=rev
Log:
[clang-rename] add basic vim integration

This patch introduces basic Vim integration for clang-rename tool.

For setup reference see clang-rename/tool/clang-rename.py

Patch by Kirill Bobyrev!

Differential revision: http://reviews.llvm.org/D22087

Added:
clang-tools-extra/trunk/clang-rename/tool/clang-rename.py

Added: clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/tool/clang-rename.py?rev=274759&view=auto
==
--- clang-tools-extra/trunk/clang-rename/tool/clang-rename.py (added)
+++ clang-tools-extra/trunk/clang-rename/tool/clang-rename.py Thu Jul  7 
09:35:32 2016
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cr'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()


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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL274759: [clang-rename] add basic vim integration (authored 
by d0k).

Changed prior to commit:
  http://reviews.llvm.org/D22087?vs=63058&id=63072#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D22087

Files:
  clang-tools-extra/trunk/clang-rename/tool/clang-rename.py

Index: clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
===
--- clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
+++ clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cr'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()


Index: clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
===
--- clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
+++ clang-tools-extra/trunk/clang-rename/tool/clang-rename.py
@@ -0,0 +1,61 @@
+'''
+Minimal clang-rename integration with Vim.
+
+Before installing make sure one of the following is satisfied:
+
+* clang-rename is in your PATH
+* `g:clang_rename_path` in ~/.vimrc points to valid clang-rename executable
+* `binary` in clang-rename.py points to valid to clang-rename executable
+
+To install, simply put this into your ~/.vimrc
+
+map ,cr :pyf /clang-rename.py
+
+IMPORTANT NOTE: Before running the tool, make sure you saved the file.
+
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cr'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''
+
+import vim
+import subprocess
+import sys
+
+def main():
+binary = 'clang-rename'
+if vim.eval('exists("g:clang_rename_path")') == "1":
+binary = vim.eval('g:clang_rename')
+
+# Get arguments for clang-rename binary.
+offset = int(vim.eval('line2byte(line("."))+col(".")')) - 2
+if offset < 0:
+print >> sys.stderr, '''Couldn\'t determine cursor position.
+Is your file empty?'''
+return
+filename = vim.current.buffer.name
+
+new_name_request_message = 'type new name:'
+new_name = vim.eval("input('{}\n')".format(new_name_request_message))
+
+# Call clang-rename.
+command = [binary,
+   filename,
+   '-i',
+   '-offset', str(offset),
+   '-new-name', str(new_name)]
+# FIXME: make it possible to run the tool on unsaved file.
+p = subprocess.Popen(command,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+stdout, stderr = p.communicate()
+
+if stderr:
+print stderr
+
+# Reload all buffers in Vim.
+vim.command("bufdo edit")
+
+
+if __name__ == '__main__':
+main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20948: [OpenCL] Fix access qualifiers handling for typedefs

2016-07-07 Thread Andrew Savonichev via cfe-commits

asavonic updated this revision to Diff 63073.
asavonic added a comment.

Merged invalid-access-qualifier.cl and images-typedef.cl tests, fixed
code style issues.


http://reviews.llvm.org/D20948

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/kernel-arg-info.cl
  test/SemaOpenCL/access-qualifier.cl
  test/SemaOpenCL/invalid-access-qualifier.cl

Index: test/SemaOpenCL/invalid-access-qualifier.cl
===
--- test/SemaOpenCL/invalid-access-qualifier.cl
+++ /dev/null
@@ -1,14 +0,0 @@
-// RUN: %clang_cc1 -verify %s
-// RUN: %clang_cc1 -verify -cl-std=CL2.0 -DCL20 %s
-
-void test1(read_only int i){} // expected-error{{access qualifier can only be used for pipe and image type}}
-
-void test2(read_only write_only image1d_t i){} // expected-error{{multiple access qualifiers}}
-
-void test3(read_only read_only image1d_t i){} // expected-error{{multiple access qualifiers}}
-
-#ifdef CL20
-void test4(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'pipe int'}}
-#else
-void test4(__read_write image1d_t i) {} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' earlier than OpenCL version 2.0}}
-#endif
Index: test/SemaOpenCL/access-qualifier.cl
===
--- /dev/null
+++ test/SemaOpenCL/access-qualifier.cl
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL1.2 %s
+// RUN: %clang_cc1 -verify -pedantic -fsyntax-only -cl-std=CL2.0 %s
+
+typedef image1d_t img1d_ro_default; // expected-note {{previously declared 'read_only' here}}
+
+typedef write_only image1d_t img1d_wo; // expected-note {{previously declared 'write_only' here}}
+typedef read_only image1d_t img1d_ro;
+
+#if __OPENCL_C_VERSION__ >= 200
+  typedef read_write image1d_t img1d_rw;
+#endif
+
+typedef int Int;
+typedef read_only int IntRO; // expected-error {{access qualifier can only be used for pipe and image type}}
+
+
+void myWrite(write_only image1d_t); // expected-note {{passing argument to parameter here}} expected-note {{passing argument to parameter here}}
+void myRead(read_only image1d_t); // expected-note {{passing argument to parameter here}}
+
+#if __OPENCL_C_VERSION__ >= 200
+void myReadWrite(read_write image1d_t);
+#else
+void myReadWrite(read_write image1d_t); // expected-error {{access qualifier 'read_write' can not be used for '__read_write image1d_t' earlier than OpenCL version 2.0}}
+#endif
+
+
+kernel void k1(img1d_wo img) {
+  myRead(img); // expected-error {{passing 'img1d_wo' (aka '__write_only image1d_t') to parameter of incompatible type '__read_only image1d_t'}}
+}
+
+kernel void k2(img1d_ro img) {
+  myWrite(img); // expected-error {{passing 'img1d_ro' (aka '__read_only image1d_t') to parameter of incompatible type '__write_only image1d_t'}}
+}
+
+kernel void k3(img1d_wo img) {
+  myWrite(img);
+}
+
+#if __OPENCL_C_VERSION__ >= 200
+kernel void k4(img1d_rw img) {
+  myReadWrite(img);
+}
+#endif
+
+kernel void k5(img1d_ro_default img) {
+  myWrite(img); // expected-error {{passing 'img1d_ro_default' (aka '__read_only image1d_t') to parameter of incompatible type '__write_only image1d_t'}}
+}
+
+kernel void k6(img1d_ro img) {
+  myRead(img);
+}
+
+kernel void k7(read_only img1d_wo img){} // expected-error {{multiple access qualifiers}}
+
+kernel void k8(write_only img1d_ro_default img){} // expected-error {{multiple access qualifiers}}
+
+kernel void k9(read_only int i){} // expected-error{{access qualifier can only be used for pipe and image type}}
+
+kernel void k10(read_only Int img){} // expected-error {{access qualifier can only be used for pipe and image type}}
+
+kernel void k11(read_only write_only image1d_t i){} // expected-error{{multiple access qualifiers}}
+
+kernel void k12(read_only read_only image1d_t i){} // expected-error{{multiple access qualifiers}}
+
+#if __OPENCL_C_VERSION__ >= 200
+kernel void k13(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'pipe int'}}
+#else
+kernel void k13(__read_write image1d_t i){} // expected-error{{access qualifier '__read_write' can not be used for '__read_write image1d_t' earlier than OpenCL version 2.0}}
+#endif
Index: test/CodeGenOpenCL/kernel-arg-info.cl
===
--- test/CodeGenOpenCL/kernel-arg-info.cl
+++ test/CodeGenOpenCL/kernel-arg-info.cl
@@ -49,7 +49,7 @@
 // ARGINFO: !kernel_arg_name ![[MD46:[0-9]+]]
 
 typedef image1d_t myImage;
-kernel void foo5(read_only myImage img1, write_only image1d_t img2) {
+kernel void foo5(myImage img1, write_only image1d_t img2) {
 }
 // CHECK: define spir_kernel void @foo5{{[^!]+}}
 // CHECK: !kernel_arg_addr_space ![[MD41:[0-9]+]]
Index: lib/Sema/SemaType.cpp

Re: [PATCH] D20948: [OpenCL] Fix access qualifiers handling for typedefs

2016-07-07 Thread Andrew Savonichev via cfe-commits

asavonic marked 4 inline comments as done.
asavonic added a comment.

http://reviews.llvm.org/D20948



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

Re: [PATCH] D22046: [clang-tidy] Add dependency on clang-headers

2016-07-07 Thread Eugene Zelenko via cfe-commits

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r274751.


http://reviews.llvm.org/D22046



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

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-07 Thread Miklos Vajna via cfe-commits

vmiklos added a subscriber: vmiklos.
vmiklos added a comment.

Can you please postpone the cleanup till http://reviews.llvm.org/D21814 is 
reviewed? The two patches conflict with each other, I fear. Thanks. :-)


http://reviews.llvm.org/D22091



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

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kim Gräsman via cfe-commits

kimgr added a subscriber: kimgr.
kimgr added a comment.

I only caught this typo after it was committed.



Comment at: clang-tools-extra/trunk/clang-rename/tool/clang-rename.py:17-18
@@ +16,4 @@
+All you have to do now is to place a cursor on a variable/function/class which
+you would like to rename and press ',cr'. You will be promted a new name if the
+cursor points to a valid symbol.
+'''

s/promted/prompted for/


Repository:
  rL LLVM

http://reviews.llvm.org/D22087



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

Re: [PATCH] D21567: [OpenCL] Generate struct type for sampler_t and function call for the initializer

2016-07-07 Thread Anastasia Stulova via cfe-commits

Anastasia added inline comments.


Comment at: include/clang/AST/BuiltinTypes.def:164
@@ +163,3 @@
+// Internal OpenCL sampler initializer type.
+BUILTIN_TYPE(OCLSamplerInit, OCLSamplerInitTy)
+

> However, we want it to be translated to __sampler_initializer* type.

This is precisely what I am not able to understand. Why do we want it to be  
__sampler_initializer* type? I don't think we want that. If it was a sampler 
type originally in OpenCL, it should be kept a sampler type in LLVM too.


Comment at: include/clang/AST/OperationKinds.def:328
@@ +327,3 @@
+// Convert an integer initializer to an OpenCL sampler initializer.
+CAST_OPERATION(IntToOCLSamplerInitializer)
+

> In codegen, we want to insert a call of __transform_sampler_initializer for 
> the reference of variable a, not the definition of variable a.
 I think the original discussion was to insert the call on the declaration of 
sampler variable as an initializer. This would allow us to use variable itself 
"as is" everywhere else.

So if let's say we compile:
  constant sampler_t a = 0;
  kernel void f() {
 g(a);
  }

the code we are generating would be equivalent to:

  kernel void f() {
__sampler* a = __transform_sampler_initializer(0);
g(a);
  }

Why are we not using this approach?

I think we discussed later that we could generate a struct of initializers 
instead of just int (0 -> struct sampler_initializer). Here is the description: 
http://lists.llvm.org/pipermail/cfe-dev/2016-June/049389.html
But general principle still was as described above...


Comment at: include/clang/Basic/DiagnosticGroups.td:876
@@ +875,3 @@
+// A warning group for warnings about code that clang accepts when
+// compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
+def SpirCompat : DiagGroup<"spir-compat">;

I see. This is just because OpenCL doesn't allocate actual constants for 
different allowed sampler modes whereas SPIR does.


Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:52
@@ +51,3 @@
+   Ctx, "__sampler"),
+   CGM.getContext().getTargetAddressSpace(
+   LangAS::opencl_constant));

If you do conversion struct ptr <-> opaque ptr in this function, it should be 
fine? I would imagine images and other OpenCL types have the same requirement.


http://reviews.llvm.org/D21567



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

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-07 Thread Benjamin Kramer via cfe-commits

bkramer added inline comments.


Comment at: clang-rename/USRLocFinder.h:38
@@ -37,2 +37,2 @@
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_LOC_FINDER_H
+#endif  // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_LOC_FINDER_H

In LLVM we usually have only one space before a // comment. was this 
clang-formatted with google style?


http://reviews.llvm.org/D22091



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

Re: [PATCH] D20948: [OpenCL] Fix access qualifiers handling for typedefs

2016-07-07 Thread Anastasia Stulova via cfe-commits

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

LGTM! Could you please address these last minor comments though before 
committing.

Thanks!



Comment at: test/SemaOpenCL/access-qualifier.cl:10
@@ +9,3 @@
+#if __OPENCL_C_VERSION__ >= 200
+  typedef read_write image1d_t img1d_rw;
+#endif

don't indent here!


Comment at: test/SemaOpenCL/access-qualifier.cl:68
@@ +67,3 @@
+#else
+kernel void k13(__read_write image1d_t i){} // expected-error{{access 
qualifier '__read_write' can not be used for '__read_write image1d_t' earlier 
than OpenCL version 2.0}}
+#endif

It reads a bit strange. Would it be better:
"earlier than OpenCL version 2.0" -> "prior to OpenCL version 2.0"


http://reviews.llvm.org/D20948



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

Re: [PATCH] D20948: [OpenCL] Fix access qualifiers handling for typedefs

2016-07-07 Thread Anastasia Stulova via cfe-commits

Anastasia added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2509
@@ -2508,3 +2508,3 @@
   "functions, methods, and parameters|classes|enums|variables|methods|"
-  "fields and global variables|structs|variables and typedefs|thread-local 
variables|"
-  "variables and fields|variables, data members and tag types|"
+  "fields and global variables|structs|parameters and typedefs|variables and 
typedefs|"
+  "thread-local variables|variables and fields|variables, data members and tag 
types|"

Is this change actually needed?


http://reviews.llvm.org/D20948



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

r274766 - Revert "[AArch64] Change the preferred alignment for char and short to word alignment"

2016-07-07 Thread Chad Rosier via cfe-commits

Author: mcrosier
Date: Thu Jul  7 11:37:19 2016
New Revision: 274766

URL: http://llvm.org/viewvc/llvm-project?rev=274766&view=rev
Log:
Revert "[AArch64] Change the preferred alignment for char and short to word 
alignment"

This reverts commit r273280 as the change was not properly approved.

Modified:
cfe/trunk/lib/Basic/Targets.cpp

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=274766&r1=274765&r2=274766&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Jul  7 11:37:19 2016
@@ -5994,7 +5994,7 @@ class AArch64leTargetInfo : public AArch
 if (getTriple().isOSBinFormatMachO())
   resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128");
 else
-  resetDataLayout("e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+  resetDataLayout("e-m:e-i64:64-i128:128-n32:64-S128");
   }
 
 public:
@@ -6012,7 +6012,7 @@ public:
 class AArch64beTargetInfo : public AArch64TargetInfo {
   void setDataLayout() override {
 assert(!getTriple().isOSBinFormatMachO());
-resetDataLayout("E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+resetDataLayout("E-m:e-i64:64-i128:128-n32:64-S128");
   }
 
 public:


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

r274770 - NVPTX: Use the nvvm builtins to read SRegs rather than the legacy ptx ones

2016-07-07 Thread Justin Bogner via cfe-commits

Author: bogner
Date: Thu Jul  7 11:41:08 2016
New Revision: 274770

URL: http://llvm.org/viewvc/llvm-project?rev=274770&view=rev
Log:
NVPTX: Use the nvvm builtins to read SRegs rather than the legacy ptx ones

The ptx spellings were removed from LLVM in r274769.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def
cfe/trunk/lib/Headers/cuda_builtin_vars.h
cfe/trunk/test/CodeGen/builtins-nvptx.c
cfe/trunk/test/CodeGenCUDA/cuda-builtin-vars.cu
cfe/trunk/test/SemaCUDA/builtins.cu

Modified: cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def?rev=274770&r1=274769&r2=274770&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def Thu Jul  7 11:41:08 2016
@@ -14,53 +14,50 @@
 
 // The format of this database matches clang/Basic/Builtins.def.
 
-// Builtins retained from previous PTX back-end
-BUILTIN(__builtin_ptx_read_tid_x, "i", "nc")
-BUILTIN(__builtin_ptx_read_tid_y, "i", "nc")
-BUILTIN(__builtin_ptx_read_tid_z, "i", "nc")
-BUILTIN(__builtin_ptx_read_tid_w, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_ntid_x, "i", "nc")
-BUILTIN(__builtin_ptx_read_ntid_y, "i", "nc")
-BUILTIN(__builtin_ptx_read_ntid_z, "i", "nc")
-BUILTIN(__builtin_ptx_read_ntid_w, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_ctaid_x, "i", "nc")
-BUILTIN(__builtin_ptx_read_ctaid_y, "i", "nc")
-BUILTIN(__builtin_ptx_read_ctaid_z, "i", "nc")
-BUILTIN(__builtin_ptx_read_ctaid_w, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_nctaid_x, "i", "nc")
-BUILTIN(__builtin_ptx_read_nctaid_y, "i", "nc")
-BUILTIN(__builtin_ptx_read_nctaid_z, "i", "nc")
-BUILTIN(__builtin_ptx_read_nctaid_w, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_laneid, "i", "nc")
-BUILTIN(__builtin_ptx_read_warpid, "i", "nc")
-BUILTIN(__builtin_ptx_read_nwarpid, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_smid, "i", "nc")
-BUILTIN(__builtin_ptx_read_nsmid, "i", "nc")
-BUILTIN(__builtin_ptx_read_gridid, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_lanemask_eq, "i", "nc")
-BUILTIN(__builtin_ptx_read_lanemask_le, "i", "nc")
-BUILTIN(__builtin_ptx_read_lanemask_lt, "i", "nc")
-BUILTIN(__builtin_ptx_read_lanemask_ge, "i", "nc")
-BUILTIN(__builtin_ptx_read_lanemask_gt, "i", "nc")
-
-BUILTIN(__builtin_ptx_read_clock, "i", "n")
-BUILTIN(__builtin_ptx_read_clock64, "LLi", "n")
-
-BUILTIN(__builtin_ptx_read_pm0, "i", "n")
-BUILTIN(__builtin_ptx_read_pm1, "i", "n")
-BUILTIN(__builtin_ptx_read_pm2, "i", "n")
-BUILTIN(__builtin_ptx_read_pm3, "i", "n")
+// Special Registers
 
-BUILTIN(__builtin_ptx_bar_sync, "vi", "n")
+BUILTIN(__nvvm_read_ptx_sreg_tid_x, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_tid_y, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_tid_z, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_tid_w, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_ntid_x, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_ntid_y, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_ntid_z, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_ntid_w, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_ctaid_x, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_ctaid_y, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_ctaid_z, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_ctaid_w, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_nctaid_x, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_nctaid_y, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_nctaid_z, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_nctaid_w, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_laneid, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_warpid, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_nwarpid, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_smid, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_nsmid, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_gridid, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_lanemask_eq, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_lanemask_le, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_lanemask_lt, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_lanemask_ge, "i", "nc")
+BUILTIN(__nvvm_read_ptx_sreg_lanemask_gt, "i", "nc")
+
+BUILTIN(__nvvm_read_ptx_sreg_clock, "i", "n")
+BUILTIN(__nvvm_read_ptx_sreg_clock64, "LLi", "n")
+
+BUILTIN(__nvvm_read_ptx_sreg_pm0, "i", "n")
+BUILTIN(__nvvm_read_ptx_sreg_pm1, "i", "n")
+BUILTIN(__nvvm_read_ptx_sreg_pm2, "i", "n")
+BUILTIN(__nvvm_read_ptx_sreg_pm3, "i", "n")
 
-
-// Builtins exposed as part of NVVM
 // MISC
 
 BUILTIN(__nvvm_clz_i, "ii", "")
@@ -396,11 +393,11 @@ BUILTIN(__nvvm_bitcast_d2ll, "LLid", "")
 
 // Sync
 
-BUILTIN(__syncthreads, "v", "")
 BUILTIN(__nvvm_bar0, "v", "")
 BUILTIN(__nvvm_bar0_popc, "ii", "")
 BUILTIN(__nvvm_bar0_and, "ii", "")
 BUILTIN(__nvvm_bar0_or, "ii", "")
+BUILTIN(__nvvm_bar_sync, "vi", "n")
 
 // Shuffle
 

Modified: cfe/trunk/lib/Headers/cuda_builtin_vars.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/cuda_builtin_vars.h?rev=274770&r1=274769&r2=274770&view=diff
==
--

r274767 - Revert "[aarch64] Update datalayout for aarch64 tests"

2016-07-07 Thread Chad Rosier via cfe-commits

Author: mcrosier
Date: Thu Jul  7 11:37:21 2016
New Revision: 274767

URL: http://llvm.org/viewvc/llvm-project?rev=274767&view=rev
Log:
Revert "[aarch64] Update datalayout for aarch64 tests"

This reverts commit r273289, which was a follow to r273280, which was
reverted because the change was not properly approved.

Modified:
cfe/trunk/test/CodeGen/aarch64-type-sizes.c
cfe/trunk/test/CodeGen/target-data.c

Modified: cfe/trunk/test/CodeGen/aarch64-type-sizes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/aarch64-type-sizes.c?rev=274767&r1=274766&r2=274767&view=diff
==
--- cfe/trunk/test/CodeGen/aarch64-type-sizes.c (original)
+++ cfe/trunk/test/CodeGen/aarch64-type-sizes.c Thu Jul  7 11:37:21 2016
@@ -1,7 +1,8 @@
-// RUN: %clang_cc1 -triple aarch64_be-none-linux-gnu -emit-llvm -w -o - %s | 
FileCheck --check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple aarch64_be-none-linux-gnu -emit-llvm -w -o - %s | 
FileCheck --check-prefix=CHECK --check-prefix=CHECK-BE %s
 // char by definition has size 1
 
-// CHECK: target datalayout = 
"E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+// CHECK-LE: target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
+// CHECK-BE: target datalayout = "E-m:e-i64:64-i128:128-n32:64-S128"
 
 int check_short() {
   return sizeof(short);
@@ -88,3 +89,4 @@ int foo() {
   return sizeof(enum Small);
 // CHECK: ret i32 4
 }
+

Modified: cfe/trunk/test/CodeGen/target-data.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-data.c?rev=274767&r1=274766&r2=274767&view=diff
==
--- cfe/trunk/test/CodeGen/target-data.c (original)
+++ cfe/trunk/test/CodeGen/target-data.c Thu Jul  7 11:37:21 2016
@@ -141,7 +141,7 @@
 
 // RUN: %clang_cc1 -triple arm64-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=AARCH64
-// AARCH64: target datalayout = 
"e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+// AARCH64: target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
 
 // RUN: %clang_cc1 -triple thumb-unknown-gnueabi -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=THUMB


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

Re: [PATCH] D21869: [CUDA] Check that our CUDA install supports the requested architectures.

2016-07-07 Thread Artem Belevich via cfe-commits

tra added inline comments.


Comment at: include/clang/Basic/DiagnosticDriverKinds.td:32
@@ -29,1 +31,3 @@
+  "Use --cuda-path to specify a different CUDA install, or pass "
+  "--nocuda-version-check.">;
 def err_drv_invalid_thread_model_for_target : Error<

Is it supposed to be two dashes or one?



Comment at: include/clang/Driver/Options.td:1722-1724
@@ -1721,2 +1721,5 @@
 def nocudalib : Flag<["-"], "nocudalib">;
+def nocuda_version_check : Flag<["-"], "nocuda-version-check">,
+  HelpText<"Don't error out if the detected version of the CUDA install is "
+   "too low for the requested CUDA gpu architecture.">;
 def nodefaultlibs : Flag<["-"], "nodefaultlibs">;

IMO we should use double-dash for this option. 

-nocudalib/-nocudainc were mimicking existing -nostdlib/-nostdinc and thus 
ended with single dash in front.
--nocuda-version-check is not constrained by this.


Comment at: lib/Driver/ToolChains.cpp:1704
@@ -1702,1 +1703,3 @@
 
+// Parses the contents of version.txt in an CUDA installation.
+static CudaVersion ParseCudaVersionFile(llvm::StringRef V) {

Might add an example of what's in that file, so it's easier to understand what 
the code below is doing.


Comment at: lib/Driver/ToolChains.cpp:1793
@@ +1792,3 @@
+FS.getBufferForFile(InstallPath + "/version.txt");
+if (!VersionFile) {
+  // CUDA 7.0 doesn't have a version.txt, so guess that's our version if

What if it's cuda-8, but we've failed to read the file due to permissions.
Perhaps we should differentiate no-such-file from other errors.


Comment at: lib/Driver/ToolChains.cpp:4711
@@ +4710,3 @@
+  // Check our CUDA version if we're going to include the CUDA headers.
+  if (!DriverArgs.hasArg(options::OPT_nocudainc) &&
+  !DriverArgs.hasArg(options::OPT_nocuda_version_check)) {

-nocudainc should not preclude CUDA version check, IMO.
Primary use of version check is to make sure we can compile for a given GPU 
architecture. I think we still want to do that even if -nocudainc is passed.


http://reviews.llvm.org/D21869



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

Re: [PATCH] D21667: [analyzer] Add rudimentary handling of AtomicExpr.

2016-07-07 Thread Anna Zaks via cfe-commits

zaks.anna accepted this revision.
This revision is now accepted and ready to land.


Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2075
@@ +2074,3 @@
+void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
+ ExplodedNodeSet &Dst) {
+  ExplodedNodeSet AfterPreSet;

alignment is off?


http://reviews.llvm.org/D21667



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

Re: [PATCH] D22075: [OpenMP] Fix incorrect diagnostics in map clause

2016-07-07 Thread Samuel Antao via cfe-commits

sfantao added a comment.

Thanks. You have to wait for Alexey to take a look at the patch too.

Thanks again,
Samuel


http://reviews.llvm.org/D22075



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

Re: [PATCH] D22075: [OpenMP] Fix incorrect diagnostics in map clause

2016-07-07 Thread David S via cfe-commits

davidsh updated this revision to Diff 63090.
davidsh added a comment.

Address comments


http://reviews.llvm.org/D22075

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/target_map_messages.cpp

Index: test/OpenMP/target_map_messages.cpp
===
--- test/OpenMP/target_map_messages.cpp
+++ test/OpenMP/target_map_messages.cpp
@@ -284,6 +284,11 @@
 {}
   }
   }
+  #pragma omp target data map(marr[:][:][:])
+  {
+#pragma omp target data map(marr)
+{}
+  }
 
   #pragma omp target data map(to: t)
   {
@@ -488,10 +493,10 @@
 #pragma omp target data map(j)
 #pragma omp target map(l) map(l[:5]) // expected-error {{variable already 
marked as mapped in current construct}} expected-note {{used here}}
   foo();
-#pragma omp target data map(k[:4], j, l[:5]) // expected-note 2 {{used here}}
+#pragma omp target data map(k[:4], j, l[:5]) // expected-note {{used here}}
 #pragma omp target data map(k) // expected-error {{pointer cannot be mapped 
along with a section derived from itself}}
 #pragma omp target data map(j)
-#pragma omp target map(l) // expected-error {{original storage of expression 
in data environment is shared but data environment do not fully contain mapped 
expression storage}}
+#pragma omp target map(l)
   foo();
 
 #pragma omp target data map(always, tofrom: x)
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -10649,6 +10649,25 @@
   if (CI->getAssociatedDeclaration() != SI->getAssociatedDeclaration())
 break;
 }
+// Check if the extra components of the expressions in the enclosing 
+// data environment are redundant for the current base declaration. 
+// If they are, the maps completely overlap, which is legal.
+for (; SI != SE; ++SI) {
+  QualType Type;
+  if (auto *ASE = 
+  dyn_cast(SI->getAssociatedExpression())) {
+Type = ASE->getBase()->IgnoreParenImpCasts()->getType();
+  } else if (auto *OASE = 
+   dyn_cast(SI->getAssociatedExpression())) {
+auto *E = OASE->getBase()->IgnoreParenImpCasts();
+Type =
+OMPArraySectionExpr::getBaseOriginalType(E).getCanonicalType();
+  }
+  if (Type.isNull() || Type->isAnyPointerType() ||
+  CheckArrayExpressionDoesNotReferToWholeSize(SemaRef,
+  SI->getAssociatedExpression(),Type))
+break;
+}
 
 // OpenMP 4.5 [2.15.5.1, map Clause, Restrictions, p.4]
 //  List items of map clauses in the same construct must not share


Index: test/OpenMP/target_map_messages.cpp
===
--- test/OpenMP/target_map_messages.cpp
+++ test/OpenMP/target_map_messages.cpp
@@ -284,6 +284,11 @@
 {}
   }
   }
+  #pragma omp target data map(marr[:][:][:])
+  {
+#pragma omp target data map(marr)
+{}
+  }
 
   #pragma omp target data map(to: t)
   {
@@ -488,10 +493,10 @@
 #pragma omp target data map(j)
 #pragma omp target map(l) map(l[:5]) // expected-error {{variable already marked as mapped in current construct}} expected-note {{used here}}
   foo();
-#pragma omp target data map(k[:4], j, l[:5]) // expected-note 2 {{used here}}
+#pragma omp target data map(k[:4], j, l[:5]) // expected-note {{used here}}
 #pragma omp target data map(k) // expected-error {{pointer cannot be mapped along with a section derived from itself}}
 #pragma omp target data map(j)
-#pragma omp target map(l) // expected-error {{original storage of expression in data environment is shared but data environment do not fully contain mapped expression storage}}
+#pragma omp target map(l)
   foo();
 
 #pragma omp target data map(always, tofrom: x)
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -10649,6 +10649,25 @@
   if (CI->getAssociatedDeclaration() != SI->getAssociatedDeclaration())
 break;
 }
+// Check if the extra components of the expressions in the enclosing 
+// data environment are redundant for the current base declaration. 
+// If they are, the maps completely overlap, which is legal.
+for (; SI != SE; ++SI) {
+  QualType Type;
+  if (auto *ASE = 
+  dyn_cast(SI->getAssociatedExpression())) {
+Type = ASE->getBase()->IgnoreParenImpCasts()->getType();
+  } else if (auto *OASE = 
+   dyn_cast(SI->getAssociatedExpression())) {
+auto *E = OASE->getBase()->IgnoreParenImpCasts();
+Type =
+OMPArraySectionExpr::getBaseOriginalType(E).getCanonicalType();
+  }
+  if (Type.isNull() || Type->isAnyPointerType() ||
+  CheckArrayExpressionDoesNotReferToWholeS

Re: [PATCH] D22048: [analyzer] Suppress false positives in std::shared_ptr

2016-07-07 Thread Devin Coughlin via cfe-commits

dcoughlin closed this revision.
dcoughlin added a comment.

This was committed in r274691. I forgot to add the Differential Revision line 
so phabricator didn't pick it up.


http://reviews.llvm.org/D22048



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

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-07 Thread Alexander Droste via cfe-commits

Alexander_Droste updated this revision to Diff 63091.
Alexander_Droste added a comment.

- make static functions free functions
- make static containers local to their corresponding functions
- only assign the buffer type name in case of an error
- fix capitalization and punctuation in error diagnostic
- usage of formatting placeholders instead of concatenation
- move buffer type name variable out of the loop


http://reviews.llvm.org/D21962

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/MpiTypeMismatchCheck.cpp
  clang-tidy/misc/MpiTypeMismatchCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-mpi-type-mismatch.rst
  test/clang-tidy/Inputs/misc-mpi-type-mismatch/mpimock.h
  test/clang-tidy/misc-mpi-type-mismatch.cpp

Index: test/clang-tidy/misc-mpi-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-mpi-type-mismatch.cpp
@@ -0,0 +1,248 @@
+// RUN: %check_clang_tidy %s misc-mpi-type-mismatch %t -- -- -I %S/Inputs/misc-mpi-type-mismatch
+
+#include "mpimock.h"
+
+void charNegativeTest() {
+  unsigned char buf;
+  MPI_Send(&buf, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsigned char' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+  
+  int buf2;
+  MPI_Send(&buf2, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'int' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+  
+  short buf3;
+  MPI_Send(&buf3, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'short' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+
+  long buf4;
+  MPI_Send(&buf4, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+  
+  int8_t buf5;
+  MPI_Send(&buf5, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'int8_t' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+
+  uint16_t buf6;
+  MPI_Send(&buf6, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'uint16_t' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+
+  long double _Complex buf7;
+  MPI_Send(&buf7, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long double _Complex' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+
+  std::complex buf8;
+  MPI_Send(&buf8, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'complex' does not match the MPI datatype 'MPI_CHAR' [misc-mpi-type-mismatch]
+}
+
+void intNegativeTest() {
+  unsigned char buf;
+  MPI_Send(&buf, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsigned char' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+  
+  unsigned buf2;
+  MPI_Send(&buf2, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsigned int' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+  
+  short buf3;
+  MPI_Send(&buf3, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'short' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+
+  long buf4;
+  MPI_Send(&buf4, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+  
+  int8_t buf5;
+  MPI_Send(&buf5, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'int8_t' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+
+  uint16_t buf6;
+  MPI_Send(&buf6, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'uint16_t' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+
+  long double _Complex buf7;
+  MPI_Send(&buf7, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'long double _Complex' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+
+  std::complex buf8;
+  MPI_Send(&buf8, 1, MPI_INT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'complex' does not match the MPI datatype 'MPI_INT' [misc-mpi-type-mismatch]
+}
+
+void longNegativeTest() {
+  char buf;
+  MPI_Send(&buf, 1, MPI_LONG, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'char' does not match the MPI datatype 'MPI_LONG' [misc-mpi-type-mismatch]
+  
+  unsigned buf2;
+  MPI_Send(&buf2, 1, MPI_LONG, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer type 'unsign

RE: [PATCH] D21567: [OpenCL] Generate struct type for sampler_t and function call for the initializer

2016-07-07 Thread Liu, Yaxun (Sam) via cfe-commits

+ Brian

Hi Anastasia,

The advantage for translating sampler variable to a global variable with 
__sampler_initializer type is that it is consistent with OpenCL C++ and SPIRV, 
so it is easier to translate the IR to SPIRV.

About the type name of sampler_t in LLVM, using a name without `.` allows 
library functions to use the concrete sampler types directly without casting. 
Casting not only affects the readability of the library code, but also causes 
extra efforts in optimizing these codes.
 
Sam

-Original Message-
From: Anastasia Stulova [mailto:anastasia.stul...@arm.com] 
Sent: Thursday, July 7, 2016 11:29 AM
To: Liu, Yaxun (Sam) ; anastasia.stul...@arm.com; 
alexey.ba...@intel.com; xiuli...@outlook.com
Cc: Stellard, Thomas ; cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D21567: [OpenCL] Generate struct type for sampler_t and 
function call for the initializer

Anastasia added inline comments.


Comment at: include/clang/AST/BuiltinTypes.def:164
@@ +163,3 @@
+// Internal OpenCL sampler initializer type.
+BUILTIN_TYPE(OCLSamplerInit, OCLSamplerInitTy)
+

> However, we want it to be translated to __sampler_initializer* type.

This is precisely what I am not able to understand. Why do we want it to be  
__sampler_initializer* type? I don't think we want that. If it was a sampler 
type originally in OpenCL, it should be kept a sampler type in LLVM too.


Comment at: include/clang/AST/OperationKinds.def:328
@@ +327,3 @@
+// Convert an integer initializer to an OpenCL sampler initializer.
+CAST_OPERATION(IntToOCLSamplerInitializer)
+

> In codegen, we want to insert a call of __transform_sampler_initializer for 
> the reference of variable a, not the definition of variable a.
 I think the original discussion was to insert the call on the declaration of 
sampler variable as an initializer. This would allow us to use variable itself 
"as is" everywhere else.

So if let's say we compile:
  constant sampler_t a = 0;
  kernel void f() {
 g(a);
  }

the code we are generating would be equivalent to:

  kernel void f() {
__sampler* a = __transform_sampler_initializer(0);
g(a);
  }

Why are we not using this approach?

I think we discussed later that we could generate a struct of initializers 
instead of just int (0 -> struct sampler_initializer). Here is the description: 
http://lists.llvm.org/pipermail/cfe-dev/2016-June/049389.html
But general principle still was as described above...


Comment at: include/clang/Basic/DiagnosticGroups.td:876
@@ +875,3 @@
+// A warning group for warnings about code that clang accepts when // 
+compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
+def SpirCompat : DiagGroup<"spir-compat">;

I see. This is just because OpenCL doesn't allocate actual constants for 
different allowed sampler modes whereas SPIR does.


Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:52
@@ +51,3 @@
+   Ctx, "__sampler"),
+   CGM.getContext().getTargetAddressSpace(
+   LangAS::opencl_constant));

If you do conversion struct ptr <-> opaque ptr in this function, it should be 
fine? I would imagine images and other OpenCL types have the same requirement.


http://reviews.llvm.org/D21567



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

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

2016-07-07 Thread Alexander Droste via cfe-commits

Alexander_Droste marked 22 inline comments as done.


Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:153
@@ +152,3 @@
+  {BuiltinType::LongDouble, "MPI_C_LONG_DOUBLE_COMPLEX"}};
+
+  const auto *Builtin =

Sure, I can do that. I marked all addressed comments with done.


Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:272
@@ +271,3 @@
+// Skip unknown MPI datatypes and void pointers.
+StringRef MPIDatatype =
+tooling::fixit::getText(*CE->getArg(DatatypeIdx), *Result.Context);

I removed the unnecessary single word comments.


Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:272
@@ +271,3 @@
+// Skip unknown MPI datatypes and void pointers.
+StringRef MPIDatatype =
+tooling::fixit::getText(*CE->getArg(DatatypeIdx), *Result.Context);

Alexander_Droste wrote:
> I removed the unnecessary single word comments.
Further, the variable now only gets assigned in case of an error.


Comment at: clang-tidy/misc/MpiTypeMismatchCheck.h:60
@@ +59,2 @@
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MPI_TYPE_MISMATCH_H

I see, this removes a lot of verbosity.


http://reviews.llvm.org/D21962



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

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 added inline comments.


Comment at: clang-rename/USRLocFinder.h:38
@@ -37,2 +37,2 @@
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_LOC_FINDER_H
+#endif  // LLVM_CLANG_TOOLS_EXTRA_CLANG_RENAME_USR_LOC_FINDER_H

bkramer wrote:
> In LLVM we usually have only one space before a // comment. was this 
> clang-formatted with google style?
Oops. Yes, seems like my machine had Google as the default style, instead of 
the LLVM code style by default, which I usually have.


http://reviews.llvm.org/D22091



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

Re: [PATCH] D22091: [clang-rename] exit code-related bugfix and code cleanup

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 updated this revision to Diff 63093.
omtcyf0 marked an inline comment as done.

http://reviews.llvm.org/D22091

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/USRLocFinder.cpp
  clang-rename/USRLocFinder.h
  clang-rename/tool/ClangRename.cpp

Index: clang-rename/tool/ClangRename.cpp
===
--- clang-rename/tool/ClangRename.cpp
+++ clang-rename/tool/ClangRename.cpp
@@ -36,7 +36,6 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/Host.h"
-#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +82,7 @@
 #define CLANG_RENAME_VERSION "0.0.1"
 
 static void PrintVersion() {
-  outs() << "clang-rename version " << CLANG_RENAME_VERSION << "\n";
+  outs() << "clang-rename version " << CLANG_RENAME_VERSION << '\n';
 }
 
 using namespace clang;
@@ -101,7 +100,6 @@
 
   if (NewName.empty()) {
 errs() << "clang-rename: no new name provided.\n\n";
-cl::PrintHelpMessage();
 exit(1);
   }
 
@@ -115,12 +113,14 @@
   const auto &USRs = USRAction.getUSRs();
   const auto &PrevName = USRAction.getUSRSpelling();
 
-  if (PrevName.empty())
+  if (PrevName.empty()) {
 // An error should have already been printed.
 exit(1);
+  }
 
-  if (PrintName)
-errs() << "clang-rename: found name: " << PrevName << "\n";
+  if (PrintName) {
+errs() << "clang-rename: found name: " << PrevName << '\n';
+  }
 
   // Perform the renaming.
   rename::RenamingAction RenameAction(NewName, PrevName, USRs,
Index: clang-rename/USRLocFinder.h
===
--- clang-rename/USRLocFinder.h
+++ clang-rename/USRLocFinder.h
@@ -29,9 +29,8 @@
 namespace rename {
 
 // FIXME: make this an AST matcher. Wouldn't that be awesome??? I agree!
-std::vector getLocationsOfUSR(llvm::StringRef usr,
-  llvm::StringRef PrevName,
-  Decl *decl);
+std::vector
+getLocationsOfUSR(llvm::StringRef usr, llvm::StringRef PrevName, Decl *decl);
 }
 }
 
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -34,8 +34,8 @@
 class USRLocFindingASTVisitor
 : public clang::RecursiveASTVisitor {
 public:
-  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName) : USR(USR), PrevName(PrevName) {
-  }
+  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName)
+  : USR(USR), PrevName(PrevName) {}
 
   // Declaration visitors:
 
@@ -60,8 +60,10 @@
 
   bool VisitCXXConstructorDecl(clang::CXXConstructorDecl *ConstructorDecl) {
 const ASTContext &Context = ConstructorDecl->getASTContext();
-for (clang::CXXConstructorDecl::init_const_iterator it = ConstructorDecl->init_begin(); it != ConstructorDecl->init_end(); ++it) {
-  const clang::CXXCtorInitializer* Initializer = *it;
+for (clang::CXXConstructorDecl::init_const_iterator it =
+ ConstructorDecl->init_begin();
+ it != ConstructorDecl->init_end(); ++it) {
+  const clang::CXXCtorInitializer *Initializer = *it;
   if (Initializer->getSourceOrder() == -1) {
 // Ignore implicit initializers.
 continue;
@@ -71,9 +73,12 @@
 if (getUSRForDecl(FieldDecl) == USR) {
   // The initializer refers to a field that is to be renamed.
   SourceLocation Location = Initializer->getSourceLocation();
-  StringRef TokenName = Lexer::getSourceText(CharSourceRange::getTokenRange(Location), Context.getSourceManager(), Context.getLangOpts());
+  StringRef TokenName = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Location),
+  Context.getSourceManager(), Context.getLangOpts());
   if (TokenName == PrevName) {
-// The token of the source location we find actually has the old name.
+// The token of the source location we find actually has the old
+// name.
 LocationsFound.push_back(Initializer->getSourceLocation());
   }
 }
@@ -183,14 +188,15 @@
   bool handleCXXNamedCastExpr(clang::CXXNamedCastExpr *Expr) {
 clang::QualType Type = Expr->getType();
 // See if this a cast of a pointer.
-const RecordDecl* Decl = Type->getPointeeCXXRecordDecl();
+const RecordDecl *Decl = Type->getPointeeCXXRecordDecl();
 if (!Decl) {
   // See if this is a cast of a reference.
   Decl = Type->getAsCXXRecordDecl();
 }
 
 if (Decl && getUSRForDecl(Decl) == USR) {
-  SourceLocation Location = Expr->getTypeInfoAsWritten()->getTypeLoc().getBeginLoc();
+  SourceLocation Location =
+  Expr->getTypeInfoAsWritten()->getTypeLoc().getBeginLoc();
   LocationsFound.push_back(Location);
 }
 
@@ -205,8 +211,7 @@
 };
 } // namespace
 
-std::vector getLocationsOfUSR(StringRef USR,
-

Re: [PATCH] D21869: [CUDA] Check that our CUDA install supports the requested architectures.

2016-07-07 Thread Justin Lebar via cfe-commits

jlebar updated this revision to Diff 63094.
jlebar marked 3 inline comments as done.
jlebar added a comment.

Address review comments.


http://reviews.llvm.org/D21869

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  test/Driver/Inputs/CUDA_80/usr/local/cuda/bin/.keep
  test/Driver/Inputs/CUDA_80/usr/local/cuda/include/.keep
  test/Driver/Inputs/CUDA_80/usr/local/cuda/lib/.keep
  test/Driver/Inputs/CUDA_80/usr/local/cuda/lib64/.keep
  
test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_20.10.bc
  
test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_35.10.bc
  test/Driver/Inputs/CUDA_80/usr/local/cuda/version.txt
  test/Driver/cuda-version-check.cu

Index: test/Driver/cuda-version-check.cu
===
--- /dev/null
+++ test/Driver/cuda-version-check.cu
@@ -0,0 +1,51 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+// RUN: %clang -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+
+// The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60
+
+// This should only complain about sm_60, not sm_35.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
+// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
+
+// We should get two errors here, one for sm_60 and one for sm_61.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
+// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
+
+// We should still get an error if we pass -nocudainc, because this compilation
+// would invoke ptxas, and we do a version check on that, too.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60
+
+// If with -nocudainc and -E, we don't touch the CUDA install, so we
+// shouldn't get an error.
+// RUN: %clang -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
+// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+
+// -nocuda-version-check should suppress all of these errors.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN:-nocuda-version-check %s | \
+// RUN:FileCheck %s --check-prefix=OK
+
+// OK-NOT: error: GPU arch
+
+// OK_SM35-NOT: error: GPU arch sm_35
+
+// We should only get one error per architecture.
+// ERR_SM60: error: GPU arch sm_60 {{.*}}
+// ERR_SM60-NOT: error: GPU arch sm_60
+
+// ERR_SM61: error: GPU arch sm_61 {{.*}}
+// ERR_SM61-NOT: error: GPU arch sm_61
Index: test/Driver/Inputs/CUDA_80/usr/local/cuda/version.txt
===
--- /dev/null
+++ test/Driver/Inputs/CUDA_80/usr/local/cuda/version.txt
@@ -0,0 +1 @@
+CUDA Version 8.0.42
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -11146,6 +11146,12 @@
   assert(gpu_archs.size() == 1 && "Exactly one GPU Arch required for ptxas.");
   const std::string& gpu_arch = gpu_archs[0];
 
+  // Check that our installation's ptxas supports gpu_arch.
+  if (!Args.hasArg(options::OPT_nocuda_version_check)) {
+TC.cudaInstallation().CheckCudaVersionSupportsArch(
+StringToCudaArch(gpu_arch));
+  }
+
   ArgStringList CmdArgs;
   CmdArgs.push_back(TC.getTriple().isArch64Bit() ? "-m64" : "-m32");
   if (Args.hasFlag(options::OPT_cuda_noopt_device_debug,
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -11,12 +11,14 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_H
 
 #include "Tools.h"
+#include "clang/Basic/Cuda.h"
 #include "clang/Basic/VersionTuple.h"
 #include "clang/Driver/Action.h"
 #include "clang/Driver/Multilib.h"
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/Compiler.h"
 #include 
 #include 
@@ -162,22 +164,35 @@
   private:
 const Driver &D;
 bool IsValid = false;
+CudaVersion Version = CudaVersion::UNKNOWN;
 std::string InstallPath;
 std::string BinPath;
 std::string LibPath;
 std::s

Re: [PATCH] D21869: [CUDA] Check that our CUDA install supports the requested architectures.

2016-07-07 Thread Justin Lebar via cfe-commits

jlebar added inline comments.


Comment at: include/clang/Driver/Options.td:1722-1724
@@ -1721,2 +1721,5 @@
 def nocudalib : Flag<["-"], "nocudalib">;
+def nocuda_version_check : Flag<["-"], "nocuda-version-check">,
+  HelpText<"Don't error out if the detected version of the CUDA install is "
+   "too low for the requested CUDA gpu architecture.">;
 def nodefaultlibs : Flag<["-"], "nodefaultlibs">;

tra wrote:
> IMO we should use double-dash for this option. 
> 
> -nocudalib/-nocudainc were mimicking existing -nostdlib/-nostdinc and thus 
> ended with single dash in front.
> --nocuda-version-check is not constrained by this.
I guess I have to do a double dash because --no-cuda-noopt-device-debug is thus 
spelled.  Notice there's also a dash after "no".

...this is so screwed up.


Comment at: lib/Driver/ToolChains.cpp:1793
@@ +1792,3 @@
+FS.getBufferForFile(InstallPath + "/version.txt");
+if (!VersionFile) {
+  // CUDA 7.0 doesn't have a version.txt, so guess that's our version if

tra wrote:
> What if it's cuda-8, but we've failed to read the file due to permissions.
> Perhaps we should differentiate no-such-file from other errors.
Urgh.

I feel like simpler is probably better.  Like, I would rather say:

- We consider /usr/local/cuda, /usr/local/cuda-8.0, ...
- We pick the first one that has a few key files
- We infer its version by looking for version.txt.  If we can't read it, for 
whatever reason, we assume 7.0.

Than the more complicated

- We infer its version by looking for version.txt.  If it's present but we 
can't read it, we try to look at the file path (/usr/local/cuda-8.0/...).

Especially because as we make things more complicated, our next step will 
probably be to change the algorithm to something like:

- For each directory D in /usr/local/cuda, /usr/local/cuda-8.0, ...
  - Infer D's version using the algorithm above.
  - If D is not compatible with all of the cuda_archs specified, break and try 
the next one...

I do agree that if someone hits a permission problem on version.txt, that would 
be very difficult for them to debug.  I suppose we could raise a proper error 
if version.txt is present but not readable?  I'm not sure it's worth doing 
that, though...


Comment at: lib/Driver/ToolChains.cpp:4711
@@ +4710,3 @@
+  // Check our CUDA version if we're going to include the CUDA headers.
+  if (!DriverArgs.hasArg(options::OPT_nocudainc) &&
+  !DriverArgs.hasArg(options::OPT_nocuda_version_check)) {

tra wrote:
> -nocudainc should not preclude CUDA version check, IMO.
> Primary use of version check is to make sure we can compile for a given GPU 
> architecture. I think we still want to do that even if -nocudainc is passed.
We also do a check if you invoke ptxas.  So the only circumstances under which 
we don't do a check is -nocudainc and no invocation of ptxas.  In which case 
the only thing we're using from the CUDA install is fatbinary, which doesn't 
care about the sm version (afaik).  We could add a version check on the fatbin 
invocation too if you want.  My goal was just that if we don't use anything 
from the CUDA install, we should not check its version.


http://reviews.llvm.org/D21869



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

Re: [PATCH] D21667: [analyzer] Add rudimentary handling of AtomicExpr.

2016-07-07 Thread Devin Coughlin via cfe-commits

dcoughlin updated this revision to Diff 63096.
dcoughlin added a comment.

Fix typo and bad indentation.


http://reviews.llvm.org/D21667

Files:
  include/clang/AST/Expr.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/atomics.c

Index: test/Analysis/atomics.c
===
--- /dev/null
+++ test/Analysis/atomics.c
@@ -0,0 +1,95 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+// Tests for c11 atomics. Many of these tests currently yield unknown
+// because we don't fully model the atomics and instead imprecisely
+// treat their arguments as escaping.
+
+typedef unsigned int uint32_t;
+typedef enum memory_order {
+  memory_order_relaxed = __ATOMIC_RELAXED,
+  memory_order_consume = __ATOMIC_CONSUME,
+  memory_order_acquire = __ATOMIC_ACQUIRE,
+  memory_order_release = __ATOMIC_RELEASE,
+  memory_order_acq_rel = __ATOMIC_ACQ_REL,
+  memory_order_seq_cst = __ATOMIC_SEQ_CST
+} memory_order;
+
+void clang_analyzer_eval(int);
+
+struct RefCountedStruct {
+  uint32_t refCount;
+  void *ptr;
+};
+
+void test_atomic_fetch_add(struct RefCountedStruct *s) {
+  s->refCount = 1;
+
+  uint32_t result = __c11_atomic_fetch_add((volatile _Atomic(uint32_t) *)&s->refCount,- 1, memory_order_relaxed);
+
+  // When we model atomics fully this should (probably) be FALSE. It should never
+  // be TRUE (because the operation mutates the passed in storage).
+  clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}}
+
+  // When fully modeled this should be TRUE
+  clang_analyzer_eval(result == 1); // expected-warning {{UNKNOWN}}
+}
+
+void test_atomic_load(struct RefCountedStruct *s) {
+  s->refCount = 1;
+
+  uint32_t result = __c11_atomic_load((volatile _Atomic(uint32_t) *)&s->refCount, memory_order_relaxed);
+
+  // When we model atomics fully this should (probably) be TRUE.
+  clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}}
+
+  // When fully modeled this should be TRUE
+  clang_analyzer_eval(result == 1); // expected-warning {{UNKNOWN}}
+}
+
+void test_atomic_store(struct RefCountedStruct *s) {
+  s->refCount = 1;
+
+  __c11_atomic_store((volatile _Atomic(uint32_t) *)&s->refCount, 2, memory_order_relaxed);
+
+  // When we model atomics fully this should (probably) be FALSE. It should never
+  // be TRUE (because the operation mutates the passed in storage).
+  clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}}
+}
+
+void test_atomic_exchange(struct RefCountedStruct *s) {
+  s->refCount = 1;
+
+  uint32_t result = __c11_atomic_exchange((volatile _Atomic(uint32_t) *)&s->refCount, 2, memory_order_relaxed);
+
+  // When we model atomics fully this should (probably) be FALSE. It should never
+  // be TRUE (because the operation mutates the passed in storage).
+  clang_analyzer_eval(s->refCount == 1); // expected-warning {{UNKNOWN}}
+
+  // When fully modeled this should be TRUE
+  clang_analyzer_eval(result == 1); // expected-warning {{UNKNOWN}}
+}
+
+
+void test_atomic_compare_exchange_strong(struct RefCountedStruct *s) {
+  s->refCount = 1;
+  uint32_t expected = 2;
+  uint32_t desired = 3;
+  _Bool result = __c11_atomic_compare_exchange_strong((volatile _Atomic(uint32_t) *)&s->refCount, &expected, desired, memory_order_relaxed, memory_order_relaxed);
+
+  // For now we expect both expected and refCount to be invalidated by the
+  // call. In the future we should model more precisely.
+  clang_analyzer_eval(s->refCount == 3); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(expected == 2); // expected-warning {{UNKNOWN}}
+}
+
+void test_atomic_compare_exchange_weak(struct RefCountedStruct *s) {
+  s->refCount = 1;
+  uint32_t expected = 2;
+  uint32_t desired = 3;
+  _Bool result = __c11_atomic_compare_exchange_weak((volatile _Atomic(uint32_t) *)&s->refCount, &expected, desired, memory_order_relaxed, memory_order_relaxed);
+
+  // For now we expect both expected and refCount to be invalidated by the
+  // call. In the future we should model more precisely.
+  clang_analyzer_eval(s->refCount == 3); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(expected == 2); // expected-warning {{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -902,7 +902,6 @@
 case Stmt::CUDAKernelCallExprClass:
 case Stmt::OpaqueValueExprClass:
 case Stmt::AsTypeExprClass:
-case Stmt::AtomicExprClass:
   // Fall through.
 
 // Cases we intentionally don't evaluate, since they don't need
@@ -1247,6 +1246,12 @@
   Bldr.addNodes(Dst);
   break;
 
+case Stmt::AtomicExprClass:
+  Bldr.takeNodes(Pred);
+  VisitAtomicExpr(cast(S), Pred, Dst);
+  Bldr.addNodes(Dst);
+  break;
+
 case Stmt::ObjCIvarRefExprClass:

Re: [PATCH] D21667: [analyzer] Add rudimentary handling of AtomicExpr.

2016-07-07 Thread Devin Coughlin via cfe-commits

dcoughlin marked an inline comment as done.


Comment at: test/Analysis/atomics.c:5
@@ +4,3 @@
+// because we don't fully model the atomics and instead imprecisely
+// treat their arguments as escaping.
+

Thanks! Fixed.


http://reviews.llvm.org/D21667



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

Re: [PATCH] D21667: [analyzer] Add rudimentary handling of AtomicExpr.

2016-07-07 Thread Devin Coughlin via cfe-commits

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

Ping.

Richard: Would you be willing to take a quick look at the change to the AST?


http://reviews.llvm.org/D21667



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

Re: [PATCH] D21869: [CUDA] Check that our CUDA install supports the requested architectures.

2016-07-07 Thread Artem Belevich via cfe-commits

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

LGTM.



Comment at: lib/Driver/ToolChains.cpp:1798
@@ +1797,3 @@
+FS.getBufferForFile(InstallPath + "/version.txt");
+if (!VersionFile) {
+  // CUDA 7.0 doesn't have a version.txt, so guess that's our version if

Good point. It may be worth a separate patch. Falling back to 7.0 on any error 
is OK for now.


Comment at: lib/Driver/ToolChains.cpp:4727
@@ +4726,3 @@
+  // Check our CUDA version if we're going to include the CUDA headers.
+  if (!DriverArgs.hasArg(options::OPT_nocudainc) &&
+  !DriverArgs.hasArg(options::OPT_nocuda_version_check)) {

OK.


http://reviews.llvm.org/D21869



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

Re: [PATCH] D21814: clang-rename: support multiple renames with one invocation

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 added a subscriber: omtcyf0.
omtcyf0 added a comment.

Hi @vmiklos!

Thank you very much for contributing to clang-rename.

The patch looks nice, but it conflicts with my understanding of the view on 
what the tool should do.

Generally, I do not support the idea of adding an option to perform multiple 
renamings at once. Here are my reasons:

- I think the tool should  mainly target editors. Most users won't use command 
line interface IMHO. Thus, performing one renaming action at once seems more 
than logical to me, because you don't do multiple in the editor.

- There's a patch I wish to put for review soon, which is about checking 
-new-name - whether it is a valid identifier or not. This will be the first 
step towards teaching the tool to understand whether the change will break the 
codebase or not (i.e. there will be name conflict, invalid identifier or 
something else). So, we'll have to check each "new-name" for being valid, check 
each "old-name" for being valid, check whether we have the same number of 
"new-name"s and "old-name"s (or offsets) [which you have in your patch], then 
for each pair check whether the renaming action would be OK.

- With this patch we're bringing tokenization of these names into 
ClangRename.cpp, which doesn't seem cool to me. I think the main file is 
supposed to be tool-logic only, not sure if putting much logic-unrelated code 
is good there.

To sum up, my main points are:

- I think we should try to make a good interface, which can be easily reused by 
the editors instead of trying to target CLI. You can try very simple vim 
integration, see latest http://reviews.llvm.org/D22087.

- I believe the tool shouldn't try to do literally everything and get features, 
which require much hardcoding and cornercases handling. Doing only one exact 
thing and doing it very good seems reasonable to me.

Anyway, if everyone is happy about moving in this direction aswell, I'm fine 
too, but these are just my thoughts.

If you want to chat, feel free to drop me an email :)


http://reviews.llvm.org/D21814



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

r274780 - [CUDA] Rename the __nvvm_bar0 builtin back to __syncthreads.

2016-07-07 Thread Justin Lebar via cfe-commits

Author: jlebar
Date: Thu Jul  7 13:15:03 2016
New Revision: 274780

URL: http://llvm.org/viewvc/llvm-project?rev=274780&view=rev
Log:
[CUDA] Rename the __nvvm_bar0 builtin back to __syncthreads.

The builtin was renamed in r274770.  But __syncthreads is part of our
user-facing API, so we need to keep the name as-is.

Patch by Justin Bogner.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def
cfe/trunk/test/CodeGen/builtins-nvptx.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def?rev=274780&r1=274779&r2=274780&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def Thu Jul  7 13:15:03 2016
@@ -393,7 +393,7 @@ BUILTIN(__nvvm_bitcast_d2ll, "LLid", "")
 
 // Sync
 
-BUILTIN(__nvvm_bar0, "v", "")
+BUILTIN(__syncthreads, "v", "")
 BUILTIN(__nvvm_bar0_popc, "ii", "")
 BUILTIN(__nvvm_bar0_and, "ii", "")
 BUILTIN(__nvvm_bar0_or, "ii", "")

Modified: cfe/trunk/test/CodeGen/builtins-nvptx.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-nvptx.c?rev=274780&r1=274779&r2=274780&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-nvptx.c (original)
+++ cfe/trunk/test/CodeGen/builtins-nvptx.c Thu Jul  7 13:15:03 2016
@@ -179,7 +179,7 @@ __device__ void nvvm_math(float f1, floa
 // CHECK: call void @llvm.nvvm.membar.sys()
   __nvvm_membar_sys();
 // CHECK: call void @llvm.nvvm.barrier0()
-  __nvvm_bar0();
+  __syncthreads();
 }
 
 __device__ int di;


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

r274781 - [CUDA] Check that our CUDA install supports the requested architectures.

2016-07-07 Thread Justin Lebar via cfe-commits

Author: jlebar
Date: Thu Jul  7 13:17:52 2016
New Revision: 274781

URL: http://llvm.org/viewvc/llvm-project?rev=274781&view=rev
Log:
[CUDA] Check that our CUDA install supports the requested architectures.

Summary:
Raise an error if you're using a CUDA installation that's too old for
the requested architectures.  In practice, this means that you need a
CUDA 8 install to compile for sm_6*.

Reviewers: tra

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D21869

Added:
cfe/trunk/test/Driver/Inputs/CUDA_80/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/bin/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/bin/.keep
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/include/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/include/.keep
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/lib/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/lib/.keep
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/lib64/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/lib64/.keep
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/

cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_20.10.bc

cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_35.10.bc
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/version.txt
cfe/trunk/test/Driver/cuda-version-check.cu
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=274781&r1=274780&r2=274781&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Jul  7 13:17:52 
2016
@@ -26,6 +26,10 @@ def err_drv_cuda_bad_gpu_arch : Error<"U
 def err_drv_no_cuda_installation : Error<
   "cannot find CUDA installation.  Provide its path via --cuda-path, or pass "
   "-nocudainc to build without CUDA includes.">;
+def err_drv_cuda_version_too_low : Error<
+  "GPU arch %1 requires CUDA version at least %3, but installation at %0 is 
%2. "
+  "Use --cuda-path to specify a different CUDA install, or pass "
+  "--no-cuda-version-check.">;
 def err_drv_invalid_thread_model_for_target : Error<
   "invalid thread model '%0' in '%1' for this target">;
 def err_drv_invalid_linker_name : Error<

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=274781&r1=274780&r2=274781&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Jul  7 13:17:52 2016
@@ -410,6 +410,9 @@ def cuda_gpu_arch_EQ : Joined<["--"], "c
   HelpText<"CUDA GPU architecture (e.g. sm_35).  May be specified more than 
once.">;
 def cuda_noopt_device_debug : Flag<["--"], "cuda-noopt-device-debug">,
   HelpText<"Enable device-side debug info generation. Disables ptxas 
optimizations.">;
+def no_cuda_version_check : Flag<["--"], "no-cuda-version-check">,
+  HelpText<"Don't error out if the detected version of the CUDA install is "
+   "too low for the requested CUDA gpu architecture.">;
 def no_cuda_noopt_device_debug : Flag<["--"], "no-cuda-noopt-device-debug">;
 def cuda_path_EQ : Joined<["--"], "cuda-path=">, Group,
   HelpText<"CUDA installation path">;

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=274781&r1=274780&r2=274781&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Jul  7 13:17:52 2016
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "ToolChains.h"
+#include "clang/Basic/Cuda.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
 #include "clang/Basic/VirtualFileSystem.h"
@@ -1703,9 +1704,33 @@ bool Generic_GCC::GCCInstallationDetecto
 BiarchTripleAliases.push_back(BiarchTriple.str());
 }
 
+// Parses the contents of version.txt in an CUDA installation.  It should
+// contain one line of the from e.g. "CUDA Version 7.5.2".
+static CudaVersion ParseCudaVersionFile(llvm::StringRef V) {
+  if (!V.startswith("

Re: [PATCH] D21869: [CUDA] Check that our CUDA install supports the requested architectures.

2016-07-07 Thread Justin Lebar via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL274781: [CUDA] Check that our CUDA install supports the 
requested architectures. (authored by jlebar).

Changed prior to commit:
  http://reviews.llvm.org/D21869?vs=63094&id=63100#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D21869

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains.cpp
  cfe/trunk/lib/Driver/ToolChains.h
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/bin/.keep
  cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/include/.keep
  cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/lib/.keep
  cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/lib64/.keep
  
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_20.10.bc
  
cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/nvvm/libdevice/libdevice.compute_35.10.bc
  cfe/trunk/test/Driver/Inputs/CUDA_80/usr/local/cuda/version.txt
  cfe/trunk/test/Driver/cuda-version-check.cu

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -410,6 +410,9 @@
   HelpText<"CUDA GPU architecture (e.g. sm_35).  May be specified more than once.">;
 def cuda_noopt_device_debug : Flag<["--"], "cuda-noopt-device-debug">,
   HelpText<"Enable device-side debug info generation. Disables ptxas optimizations.">;
+def no_cuda_version_check : Flag<["--"], "no-cuda-version-check">,
+  HelpText<"Don't error out if the detected version of the CUDA install is "
+   "too low for the requested CUDA gpu architecture.">;
 def no_cuda_noopt_device_debug : Flag<["--"], "no-cuda-noopt-device-debug">;
 def cuda_path_EQ : Joined<["--"], "cuda-path=">, Group,
   HelpText<"CUDA installation path">;
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -26,6 +26,10 @@
 def err_drv_no_cuda_installation : Error<
   "cannot find CUDA installation.  Provide its path via --cuda-path, or pass "
   "-nocudainc to build without CUDA includes.">;
+def err_drv_cuda_version_too_low : Error<
+  "GPU arch %1 requires CUDA version at least %3, but installation at %0 is %2. "
+  "Use --cuda-path to specify a different CUDA install, or pass "
+  "--no-cuda-version-check.">;
 def err_drv_invalid_thread_model_for_target : Error<
   "invalid thread model '%0' in '%1' for this target">;
 def err_drv_invalid_linker_name : Error<
Index: cfe/trunk/test/Driver/cuda-version-check.cu
===
--- cfe/trunk/test/Driver/cuda-version-check.cu
+++ cfe/trunk/test/Driver/cuda-version-check.cu
@@ -0,0 +1,51 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+// RUN: %clang -v -### --cuda-gpu-arch=sm_20 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA_80 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+
+// The installation at Inputs/CUDA is CUDA 7.0, which doesn't support sm_60.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60
+
+// This should only complain about sm_60, not sm_35.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_35 \
+// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=OK_SM35
+
+// We should get two errors here, one for sm_60 and one for sm_61.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --cuda-gpu-arch=sm_61 \
+// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60 --check-prefix=ERR_SM61
+
+// We should still get an error if we pass -nocudainc, because this compilation
+// would invoke ptxas, and we do a version check on that, too.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 -nocudainc --sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=ERR_SM60
+
+// If with -nocudainc and -E, we don't touch the CUDA install, so we
+// shouldn't get an error.
+// RUN: %clang -v -### -E --cuda-device-only --cuda-gpu-arch=sm_60 -nocudainc \
+// RUN:--sysroot=%S/Inputs/CUDA 2>&1 %s | \
+// RUN:FileCheck %s --check-prefix=OK
+
+// -nocuda-version-check should suppress all of these errors.
+// RUN: %clang -v -### --cuda-gpu-arch=sm_60 --sysroot=%S/Inputs/CUDA 2>&1 \
+// RUN:-nocuda-version-check %s | \
+// RUN:File

Re: [PATCH] D21912: [CUDA] Don't assume that destructors can't be overloaded.

2016-07-07 Thread Justin Lebar via cfe-commits

jlebar added a comment.

Friendly ping


http://reviews.llvm.org/D21912



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

Re: [PATCH] D20196: [clang-tidy] Inefficient string operation

2016-07-07 Thread Bittner Barni via cfe-commits

bittnerbarni updated this revision to Diff 63098.
bittnerbarni added a comment.

Thank you, for your valuable comments Alexander!


http://reviews.llvm.org/D20196

Files:
  clang-tidy/performance/CMakeLists.txt
  clang-tidy/performance/InefficientStringConcatenationCheck.cpp
  clang-tidy/performance/InefficientStringConcatenationCheck.h
  clang-tidy/performance/PerformanceTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
  test/clang-tidy/performance-inefficient-string-concatenation.cpp

Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t
+
+namespace std {
+template 
+class basic_string {
+public:
+  basic_string() {}
+  ~basic_string() {}
+  basic_string *operator+=(const basic_string &) {}
+  friend basic_string operator+(const basic_string &, const basic_string &) {}
+};
+typedef basic_string string;
+typedef basic_string wstring;
+}
+
+void f(std::string) {}
+std::string g(std::string) {}
+
+int main() {
+  std::string mystr1, mystr2;
+  std::wstring mywstr1, mywstr2;
+
+  for (int i = 0; i < 10; ++i) {
+f(mystr1 + mystr2 + mystr1);
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead
+mystr1 = mystr1 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
+mystr1 = mystr2 + mystr2 + mystr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation
+mystr1 = mystr2 + mystr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
+mywstr1 = mywstr2 + mywstr1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
+mywstr1 = mywstr2 + mywstr2 + mywstr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation
+
+mywstr1 = mywstr2 + mywstr2;
+mystr1 = mystr2 + mystr2;
+mystr1 += mystr2;
+f(mystr2 + mystr1);
+mystr1 = g(mystr1);
+  }
+  return 0;
+}
Index: docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/performance-inefficient-string-concatenation.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - performance-inefficient-string-concatenation
+
+performance-inefficient-string-concatenation
+
+
+This check warns about the performance overhead arising from concatenating strings using the ``operator+``, for instance:
+
+.. code:: c++
+
+std::string a("Foo"), b("Bar");
+a = a + b;
+
+Instead of this structure you should use ``operator+=`` or ``std::string``'s (``std::basic_string``) class member function ``append()``. For instance:
+   
+.. code:: c++
+
+   std::string a("Foo"), b("Baz");
+   for (int i = 0; i < 2; ++i) {
+   a = a + "Bar" + b;
+   }
+
+Could be rewritten in a greatly more efficient way like:
+
+.. code:: c++
+
+   std::string a("Foo"), b("Baz");
+   for (int i = 0; i < 2; ++i) {
+   a.append("Bar").append(b);
+   } 
+
+And this can be rewritten too:
+
+.. code:: c++
+
+   void f(const std::string&) {}
+   std::string a("Foo"), b("Baz");
+   void g() {
+   f(a + "Bar" + b);
+   }
+
+In a slightly more efficient way like:
+
+.. code:: c++
+
+   void f(const std::string&) {}
+   std::string a("Foo"), b("Baz");
+   void g() {
+   f(std::string(a).append("Bar").append(b));
+   }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -102,6 +102,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+   performance-inefficient-string-concatenation
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
readability-avoid-const-params-in-decls
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -184,6 +184,12 @@
   Warns about range-based loop with a loop variable of const ref type where the
   type of the variable does not match the one returned by the iterator.
 
+- New `performance-inefficient-string-concatenation
+  `_ check
+
+  This check is to warn about the performance overhead arising from concatenating 
+  strings, using the ``operator+``, instead of ``operator+=``.
+  
 - New `performance-unnecessary-value-param

Re: [PATCH] D22087: [clang-rename] add basic vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 added a comment.

@kimgr oops, sorry.

I'll send a cleanup patch.

Thanks for noticing.


Repository:
  rL LLVM

http://reviews.llvm.org/D22087



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

[PATCH] D22100: [clang-rename] fix typo in Python script for Vim integration

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 created this revision.
omtcyf0 added reviewers: kimgr, alexfh, bkramer, hokein, ioeric.
omtcyf0 added a subscriber: cfe-commits.

http://reviews.llvm.org/D22100

Files:
  clang-rename/tool/clang-rename.py

Index: clang-rename/tool/clang-rename.py
===
--- clang-rename/tool/clang-rename.py
+++ clang-rename/tool/clang-rename.py
@@ -14,8 +14,8 @@
 IMPORTANT NOTE: Before running the tool, make sure you saved the file.
 
 All you have to do now is to place a cursor on a variable/function/class which
-you would like to rename and press ',cr'. You will be promted a new name if the
-cursor points to a valid symbol.
+you would like to rename and press ',cr'. You will be prompted for a new name 
if
+the cursor points to a valid symbol.
 '''
 
 import vim


Index: clang-rename/tool/clang-rename.py
===
--- clang-rename/tool/clang-rename.py
+++ clang-rename/tool/clang-rename.py
@@ -14,8 +14,8 @@
 IMPORTANT NOTE: Before running the tool, make sure you saved the file.
 
 All you have to do now is to place a cursor on a variable/function/class which
-you would like to rename and press ',cr'. You will be promted a new name if the
-cursor points to a valid symbol.
+you would like to rename and press ',cr'. You will be prompted for a new name if
+the cursor points to a valid symbol.
 '''
 
 import vim
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r274782 - [CUDA] s/OPT_nocuda_version_chec/OPT_no_cuda_version_check/.

2016-07-07 Thread Justin Lebar via cfe-commits

Author: jlebar
Date: Thu Jul  7 13:24:28 2016
New Revision: 274782

URL: http://llvm.org/viewvc/llvm-project?rev=274782&view=rev
Log:
[CUDA] s/OPT_nocuda_version_chec/OPT_no_cuda_version_check/.

Fix build breakage.

Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=274782&r1=274781&r2=274782&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Jul  7 13:24:28 2016
@@ -4725,7 +4725,7 @@ void CudaToolChain::AddCudaIncludeArgs(c
ArgStringList &CC1Args) const {
   // Check our CUDA version if we're going to include the CUDA headers.
   if (!DriverArgs.hasArg(options::OPT_nocudainc) &&
-  !DriverArgs.hasArg(options::OPT_nocuda_version_check)) {
+  !DriverArgs.hasArg(options::OPT_no_cuda_version_check)) {
 StringRef Arch = DriverArgs.getLastArgValue(options::OPT_march_EQ);
 assert(!Arch.empty() && "Must have an explicit GPU arch.");
 CudaInstallation.CheckCudaVersionSupportsArch(StringToCudaArch(Arch));

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=274782&r1=274781&r2=274782&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Thu Jul  7 13:24:28 2016
@@ -11147,7 +11147,7 @@ void NVPTX::Assembler::ConstructJob(Comp
   const std::string& gpu_arch = gpu_archs[0];
 
   // Check that our installation's ptxas supports gpu_arch.
-  if (!Args.hasArg(options::OPT_nocuda_version_check)) {
+  if (!Args.hasArg(options::OPT_no_cuda_version_check)) {
 TC.cudaInstallation().CheckCudaVersionSupportsArch(
 StringToCudaArch(gpu_arch));
   }


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

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Reid Kleckner via cfe-commits

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

Looks good from a driver perspective. Aaron, you're happy with the attribute 
spelling stuff, right?

It looks like you can actually land this before landing the two dependent 
changes you mention and the tests will pass. I'd go ahead and land it to avoid 
the need to rebase, but it's up to you.



Comment at: include/clang/Basic/AttrDocs.td:2459
@@ +2458,3 @@
+
+Conversely, ``__attribute__((xray_never_instrument))`` or 
``[[clang:xray_never_instrument]]`` will inhibit the insertion of these 
instrumentation points.
+

These paragraphs can be wrapped to 80 cols like the others.


http://reviews.llvm.org/D20352



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

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Aaron Ballman via cfe-commits

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

In http://reviews.llvm.org/D20352#477038, @rnk wrote:

> Looks good from a driver perspective. Aaron, you're happy with the attribute 
> spelling stuff, right?

No, I'm not. I am worried about how this conflicts with another in-flight patch 
for supporting MS hotpatchable functions since it seems these two attributes do 
roughly the same thing. I'd like to understand how these two user-facing 
attributes will not be confusing to users, what should happen if both 
attributes wind up on a function, etc. I am hoping that we can wind up with 
only one attribute that covers both cases, if possible.

http://reviews.llvm.org/D20352

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

[PATCH] D22102: [clang-rename] extend testset

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 created this revision.
omtcyf0 added reviewers: alexfh, klimek, bkramer, ioeric.
omtcyf0 added a subscriber: cfe-commits.

This patch introduces few additional tests including one case the tool does not 
handle yet, which should be fixed in the future.

http://reviews.llvm.org/D22102

Files:
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/VariableMacro.cpp

Index: test/clang-rename/VariableMacro.cpp
===
--- /dev/null
+++ test/clang-rename/VariableMacro.cpp
@@ -0,0 +1,18 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+#define Y X // CHECK: #define Y Z
+
+void foo(int value) {}
+
+void macro() {
+  int X;// CHECK: int Z;
+  X = 42;   // CHECK: Z = 42;
+  Y -= 0;
+  foo(X);   // CHECK: foo(Z);
+  foo(Y);
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/TemplateTypename.cpp
===
--- /dev/null
+++ test/clang-rename/TemplateTypename.cpp
@@ -0,0 +1,16 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=152 -new-name=U %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+// XFAIL: *
+
+template 
+T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
+  T value;// CHECK: U value;
+  int number = 42;
+  value = (T)number;  // CHECK: value = (U)number;
+  value = static_cast(number); // CHECK: value = static_cast(number);
+  return value;
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/Namespace.cpp
===
--- /dev/null
+++ test/clang-rename/Namespace.cpp
@@ -0,0 +1,14 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=143 -new-name=llvm %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+namespace foo { // CHECK: namespace llvm {
+  int x;
+}
+
+void boo() {
+  foo::x = 42;  // CHECK: llvm::x = 42;
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/FunctionMacro.cpp
===
--- /dev/null
+++ test/clang-rename/FunctionMacro.cpp
@@ -0,0 +1,21 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=199 -new-name=macro_function %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+#define moo foo // CHECK: #define moo macro_function
+
+int foo() { // CHECK: int macro_function() {
+  return 42;
+}
+
+void boo(int value) {}
+
+void qoo() {
+  foo();// CHECK: macro_function();
+  boo(foo());   // CHECK: boo(macro_function());
+  moo();
+  boo(moo());
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.


Index: test/clang-rename/VariableMacro.cpp
===
--- /dev/null
+++ test/clang-rename/VariableMacro.cpp
@@ -0,0 +1,18 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+#define Y X // CHECK: #define Y Z
+
+void foo(int value) {}
+
+void macro() {
+  int X;// CHECK: int Z;
+  X = 42;   // CHECK: Z = 42;
+  Y -= 0;
+  foo(X);   // CHECK: foo(Z);
+  foo(Y);
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/TemplateTypename.cpp
===
--- /dev/null
+++ test/clang-rename/TemplateTypename.cpp
@@ -0,0 +1,16 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=152 -new-name=U %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+// XFAIL: *
+
+template 
+T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
+  T value;// CHECK: U value;
+  int number = 42;
+  value = (T)number;  // CHECK: value = (U)number;
+  value = static_cast(number); // CHECK: value = static_cast(number);
+  return value;
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/Namespace.cpp
===
--- /dev/null
+++ test/clang-rename/Namespace.cpp
@@ -0,0 +1,14 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=143 -new-name=llvm %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+namespace foo { // CHECK: namespace llvm {
+  int x;
+}
+
+void boo() {
+  foo::x = 42;  // CHECK: llvm::x = 42;
+}
+
+// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
+// this file.
Index: test/clang-rename/FunctionMacro.cpp
===
--- /dev/nu

[PATCH] D22105: [X86][SSE] Reimplement SSE fp2si conversion intrinsics instead of using generic IR

2016-07-07 Thread Simon Pilgrim via cfe-commits

RKSimon created this revision.
RKSimon added reviewers: eli.friedman, mkuper, craig.topper, spatel, andreadb.
RKSimon added a subscriber: cfe-commits.
RKSimon set the repository for this revision to rL LLVM.

D20859 and D20860 attempted to replace the SSE (V)CVTTPS2DQ and VCVTTPD2DQ 
truncating conversions with generic IR instead. 

It turns out that the behaviour of these intrinsics is different enough from 
generic IR that this will cause problems, INF/NAN/out of range values are 
guaranteed to result in a 0x8000 value - which plays havoc with constant 
folding which converts them to either zero or UNDEF. This is also an issue with 
the scalar implementations (which were already generic IR and what I was trying 
to match).

This patch changes both scalar and packed versions back to using x86-specific 
builtins.

It also deals with the other scalar conversion cases that are runtime rounding 
mode dependent and can have similar issues with constant folding.

A companion llvm patch will be submitted shortly.

Repository:
  rL LLVM

http://reviews.llvm.org/D22105

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/Headers/avxintrin.h
  lib/Headers/emmintrin.h
  lib/Headers/xmmintrin.h
  test/CodeGen/avx-builtins.c
  test/CodeGen/avx512f-builtins.c
  test/CodeGen/builtins-x86.c
  test/CodeGen/sse-builtins.c
  test/CodeGen/sse2-builtins.c

Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -507,7 +507,7 @@
 
 __m128 test_mm_cvtsd_ss(__m128 A, __m128d B) {
   // CHECK-LABEL: test_mm_cvtsd_ss
-  // CHECK: fptrunc double %{{.*}} to float
+  // CHECK: call <4 x float> @llvm.x86.sse2.cvtsd2ss(<4 x float> %{{.*}}, <2 x double> %{{.*}})
   return _mm_cvtsd_ss(A, B);
 }
 
@@ -541,8 +541,7 @@
 
 __m128d test_mm_cvtsi64_sd(__m128d A, long long B) {
   // CHECK-LABEL: test_mm_cvtsi64_sd
-  // CHECK: sitofp i64 %{{.*}} to double
-  // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i32 0
+  // CHECK: call <2 x double> @llvm.x86.sse2.cvtsi642sd(<2 x double> %{{.*}}, i64 %{{.*}})
   return _mm_cvtsi64_sd(A, B);
 }
 
@@ -569,21 +568,19 @@
 
 __m128i test_mm_cvttps_epi32(__m128 A) {
   // CHECK-LABEL: test_mm_cvttps_epi32
-  // CHECK: fptosi <4 x float> %{{.*}} to <4 x i32>
+  // CHECK: call <4 x i32> @llvm.x86.sse2.cvttps2dq(<4 x float> %{{.*}})
   return _mm_cvttps_epi32(A);
 }
 
 int test_mm_cvttsd_si32(__m128d A) {
   // CHECK-LABEL: test_mm_cvttsd_si32
-  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
-  // CHECK: fptosi double %{{.*}} to i32
+  // CHECK: call i32 @llvm.x86.sse2.cvttsd2si(<2 x double> %{{.*}})
   return _mm_cvttsd_si32(A);
 }
 
 long long test_mm_cvttsd_si64(__m128d A) {
   // CHECK-LABEL: test_mm_cvttsd_si64
-  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
-  // CHECK: fptosi double %{{.*}} to i64
+  // CHECK: call i64 @llvm.x86.sse2.cvttsd2si64(<2 x double> %{{.*}})
   return _mm_cvttsd_si64(A);
 }
 
Index: test/CodeGen/sse-builtins.c
===
--- test/CodeGen/sse-builtins.c
+++ test/CodeGen/sse-builtins.c
@@ -263,15 +263,13 @@
 
 __m128 test_mm_cvtsi32_ss(__m128 A, int B) {
   // CHECK-LABEL: test_mm_cvtsi32_ss
-  // CHECK: sitofp i32 %{{.*}} to float
-  // CHECK: insertelement <4 x float> %{{.*}}, float %{{.*}}, i32 0
+  // CHECK: call <4 x float> @llvm.x86.sse.cvtsi2ss(<4 x float> %{{.*}}, i32 %{{.*}})
   return _mm_cvtsi32_ss(A, B);
 }
 
 __m128 test_mm_cvtsi64_ss(__m128 A, long long B) {
   // CHECK-LABEL: test_mm_cvtsi64_ss
-  // CHECK: sitofp i64 %{{.*}} to float
-  // CHECK: insertelement <4 x float> %{{.*}}, float %{{.*}}, i32 0
+  // CHECK: call <4 x float> @llvm.x86.sse.cvtsi642ss(<4 x float> %{{.*}}, i64 %{{.*}})
   return _mm_cvtsi64_ss(A, B);
 }
 
@@ -295,22 +293,19 @@
 
 int test_mm_cvtt_ss2si(__m128 A) {
   // CHECK-LABEL: test_mm_cvtt_ss2si
-  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
-  // CHECK: fptosi float %{{.*}} to i32
+  // CHECK: call i32 @llvm.x86.sse.cvttss2si(<4 x float> %{{.*}})
   return _mm_cvtt_ss2si(A);
 }
 
 int test_mm_cvttss_si32(__m128 A) {
   // CHECK-LABEL: test_mm_cvttss_si32
-  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
-  // CHECK: fptosi float %{{.*}} to i32
+  // CHECK: call i32 @llvm.x86.sse.cvttss2si(<4 x float> %{{.*}})
   return _mm_cvttss_si32(A);
 }
 
 long long test_mm_cvttss_si64(__m128 A) {
   // CHECK-LABEL: test_mm_cvttss_si64
-  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
-  // CHECK: fptosi float %{{.*}} to i64
+  // CHECK: call i64 @llvm.x86.sse.cvttss2si64(<4 x float> %{{.*}})
   return _mm_cvttss_si64(A);
 }
 
Index: test/CodeGen/builtins-x86.c
===
--- test/CodeGen/builtins-x86.c
+++ test/CodeGen/builtins-x86.c
@@ -286,13 +286,17 @@
 
   tmp_V4f = __builtin_ia32_cvtpi2ps(tmp_V4f, tmp_V2i);
   tmp_V2i = __builtin_ia32_cvtps2pi(tmp_V4f);
+  tmp_V4f =

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Reid Kleckner via cfe-commits

rnk added a comment.

In http://reviews.llvm.org/D20352#477084, @aaron.ballman wrote:

> No, I'm not. I am worried about how this conflicts with another in-flight 
> patch for supporting MS hotpatchable functions since it seems these two 
> attributes do roughly the same thing. I'd like to understand how these two 
> user-facing attributes will not be confusing to users, what should happen if 
> both attributes wind up on a function, etc. I am hoping that we can wind up 
> with only one attribute that covers both cases, if possible.

From a user perspective, I would definitely want to keep these attributes 
separate. It's only possible to completely replace an MS hotpatchable function, 
whereas XRay allows you to instrument function entry and exit with low 
overhead. You can achieve the same effect as XRay with MS hotpatchable 
prologues and a full trampoline with a stack frame, but probably not at the 
same runtime cost. They really are suited for different tasks.

http://reviews.llvm.org/D20352

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

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Eric Christopher via cfe-commits

echristo added a comment.

In http://reviews.llvm.org/D20352#477203, @rnk wrote:

> In http://reviews.llvm.org/D20352#477084, @aaron.ballman wrote:
>
> > No, I'm not. I am worried about how this conflicts with another in-flight 
> > patch for supporting MS hotpatchable functions since it seems these two 
> > attributes do roughly the same thing. I'd like to understand how these two 
> > user-facing attributes will not be confusing to users, what should happen 
> > if both attributes wind up on a function, etc. I am hoping that we can wind 
> > up with only one attribute that covers both cases, if possible.
>
>
> From a user perspective, I would definitely want to keep these attributes 
> separate. It's only possible to completely replace an MS hotpatchable 
> function, whereas XRay allows you to instrument function entry and exit with 
> low overhead. You can achieve the same effect as XRay with MS hotpatchable 
> prologues and a full trampoline with a stack frame, but probably not at the 
> same runtime cost. They really are suited for different tasks.


Agreed.

-eric


http://reviews.llvm.org/D20352



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

Re: [PATCH] D20352: Add XRay flags to Clang

2016-07-07 Thread Aaron Ballman via cfe-commits

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

In http://reviews.llvm.org/D20352#477203, @rnk wrote:

> In http://reviews.llvm.org/D20352#477084, @aaron.ballman wrote:
>
> > No, I'm not. I am worried about how this conflicts with another in-flight 
> > patch for supporting MS hotpatchable functions since it seems these two 
> > attributes do roughly the same thing. I'd like to understand how these two 
> > user-facing attributes will not be confusing to users, what should happen 
> > if both attributes wind up on a function, etc. I am hoping that we can wind 
> > up with only one attribute that covers both cases, if possible.
>
>
> From a user perspective, I would definitely want to keep these attributes 
> separate. It's only possible to completely replace an MS hotpatchable 
> function, whereas XRay allows you to instrument function entry and exit with 
> low overhead. You can achieve the same effect as XRay with MS hotpatchable 
> prologues and a full trampoline with a stack frame, but probably not at the 
> same runtime cost. They really are suited for different tasks.

Okay, that was the key piece I was missing -- I was under the impression that 
this was two different ways to achieve the same task. Thank you for the 
explanation (and sorry if I was being dense). :-)

When the MS hot patching attribute lands, that will be the time to discuss and 
document conflicts between the two (I am assuming you can't enable XRay and MS 
hot patching for the same function, which may be an incorrect assumption). So I 
think this LGTM now.

http://reviews.llvm.org/D20352

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

Re: r274748 - Fix "not all control paths return a value" warning on MSVC

2016-07-07 Thread Sean Silva via cfe-commits

Btw we do have some advice for these situations in the programmer's manual:
http://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations

-- Sean Silva

On Thu, Jul 7, 2016 at 4:24 AM, Simon Pilgrim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rksimon
> Date: Thu Jul  7 06:24:38 2016
> New Revision: 274748
>
> URL: http://llvm.org/viewvc/llvm-project?rev=274748&view=rev
> Log:
> Fix "not all control paths return a value" warning on MSVC
>
> This time without causing a 'all enums handled' warning on other compilers.
>
> Modified:
> cfe/trunk/lib/Basic/Targets.cpp
>
> Modified: cfe/trunk/lib/Basic/Targets.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=274748&r1=274747&r2=274748&view=diff
>
> ==
> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> +++ cfe/trunk/lib/Basic/Targets.cpp Thu Jul  7 06:24:38 2016
> @@ -1777,7 +1777,7 @@ public:
>// Set __CUDA_ARCH__ for the GPU specified.
>std::string CUDAArchCode = [this] {
>  switch (GPU) {
> -case CudaArch::UNKNOWN:
> +default:
>assert(false && "No GPU arch when compiling CUDA device code.");
>return "";
>  case CudaArch::SM_20:
> @@ -1804,8 +1804,6 @@ public:
>return "610";
>  case CudaArch::SM_62:
>return "620";
> - default:
> -   llvm_unreachable("unhandled CudaArch");
>  }
>}();
>Builder.defineMacro("__CUDA_ARCH__", CUDAArchCode);
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r274791 - [AArch64] Change the preferred alignment for char and short.

2016-07-07 Thread Chad Rosier via cfe-commits

Author: mcrosier
Date: Thu Jul  7 15:02:25 2016
New Revision: 274791

URL: http://llvm.org/viewvc/llvm-project?rev=274791&view=rev
Log:
[AArch64] Change the preferred alignment for char and short.

This reinstates commits r273280 and r273289.

Original Review: http://reviews.llvm.org/D21414.

Modified:
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/test/CodeGen/aarch64-type-sizes.c
cfe/trunk/test/CodeGen/target-data.c

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=274791&r1=274790&r2=274791&view=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Jul  7 15:02:25 2016
@@ -5994,7 +5994,7 @@ class AArch64leTargetInfo : public AArch
 if (getTriple().isOSBinFormatMachO())
   resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128");
 else
-  resetDataLayout("e-m:e-i64:64-i128:128-n32:64-S128");
+  resetDataLayout("e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
   }
 
 public:
@@ -6012,7 +6012,7 @@ public:
 class AArch64beTargetInfo : public AArch64TargetInfo {
   void setDataLayout() override {
 assert(!getTriple().isOSBinFormatMachO());
-resetDataLayout("E-m:e-i64:64-i128:128-n32:64-S128");
+resetDataLayout("E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
   }
 
 public:

Modified: cfe/trunk/test/CodeGen/aarch64-type-sizes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/aarch64-type-sizes.c?rev=274791&r1=274790&r2=274791&view=diff
==
--- cfe/trunk/test/CodeGen/aarch64-type-sizes.c (original)
+++ cfe/trunk/test/CodeGen/aarch64-type-sizes.c Thu Jul  7 15:02:25 2016
@@ -1,8 +1,7 @@
-// RUN: %clang_cc1 -triple aarch64_be-none-linux-gnu -emit-llvm -w -o - %s | 
FileCheck --check-prefix=CHECK --check-prefix=CHECK-BE %s
+// RUN: %clang_cc1 -triple aarch64_be-none-linux-gnu -emit-llvm -w -o - %s | 
FileCheck --check-prefix=CHECK %s
 // char by definition has size 1
 
-// CHECK-LE: target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
-// CHECK-BE: target datalayout = "E-m:e-i64:64-i128:128-n32:64-S128"
+// CHECK: target datalayout = 
"E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 
 int check_short() {
   return sizeof(short);
@@ -89,4 +88,3 @@ int foo() {
   return sizeof(enum Small);
 // CHECK: ret i32 4
 }
-

Modified: cfe/trunk/test/CodeGen/target-data.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-data.c?rev=274791&r1=274790&r2=274791&view=diff
==
--- cfe/trunk/test/CodeGen/target-data.c (original)
+++ cfe/trunk/test/CodeGen/target-data.c Thu Jul  7 15:02:25 2016
@@ -141,7 +141,7 @@
 
 // RUN: %clang_cc1 -triple arm64-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=AARCH64
-// AARCH64: target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
+// AARCH64: target datalayout = 
"e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 
 // RUN: %clang_cc1 -triple thumb-unknown-gnueabi -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=THUMB


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

Re: [PATCH] D21814: clang-rename: support multiple renames with one invocation

2016-07-07 Thread Miklos Vajna via cfe-commits

vmiklos added a comment.

Kirill: OK, so you're in the camp marked as b) by Manuel. Sure, the vim 
integration is nice (I'm also a vim user), now that you mentioned it, I need to 
go and try it myself. ;-) Given the above patch, probably it's obvious that I'm 
more in camp a). I don't insist on having that in a tool named `clang-rename`, 
though.

Hmm, so here is the summary of the comments so far, as far as I understand:

- Manuel: it's OK to handle multiple renames with a single command-line 
invocation, but not sure if clang-rename should do that, or if it should be a 
separate tool
- Benjamin: no problem with clang-rename having this feature
- Kirill: clang-rename should definitely not have this feature.

What's the consensus, should I rework this patch, so that it doesn't touch 
`tool/ClangRename.cpp`, but adds a new tool (named e.g. `clang-multi-rename`, 
still linking to `clangRename`)? I can do that, but it would be good to hear 
first if it is worth the effort, or there would be still "the patch is correct, 
but it's not the way to go" style comments. :-)

Thanks.


http://reviews.llvm.org/D21814



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

Re: [PATCH] D21814: clang-rename: support multiple renames with one invocation

2016-07-07 Thread Kirill Bobyrev via cfe-commits

omtcyf0 added a comment.

Miklos, thanks for the feedback!

Hm, I'm not sure about a) and b) camps here. I think we can have both. It may 
be that I haven't looked too much into the code or I am missing something, but 
so far both integration and cross-TU analysis seem OK together in one tool as 
for me.

So far, integration (in it's very very simple way) is very straightforward: we 
just have a binary and we call it via passing two parameters (offset and 
new-name). This doesn't require neither changes in the tool as it is nor any 
difficult tweaks.

As for cross-TU stuff, why can't it be here, too? We'll still need it at some 
point.

Creating `clang-multi-rename` doesn't seem right at all - there are already too 
many tools which some users don't understand. If the command line interface 
with this feature is a useful thing to have, then we should definitely just add 
it here. One issue I see is that I think it won't be useful to most users, but 
a) I can be wrong b) It's not the main one :)

Probably my main point is: **let's not bring too much features into the 
interface we don't know anything about yet**. It feels right to me to ensure 
that //the tool actually works correctly//. Because it does not at the moment 
(see http://reviews.llvm.org/D22102 XFAIL test for example).

After we do that, we will have the feedback from the community, because they 
will try the first version, which actually works. We might get at least few 
people just trying it at least and saying whether for example it's not usable 
at all in the editor and that they would like an extended CLI. I'll be all for 
the changes similar to these proposed by you then!

In my opinion we should (right now) only support a single use scenario: 
`clang-rename -offset=[OFFSET] -new-name=[NAME]`. Not only because we would 
keep things easy (which I'd honestly be very happy about, but, I know, people 
hate me for that :D), but because when we get our first users, chances of them 
finding some configuration of parameters which breaks than simples single 
scenario is very high, not talking about the multiple scenarios you're 
proposing. Why would we need many ways to interact with an incorrect tool? We 
should probably have a single way to interact with a completely correct tool.

At the end I think we can both be happy if we separate the CLI and the renaming 
interface completely. The editors can interact with a C++ Library then and 
`clang-rename` would become a (probably) sophisticated command line interface 
to that library, only dealing with user-library interaction thingie. We can't 
have both at the moment, of that I am certain.

My proposal is to:

- Improve Vim interface just a little, I think I can do that tomorrow.
- Write a letter to the cfe-dev, explaining that we want `clang-rename` to 
become useful and that we want their feedback. I'll send it tomorrow, right 
after I'll be done with the Vim interface slight improvements. We'll probably 
get more interesting ideas and, most importantly, at least few first users, who 
will help us to understand what they need. Discussions on reviews get lost and 
forgotten, cfe-dev is always there and living.
- Get a proper test set for clang-rename. Really. Because now it's not tested 
at all. Fill in bugs (hopefully the guys who will try it at least will help), 
get suggestions, get opinions. Probably make a list/plan of them.
- Address the issues and then think of the additional stuff we can add (like 
separating the interface and routine and all that).

As I'm going to invest very much time into the project I hope we can do that!

I'll be very happy to hear more opinions from the others both here and in the 
cfe-dev list after I send the message!

@klimek, @bkramer, what do you think of this?


http://reviews.llvm.org/D21814



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

1 2 >

1 - 100 of 133 matches

Mail list logo