date:20191112

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

2019-11-12 Thread David Zarzycki via Phabricator via cfe-commits

davezarzycki added subscribers: libcxx-commits, davezarzycki.
davezarzycki added a comment.

This broke the libcxx test suite:

  FAIL: libc++ :: 
libcxx/debug/containers/db_sequence_container_iterators.multithread.pass.cpp 
(50769 of 59296)
   TEST 'libc++ :: 
libcxx/debug/containers/db_sequence_container_iterators.multithread.pass.cpp' 
FAILED 
  Command: ['/p/tllvm/bin/clang++', '-o', 
'/tmp/_update_lc/t/projects/libcxx/test/libcxx/debug/containers/Output/db_sequence_container_iterators.multithread.pass.cpp.o',
 '-x', 'c++', 
'/home/dave/s/lp/libcxx/test/libcxx/debug/containers/db_sequence_container_iterators.multithread.pass.cpp',
 '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', 
'-include', '/home/dave/s/lp/libcxx/test/support/nasty_macros.h', 
'-nostdinc++', '-I/home/dave/s/lp/libcxx/include', 
'-I/tmp/_update_lc/t/projects/libcxx/include/c++build', 
'-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', 
'-I/home/dave/s/lp/libcxx/test/support', 
'-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/home/dave/s/lp/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"',
 
'-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/tmp/_update_lc/t/projects/libcxx/test/filesystem/Output/dynamic_env"',
 '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python 
/home/dave/s/lp/libcxx/test/support/filesystem_dynamic_test_helper.py"', 
'-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', 
'-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', 
'-Wno-attributes', '-Wno-constant-evaluated', '-Wno-pessimizing-move', 
'-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', 
'-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', 
'-Wunreachable-code', '-c']
  Exit Code: 1
  Standard Error:
  --
  clang version 10.0.0 (https://github.com/llvm/llvm-project.git 
d8b6b1114307558a5245de3806bb70f53f6f3efe)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: /p/tllvm/bin
  Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/9
  Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/9
  Candidate multilib: .;@m64
  Candidate multilib: 32;@m32
  Selected multilib: .;@m64
   "/p/tllvm/bin/clang-10" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj 
-mrelax-all -disable-free -disable-llvm-verifier -discard-value-names 
-main-file-name db_sequence_container_iterators.multithread.pass.cpp 
-mrelocation-model static -mthread-model posix -mframe-pointer=all -fmath-errno 
-fno-rounding-math -masm-verbose -mconstructor-aliases -munwind-tables 
-fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -v 
-nostdinc++ -resource-dir /p/tllvm/lib64/clang/10.0.0 -include 
/home/dave/s/lp/libcxx/test/support/nasty_macros.h -I 
/home/dave/s/lp/libcxx/include -I 
/tmp/_update_lc/t/projects/libcxx/include/c++build -D __STDC_FORMAT_MACROS -D 
__STDC_LIMIT_MACROS -D __STDC_CONSTANT_MACROS -I 
/home/dave/s/lp/libcxx/test/support -D 
"LIBCXX_FILESYSTEM_STATIC_TEST_ROOT=\"/home/dave/s/lp/libcxx/test/std/input.output/filesystems/Inputs/static_test_env\""
 -D 
"LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT=\"/tmp/_update_lc/t/projects/libcxx/test/filesystem/Output/dynamic_env\""
 -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER=\"/usr/bin/python 
/home/dave/s/lp/libcxx/test/support/filesystem_dynamic_test_helper.py\"" -D 
_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -internal-isystem /usr/local/include 
-internal-isystem /p/tllvm/lib64/clang/10.0.0/include -internal-externc-isystem 
/include -internal-externc-isystem /usr/include -Werror=thread-safety -Wall 
-Wextra -Werror -Wuser-defined-warnings -Wshadow 
-Wno-unused-command-line-argument -Wno-attributes -Wno-constant-evaluated 
-Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals 
-Wno-noexcept-type -Wsign-compare -Wunused-variable -Wunused-parameter 
-Wunreachable-code -std=c++2a -fdeprecated-macro -fdebug-compilation-dir 
/tmp/_update_lc/t/projects/libcxx/test/libcxx/debug/containers -ftemplate-depth 
270 -ferror-limit 19 -fmessage-length 0 -fgnuc-version=4.2.1 
-fno-implicit-modules -fobjc-runtime=gcc -fcxx-exceptions -fexceptions 
-fdiagnostics-show-option -faddrsig -o 
/tmp/_update_lc/t/projects/libcxx/test/libcxx/debug/containers/Output/db_sequence_container_iterators.multithread.pass.cpp.o
 -x c++ 
/home/dave/s/lp/libcxx/test/libcxx/debug/containers/db_sequence_container_iterators.multithread.pass.cpp
  clang -cc1 version 10.0.0 based upon LLVM 10.0.0git default target 
x86_64-unknown-linux-gnu
  ignoring nonexistent directory "/include"
  #include "..." search starts here:
  #include <...> search starts here:
   /home/dave/s/lp/libcxx/include
   /tmp/_update_lc/t/projects/libcxx/include/c++build
   /home/dave/s/lp/libcxx/test/support
   /usr/local/include
   /p/tllvm/lib64/clang/10.0.0/include
   /usr/include
  End of search list.
  In file included from

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

2019-11-12 Thread Gleb Popov via Phabricator via cfe-commits

6yearold added a comment.

Bump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69990



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

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

2019-11-12 Thread Pierre Habouzit via Phabricator via cfe-commits

MadCoder marked an inline comment as not done.
MadCoder added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+called directly. It can also be used on Objective-C Categories and Extensions,
+in which case it applies to all methods declared in such an ``@interface``.
+

rjmccall wrote:
> If this is still true (rather than being done with `objc_direct_members`),
> I feel it should be documented next to the statement at the end about
> `objc_direct_members` in order to make the contrasting use cases clear.
> But we should discuss whether this should be using `objc_direct_members`
> for that use case.
`objc_direct_members` is for the @implementation only right now. 
`objc_direct_members` but it could make sense to be used on `@interface` 
instead I agree. It would make sense.

EDIT: actually I quite like this, it is much cleaner. I'll work on updating the 
patch.



Comment at: clang/include/clang/Basic/AttrDocs.td:3912
+Properties can also be declared with the ``direct`` property attribute
+which causes the property getter and setter to be direct methods as well.
+

rjmccall wrote:
> "If an Objective-C property is declared with the ``direct`` property 
> attribute, then its getter and setter are declared to be direct unless they 
> are explicitly declared."
> 
> I assume that's correct w.r.t the treatment of explicit declarations of 
> getters and setters?
I would expect so, I shall have a test for it



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:3885
+{
+  // GNU runtime doesn't support direct calls at this time
+}

rjmccall wrote:
> This doesn't seem to be diagnosed in Sema.
how should I do it, is there an example I can follow?



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:1588
 // interface, we cannot perform this check.
+   //
+   // Note that for direct methods, because objc_msgSend is skipped,

rjmccall wrote:
> MadCoder wrote:
> > doh I'll fix the tabs
> The indentation still seems wrong.
yeah I fixed it in my checkout but didn't update the diff here yet



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4095
+Builder.CreateStore(result.getScalarVal(),
+CGF.GetAddrOfLocalVar(OMD->getSelfDecl()));
+

rjmccall wrote:
> We should also be changing `selfValue` so that the null check below will do 
> the right thing with e.g. subclasses of weak-linked classes, where IIUC the 
> input value might be non-null but the initialized class pointer might be null.
I'm not sure what you mean, wouldn't storing to 
`CGF.GetAddrOfLocalVar(OMD->getSelfDecl())` actually affect selfValue itself? 
I'm not 100% sure I understand what you mean here.


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

https://reviews.llvm.org/D69991



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

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

2019-11-12 Thread Pierre Habouzit via Phabricator via cfe-commits

MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:2169
+// Direct methods will synthesize the proper `_cmd` internally,
+   // so just don't bother with setting the `_cmd` argument.
+assert(!IsSuper);

jeez will fix the indent here too


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

https://reviews.llvm.org/D69991



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

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

2019-11-12 Thread Conrad Poelman via Phabricator via cfe-commits

poelmanc created this revision.
poelmanc added reviewers: alexfh, djasper.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

In addition to adding `override` wherever possible, clang-tidy's 
`modernize-use-override` nicely removes `virtual` when `override` or `final` is 
specified, and further removes `override` when `final` is specified. While this 
is great default behavior, when code needs to be compiled with `gcc` at high 
warning levels that include `gcc -Wsuggest-override` or `gcc 
-Werror=suggest-override`, clang-tidy's removal of the redundant `override` 
keyword causes gcc to emit a warning or error. This discrepancy / conflict has 
been noted by others including a comment on Stack Overflow 

 and by Mozzilla's Firefox developers 
.

This patch adds an AllowOverrideAndFinal option defaulting to `0` - thus 
preserving current behavior - that when enabled allows both `override` and 
`final` to co-exist, while still fixing all other issues.

The patch includes a test file verifying all combinations of 
``virtual``/``override``/``final``, and mentions the new option in the release 
notes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D70165

Files:
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.AllowOverrideAndFinal, value: 1}]}"
+
+struct Base {
+  virtual ~Base();
+  virtual void a();
+  virtual void b();
+  virtual void c();
+  virtual void d();
+  virtual void e();
+  virtual void f();
+  virtual void g();
+  virtual void h();
+  virtual void i();
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  ~Simple() override;
+  virtual void a() override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'override' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+  virtual void b() final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void b() final;
+  virtual void c() final override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void c() final override;
+  virtual void d() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void d() override final;
+  void e() final override;
+  void f() override final;
+  void g() final;
+  void h() override;
+  void i();
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void i() override;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
@@ -27,6 +27,14 @@
 
If set to non-zero, this check will not diagnose destructors. Default is `0`.
 
+.. option:: AllowOverrideAndFinal
+
+   If set to non-zero, this check will not diagnose ``override`` as redundant
+   with ``final``. This is useful when code will be compiled by a compiler with
+   warning/error checking flags requiring ``override`` explicitly on overriden
+   members, such as ``gcc -Wsuggest-override``/``gcc -Werror=suggest-override``.
+   Default is `0`.
+
 .. option:: OverrideSpelling
 
Specifies a macro to use instead of ``override``. This is useful when
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,6 +153,12 @@
   Finds non-static member functions that can be made ``const``

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

2019-11-12 Thread Pierre Habouzit via Phabricator via cfe-commits

MadCoder updated this revision to Diff 229015.
MadCoder marked 16 inline comments as done and an inline comment as not done.
MadCoder added a comment.

Handled a bunch of comments (marked done).

will update again to implement the deeper requested change around 
`objc_direct_members` and some more unaddressed comments.


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

https://reviews.llvm.org/D69991

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

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod; // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{objc_direct attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{objc_direct attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular; // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct))
+@interface Root ()
+- (void)rootExtensionDirect; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct))
+@interface Root (Direct)
+- (void)rootCategoryDirect; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular; // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular; // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_root_class, objc_direct)) // expected-error {{'objc_direct' attribute only applies to Objective-C methods and Objective-C containers}}
+@interface SubDirectFail : Root
+- (instancetype)init;
+@end
+
+@interface Sub : Root 
+/* invalid overrides with directs */
+- (void)rootRegular __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
++ (void)classRootRegular __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
+- (void)rootExtensionRegular __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
++ (void)classRootExtensionRegular __attribute__((objc_direct)); //

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-12 Thread Awanish Pandey via Phabricator via cfe-commits

awpandey added inline comments.

Comment at: clang/test/CodeGenCXX/debug-info-template-align.cpp:8
+
+// CHECK: DIDerivedType(tag: DW_TAG_typedef, {{.*}}, align:
+

aprantl wrote:
> Do we need to hardcode the target here? Could we check for the specific align 
> value?
Sorry, I could not get your first comment

> Do we need to hardcode the target here?

I have incorporated your rest of the suggestions.

Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ uint32_t AlignInBits = 0);

aprantl wrote:
> This should be `Optional` AlignInBits. Even better perhaps 
> `llvm::Align` but perhaps that's too restrictive.
Yes @aprantl  this should be of `Optional`  type but changing this 
will require change in the `DIDerivedType::get` macro and this macro is used by 
many other functions. Please correct me if I am wrong. ??

Current functionality of the `createTypeDef` is like the default value of the 
`alignInBits`  will be `0`. Consider below comment in `DIBuilder.cpp`.

Comment at: llvm/lib/IR/DIBuilder.cpp:309
-DIScope *Context) {
+DIScope *Context,
+uint32_t AlignInBits) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
-LineNo, getNonCompileUnitScope(Context), Ty, 0, 0,

Consider the 9th argument here 
```
return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,LineNo, 
getNonCompileUnitScope(Context), Ty, 0, 0, --Hard coded alignment for the 
typedef field in existing API.
0, None, DINode::FlagZero);
}
```
 That is why rather than changing the entire API by using `Option` I 
used `uint_32`

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

https://reviews.llvm.org/D70111

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

[PATCH] D70164: [JSONCompilationDatabase] Fix backslash escaping on Windows

2019-11-12 Thread liu hui via Phabricator via cfe-commits

lh123 created this revision.
lh123 added reviewers: sammccall, ilya-biryukov, hokein.
Herald added subscribers: cfe-commits, usaxena95, mstorsjo, kadircet.
Herald added a project: clang.

We should always use `TokenizeWindowsCommandLine` on Windows.

When compiling clangd with `MinGW-W64`, `Triple.getEnvironment` will return 
`EnvironmentType::GNU`.

fixes https://github.com/clangd/clangd/issues/92.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70164

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -137,11 +137,7 @@
 Syntax = JSONCommandLineSyntax::Gnu;
 llvm::Triple Triple(llvm::sys::getProcessTriple());
 if (Triple.getOS() == llvm::Triple::OSType::Win32) {
-  // Assume Windows command line parsing on Win32 unless the triple
-  // explicitly tells us otherwise.
-  if (!Triple.hasEnvironment() ||
-  Triple.getEnvironment() == llvm::Triple::EnvironmentType::MSVC)
-Syntax = JSONCommandLineSyntax::Windows;
+  Syntax = JSONCommandLineSyntax::Windows;
 }
   }
 


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -137,11 +137,7 @@
 Syntax = JSONCommandLineSyntax::Gnu;
 llvm::Triple Triple(llvm::sys::getProcessTriple());
 if (Triple.getOS() == llvm::Triple::OSType::Win32) {
-  // Assume Windows command line parsing on Win32 unless the triple
-  // explicitly tells us otherwise.
-  if (!Triple.hasEnvironment() ||
-  Triple.getEnvironment() == llvm::Triple::EnvironmentType::MSVC)
-Syntax = JSONCommandLineSyntax::Windows;
+  Syntax = JSONCommandLineSyntax::Windows;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

2019-11-12 Thread Awanish Pandey via Phabricator via cfe-commits

awpandey updated this revision to Diff 229013.
awpandey marked 3 inline comments as done.
awpandey added a comment.
Herald added a reviewer: deadalnix.

@aprantl  Thanks for the suggestion. Based on your suggestion I have done the 
following changes

1. I have added alignment value as a check in the test case
2. I have added C-Binding changes.


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

https://reviews.llvm.org/D70111

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-align.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/test/DebugInfo/X86/debug-info-template-align.ll
  llvm/tools/llvm-c-test/debuginfo.c

Index: llvm/tools/llvm-c-test/debuginfo.c
===
--- llvm/tools/llvm-c-test/debuginfo.c
+++ llvm/tools/llvm-c-test/debuginfo.c
@@ -69,7 +69,7 @@
   LLVMMetadataRef Int64Ty =
   LLVMDIBuilderCreateBasicType(DIB, "Int64", 5, 64, 0, LLVMDIFlagZero);
   LLVMMetadataRef Int64TypeDef =
-LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File);
+  LLVMDIBuilderCreateTypedef(DIB, Int64Ty, "int64_t", 7, File, 42, File, 0);
 
   LLVMMetadataRef GlobalVarValueExpr =
   LLVMDIBuilderCreateConstantValueExpression(DIB, 0);
Index: llvm/test/DebugInfo/X86/debug-info-template-align.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-template-align.ll
@@ -0,0 +1,63 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;typedef char  __attribute__((__aligned__(64))) alchar;
+
+;int main(){
+;alchar newChar;
+;}
+; $ clang++ -O0 -g -gdwarf-5 debug-info-template-align.cpp -c
+
+; CHECK: .debug_abbrev contents:
+
+; CHECK: [5] DW_TAG_typedef  DW_CHILDREN_no
+; CHECK:  DW_AT_alignment DW_FORM_udata
+
+; CHECK: .debug_info contents:
+
+;CHECK: DW_TAG_typedef [5]
+;CHECK: DW_AT_name {{.*}} "alchar"
+;CHECK-NEXT: DW_AT_alignment [DW_FORM_udata] (64)
+
+
+; ModuleID = '/dir/test.cpp'
+source_filename = "/dir/test.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone uwtable
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %newChar = alloca i8, align 64
+  call void @llvm.dbg.declare(metadata i8* %newChar, metadata !12, metadata !DIExpression()), !dbg !15
+  ret i32 0, !dbg !16
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "/dir/test.cpp", directory: "/home/awpandey/tools/llvm/test/DebugInfo", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 "}
+!7 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 12, type: !9, scopeLine: 12, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!8 = !DIFile(filename: "test.cpp", directory: "/dir", checksumkind: CSK_MD5, checksum: "872e252efdfcb9480b4bfaf8437f58ab")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocalVariable(name: "newChar", scope: !7, file: !8, line: 13, type: !13)
+!13 = !DIDerivedType(tag: DW_TAG_typedef, name: "alchar", file: !8, line: 10, baseType: !14, align: 512)
+!14 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!15 = !DILocation(line: 13, column: 10, scope: !7)
+!16 = !DILocation(line: 14, column: 1, scope: !7)
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1108,11 +1108,10 @@
 LLVMDIBuilderCreateTypedef(LLVMDIBuilderRef Builder, LLVMMetadataRef Type,
const char *Name, size_t NameLen,
LLVMMetadataRef File, unsigned LineNo,
-   LLVMMetadataRef Scope) {
+   LLVMMetadataRef Scope, uint32_t AlignInBits) {
   return wrap(unwrap(Builder)->createTypedef(
-  unwrapDI(Type), {Name, NameLen},
-  unwrapDI(File), LineNo,
-

[PATCH] D69841: Target Ivy bridge on macOS Mojave and later

2019-11-12 Thread David Zarzycki via Phabricator via cfe-commits

davezarzycki abandoned this revision.
davezarzycki added a comment.

Okay. I'm going to abandon this patch on the assumption that Apple has an 
internal bug tracking this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69841



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

[PATCH] D70157: Align branches within 32-Byte boundary

2019-11-12 Thread Kan Shengchen via Phabricator via cfe-commits

skan marked an inline comment as done.
skan added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:134
+  /// macro-fusion.
+  inline FirstMFInstKind classifyFirstOpcode(unsigned Opcode) {
+switch (Opcode) {

craig.topper wrote:
> xiangzhangllvm wrote:
> > xiangzhangllvm wrote:
> > > We rarely put function definition at *.h,  if putting it into 
> > > X86MacroFusion.cpp will cause compile problem, X86AsmBackend.cpp maybe a 
> > > good place to put it.
> > Seems not big function, just many "case", it is fine for me if you don't 
> > want to change it.
> If you want to put it in a cpp, i'd say X86MCTargetDesc.cpp. There are 
> already a few random functions in there like getX86SubSuperRegister or 
> whatever its called.
I think the function is small enough to be put in header file, I will keep it 
unchanged if you are fine with it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

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

2019-11-12 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3905
+categories, or extensions (for the latter two it applies the attribute
+to all methods declared in the category/implementation),
+

Editing error?



Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+called directly. It can also be used on Objective-C Categories and Extensions,
+in which case it applies to all methods declared in such an ``@interface``.
+

If this is still true (rather than being done with `objc_direct_members`),
I feel it should be documented next to the statement at the end about
`objc_direct_members` in order to make the contrasting use cases clear.
But we should discuss whether this should be using `objc_direct_members`
for that use case.



Comment at: clang/include/clang/Basic/AttrDocs.td:3912
+Properties can also be declared with the ``direct`` property attribute
+which causes the property getter and setter to be direct methods as well.
+

"If an Objective-C property is declared with the ``direct`` property attribute, 
then its getter and setter are declared to be direct unless they are explicitly 
declared."

I assume that's correct w.r.t the treatment of explicit declarations of getters 
and setters?



Comment at: clang/include/clang/Basic/AttrDocs.td:3918
+
+Some sematics of the message send are however preserved:
+

A message send to a direct method calls the implementation directly, as if it 
were a C function, rather than using ordinary Objective-C method dispatch. This 
is substantially faster and potentially allows the implementation to be 
inlined, but it also means the method cannot be overridden in subclasses or 
replaced dynamically, as ordinary Objective-C methods can.

Furthermore, a direct method is not listed in the class's method lists.  This 
substantially reduces the code-size overhead of the method but also means it 
cannot be called dynamically using ordinary Objective-C method dispatch at all; 
in particular, this means that it cannot override a superclass method or 
satisfy a protocol requirement.

Clang will diagnose attempts to override direct methods, override non-direct 
methods with direct methods, implement a protocol requirement with a direct 
method, or use a selector that is only known to be used for direct methods.

Symbols for direct method implementations are implicitly given hidden 
visibility, meaning that they can only be called within the same linkage unit.

Although a direct method is called directly as if it were a C function, it 
still obeys Objective-C semantics in other ways:

- If the receiver is `nil`, the call does nothing and returns the zero value 
for the return type.

- Calling a direct class method will cause the class to be initialized, 
including calling the `+initialize` method if present.

- The method still receives an implicit `_cmd` argument containing its selector.



Comment at: clang/include/clang/Basic/AttrDocs.td:3938
+and causes any method that has not been declared in any ``@interface``
+or ``@protocol`` this class conforms to to be direct methods.
+  }];

"decorage"

I think you could be more precise about "has not been declared in any 
``@interface`` or ``@protocol``".  It's specifically the interfaces, class 
extensions, categories, and protocols of this class and its superclasses.

This should refer the user to the documentation about the ``objc_direct`` 
attribute.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:994
+def err_objc_direct_missing_on_decl : Error<
+  "method implementation is direct but its declaration was not">;
+def err_objc_direct_on_override : Error<

"direct method implementation was previously declared not direct"



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:996
+def err_objc_direct_on_override : Error<
+  "method overrides or implementing protocol conformance cannot be direct">;
+def err_objc_override_direct_method : Error<

"methods that %select{override superclass methods|implement protocol 
requirements}0 cannot be direct"



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:998
+def err_objc_override_direct_method : Error<
+  "cannot override direct methods">;
+

"cannot override a method that is declared direct by a superclass"



Comment at: clang/lib/CodeGen/CGObjC.cpp:689
+// This function is a direct call, it has to implement a nil check
+// on entry.
+CGM.getObjCRuntime().GenerateDirectMethodPrologue(*this, Fn, OMD, CD);

Please add a TODO about the idea of emitting separate entry points that elide 
the check.



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:3885
+{
+  // GNU runtime doesn't support direct calls at this time
+}

[PATCH] D70157: Align branches within 32-Byte boundary

2019-11-12 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:134
+  /// macro-fusion.
+  inline FirstMFInstKind classifyFirstOpcode(unsigned Opcode) {
+switch (Opcode) {

xiangzhangllvm wrote:
> xiangzhangllvm wrote:
> > We rarely put function definition at *.h,  if putting it into 
> > X86MacroFusion.cpp will cause compile problem, X86AsmBackend.cpp maybe a 
> > good place to put it.
> Seems not big function, just many "case", it is fine for me if you don't want 
> to change it.
If you want to put it in a cpp, i'd say X86MCTargetDesc.cpp. There are already 
a few random functions in there like getX86SubSuperRegister or whatever its 
called.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70157: Align branches within 32-Byte boundary

2019-11-12 Thread Xiang Zhang via Phabricator via cfe-commits

xiangzhangllvm added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:134
+  /// macro-fusion.
+  inline FirstMFInstKind classifyFirstOpcode(unsigned Opcode) {
+switch (Opcode) {

xiangzhangllvm wrote:
> We rarely put function definition at *.h,  if putting it into 
> X86MacroFusion.cpp will cause compile problem, X86AsmBackend.cpp maybe a good 
> place to put it.
Seems not big function, just many "case", it is fine for me if you don't want 
to change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70157: Align branches within 32-Byte boundary

2019-11-12 Thread Xiang Zhang via Phabricator via cfe-commits

xiangzhangllvm added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:134
+  /// macro-fusion.
+  inline FirstMFInstKind classifyFirstOpcode(unsigned Opcode) {
+switch (Opcode) {

We rarely put function definition at *.h,  if putting it into 
X86MacroFusion.cpp will cause compile problem, X86AsmBackend.cpp maybe a good 
place to put it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70158: [analyzer] Fix Objective-C accessor body farms after D68108.

2019-11-12 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ marked 2 inline comments as done.
NoQ added inline comments.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist:225

-14
-16
-17
+26
+30

`26` is the new `10`.



Comment at: clang/test/Analysis/nullability-notes.m:31
 -(void) method {
+  clang_analyzer_warnOnDeadSymbol(self);
   // no-crash

D68108 caused this symbol to never die because in the inlined stack frame of 
the accessor it was bound to implicit parameter variable `self` in the Store, 
and that variable was put into `UnknownSpaceRegion` due to not being part of 
its stack frame's declaration, so it was held alive forever by the Store as if 
it's a static variable.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70158



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

[PATCH] D70158: [analyzer] Fix Objective-C accessor body farms after D68108.

2019-11-12 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ created this revision.
NoQ added reviewers: dcoughlin, aprantl.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, JDevlieghere, szepet, baloghadamsoftware, 
kristof.beyls, xazax.hun.
Herald added a project: clang.
NoQ marked 2 inline comments as done.
NoQ added inline comments.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist:225

-14
-16
-17
+26
+30

`26` is the new `10`.



Comment at: clang/test/Analysis/nullability-notes.m:31
 -(void) method {
+  clang_analyzer_warnOnDeadSymbol(self);
   // no-crash

D68108 caused this symbol to never die because in the inlined stack frame of 
the accessor it was bound to implicit parameter variable `self` in the Store, 
and that variable was put into `UnknownSpaceRegion` due to not being part of 
its stack frame's declaration, so it was held alive forever by the Store as if 
it's a static variable.


In the affected test D68108  causes stubs of 
getter and setter methods for property 'x' appear in both `ObjCInterfaceDecl` 
and `ObjCImplementationDecl` for our toy `ClassWithProperties`. Previously they 
were only present in `ObjCInterfaceDecl`. I guess that's the intended behavior.

`ObjCInterfaceDecl::lookupPrivateMethod()` preferes looking up the method in 
the implementation. `CallEvent::getRuntimeDefinition()` trusts it and starts 
using the implementation stub instead of the interface stub. This explains the 
disappearance of "executed line" 10: implementation stubs, unlike interface 
stubs, have invalid source locations.

On the other hand, bodyFarm's `createObjCPropertyGetter()` function queries 
`ObjCPropertyDecl` when it is looking for the declaration of variable '`self`' 
within the method. This is no longer valid, because the accessor declaration 
for `ObjCPropertyDecl` is the interface stub, while the newly farmed body is 
going to be attached to the implementation stub. This explains the change in 
dead symbol elimination: basically, the 'self' variable is from a different 
Decl, so liveness analysis becomes confused. If we are to keep inlining and 
farming the implementation stub, we should use the 'self' from the 
implementation stub, not the 'self' from the interface stub.

In this patch i basically change `CallEvent` back to use the interface stub. 
Generally, i believe that `CallEvent::getRuntimeDefinition()` should always 
return either the declaration that has a body (if the body is at all present), 
or the canonical declaration (in all other cases), but i didn't verify that. In 
any case, body farms always farm the body for the canonical declaration.

I think i made the opposite choice in my previous patch on the subject, D60899 
. I guess i'll need to make up my mind. I'll 
think a bit more about this.

Another useful assertion to add would be to always check that the farmed body 
only refers to `ParmVarDecl`s (or `ImplicitParamDecl`s) that belong to the 
correct function/method decl.


Repository:
  rC Clang

https://reviews.llvm.org/D70158

Files:
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
  clang/test/Analysis/nullability-notes.m

Index: clang/test/Analysis/nullability-notes.m
===
--- clang/test/Analysis/nullability-notes.m
+++ clang/test/Analysis/nullability-notes.m
@@ -1,6 +1,22 @@
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-output=text -verify %s
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-output=plist -o %t.plist %s
-// RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/nullability-notes.m.plist -
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullableDereferenced \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull \
+// RUN:

[PATCH] D69758: [Gnu toolchain] Look at standard GCC paths for libstdcxx by default

2019-11-12 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added reviewers: mclow.lists, ldionne, EricWF, rsmith.
kristina added a comment.
Herald added a subscriber: dexonsmith.

Added libcxx maintainers, would like one of them to sign off on this. I presume 
this is NFCI for Linux so it should be covered by existing regression tests?




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2623
+  int MaxVersion = 0;
+  std::string MaxVersionString = "";
+  for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;

No need for `= ""` here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69758



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

[PATCH] D70157: Align branches within 32-Byte boundary

2019-11-12 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added reviewers: MaskRay, jyknight.
craig.topper added a comment.

Adding some others who may be be more familiar with the Fragments in the MC 
layer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D67787: Add 8548 CPU definition and attributes

2019-11-12 Thread Justin Hibbits via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc4bc5aa0d84: Add 8548 CPU definition and attributes 
(authored by chmeee).

Changed prior to commit:
  https://reviews.llvm.org/D67787?vs=228589=228995#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67787

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/test/Driver/clang-translation.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/init.c

Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -7604,6 +7604,12 @@
 //
 // PPC32-SPE:#define __NO_FPRS__ 1
 // PPC32-SPE:#define __SPE__ 1
+// 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -target-cpu 8548 < /dev/null | FileCheck -match-full-lines -check-prefix PPC8548 %s
+//
+// PPC8548:#define __NO_FPRS__ 1
+// PPC8548:#define __NO_LWSYNC__ 1
+// PPC8548:#define __SPE__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-apple-darwin8 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-DARWIN %s
 //
Index: clang/test/Misc/target-invalid-cpu-note.c
===
--- clang/test/Misc/target-invalid-cpu-note.c
+++ clang/test/Misc/target-invalid-cpu-note.c
@@ -79,10 +79,10 @@
 // PPC: error: unknown target CPU 'not-a-cpu'
 // PPC: note: valid target CPU values are: generic, 440, 450, 601, 602, 603,
 // PPC-SAME: 603e, 603ev, 604, 604e, 620, 630, g3, 7400, g4, 7450, g4+, 750,
-// PPC-SAME: 970, g5, a2, a2q, e500mc, e5500, power3, pwr3, power4, pwr4,
-// PPC-SAME: power5, pwr5, power5x, pwr5x, power6, pwr6, power6x, pwr6x, power7,
-// PPC-SAME: pwr7, power8, pwr8, power9, pwr9, powerpc, ppc, powerpc64, ppc64,
-// PPC-SAME: powerpc64le, ppc64le
+// PPC-SAME: 8548, 970, g5, a2, a2q, e500, e500mc, e5500, power3, pwr3, power4,
+// PPC-SAME: pwr4, power5, pwr5, power5x, pwr5x, power6, pwr6, power6x, pwr6x,
+// PPC-SAME: power7, pwr7, power8, pwr8, power9, pwr9, powerpc, ppc, powerpc64,
+// PPC-SAME: ppc64, powerpc64le, ppc64le
 
 // RUN: not %clang_cc1 -triple mips--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix MIPS
 // MIPS: error: unknown target CPU 'not-a-cpu'
Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -277,6 +277,18 @@
 // PPC64NS: "-target-cpu" "ppc64"
 
 // RUN: %clang -target powerpc-fsl-linux -### -S %s \
+// RUN: -mcpu=e500 2>&1 | FileCheck -check-prefix=PPCE500 %s
+// PPCE500: clang
+// PPCE500: "-cc1"
+// PPCE500: "-target-cpu" "e500"
+
+// RUN: %clang -target powerpc-fsl-linux -### -S %s \
+// RUN: -mcpu=8548 2>&1 | FileCheck -check-prefix=PPC8548 %s
+// PPC8548: clang
+// PPC8548: "-cc1"
+// PPC8548: "-target-cpu" "e500"
+
+// RUN: %clang -target powerpc-fsl-linux -### -S %s \
 // RUN: -mcpu=e500mc 2>&1 | FileCheck -check-prefix=PPCE500MC %s
 // PPCE500MC: clang
 // PPCE500MC: "-cc1"
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -53,10 +53,12 @@
 .Case("7450", "7450")
 .Case("G4+", "g4+")
 .Case("750", "750")
+.Case("8548", "e500")
 .Case("970", "970")
 .Case("G5", "g5")
 .Case("a2", "a2")
 .Case("a2q", "a2q")
+.Case("e500", "e500")
 .Case("e500mc", "e500mc")
 .Case("e5500", "e5500")
 .Case("power3", "pwr3")
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -44,7 +44,8 @@
 ArchDefinePwr8 = 1 << 12,
 ArchDefinePwr9 = 1 << 13,
 ArchDefineA2 = 1 << 14,
-ArchDefineA2q = 1 << 15
+ArchDefineA2q = 1 << 15,
+ArchDefineE500 = 1 << 16
   } ArchDefineTypes;
 
 
@@ -145,6 +146,7 @@
  ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
  ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
  ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
+  .Cases("8548", "e500", ArchDefineE500)
   .Default(ArchDefineNone);
 }
 return CPUKnown;
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -157,6 +157,8 @@
 Builder.defineMacro("_ARCH_A2Q");
 Builder.defineMacro("_ARCH_QP");
   }
+  if (ArchDefs & ArchDefineE500)
+Builder.defineMacro("__NO_LWSYNC__");
 
   if

[clang] bc4bc5a - Add 8548 CPU definition and attributes

2019-11-12 Thread Justin Hibbits via cfe-commits


Author: Justin Hibbits
Date: 2019-11-12T20:34:34-06:00
New Revision: bc4bc5aa0d84413e4e3f082dee0d30cf839fb7ea

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

LOG: Add 8548 CPU definition and attributes

8548 CPU is GCC's name for the e500v2, so accept this in clang.  The
e500v2 doesn't support lwsync, so define __NO_LWSYNC__ for this as well,
as GCC does.

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

Added: 


Modified: 
clang/lib/Basic/Targets/PPC.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/Driver/ToolChains/Arch/PPC.cpp
clang/test/Driver/clang-translation.c
clang/test/Misc/target-invalid-cpu-note.c
clang/test/Preprocessor/init.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index a40991048873..baa96e21707b 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -157,6 +157,8 @@ void PPCTargetInfo::getTargetDefines(const LangOptions 
,
 Builder.defineMacro("_ARCH_A2Q");
 Builder.defineMacro("_ARCH_QP");
   }
+  if (ArchDefs & ArchDefineE500)
+Builder.defineMacro("__NO_LWSYNC__");
 
   if (getTriple().getVendor() == llvm::Triple::BGQ) {
 Builder.defineMacro("__bg__");
@@ -312,6 +314,11 @@ bool PPCTargetInfo::initFeatureMap(
 .Case("pwr8", true)
 .Default(false);
 
+  Features["spe"] = llvm::StringSwitch(CPU)
+.Case("8548", true)
+.Case("e500", true)
+.Default(false);
+
   if (!ppcUserFeaturesCheck(Diags, FeaturesVec))
 return false;
 
@@ -449,16 +456,16 @@ ArrayRef 
PPCTargetInfo::getGCCAddlRegNames() const {
 }
 
 static constexpr llvm::StringLiteral ValidCPUNames[] = {
-{"generic"}, {"440"}, {"450"}, {"601"},{"602"},
-{"603"}, {"603e"},{"603ev"},   {"604"},{"604e"},
-{"620"}, {"630"}, {"g3"},  {"7400"},   {"g4"},
-{"7450"},{"g4+"}, {"750"}, {"970"},{"g5"},
-{"a2"},  {"a2q"}, {"e500mc"},  {"e5500"},  {"power3"},
-{"pwr3"},{"power4"},  {"pwr4"},{"power5"}, {"pwr5"},
-{"power5x"}, {"pwr5x"},   {"power6"},  {"pwr6"},   {"power6x"},
-{"pwr6x"},   {"power7"},  {"pwr7"},{"power8"}, {"pwr8"},
-{"power9"},  {"pwr9"},{"powerpc"}, {"ppc"},{"powerpc64"},
-{"ppc64"},   {"powerpc64le"}, {"ppc64le"},
+{"generic"},   {"440"},   {"450"}, {"601"}, {"602"},
+{"603"},   {"603e"},  {"603ev"},   {"604"}, {"604e"},
+{"620"},   {"630"},   {"g3"},  {"7400"},{"g4"},
+{"7450"},  {"g4+"},   {"750"}, {"8548"},{"970"},
+{"g5"},{"a2"},{"a2q"}, {"e500"},{"e500mc"},
+{"e5500"}, {"power3"},{"pwr3"},{"power4"},  {"pwr4"},
+{"power5"},{"pwr5"},  {"power5x"}, {"pwr5x"},   {"power6"},
+{"pwr6"},  {"power6x"},   {"pwr6x"},   {"power7"},  {"pwr7"},
+{"power8"},{"pwr8"},  {"power9"},  {"pwr9"},
{"powerpc"},
+{"ppc"},   {"powerpc64"}, {"ppc64"},   {"powerpc64le"}, 
{"ppc64le"},
 };
 
 bool PPCTargetInfo::isValidCPUName(StringRef Name) const {

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 6c6421c28e23..847338bbac1b 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -44,7 +44,8 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public 
TargetInfo {
 ArchDefinePwr8 = 1 << 12,
 ArchDefinePwr9 = 1 << 13,
 ArchDefineA2 = 1 << 14,
-ArchDefineA2q = 1 << 15
+ArchDefineA2q = 1 << 15,
+ArchDefineE500 = 1 << 16
   } ArchDefineTypes;
 
 
@@ -145,6 +146,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public 
TargetInfo {
  ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
  ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
  ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
+  .Cases("8548", "e500", ArchDefineE500)
   .Default(ArchDefineNone);
 }
 return CPUKnown;

diff  --git a/clang/lib/Driver/ToolChains/Arch/PPC.cpp 
b/clang/lib/Driver/ToolChains/Arch/PPC.cpp
index 3e02e57e0f6c..e1b0311f2ad0 100644
--- a/clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -53,10 +53,12 @@ std::string ppc::getPPCTargetCPU(const ArgList ) {
 .Case("7450", "7450")
 .Case("G4+", "g4+")
 .Case("750", "750")
+.Case("8548", "e500")
 .Case("970", "970")
 .Case("G5", "g5")
 .Case("a2", "a2")
 .Case("a2q", "a2q")
+

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > ABataev wrote:
> > > > > > > > jdoerfert wrote:
> > > > > > > > > ABataev wrote:
> > > > > > > > > > Maybe add an assert when the cancellation version is 
> > > > > > > > > > requested but the cancellation block is not set? Instead of 
> > > > > > > > > > the generating simple version of barrier.
> > > > > > > > > The interface doesn't work that way as we do not know here if 
> > > > > > > > > the cancellation was requested except if the block was set. 
> > > > > > > > > That is basically the flag (and I expect it to continue to be 
> > > > > > > > > that way).
> > > > > > > > Maybe instead of `ForceSimpleBarrier` add a flag 
> > > > > > > > `EmitCancelBarrier` and if it set to true, always emit cancel 
> > > > > > > > barrier, otherwise always emit simple barrier? And add an 
> > > > > > > > assertion for non-set cancellation block or even accept it as a 
> > > > > > > > parameter here.
> > > > > > > > 
> > > > > > > > Also, what if we have inner exception handling in the region? 
> > > > > > > > Will you handle the cleanup correctly in case of the 
> > > > > > > > cancelation barrier?
> > > > > > > > Maybe instead of ForceSimpleBarrier add a flag 
> > > > > > > > EmitCancelBarrier and if it set to true, always emit cancel 
> > > > > > > > barrier, otherwise always emit simple barrier? And add an 
> > > > > > > > assertion for non-set cancellation block or even accept it as a 
> > > > > > > > parameter here.
> > > > > > > 
> > > > > > > What is the difference in moving some of the boolean logic to the 
> > > > > > > caller? Also, we have test to verify we get cancellation barriers 
> > > > > > > if we need them, both unit tests and clang lit tests.
> > > > > > > 
> > > > > > > 
> > > > > > > > Also, what if we have inner exception handling in the region? 
> > > > > > > > Will you handle the cleanup correctly in case of the 
> > > > > > > > cancelation barrier?
> > > > > > > 
> > > > > > > I think so. Right now through the code in clang that does the set 
> > > > > > > up of the cancellation block, later through callbacks but we only 
> > > > > > > need that for regions where we actually go out of some scope, 
> > > > > > > e.g., parallel.
> > > > > > 1. I'm just thinking about future users of thus interface. It woild 
> > > > > > be good if we could provide safe interface for all the users, not 
> > > > > > only clang.
> > > > > > 2. Exit out of the OpenMP region is not allowed. But you may have 
> > > > > > inner try...catch or just simple compound statement with local vars 
> > > > > > that require constructors/destructors. And the cancellation barrier 
> > > > > > may exit out of these regions. And you need to call all required 
> > > > > > destructors. You'd better to think about it now, not later.
> > > > > > 2. [...] You'd better to think about it now, not later.
> > > > > 
> > > > > First, I do think about it now and I hope this was not an insinuation 
> > > > > to suggest otherwise.
> > > > > 
> > > > > > 1. I'm just thinking about future users of thus interface. It woild 
> > > > > > be good if we could provide safe interface for all the users, not 
> > > > > > only clang.
> > > > > > 2. Exit out of the OpenMP region is not allowed. But you may have 
> > > > > > inner try...catch or just simple compound statement with local vars 
> > > > > > that require constructors/destructors. And the cancellation barrier 
> > > > > > may exit out of these regions. And you need to call all required 
> > > > > > destructors.
> > > > > 
> > > > > Generally speaking, we shall not add features that we cannot use or 
> > > > > test with the assumption we will use them in the future. This is 
> > > > > suggested by the LLVM best practices. If you have specific changes in 
> > > > > mind that are testable and better than what I suggested so far, 
> > > > > please bring them forward. You can also bring forward suggestions on 
> > > > > how it might look in the future but without a real use case now it is 
> > > > > not practical to block a review based on that, given that we can 
> > > > > change the interface once the time has come.
> > > > > 
> > > > > I said before, we will need callbacks for destructors, actual 
> > > > > handling of cancellation blocks, and there are various other features 
> > > > > missing right now. Nevertheless, we cannot build them into the 
> > > > > current interface, or even try to prepare for all of them, while 
> > > > > keeping the patches small and concise.
> > > > It won't work for clang, I'm afraid. You need a list of

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > ABataev wrote:
> > > > > > > jdoerfert wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > jdoerfert wrote:
> > > > > > > > > > ABataev wrote:
> > > > > > > > > > > Maybe add an assert when the cancellation version is 
> > > > > > > > > > > requested but the cancellation block is not set? Instead 
> > > > > > > > > > > of the generating simple version of barrier.
> > > > > > > > > > The interface doesn't work that way as we do not know here 
> > > > > > > > > > if the cancellation was requested except if the block was 
> > > > > > > > > > set. That is basically the flag (and I expect it to 
> > > > > > > > > > continue to be that way).
> > > > > > > > > Maybe instead of `ForceSimpleBarrier` add a flag 
> > > > > > > > > `EmitCancelBarrier` and if it set to true, always emit cancel 
> > > > > > > > > barrier, otherwise always emit simple barrier? And add an 
> > > > > > > > > assertion for non-set cancellation block or even accept it as 
> > > > > > > > > a parameter here.
> > > > > > > > > 
> > > > > > > > > Also, what if we have inner exception handling in the region? 
> > > > > > > > > Will you handle the cleanup correctly in case of the 
> > > > > > > > > cancelation barrier?
> > > > > > > > > Maybe instead of ForceSimpleBarrier add a flag 
> > > > > > > > > EmitCancelBarrier and if it set to true, always emit cancel 
> > > > > > > > > barrier, otherwise always emit simple barrier? And add an 
> > > > > > > > > assertion for non-set cancellation block or even accept it as 
> > > > > > > > > a parameter here.
> > > > > > > > 
> > > > > > > > What is the difference in moving some of the boolean logic to 
> > > > > > > > the caller? Also, we have test to verify we get cancellation 
> > > > > > > > barriers if we need them, both unit tests and clang lit tests.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > Also, what if we have inner exception handling in the region? 
> > > > > > > > > Will you handle the cleanup correctly in case of the 
> > > > > > > > > cancelation barrier?
> > > > > > > > 
> > > > > > > > I think so. Right now through the code in clang that does the 
> > > > > > > > set up of the cancellation block, later through callbacks but 
> > > > > > > > we only need that for regions where we actually go out of some 
> > > > > > > > scope, e.g., parallel.
> > > > > > > 1. I'm just thinking about future users of thus interface. It 
> > > > > > > woild be good if we could provide safe interface for all the 
> > > > > > > users, not only clang.
> > > > > > > 2. Exit out of the OpenMP region is not allowed. But you may have 
> > > > > > > inner try...catch or just simple compound statement with local 
> > > > > > > vars that require constructors/destructors. And the cancellation 
> > > > > > > barrier may exit out of these regions. And you need to call all 
> > > > > > > required destructors. You'd better to think about it now, not 
> > > > > > > later.
> > > > > > > 2. [...] You'd better to think about it now, not later.
> > > > > > 
> > > > > > First, I do think about it now and I hope this was not an 
> > > > > > insinuation to suggest otherwise.
> > > > > > 
> > > > > > > 1. I'm just thinking about future users of thus interface. It 
> > > > > > > woild be good if we could provide safe interface for all the 
> > > > > > > users, not only clang.
> > > > > > > 2. Exit out of the OpenMP region is not allowed. But you may have 
> > > > > > > inner try...catch or just simple compound statement with local 
> > > > > > > vars that require constructors/destructors. And the cancellation 
> > > > > > > barrier may exit out of these regions. And you need to call all 
> > > > > > > required destructors.
> > > > > > 
> > > > > > Generally speaking, we shall not add features that we cannot use or 
> > > > > > test with the assumption we will use them in the future. This is 
> > > > > > suggested by the LLVM best practices. If you have specific changes 
> > > > > > in mind that are testable and better than what I suggested so far, 
> > > > > > please bring them forward. You can also bring forward suggestions 
> > > > > > on how it might look in the future but without a real use case now 
> > > > > > it is not practical to block a review based on that, given that we 
> > > > > > can change the interface once the time has come.
> > > > > > 
> > > > > > I said before, we will need callbacks for destructors, actual 
> > > > > > handling of cancellation blocks, and there are various other 
> > > > > > features missing right now. Nevertheless, we cannot build them into 
> > > > > >

[PATCH] D65184: [Sema] Thread Safety Analysis: Make negative capability typeless.

2019-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

> This is a larger effort so it will take a little bit more time on me.

This is fine, but could you add a "Plan changes" action then? This will make 
the revision disappear from the active review queue.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65184



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

So I /think/ what's happening here is the extra addresses needed for the 
call_site low_pcs are creating new basic blocks (or whatever the logic is in 
the backend that says "use line zero at the start of each basic block-like 
thing" - possible that logic is overly conservative even though we just labeled 
something without ever needing to jump to it - guess we could test that with 
plain scopes).

Now, the test we have internally might be brittle & depending on an accidental 
source location - and/or this has shown an upstream bug where we drop a 
location that we should be preserving somewhere along the way.


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

https://reviews.llvm.org/D69970



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

I can confirm we still have the same internal failure with this revised/updated 
patch.

Trying to reduce/reproduce it.


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

https://reviews.llvm.org/D69970



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

[PATCH] D69978: Separately track input and output denormal mode

2019-11-12 Thread Andy Kaylor via Phabricator via cfe-commits

andrew.w.kaylor added inline comments.



Comment at: llvm/docs/LangRef.rst:1822
 
 ``"denorm-fp-mode"``
+   This indicates the subnormal handling that may be assumed for the

I don't like the definition of this attribute. It's not reader-friendly. The 
comma-separated pair format has no indication which value refers to inputs and 
which refers to outputs. Also, while this predates your changes, I think the 
meanings of the current choices are unclear. 

What would you think of a comma-separated list with the following possibilities?


```
allow-denormals (default)
inputs-are-zero (outputs not flushed)
inputs-are-zero, outputs-are-zero
inputs-are-zero, outputs-are-positive-zero
inputs-are-positivezero (outputs not flushed)
inputs-are-positivezero, outputs-are-zero
inputs-are-positivezero, outputs-are-positive-zero
denormal-outputs-are-zero (inputs are unchanged)
denormal-outputs-are-positive-zero (inputs are unchanged)

```
I'd also be open to abbreviations. I don't know if "daz" and "ftz" are readable 
to everyone, but I'm more comfortable with them. That would make the options 
something like this.


```
allow-denormals
daz
daz, ftz
daz, ftz+
daz+
daz+, ftz
daz+, ftz+
ftz
ftz+
```


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

https://reviews.llvm.org/D69978



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

In D69970#1742575 , @vsk wrote:

> @dblaikie do you need more time to investigate? fwiw I didn't uncover any new 
> failures in a stage2 build with this patch applied.


I haven't figured this out yet, sorry - bit inundated with a few things, but 
I'm coming back to it. Apologies for the delay.


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

https://reviews.llvm.org/D69970



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

[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added inline comments.



Comment at: clang-tools-extra/clang-move/tool/ClangMove.cpp:113
   move::MoveDefinitionSpec Spec;
-  Spec.Names = {Names.begin(), Names.end()};
+  Spec.Names = (std::vector &)Names;
   Spec.OldHeader = OldHeader;

rnk wrote:
> Converting from std::list to SmallVector by way of std::vector with a C-style 
> cast is a bit surprising. Is there a simpler way to write this, like 
> `Spec.Names.assign(Names.begin(), Names.end());`?
Yep, +1 to assign(begin, end) & leaving SmallVector's API alone/not adding a 
converting assignment operator.


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

https://reviews.llvm.org/D69959



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

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

2019-11-12 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added a comment.

One style comment, patch looks conceptually fine to me otherwise. I'll wait to 
accept on how we fall on the test issue: I'd opt for an `opt` test if possible.




Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:370
+: nullptr;
+}
 return nullptr;

Consistent style please:

```
if (Value *StrLen = emitStrLen(SrcStr, B, DL, TLI)
  return B.CreateGEP(B.getInt8Ty(), SrcStr, StrLen, "strchr");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70143



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > ABataev wrote:
> > > > > > > jdoerfert wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > Maybe add an assert when the cancellation version is 
> > > > > > > > > requested but the cancellation block is not set? Instead of 
> > > > > > > > > the generating simple version of barrier.
> > > > > > > > The interface doesn't work that way as we do not know here if 
> > > > > > > > the cancellation was requested except if the block was set. 
> > > > > > > > That is basically the flag (and I expect it to continue to be 
> > > > > > > > that way).
> > > > > > > Maybe instead of `ForceSimpleBarrier` add a flag 
> > > > > > > `EmitCancelBarrier` and if it set to true, always emit cancel 
> > > > > > > barrier, otherwise always emit simple barrier? And add an 
> > > > > > > assertion for non-set cancellation block or even accept it as a 
> > > > > > > parameter here.
> > > > > > > 
> > > > > > > Also, what if we have inner exception handling in the region? 
> > > > > > > Will you handle the cleanup correctly in case of the cancelation 
> > > > > > > barrier?
> > > > > > > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier 
> > > > > > > and if it set to true, always emit cancel barrier, otherwise 
> > > > > > > always emit simple barrier? And add an assertion for non-set 
> > > > > > > cancellation block or even accept it as a parameter here.
> > > > > > 
> > > > > > What is the difference in moving some of the boolean logic to the 
> > > > > > caller? Also, we have test to verify we get cancellation barriers 
> > > > > > if we need them, both unit tests and clang lit tests.
> > > > > > 
> > > > > > 
> > > > > > > Also, what if we have inner exception handling in the region? 
> > > > > > > Will you handle the cleanup correctly in case of the cancelation 
> > > > > > > barrier?
> > > > > > 
> > > > > > I think so. Right now through the code in clang that does the set 
> > > > > > up of the cancellation block, later through callbacks but we only 
> > > > > > need that for regions where we actually go out of some scope, e.g., 
> > > > > > parallel.
> > > > > 1. I'm just thinking about future users of thus interface. It woild 
> > > > > be good if we could provide safe interface for all the users, not 
> > > > > only clang.
> > > > > 2. Exit out of the OpenMP region is not allowed. But you may have 
> > > > > inner try...catch or just simple compound statement with local vars 
> > > > > that require constructors/destructors. And the cancellation barrier 
> > > > > may exit out of these regions. And you need to call all required 
> > > > > destructors. You'd better to think about it now, not later.
> > > > > 2. [...] You'd better to think about it now, not later.
> > > > 
> > > > First, I do think about it now and I hope this was not an insinuation 
> > > > to suggest otherwise.
> > > > 
> > > > > 1. I'm just thinking about future users of thus interface. It woild 
> > > > > be good if we could provide safe interface for all the users, not 
> > > > > only clang.
> > > > > 2. Exit out of the OpenMP region is not allowed. But you may have 
> > > > > inner try...catch or just simple compound statement with local vars 
> > > > > that require constructors/destructors. And the cancellation barrier 
> > > > > may exit out of these regions. And you need to call all required 
> > > > > destructors.
> > > > 
> > > > Generally speaking, we shall not add features that we cannot use or 
> > > > test with the assumption we will use them in the future. This is 
> > > > suggested by the LLVM best practices. If you have specific changes in 
> > > > mind that are testable and better than what I suggested so far, please 
> > > > bring them forward. You can also bring forward suggestions on how it 
> > > > might look in the future but without a real use case now it is not 
> > > > practical to block a review based on that, given that we can change the 
> > > > interface once the time has come.
> > > > 
> > > > I said before, we will need callbacks for destructors, actual handling 
> > > > of cancellation blocks, and there are various other features missing 
> > > > right now. Nevertheless, we cannot build them into the current 
> > > > interface, or even try to prepare for all of them, while keeping the 
> > > > patches small and concise.
> > > It won't work for clang, I'm afraid. You need a list of desructors here. 
> > > But clang uses recursive codegen and it is very hard to walk over the 
> > > call tree and gather all required destructors into a

[clang] 5ad6f27 - Don't assume that the clang binary's resolved name includes the string

2019-11-12 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2019-11-12T16:37:05-08:00
New Revision: 5ad6f279f26cd6ce77e4fa6b8df2b23be73d7beb

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

LOG: Don't assume that the clang binary's resolved name includes the string
'clang'.

This is not true in practice in some content-addressed file systems.

Added: 


Modified: 
clang/test/Driver/arm64_32-link.c

Removed: 




diff  --git a/clang/test/Driver/arm64_32-link.c 
b/clang/test/Driver/arm64_32-link.c
index 47f0ccf0cecb..972ae6a234e1 100644
--- a/clang/test/Driver/arm64_32-link.c
+++ b/clang/test/Driver/arm64_32-link.c
@@ -1,4 +1,4 @@
 // RUN: %clang -target x86_64-apple-darwin -arch arm64_32 
-miphoneos-version-min=8.0 %s -### 2>&1 | FileCheck %s
 
-// CHECK: clang{{.*}} "-triple" "aarch64_32-apple-ios8.0.0"
+// CHECK: "-cc1"{{.*}} "-triple" "aarch64_32-apple-ios8.0.0"
 // CHECK: ld{{.*}} "-arch" "arm64_32"



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

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

2019-11-12 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

It wouldn't be that hard to add an equivalent flag to opt; just a matter of 
calling the API on TargetLibraryInfo.  -disable-simplify-libcalls already 
exists, but I guess that's not quite what you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70143



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

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

2019-11-12 Thread Kousik Kumar via Phabricator via cfe-commits

kousikk added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69122



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

[PATCH] D67992: [Sema] Add MacroQualified case for FunctionTypeUnwrapper

2019-11-12 Thread Leonard Chan via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe278c138a937: [Sema] Add MacroQualified case for 
FunctionTypeUnwrapper (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67992

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/Frontend/macro_defined_type.cpp


Index: clang/test/Frontend/macro_defined_type.cpp
===
--- clang/test/Frontend/macro_defined_type.cpp
+++ clang/test/Frontend/macro_defined_type.cpp
@@ -19,3 +19,7 @@
 struct A {
   _LIBCPP_FLOAT_ABI int operator()() throw(); // expected-warning{{'pcs' 
calling convention is not supported for this target}}
 };
+
+// Added test for fix for PR43315
+#define a __attribute__((__cdecl__, __regparm__(0)))
+int(a b)();
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6325,7 +6325,8 @@
   Pointer,
   BlockPointer,
   Reference,
-  MemberPointer
+  MemberPointer,
+  MacroQualified,
 };
 
 QualType Original;
@@ -6356,6 +6357,9 @@
 } else if (isa(Ty)) {
   T = cast(Ty)->getEquivalentType();
   Stack.push_back(Attributed);
+} else if (isa(Ty)) {
+  T = cast(Ty)->getUnderlyingType();
+  Stack.push_back(MacroQualified);
 } else {
   const Type *DTy = Ty->getUnqualifiedDesugaredType();
   if (Ty == DTy) {
@@ -6412,6 +6416,9 @@
 return C.getParenType(New);
   }
 
+  case MacroQualified:
+return wrap(C, cast(Old)->getUnderlyingType(), I);
+
   case Pointer: {
 QualType New = wrap(C, cast(Old)->getPointeeType(), I);
 return C.getPointerType(New);


Index: clang/test/Frontend/macro_defined_type.cpp
===
--- clang/test/Frontend/macro_defined_type.cpp
+++ clang/test/Frontend/macro_defined_type.cpp
@@ -19,3 +19,7 @@
 struct A {
   _LIBCPP_FLOAT_ABI int operator()() throw(); // expected-warning{{'pcs' calling convention is not supported for this target}}
 };
+
+// Added test for fix for PR43315
+#define a __attribute__((__cdecl__, __regparm__(0)))
+int(a b)();
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6325,7 +6325,8 @@
   Pointer,
   BlockPointer,
   Reference,
-  MemberPointer
+  MemberPointer,
+  MacroQualified,
 };
 
 QualType Original;
@@ -6356,6 +6357,9 @@
 } else if (isa(Ty)) {
   T = cast(Ty)->getEquivalentType();
   Stack.push_back(Attributed);
+} else if (isa(Ty)) {
+  T = cast(Ty)->getUnderlyingType();
+  Stack.push_back(MacroQualified);
 } else {
   const Type *DTy = Ty->getUnqualifiedDesugaredType();
   if (Ty == DTy) {
@@ -6412,6 +6416,9 @@
 return C.getParenType(New);
   }
 
+  case MacroQualified:
+return wrap(C, cast(Old)->getUnderlyingType(), I);
+
   case Pointer: {
 QualType New = wrap(C, cast(Old)->getPointeeType(), I);
 return C.getPointerType(New);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70150: [analyzer] Don't clean up dead symbols from constraints twice.

2019-11-12 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ marked an inline comment as done.
NoQ added inline comments.

Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:763-764
 // generate a transition to that state.
 ProgramStateRef CleanedCheckerSt =
 StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
 Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, , K);

NoQ wrote:
> Note: The results of the first invocation are discarded here, as the updated 
> state is getting frankensteined by attaching `ExprEngine`'s environment and 
> store to checker's GDM, while range constraints also reside in the GDM.
This whole frankensteining process is kinda necessary because `SymbolReaper` 
needs to be populated with the data from the Environment and the Store. I.e., 
they're supposed to issue their `markLive()` calls, otherwise we wouldn't be 
able to properly judge whether something is live in `checkDeadSymbols`; but at 
the same time the intent is to provide the uncleaned state to the callback, so 
that the checkers had access to full information about the dying symbol.

I'm not sure whether any checkers actually take advantage of such information, 
but the intent to provide this information looks valid, so i don't plan to undo 
this decision.

Repository:
  rC Clang

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

https://reviews.llvm.org/D70150

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

[clang] e278c13 - [Sema] Add MacroQualified case for FunctionTypeUnwrapper

2019-11-12 Thread Leonard Chan via cfe-commits


Author: Leonard Chan
Date: 2019-11-12T16:22:13-08:00
New Revision: e278c138a937a68f3e6c89df8eaeffa913f9b0f7

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

LOG: [Sema] Add MacroQualified case for FunctionTypeUnwrapper

This is a fix for PR43315. An assertion error is hit for this minimal example:

```
//clang -cc1 -triple x86_64-- -S tstVMStructRC-min.cpp
int (a b)();  // Assertion `Chunk.Kind == DeclaratorChunk::Function' failed.
```

This is because we do not cover the case in the FunctionTypeUnwrapper where it
receives a MacroQualifiedType. We have not run into this earlier because this
is a unique case where the __attribute__ contains both __cdecl__ and
__regparm__ (in that order), and we are compiling for x86_64. Changing the
architecture or the order of __cdecl__ and __regparm__ does not raise the
assertion.

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

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp
clang/test/Frontend/macro_defined_type.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index abee6e68c0eb..06f5e6f9ee34 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6325,7 +6325,8 @@ namespace {
   Pointer,
   BlockPointer,
   Reference,
-  MemberPointer
+  MemberPointer,
+  MacroQualified,
 };
 
 QualType Original;
@@ -6356,6 +6357,9 @@ namespace {
 } else if (isa(Ty)) {
   T = cast(Ty)->getEquivalentType();
   Stack.push_back(Attributed);
+} else if (isa(Ty)) {
+  T = cast(Ty)->getUnderlyingType();
+  Stack.push_back(MacroQualified);
 } else {
   const Type *DTy = Ty->getUnqualifiedDesugaredType();
   if (Ty == DTy) {
@@ -6412,6 +6416,9 @@ namespace {
 return C.getParenType(New);
   }
 
+  case MacroQualified:
+return wrap(C, cast(Old)->getUnderlyingType(), I);
+
   case Pointer: {
 QualType New = wrap(C, cast(Old)->getPointeeType(), I);
 return C.getPointerType(New);

diff  --git a/clang/test/Frontend/macro_defined_type.cpp 
b/clang/test/Frontend/macro_defined_type.cpp
index d4f54b65a8d6..71a0ff18477d 100644
--- a/clang/test/Frontend/macro_defined_type.cpp
+++ b/clang/test/Frontend/macro_defined_type.cpp
@@ -19,3 +19,7 @@ void Func() {
 struct A {
   _LIBCPP_FLOAT_ABI int operator()() throw(); // expected-warning{{'pcs' 
calling convention is not supported for this target}}
 };
+
+// Added test for fix for PR43315
+#define a __attribute__((__cdecl__, __regparm__(0)))
+int(a b)();



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

[PATCH] D70150: [analyzer] Don't clean up dead symbols from constraints twice.

2019-11-12 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:763-764
 // generate a transition to that state.
 ProgramStateRef CleanedCheckerSt =
 StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
 Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, , K);

Note: The results of the first invocation are discarded here, as the updated 
state is getting frankensteined by attaching `ExprEngine`'s environment and 
store to checker's GDM, while range constraints also reside in the GDM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70150



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

[PATCH] D70150: [analyzer] Don't clean up dead symbols from constraints twice.

2019-11-12 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ marked an inline comment as done.
NoQ added inline comments.
NoQ marked an inline comment as done.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:750-752
 // The constraint manager has not been cleaned up yet, so clean up now.
 CheckerState =
 getConstraintManager().removeDeadBindings(CheckerState, SymReaper);

Note: the second invocation is here.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:763-764
 // generate a transition to that state.
 ProgramStateRef CleanedCheckerSt =
 StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
 Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, , K);

Note: The results of the first invocation are discarded here, as the updated 
state is getting frankensteined by attaching `ExprEngine`'s environment and 
store to checker's GDM, while range constraints also reside in the GDM.


A solid 2% analysis speed improvement! Not really - such amounts are barely 
measurable.

But, i mean, everybody knows that dead symbol elimination is the biggest 
performance bottleneck, so many people tried to improve it, how come nobody 
noticed this bug before?


Repository:
  rC Clang

https://reviews.llvm.org/D70150

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ProgramState.cpp


Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -91,10 +91,9 @@
 I->second.second(I->second.first);
 }
 
-ProgramStateRef
-ProgramStateManager::removeDeadBindings(ProgramStateRef state,
-   const StackFrameContext *LCtx,
-   SymbolReaper& SymReaper) {
+ProgramStateRef ProgramStateManager::removeDeadBindingsFromEnvironmentAndStore(
+ProgramStateRef state, const StackFrameContext *LCtx,
+SymbolReaper ) {
 
   // This code essentially performs a "mark-and-sweep" of the VariableBindings.
   // The roots are any Block-level exprs and Decls that our liveness algorithm
@@ -112,8 +111,7 @@
   NewState.setStore(newStore);
   SymReaper.setReapedStore(newStore);
 
-  ProgramStateRef Result = getPersistentState(NewState);
-  return ConstraintMgr->removeDeadBindings(Result, SymReaper);
+  return getPersistentState(NewState);
 }
 
 ProgramStateRef ProgramState::bindLoc(Loc LV,
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -728,7 +728,8 @@
   // Create a state in which dead bindings are removed from the environment
   // and the store. TODO: The function should just return new env and store,
   // not a new state.
-  CleanedState = StateMgr.removeDeadBindings(CleanedState, SFC, SymReaper);
+  CleanedState = StateMgr.removeDeadBindingsFromEnvironmentAndStore(
+  CleanedState, SFC, SymReaper);
 
   // Process any special transfer function for dead symbols.
   // A tag to track convenience transitions, which can be removed at cleanup.
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -533,9 +533,10 @@
   ConstraintManager () { return *ConstraintMgr; }
   SubEngine () { return *Eng; }
 
-  ProgramStateRef removeDeadBindings(ProgramStateRef St,
-const StackFrameContext *LCtx,
-SymbolReaper& SymReaper);
+  ProgramStateRef
+  removeDeadBindingsFromEnvironmentAndStore(ProgramStateRef St,
+const StackFrameContext *LCtx,
+SymbolReaper );
 
 public:
 


Index: clang/lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -91,10 +91,9 @@
 I->second.second(I->second.first);
 }
 
-ProgramStateRef
-ProgramStateManager::removeDeadBindings(ProgramStateRef state,
-   const StackFrameContext *LCtx,
-   SymbolReaper& SymReaper) {
+ProgramStateRef

[PATCH] D70150: [analyzer] Don't clean up dead symbols from constraints twice.

2019-11-12 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:750-752
 // The constraint manager has not been cleaned up yet, so clean up now.
 CheckerState =
 getConstraintManager().removeDeadBindings(CheckerState, SymReaper);

Note: the second invocation is here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70150



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Maybe add an assert when the cancellation version is requested 
> > > > > > > > but the cancellation block is not set? Instead of the 
> > > > > > > > generating simple version of barrier.
> > > > > > > The interface doesn't work that way as we do not know here if the 
> > > > > > > cancellation was requested except if the block was set. That is 
> > > > > > > basically the flag (and I expect it to continue to be that way).
> > > > > > Maybe instead of `ForceSimpleBarrier` add a flag 
> > > > > > `EmitCancelBarrier` and if it set to true, always emit cancel 
> > > > > > barrier, otherwise always emit simple barrier? And add an assertion 
> > > > > > for non-set cancellation block or even accept it as a parameter 
> > > > > > here.
> > > > > > 
> > > > > > Also, what if we have inner exception handling in the region? Will 
> > > > > > you handle the cleanup correctly in case of the cancelation barrier?
> > > > > > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier 
> > > > > > and if it set to true, always emit cancel barrier, otherwise always 
> > > > > > emit simple barrier? And add an assertion for non-set cancellation 
> > > > > > block or even accept it as a parameter here.
> > > > > 
> > > > > What is the difference in moving some of the boolean logic to the 
> > > > > caller? Also, we have test to verify we get cancellation barriers if 
> > > > > we need them, both unit tests and clang lit tests.
> > > > > 
> > > > > 
> > > > > > Also, what if we have inner exception handling in the region? Will 
> > > > > > you handle the cleanup correctly in case of the cancelation barrier?
> > > > > 
> > > > > I think so. Right now through the code in clang that does the set up 
> > > > > of the cancellation block, later through callbacks but we only need 
> > > > > that for regions where we actually go out of some scope, e.g., 
> > > > > parallel.
> > > > 1. I'm just thinking about future users of thus interface. It woild be 
> > > > good if we could provide safe interface for all the users, not only 
> > > > clang.
> > > > 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> > > > try...catch or just simple compound statement with local vars that 
> > > > require constructors/destructors. And the cancellation barrier may exit 
> > > > out of these regions. And you need to call all required destructors. 
> > > > You'd better to think about it now, not later.
> > > > 2. [...] You'd better to think about it now, not later.
> > > 
> > > First, I do think about it now and I hope this was not an insinuation to 
> > > suggest otherwise.
> > > 
> > > > 1. I'm just thinking about future users of thus interface. It woild be 
> > > > good if we could provide safe interface for all the users, not only 
> > > > clang.
> > > > 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> > > > try...catch or just simple compound statement with local vars that 
> > > > require constructors/destructors. And the cancellation barrier may exit 
> > > > out of these regions. And you need to call all required destructors.
> > > 
> > > Generally speaking, we shall not add features that we cannot use or test 
> > > with the assumption we will use them in the future. This is suggested by 
> > > the LLVM best practices. If you have specific changes in mind that are 
> > > testable and better than what I suggested so far, please bring them 
> > > forward. You can also bring forward suggestions on how it might look in 
> > > the future but without a real use case now it is not practical to block a 
> > > review based on that, given that we can change the interface once the 
> > > time has come.
> > > 
> > > I said before, we will need callbacks for destructors, actual handling of 
> > > cancellation blocks, and there are various other features missing right 
> > > now. Nevertheless, we cannot build them into the current interface, or 
> > > even try to prepare for all of them, while keeping the patches small and 
> > > concise.
> > It won't work for clang, I'm afraid. You need a list of desructors here. 
> > But clang uses recursive codegen and it is very hard to walk over the call 
> > tree and gather all required destructors into a list. At least, it will 
> > require significant rework in clang frontend.
> > Instead of generating the branch to cancellation block in the builder, I 
> > would suggest to call a single callback function provided by the frontend, 
> > which will generate correct branch

LLVM buildmaster will be updated and restarted tonight

2019-11-12 Thread Galina Kistanova via cfe-commits

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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

[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

2019-11-12 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a subscriber: dblaikie.
rnk added a comment.

For reference, this is the code sequence that needs the SmallVector change:
https://github.com/llvm/llvm-project/blob/99e2cba219aea80b3f11de2aa4e0192b28852de4/clang/lib/Frontend/CompilerInvocation.cpp#L1872




Comment at: clang-tools-extra/clang-move/tool/ClangMove.cpp:113
   move::MoveDefinitionSpec Spec;
-  Spec.Names = {Names.begin(), Names.end()};
+  Spec.Names = (std::vector &)Names;
   Spec.OldHeader = OldHeader;

Converting from std::list to SmallVector by way of std::vector with a C-style 
cast is a bit surprising. Is there a simpler way to write this, like 
`Spec.Names.assign(Names.begin(), Names.end());`?



Comment at: llvm/include/llvm/ADT/SmallVector.h:905
+
+  const SmallVector =(const std::vector ) {
+this->assign(Vec.begin(), Vec.end());

+@dblaikie, who knows more about STL container details than I do.


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

https://reviews.llvm.org/D69959



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

[PATCH] D70149: [C-index] Fix annotate-deep-statements test in Debug target

2019-11-12 Thread Alexandre Ganea via Phabricator via cfe-commits

aganea created this revision.
aganea added reviewers: rsmith, aaron.ballman, rnk.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

As per title: previously, running the test 
`clang/test/Index/annotate-deep-statements.cpp` used to fail because of stack 
exhaustion (Debug target on Windows compiled with clang-cl)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70149

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3595,6 +3595,7 @@
 const char *const *command_line_args, int num_command_line_args,
 struct CXUnsavedFile *unsaved_files, unsigned num_unsaved_files,
 unsigned options, CXTranslationUnit *out_TU) {
+  noteBottomOfStack();
   SmallVector Args;
   Args.push_back("clang");
   Args.append(command_line_args, command_line_args + num_command_line_args);
@@ -3619,6 +3620,7 @@
 
   CXErrorCode result = CXError_Failure;
   auto ParseTranslationUnitImpl = [=, ] {
+noteBottomOfStack();
 result = clang_parseTranslationUnit_Impl(
 CIdx, source_filename, command_line_args, num_command_line_args,
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
@@ -6621,9 +6623,10 @@
 
 void clang_executeOnThread(void (*fn)(void*), void *user_data,
unsigned stack_size) {
-  llvm::llvm_execute_on_thread(
-  fn, user_data,
-  stack_size == 0 ? llvm::None : llvm::Optional(stack_size));
+  llvm::llvm_execute_on_thread(fn, user_data,
+   stack_size == 0
+   ? clang::DesiredStackSize
+   : llvm::Optional(stack_size));
 }
 
 
//===--===//
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12930,7 +12930,8 @@
 //   expression or statement in the body of the function [and thus before
 //   the value computation of its result].
 SequencedSubexpression Sequenced(*this);
-Base::VisitCallExpr(CE);
+SemaRef.runWithSufficientStackSpace(CE->getExprLoc(),
+[&] { Base::VisitCallExpr(CE); });
 
 // FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
   }


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3595,6 +3595,7 @@
 const char *const *command_line_args, int num_command_line_args,
 struct CXUnsavedFile *unsaved_files, unsigned num_unsaved_files,
 unsigned options, CXTranslationUnit *out_TU) {
+  noteBottomOfStack();
   SmallVector Args;
   Args.push_back("clang");
   Args.append(command_line_args, command_line_args + num_command_line_args);
@@ -3619,6 +3620,7 @@
 
   CXErrorCode result = CXError_Failure;
   auto ParseTranslationUnitImpl = [=, ] {
+noteBottomOfStack();
 result = clang_parseTranslationUnit_Impl(
 CIdx, source_filename, command_line_args, num_command_line_args,
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
@@ -6621,9 +6623,10 @@
 
 void clang_executeOnThread(void (*fn)(void*), void *user_data,
unsigned stack_size) {
-  llvm::llvm_execute_on_thread(
-  fn, user_data,
-  stack_size == 0 ? llvm::None : llvm::Optional(stack_size));
+  llvm::llvm_execute_on_thread(fn, user_data,
+   stack_size == 0
+   ? clang::DesiredStackSize
+   : llvm::Optional(stack_size));
 }
 
 //===--===//
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12930,7 +12930,8 @@
 //   expression or statement in the body of the function [and thus before
 //   the value computation of its result].
 SequencedSubexpression Sequenced(*this);
-Base::VisitCallExpr(CE);
+SemaRef.runWithSufficientStackSpace(CE->getExprLoc(),
+[&] { Base::VisitCallExpr(CE); });
 
 // FIXME: CXXNewExpr and CXXDeleteExpr implicitly call functions.
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > ABataev wrote:
> > > > > > > Maybe add an assert when the cancellation version is requested 
> > > > > > > but the cancellation block is not set? Instead of the generating 
> > > > > > > simple version of barrier.
> > > > > > The interface doesn't work that way as we do not know here if the 
> > > > > > cancellation was requested except if the block was set. That is 
> > > > > > basically the flag (and I expect it to continue to be that way).
> > > > > Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` 
> > > > > and if it set to true, always emit cancel barrier, otherwise always 
> > > > > emit simple barrier? And add an assertion for non-set cancellation 
> > > > > block or even accept it as a parameter here.
> > > > > 
> > > > > Also, what if we have inner exception handling in the region? Will 
> > > > > you handle the cleanup correctly in case of the cancelation barrier?
> > > > > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and 
> > > > > if it set to true, always emit cancel barrier, otherwise always emit 
> > > > > simple barrier? And add an assertion for non-set cancellation block 
> > > > > or even accept it as a parameter here.
> > > > 
> > > > What is the difference in moving some of the boolean logic to the 
> > > > caller? Also, we have test to verify we get cancellation barriers if we 
> > > > need them, both unit tests and clang lit tests.
> > > > 
> > > > 
> > > > > Also, what if we have inner exception handling in the region? Will 
> > > > > you handle the cleanup correctly in case of the cancelation barrier?
> > > > 
> > > > I think so. Right now through the code in clang that does the set up of 
> > > > the cancellation block, later through callbacks but we only need that 
> > > > for regions where we actually go out of some scope, e.g., parallel.
> > > 1. I'm just thinking about future users of thus interface. It woild be 
> > > good if we could provide safe interface for all the users, not only clang.
> > > 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> > > try...catch or just simple compound statement with local vars that 
> > > require constructors/destructors. And the cancellation barrier may exit 
> > > out of these regions. And you need to call all required destructors. 
> > > You'd better to think about it now, not later.
> > > 2. [...] You'd better to think about it now, not later.
> > 
> > First, I do think about it now and I hope this was not an insinuation to 
> > suggest otherwise.
> > 
> > > 1. I'm just thinking about future users of thus interface. It woild be 
> > > good if we could provide safe interface for all the users, not only clang.
> > > 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> > > try...catch or just simple compound statement with local vars that 
> > > require constructors/destructors. And the cancellation barrier may exit 
> > > out of these regions. And you need to call all required destructors.
> > 
> > Generally speaking, we shall not add features that we cannot use or test 
> > with the assumption we will use them in the future. This is suggested by 
> > the LLVM best practices. If you have specific changes in mind that are 
> > testable and better than what I suggested so far, please bring them 
> > forward. You can also bring forward suggestions on how it might look in the 
> > future but without a real use case now it is not practical to block a 
> > review based on that, given that we can change the interface once the time 
> > has come.
> > 
> > I said before, we will need callbacks for destructors, actual handling of 
> > cancellation blocks, and there are various other features missing right 
> > now. Nevertheless, we cannot build them into the current interface, or even 
> > try to prepare for all of them, while keeping the patches small and concise.
> It won't work for clang, I'm afraid. You need a list of desructors here. But 
> clang uses recursive codegen and it is very hard to walk over the call tree 
> and gather all required destructors into a list. At least, it will require 
> significant rework in clang frontend.
> Instead of generating the branch to cancellation block in the builder, I 
> would suggest to call a single callback function provided by the frontend, 
> which will generate correct branch over a chain of the destructor blocks. In 
> this case, you won't need this cancellation block at all. This is what I 
> meant when said that you need to think about this

[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

2019-11-12 Thread Alexandre Ganea via Phabricator via cfe-commits

aganea updated this revision to Diff 228971.
aganea edited the summary of this revision.
aganea added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Using `SmallVector` as requested.

The change in SmallVector.h is to support things such as `Opts.AddPluginActions 
= Args.getAllArgValues(OPT_add_plugin);` where `getAllArgValues()` returns a 
`std::vector`.


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

https://reviews.llvm.org/D69959

Files:
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang/include/clang/Frontend/FrontendOptions.h
  llvm/include/llvm/ADT/SmallVector.h

Index: llvm/include/llvm/ADT/SmallVector.h
===
--- llvm/include/llvm/ADT/SmallVector.h
+++ llvm/include/llvm/ADT/SmallVector.h
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 namespace llvm {
 
@@ -900,6 +901,11 @@
 this->assign(IL);
 return *this;
   }
+
+  const SmallVector =(const std::vector ) {
+this->assign(Vec.begin(), Vec.end());
+return *this;
+  }
 };
 
 template 
Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -18,7 +18,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 namespace llvm {
@@ -366,7 +365,7 @@
   std::string ARCMTMigrateReportOut;
 
   /// The input files and their types.
-  std::vector Inputs;
+  SmallVector Inputs;
 
   /// When the input is a module map, the original module map file from which
   /// that map was inferred, if any (for umbrella modules).
@@ -391,33 +390,33 @@
   std::string ActionName;
 
   /// Args to pass to the plugins
-  std::unordered_map> PluginArgs;
+  llvm::StringMap> PluginArgs;
 
   /// The list of plugin actions to run in addition to the normal action.
-  std::vector AddPluginActions;
+  SmallVector AddPluginActions;
 
   /// The list of plugins to load.
-  std::vector Plugins;
+  SmallVector Plugins;
 
   /// The list of module file extensions.
-  std::vector> ModuleFileExtensions;
+  SmallVector, 0> ModuleFileExtensions;
 
   /// The list of module map files to load before processing the input.
-  std::vector ModuleMapFiles;
+  SmallVector ModuleMapFiles;
 
   /// The list of additional prebuilt module files to load before
   /// processing the input.
-  std::vector ModuleFiles;
+  SmallVector ModuleFiles;
 
   /// The list of files to embed into the compiled module file.
-  std::vector ModulesEmbedFiles;
+  SmallVector ModulesEmbedFiles;
 
   /// The list of AST files to merge.
-  std::vector ASTMergeFiles;
+  SmallVector ASTMergeFiles;
 
   /// A list of arguments to forward to LLVM's option processing; this
   /// should only be used for debugging and experimental features.
-  std::vector LLVMArgs;
+  SmallVector LLVMArgs;
 
   /// File name of the file that will provide record layouts
   /// (in the format produced by -fdump-record-layouts).
Index: clang-tools-extra/clang-move/tool/ClangMove.cpp
===
--- clang-tools-extra/clang-move/tool/ClangMove.cpp
+++ clang-tools-extra/clang-move/tool/ClangMove.cpp
@@ -110,7 +110,7 @@
   Tool.appendArgumentsAdjuster(tooling::getInsertArgumentAdjuster(
   "-fparse-all-comments", tooling::ArgumentInsertPosition::BEGIN));
   move::MoveDefinitionSpec Spec;
-  Spec.Names = {Names.begin(), Names.end()};
+  Spec.Names = (std::vector &)Names;
   Spec.OldHeader = OldHeader;
   Spec.NewHeader = NewHeader;
   Spec.OldCC = OldCC;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > Maybe add an assert when the cancellation version is requested but 
> > > > > > the cancellation block is not set? Instead of the generating simple 
> > > > > > version of barrier.
> > > > > The interface doesn't work that way as we do not know here if the 
> > > > > cancellation was requested except if the block was set. That is 
> > > > > basically the flag (and I expect it to continue to be that way).
> > > > Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` 
> > > > and if it set to true, always emit cancel barrier, otherwise always 
> > > > emit simple barrier? And add an assertion for non-set cancellation 
> > > > block or even accept it as a parameter here.
> > > > 
> > > > Also, what if we have inner exception handling in the region? Will you 
> > > > handle the cleanup correctly in case of the cancelation barrier?
> > > > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and if 
> > > > it set to true, always emit cancel barrier, otherwise always emit 
> > > > simple barrier? And add an assertion for non-set cancellation block or 
> > > > even accept it as a parameter here.
> > > 
> > > What is the difference in moving some of the boolean logic to the caller? 
> > > Also, we have test to verify we get cancellation barriers if we need 
> > > them, both unit tests and clang lit tests.
> > > 
> > > 
> > > > Also, what if we have inner exception handling in the region? Will you 
> > > > handle the cleanup correctly in case of the cancelation barrier?
> > > 
> > > I think so. Right now through the code in clang that does the set up of 
> > > the cancellation block, later through callbacks but we only need that for 
> > > regions where we actually go out of some scope, e.g., parallel.
> > 1. I'm just thinking about future users of thus interface. It woild be good 
> > if we could provide safe interface for all the users, not only clang.
> > 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> > try...catch or just simple compound statement with local vars that require 
> > constructors/destructors. And the cancellation barrier may exit out of 
> > these regions. And you need to call all required destructors. You'd better 
> > to think about it now, not later.
> > 2. [...] You'd better to think about it now, not later.
> 
> First, I do think about it now and I hope this was not an insinuation to 
> suggest otherwise.
> 
> > 1. I'm just thinking about future users of thus interface. It woild be good 
> > if we could provide safe interface for all the users, not only clang.
> > 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> > try...catch or just simple compound statement with local vars that require 
> > constructors/destructors. And the cancellation barrier may exit out of 
> > these regions. And you need to call all required destructors.
> 
> Generally speaking, we shall not add features that we cannot use or test with 
> the assumption we will use them in the future. This is suggested by the LLVM 
> best practices. If you have specific changes in mind that are testable and 
> better than what I suggested so far, please bring them forward. You can also 
> bring forward suggestions on how it might look in the future but without a 
> real use case now it is not practical to block a review based on that, given 
> that we can change the interface once the time has come.
> 
> I said before, we will need callbacks for destructors, actual handling of 
> cancellation blocks, and there are various other features missing right now. 
> Nevertheless, we cannot build them into the current interface, or even try to 
> prepare for all of them, while keeping the patches small and concise.
It won't work for clang, I'm afraid. You need a list of desructors here. But 
clang uses recursive codegen and it is very hard to walk over the call tree and 
gather all required destructors into a list. At least, it will require 
significant rework in clang frontend.
Instead of generating the branch to cancellation block in the builder, I would 
suggest to call a single callback function provided by the frontend, which will 
generate correct branch over a chain of the destructor blocks. In this case, 
you won't need this cancellation block at all. This is what I meant when said 
that you need to think about this problem right now. That current solution is 
not very suitable for the use in the frontend.


Repository:
  rG LLVM Github Monorepo

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

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

2019-11-12 Thread Richard Trieu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9740f9f0b6e5: Add -Wtautological-compare to -Wall (authored 
by rtrieu).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D69292?vs=228361=228970#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69292

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/Misc/warning-wall.c
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/Sema/warn-overlap.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp

Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 void test(int x) {
   bool b1 = (8 & x) == 3;
Index: clang/test/Sema/warn-overlap.c
===
--- clang/test/Sema/warn-overlap.c
+++ clang/test/Sema/warn-overlap.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-overlap-compare %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused -Wno-loop-analysis %s
 
 #define mydefine 2
 
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
 
Index: clang/test/Misc/warning-wall.c
===
--- /dev/null
+++ clang/test/Misc/warning-wall.c
@@ -0,0 +1,95 @@
+RUN: diagtool tree -Wall > %t 2>&1
+RUN: FileCheck --input-file=%t %s
+
+ CHECK:-Wall
+CHECK-NEXT:  -Wmost
+CHECK-NEXT:-Wchar-subscripts
+CHECK-NEXT:-Wcomment
+CHECK-NEXT:-Wdelete-non-virtual-dtor
+CHECK-NEXT:  -Wdelete-non-abstract-non-virtual-dtor
+CHECK-NEXT:  -Wdelete-abstract-non-virtual-dtor
+CHECK-NEXT:-Wfor-loop-analysis
+CHECK-NEXT:-Wformat
+CHECK-NEXT:  -Wformat-extra-args
+CHECK-NEXT:  -Wformat-zero-length
+CHECK-NEXT:  -Wnonnull
+CHECK-NEXT:  -Wformat-security
+CHECK-NEXT:  -Wformat-y2k
+CHECK-NEXT:  -Wformat-invalid-specifier
+CHECK-NEXT:-Wimplicit
+CHECK-NEXT:  -Wimplicit-function-declaration
+CHECK-NEXT:  -Wimplicit-int
+CHECK-NEXT:-Winfinite-recursion
+CHECK-NEXT:-Wint-in-bool-context
+CHECK-NEXT:-Wmismatched-tags
+CHECK-NEXT:-Wmissing-braces
+CHECK-NEXT:-Wmove
+CHECK-NEXT:  -Wpessimizing-move
+CHECK-NEXT:  -Wredundant-move
+CHECK-NEXT:  -Wreturn-std-move
+CHECK-NEXT:  -Wself-move
+CHECK-NEXT:-Wmultichar
+CHECK-NEXT:-Wreorder
+CHECK-NEXT:  -Wreorder-ctor
+CHECK-NEXT:  -Wreorder-init-list
+CHECK-NEXT:-Wreturn-type
+CHECK-NEXT:  -Wreturn-type-c-linkage
+CHECK-NEXT:-Wself-assign
+CHECK-NEXT:  -Wself-assign-overloaded
+CHECK-NEXT:  -Wself-assign-field
+CHECK-NEXT:-Wself-move
+CHECK-NEXT:-Wsizeof-array-argument
+CHECK-NEXT:-Wsizeof-array-decay
+CHECK-NEXT:-Wstring-plus-int
+CHECK-NEXT:-Wtautological-compare
+CHECK-NEXT:  -Wtautological-constant-compare
+CHECK-NEXT:-Wtautological-constant-out-of-range-compare
+CHECK-NEXT:  -Wtautological-pointer-compare
+CHECK-NEXT:  -Wtautological-overlap-compare
+CHECK-NEXT:  -Wtautological-bitwise-compare
+CHECK-NEXT:  -Wtautological-undefined-compare
+CHECK-NEXT:  -Wtautological-objc-bool-compare
+CHECK-NEXT:-Wtrigraphs
+CHECK-NEXT:-Wuninitialized
+CHECK-NEXT:  -Wsometimes-uninitialized
+CHECK-NEXT:  -Wstatic-self-init
+CHECK-NEXT:-Wunknown-pragmas
+CHECK-NEXT:-Wunused
+CHECK-NEXT:  -Wunused-argument
+CHECK-NEXT:  -Wunused-function
+CHECK-NEXT:-Wunneeded-internal-declaration
+CHECK-NEXT:  -Wunused-label
+CHECK-NEXT:  -Wunused-private-field
+CHECK-NEXT:  -Wunused-lambda-capture
+CHECK-NEXT:  -Wunused-local-typedef
+CHECK-NEXT:  -Wunused-value
+CHECK-NEXT:-Wunused-comparison
+CHECK-NEXT:-Wunused-result
+CHECK-NEXT:-Wunevaluated-expression
+CHECK-NEXT:  -Wpotentially-evaluated-expression
+CHECK-NEXT:  -Wunused-variable
+CHECK-NEXT:-Wunused-const-variable
+CHECK-NEXT:  -Wunused-property-ivar
+CHECK-NEXT:-Wvolatile-register-var
+CHECK-NEXT:-Wobjc-missing-super-calls
+CHECK-NEXT:-Wobjc-designated-initializers
+CHECK-NEXT:-Wobjc-flexible-array
+CHECK-NEXT:-Woverloaded-virtual
+CHECK-NEXT:-Wprivate-extern
+CHECK-NEXT:-Wcast-of-sel-type
+CHECK-NEXT:-Wextern-c-compat
+CHECK-NEXT:-Wuser-defined-warnings

[clang] 9740f9f - Add -Wtautological-compare to -Wall

2019-11-12 Thread via cfe-commits


Author: Weverything
Date: 2019-11-12T14:36:57-08:00
New Revision: 9740f9f0b6e5d7d5104027aee7edc9c5202dd052

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

LOG: Add -Wtautological-compare to -Wall

Some warnings in -Wtautological-compare subgroups are DefaultIgnore.
Adding this group to -Wmost, which is part of -Wall, will aid in their
discoverability.

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

Added: 
clang/test/Misc/warning-wall.c

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/test/Sema/warn-bitwise-compare.c
clang/test/Sema/warn-overlap.c
clang/test/SemaCXX/warn-bitwise-compare.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 29d27ec681f5..bc66a8253074 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -843,6 +843,7 @@ def Most : DiagGroup<"most", [
 SizeofArrayArgument,
 SizeofArrayDecay,
 StringPlusInt,
+TautologicalCompare,
 Trigraphs,
 Uninitialized,
 UnknownPragmas,

diff  --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
new file mode 100644
index ..fadcceefe297
--- /dev/null
+++ b/clang/test/Misc/warning-wall.c
@@ -0,0 +1,95 @@
+RUN: diagtool tree -Wall > %t 2>&1
+RUN: FileCheck --input-file=%t %s
+
+ CHECK:-Wall
+CHECK-NEXT:  -Wmost
+CHECK-NEXT:-Wchar-subscripts
+CHECK-NEXT:-Wcomment
+CHECK-NEXT:-Wdelete-non-virtual-dtor
+CHECK-NEXT:  -Wdelete-non-abstract-non-virtual-dtor
+CHECK-NEXT:  -Wdelete-abstract-non-virtual-dtor
+CHECK-NEXT:-Wfor-loop-analysis
+CHECK-NEXT:-Wformat
+CHECK-NEXT:  -Wformat-extra-args
+CHECK-NEXT:  -Wformat-zero-length
+CHECK-NEXT:  -Wnonnull
+CHECK-NEXT:  -Wformat-security
+CHECK-NEXT:  -Wformat-y2k
+CHECK-NEXT:  -Wformat-invalid-specifier
+CHECK-NEXT:-Wimplicit
+CHECK-NEXT:  -Wimplicit-function-declaration
+CHECK-NEXT:  -Wimplicit-int
+CHECK-NEXT:-Winfinite-recursion
+CHECK-NEXT:-Wint-in-bool-context
+CHECK-NEXT:-Wmismatched-tags
+CHECK-NEXT:-Wmissing-braces
+CHECK-NEXT:-Wmove
+CHECK-NEXT:  -Wpessimizing-move
+CHECK-NEXT:  -Wredundant-move
+CHECK-NEXT:  -Wreturn-std-move
+CHECK-NEXT:  -Wself-move
+CHECK-NEXT:-Wmultichar
+CHECK-NEXT:-Wreorder
+CHECK-NEXT:  -Wreorder-ctor
+CHECK-NEXT:  -Wreorder-init-list
+CHECK-NEXT:-Wreturn-type
+CHECK-NEXT:  -Wreturn-type-c-linkage
+CHECK-NEXT:-Wself-assign
+CHECK-NEXT:  -Wself-assign-overloaded
+CHECK-NEXT:  -Wself-assign-field
+CHECK-NEXT:-Wself-move
+CHECK-NEXT:-Wsizeof-array-argument
+CHECK-NEXT:-Wsizeof-array-decay
+CHECK-NEXT:-Wstring-plus-int
+CHECK-NEXT:-Wtautological-compare
+CHECK-NEXT:  -Wtautological-constant-compare
+CHECK-NEXT:-Wtautological-constant-out-of-range-compare
+CHECK-NEXT:  -Wtautological-pointer-compare
+CHECK-NEXT:  -Wtautological-overlap-compare
+CHECK-NEXT:  -Wtautological-bitwise-compare
+CHECK-NEXT:  -Wtautological-undefined-compare
+CHECK-NEXT:  -Wtautological-objc-bool-compare
+CHECK-NEXT:-Wtrigraphs
+CHECK-NEXT:-Wuninitialized
+CHECK-NEXT:  -Wsometimes-uninitialized
+CHECK-NEXT:  -Wstatic-self-init
+CHECK-NEXT:-Wunknown-pragmas
+CHECK-NEXT:-Wunused
+CHECK-NEXT:  -Wunused-argument
+CHECK-NEXT:  -Wunused-function
+CHECK-NEXT:-Wunneeded-internal-declaration
+CHECK-NEXT:  -Wunused-label
+CHECK-NEXT:  -Wunused-private-field
+CHECK-NEXT:  -Wunused-lambda-capture
+CHECK-NEXT:  -Wunused-local-typedef
+CHECK-NEXT:  -Wunused-value
+CHECK-NEXT:-Wunused-comparison
+CHECK-NEXT:-Wunused-result
+CHECK-NEXT:-Wunevaluated-expression
+CHECK-NEXT:  -Wpotentially-evaluated-expression
+CHECK-NEXT:  -Wunused-variable
+CHECK-NEXT:-Wunused-const-variable
+CHECK-NEXT:  -Wunused-property-ivar
+CHECK-NEXT:-Wvolatile-register-var
+CHECK-NEXT:-Wobjc-missing-super-calls
+CHECK-NEXT:-Wobjc-designated-initializers
+CHECK-NEXT:-Wobjc-flexible-array
+CHECK-NEXT:-Woverloaded-virtual
+CHECK-NEXT:-Wprivate-extern
+CHECK-NEXT:-Wcast-of-sel-type
+CHECK-NEXT:-Wextern-c-compat
+CHECK-NEXT:-Wuser-defined-warnings
+CHECK-NEXT:  -Wparentheses
+CHECK-NEXT:-Wlogical-op-parentheses
+CHECK-NEXT:-Wlogical-not-parentheses
+CHECK-NEXT:-Wbitwise-conditional-parentheses
+CHECK-NEXT:-Wbitwise-op-parentheses
+CHECK-NEXT:-Wshift-op-parentheses
+CHECK-NEXT:-Woverloaded-shift-op-parentheses
+CHECK-NEXT:-Wparentheses-equality
+CHECK-NEXT:-Wdangling-else
+CHECK-NEXT:  -Wswitch
+CHECK-NEXT:  -Wswitch-bool
+
+
+CHECK-NOT:-W

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

2019-11-12 Thread Chi Chun Chen via Phabricator via cfe-commits

cchen added a comment.

Not sure how to test this change since the issue only happens after I add more 
implicit-behaviors for extended defaultmap. Now `ActOnOpenMPDefaultmapClause` 
is doing `M != OMPC_DEFAULTMAP_MODIFIER_tofrom`, so even the modifier is set  
to be "scalar", the code is still right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70142



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

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

2019-11-12 Thread Michael Kruse via Phabricator via cfe-commits

Meinersbur added a comment.

Sorry for the delay, for some reason I did not see a notification for the 
update.




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

[serious] Why not reusing the `Args.push_back` code from above? I think it is 
important `vectorize_predicate` and `vectorize_width` (and ever additional 
property we introduce in the future) the same way. IMHO everything else becomes 
more and more confusing.
I have the following in mind:

```
if (Attrs.VectorizeEnable != LoopAttributes::Unspecified ||
  IsVectorPredicateEnabled || Attrs.VectorizeWidth > 1) {
auto AttrVal = Attrs.VectorizeEnable != LoopAttributes::Disable;
Args.push_back(..., ConstantInt::get(AttrVal));
}
```



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

https://reviews.llvm.org/D69628



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > Maybe add an assert when the cancellation version is requested but 
> > > > > the cancellation block is not set? Instead of the generating simple 
> > > > > version of barrier.
> > > > The interface doesn't work that way as we do not know here if the 
> > > > cancellation was requested except if the block was set. That is 
> > > > basically the flag (and I expect it to continue to be that way).
> > > Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and 
> > > if it set to true, always emit cancel barrier, otherwise always emit 
> > > simple barrier? And add an assertion for non-set cancellation block or 
> > > even accept it as a parameter here.
> > > 
> > > Also, what if we have inner exception handling in the region? Will you 
> > > handle the cleanup correctly in case of the cancelation barrier?
> > > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and if 
> > > it set to true, always emit cancel barrier, otherwise always emit simple 
> > > barrier? And add an assertion for non-set cancellation block or even 
> > > accept it as a parameter here.
> > 
> > What is the difference in moving some of the boolean logic to the caller? 
> > Also, we have test to verify we get cancellation barriers if we need them, 
> > both unit tests and clang lit tests.
> > 
> > 
> > > Also, what if we have inner exception handling in the region? Will you 
> > > handle the cleanup correctly in case of the cancelation barrier?
> > 
> > I think so. Right now through the code in clang that does the set up of the 
> > cancellation block, later through callbacks but we only need that for 
> > regions where we actually go out of some scope, e.g., parallel.
> 1. I'm just thinking about future users of thus interface. It woild be good 
> if we could provide safe interface for all the users, not only clang.
> 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> try...catch or just simple compound statement with local vars that require 
> constructors/destructors. And the cancellation barrier may exit out of these 
> regions. And you need to call all required destructors. You'd better to think 
> about it now, not later.
> 2. [...] You'd better to think about it now, not later.

First, I do think about it now and I hope this was not an insinuation to 
suggest otherwise.

> 1. I'm just thinking about future users of thus interface. It woild be good 
> if we could provide safe interface for all the users, not only clang.
> 2. Exit out of the OpenMP region is not allowed. But you may have inner 
> try...catch or just simple compound statement with local vars that require 
> constructors/destructors. And the cancellation barrier may exit out of these 
> regions. And you need to call all required destructors.

Generally speaking, we shall not add features that we cannot use or test with 
the assumption we will use them in the future. This is suggested by the LLVM 
best practices. If you have specific changes in mind that are testable and 
better than what I suggested so far, please bring them forward. You can also 
bring forward suggestions on how it might look in the future but without a real 
use case now it is not practical to block a review based on that, given that we 
can change the interface once the time has come.

I said before, we will need callbacks for destructors, actual handling of 
cancellation blocks, and there are various other features missing right now. 
Nevertheless, we cannot build them into the current interface, or even try to 
prepare for all of them, while keeping the patches small and concise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 228959.
nridge added a comment.

Add unit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -140,7 +140,7 @@
   }
   for (auto  : ExpectedLines)
 ExpectedLinePairHighlighting.push_back(
-{LineTokens.first, LineTokens.second});
+{LineTokens.first, LineTokens.second, /*IsInactive = */ false});
 
   std::vector ActualDiffed =
   diffHighlightings(NewTokens, OldTokens);
@@ -589,6 +589,33 @@
   R"cpp(
   void $Function[[foo]]();
   using ::$Function[[foo]];
+)cpp",
+  // Inactive code highlighting
+  R"cpp(
+  // FIXME: In the preamble, no inactive code highlightings are produced.
+  #ifdef $Macro[[test]]
+  #endif
+
+  // A declaration to cause the preamble to end.
+  int $Variable[[EndPreamble]];
+
+  // Code after the preamble.
+  // Inactive lines get an empty InactiveCode token at the beginning.
+  // Code inside inactive blocks does not get regular highlightings
+  // because it's not part of the AST.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive1;
+$InactiveCode[[]]  #endif
+
+  #ifndef $Macro[[test]]
+  int $Variable[[Active1]];
+  #endif
+
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive2;
+$InactiveCode[[]]  #else
+  int $Variable[[Active2]];
+  #endif
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
@@ -656,10 +683,12 @@
{{HighlightingKind::Variable,
  Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
 {HighlightingKind::Function,
- Range{CreatePosition(3, 4), CreatePosition(3, 7),
+ Range{CreatePosition(3, 4), CreatePosition(3, 7)}}},
+   /* IsInactive = */ false},
   {1,
{{HighlightingKind::Variable,
- Range{CreatePosition(1, 1), CreatePosition(1, 5)};
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)}}},
+   /* IsInactive = */ true}};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -57,6 +57,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
@@ -66,6 +69,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -81,10 +85,12 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -100,6 +106,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -115,6 +122,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": ""
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -44,7 +44,11 @@
   Primitive,
   Macro,
 
-  LastKind = Macro
+  // This one is different from the other kinds as it's a line style
+  // rather than a token style.
+

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:143
+for (const SourceRange  :
+ AST.getPreprocessor().getPreprocessingRecord()->getSkippedRanges()) {
+  if (isInsideMainFile(R.getBegin(), SM)) {

hokein wrote:
> I think the current implementation only collects the skipped preprocessing 
> ranges **after preamble** region in the main file.  We probably need to store 
> these ranges in PreambleData to make the ranges in the preamble region work, 
> no need to do it in this patch, but we'd better have a test reflecting this 
> fact. 
> 
> ```
> #ifdef ActiveCOde
> // inactive code here
> #endif
> 
> #include "foo.h"
> // preamble ends here
> 
> namespace ns {
> // ..
> }  
> ```
Your observation is correct: the current implementation only highlights 
inactive lines after the preamble. For now, I added a test case with a FIXME 
for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

[PATCH] D67536: [clangd] Inactive regions support as an extension to semantic highlighting

2019-11-12 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 228960.
nridge marked 11 inline comments as done.
nridge added a comment.

Address one minor remaining comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -140,7 +140,7 @@
   }
   for (auto  : ExpectedLines)
 ExpectedLinePairHighlighting.push_back(
-{LineTokens.first, LineTokens.second});
+{LineTokens.first, LineTokens.second, /*IsInactive = */ false});
 
   std::vector ActualDiffed =
   diffHighlightings(NewTokens, OldTokens);
@@ -589,6 +589,33 @@
   R"cpp(
   void $Function[[foo]]();
   using ::$Function[[foo]];
+)cpp",
+  // Inactive code highlighting
+  R"cpp(
+  // FIXME: In the preamble, no inactive code highlightings are produced.
+  #ifdef $Macro[[test]]
+  #endif
+
+  // A declaration to cause the preamble to end.
+  int $Variable[[EndPreamble]];
+
+  // Code after the preamble.
+  // Inactive lines get an empty InactiveCode token at the beginning.
+  // Code inside inactive blocks does not get regular highlightings
+  // because it's not part of the AST.
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive1;
+$InactiveCode[[]]  #endif
+
+  #ifndef $Macro[[test]]
+  int $Variable[[Active1]];
+  #endif
+
+$InactiveCode[[]]  #ifdef $Macro[[test]]
+$InactiveCode[[]]  int Inactive2;
+$InactiveCode[[]]  #else
+  int $Variable[[Active2]];
+  #endif
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
@@ -656,10 +683,12 @@
{{HighlightingKind::Variable,
  Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
 {HighlightingKind::Function,
- Range{CreatePosition(3, 4), CreatePosition(3, 7),
+ Range{CreatePosition(3, 4), CreatePosition(3, 7)}}},
+   /* IsInactive = */ false},
   {1,
{{HighlightingKind::Variable,
- Range{CreatePosition(1, 1), CreatePosition(1, 5)};
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)}}},
+   /* IsInactive = */ true}};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -57,6 +57,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
@@ -66,6 +69,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -81,10 +85,12 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -100,6 +106,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -115,6 +122,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": ""
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -44,7 +44,11 @@
   Primitive,
   Macro,
 
-  LastKind = Macro
+  // This one is different from the other kinds as

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > Maybe add an assert when the cancellation version is requested but the 
> > > > cancellation block is not set? Instead of the generating simple version 
> > > > of barrier.
> > > The interface doesn't work that way as we do not know here if the 
> > > cancellation was requested except if the block was set. That is basically 
> > > the flag (and I expect it to continue to be that way).
> > Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and if 
> > it set to true, always emit cancel barrier, otherwise always emit simple 
> > barrier? And add an assertion for non-set cancellation block or even accept 
> > it as a parameter here.
> > 
> > Also, what if we have inner exception handling in the region? Will you 
> > handle the cleanup correctly in case of the cancelation barrier?
> > Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and if it 
> > set to true, always emit cancel barrier, otherwise always emit simple 
> > barrier? And add an assertion for non-set cancellation block or even accept 
> > it as a parameter here.
> 
> What is the difference in moving some of the boolean logic to the caller? 
> Also, we have test to verify we get cancellation barriers if we need them, 
> both unit tests and clang lit tests.
> 
> 
> > Also, what if we have inner exception handling in the region? Will you 
> > handle the cleanup correctly in case of the cancelation barrier?
> 
> I think so. Right now through the code in clang that does the set up of the 
> cancellation block, later through callbacks but we only need that for regions 
> where we actually go out of some scope, e.g., parallel.
1. I'm just thinking about future users of thus interface. It woild be good if 
we could provide safe interface for all the users, not only clang.
2. Exit out of the OpenMP region is not allowed. But you may have inner 
try...catch or just simple compound statement with local vars that require 
constructors/destructors. And the cancellation barrier may exit out of these 
regions. And you need to call all required destructors. You'd better to think 
about it now, not later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

2019-11-12 Thread Dimitry Andric via Phabricator via cfe-commits

dim added a comment.

In D70143#1742885 , @lebedev.ri wrote:

> This should have a llvm ir test in `llvm/test/transforms/instcombine` i 
> think, not a clang test.

Yes, I thought so too, but I could not get it to work (i.e. crash) with LLVM 
IR.  I just don't understand how `opt` works, and it does not have a 
`-fno-builtin-strlen` option either.  Therefore, I made it work with clang, as 
having a working test is better than no test at all.  But I'm fine with leaving 
out the test, it was just for completeness' sake.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70143

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

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-12 Thread Conrad Poelman via Phabricator via cfe-commits

poelmanc created this revision.
poelmanc added reviewers: malcolm.parsons, angelgarcia, aaron.ballman.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

`modernize-use-equals-default` replaces default constructors/destructors with 
`= default;`. When the optional semicolon after a member function 
 is present, this results in 
two consecutive semicolons.

This small patch checks to see if the next non-comment token after the code to 
be replaced is a semicolon, and if so offers a replacement of `= default` 
rather than `= default;`.

This patch adds trailing comments and semicolons to about 5 existing tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D70144

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
@@ -7,7 +7,7 @@
   ~OL();
 };
 
-OL::OL() {}
+OL::OL() {};
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a 
trivial default constructor [modernize-use-equals-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
@@ -17,9 +17,9 @@
 // Inline definitions.
 class IL {
 public:
-  IL() {}
+  IL() {}   ; // Note embedded tab on this line
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: IL() = default;
+  // CHECK-FIXES: IL() = default; // Note embedded tab on this line
   ~IL() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~IL() = default;
@@ -46,18 +46,20 @@
 // Default member initializer
 class DMI {
 public:
-  DMI() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: DMI() = default;
+  DMI() {} // Comment before semi-colon on next line
+  ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+  // CHECK-FIXES: DMI() = default // Comment before semi-colon on next line
+  // CHECK-FIXES-NEXT:   ;
   int Field = 5;
 };
 
 // Class member
 class CM {
 public:
-  CM() {}
+  CM() {} /* Comments */ /* before */ /* semicolon */;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: CM() = default;
+  // CHECK-FIXES: CM() = default /* Comments */ /* before */ /* semicolon */;
   OL o;
 };
 
@@ -66,7 +68,7 @@
   Priv() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: Priv() = default;
-  ~Priv() {}
+  ~Priv() {};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~Priv() = default;
 };
Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
@@ -119,7 +119,7 @@
 struct BF {
   BF() = default;
   BF(const BF ) : Field1(Other.Field1), Field2(Other.Field2), 
Field3(Other.Field3),
-Field4(Other.Field4) {}
+Field4(Other.Field4) {};
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
   // CHECK-FIXES: BF(const BF ) {{$}}
   // CHECK-FIXES: = default;
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -302,9 +302,19 @@
   auto Diag = diag(Location, "use '= default' to define a trivial " +
  SpecialFunctionName);
 
-  if (ApplyFix)
-Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+  if (ApplyFix) {
+// Peek ahead to see if there's a semicolon after Body->getSourceRange()
+Optional Token;
+do {
+  Token = Lexer::findNextToken(
+  Body->getSourceRange().getEnd().getLocWithOffset(1),
+  Result.Context->getSourceManager(), Result.Context->getLangOpts());
+} while (Token && Token->is(tok::comment));
+StringRef Replacement =
+Token && Token->is(tok::semi) ? "= default" : "= default;";
+Diag << FixItHint::CreateReplacement(Body->getSourceRange(), Replacement)
  << RemoveInitializers;
+  }
 }
 
 } // namespace modernize


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===
---

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > Maybe add an assert when the cancellation version is requested but the 
> > > cancellation block is not set? Instead of the generating simple version 
> > > of barrier.
> > The interface doesn't work that way as we do not know here if the 
> > cancellation was requested except if the block was set. That is basically 
> > the flag (and I expect it to continue to be that way).
> Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and if 
> it set to true, always emit cancel barrier, otherwise always emit simple 
> barrier? And add an assertion for non-set cancellation block or even accept 
> it as a parameter here.
> 
> Also, what if we have inner exception handling in the region? Will you handle 
> the cleanup correctly in case of the cancelation barrier?
> Maybe instead of ForceSimpleBarrier add a flag EmitCancelBarrier and if it 
> set to true, always emit cancel barrier, otherwise always emit simple 
> barrier? And add an assertion for non-set cancellation block or even accept 
> it as a parameter here.

What is the difference in moving some of the boolean logic to the caller? Also, 
we have test to verify we get cancellation barriers if we need them, both unit 
tests and clang lit tests.


> Also, what if we have inner exception handling in the region? Will you handle 
> the cleanup correctly in case of the cancelation barrier?

I think so. Right now through the code in clang that does the set up of the 
cancellation block, later through callbacks but we only need that for regions 
where we actually go out of some scope, e.g., parallel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

2019-11-12 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

Yes, i will prepare a new patch and add him as a reviewer.


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

https://reviews.llvm.org/D68185



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

2019-11-12 Thread Paul Taylor via Phabricator via cfe-commits

ptaylor added a comment.

A similar PR has been submitted to the `vscode-cpptools` repo to enable 
debugging CUDA files via `cuda-gdb` with the official Microsoft VSCode C++ 
plugin: https://github.com/microsoft/vscode-cpptools/pull/4585


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

jdoerfert wrote:
> ABataev wrote:
> > Maybe add an assert when the cancellation version is requested but the 
> > cancellation block is not set? Instead of the generating simple version of 
> > barrier.
> The interface doesn't work that way as we do not know here if the 
> cancellation was requested except if the block was set. That is basically the 
> flag (and I expect it to continue to be that way).
Maybe instead of `ForceSimpleBarrier` add a flag `EmitCancelBarrier` and if it 
set to true, always emit cancel barrier, otherwise always emit simple barrier? 
And add an assertion for non-set cancellation block or even accept it as a 
parameter here.

Also, what if we have inner exception handling in the region? Will you handle 
the cleanup correctly in case of the cancelation barrier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

2019-11-12 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

This should have a llvm ir test in `llvm/test/transforms/instcombine` i think, 
not a clang test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70143



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

[PATCH] D67750: Allow additional file suffixes/extensions considered as source in main include grouping

2019-11-12 Thread MyDeveloperDay via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG335ac2eb662c: Allow additional file suffixes/extensions 
considered as source in main include… (authored by MyDeveloperDay).

Changed prior to commit:
  https://reviews.llvm.org/D67750?vs=228872=228953#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67750

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -466,6 +466,57 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, LeavesMainHeaderFirstInAdditionalExtensions) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?|(Impl)?$";
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"b.h\"\n"
+"#include \"c.h\"\n"
+"#include \"llvm/a.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+
+  // .cpp extension is considered "main" by default
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.cpp"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.cpp"));
+
+  // Allow additional filenames / extensions
+  Style.IncludeIsMainSourceRegex = "(Impl\\.hpp)|(\\.xxx)$";
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "a_test.xxx"));
+  EXPECT_EQ("#include \"llvm/a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"llvm/a.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"b.h\"\n",
+ "aImpl.hpp"));
+}
+
 TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Merge;
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12847,6 +12847,8 @@
   IncludeStyle.IncludeCategories, ExpectedCategories);
   CHECK_PARSE("IncludeIsMainRegex: 'abc$'", IncludeStyle.IncludeIsMainRegex,
   "abc$");
+  CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'",
+  IncludeStyle.IncludeIsMainSourceRegex, "abc$");
 
   Style.RawStringFormats.clear();
   std::vector ExpectedRawStringFormats = {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -75,9 +75,9 @@
 
 static cl::opt AssumeFileName(
 "assume-filename",
-cl::desc("When reading from stdin, clang-format assumes this\n"
- "filename to look for a style config file (with\n"
- "-style=file) and to determine the language."),
+cl::desc("Override filename used to determine the language.\n"
+ "When reading from stdin, clang-format assumes this\n"
+ "filename to determine the language."),
 cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt Inplace("i",
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -184,6 +184,10 @@
FileName.endswith(".cpp") || FileName.endswith(".c++") ||
FileName.endswith(".cxx") || FileName.endswith(".m") ||
FileName.endswith(".mm");
+  if

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added a comment.

Add a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70142



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

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

2019-11-12 Thread Dimitry Andric via Phabricator via cfe-commits

dim created this revision.
dim added reviewers: xbolva00, spatel, jdoerfert, efriedma.
Herald added subscribers: cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

This fixes PR43081, where the transformation of `strchr(p, 0) -> p +
strlen(p)` can cause a segfault, if `-fno-builtin-strlen` is used.  In
that case, `emitStrLen` returns nullptr, which CreateGEP is not designed
to handle.  Also add the minimized code from the PR as a test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70143

Files:
  clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp


Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -363,9 +363,11 @@
   // a string literal.  If so, we can constant fold.
   StringRef Str;
   if (!getConstantStringInfo(SrcStr, Str)) {
-if (CharC->isZero()) // strchr(p, 0) -> p + strlen(p)
-  return B.CreateGEP(B.getInt8Ty(), SrcStr, emitStrLen(SrcStr, B, DL, TLI),
- "strchr");
+if (CharC->isZero()) { // strchr(p, 0) -> p + strlen(p)
+  Value *StrLen = emitStrLen(SrcStr, B, DL, TLI);
+  return StrLen ? B.CreateGEP(B.getInt8Ty(), SrcStr, StrLen, "strchr")
+: nullptr;
+}
 return nullptr;
   }
 
Index: clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-- -S -O1 -fno-builtin-strlen %s -o - | 
FileCheck %s
+char *strchr(const char *, int);
+char *b(char *a) {
+  return strchr(a, '\0');
+// CHECK: jmp  strchr
+}


Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -363,9 +363,11 @@
   // a string literal.  If so, we can constant fold.
   StringRef Str;
   if (!getConstantStringInfo(SrcStr, Str)) {
-if (CharC->isZero()) // strchr(p, 0) -> p + strlen(p)
-  return B.CreateGEP(B.getInt8Ty(), SrcStr, emitStrLen(SrcStr, B, DL, TLI),
- "strchr");
+if (CharC->isZero()) { // strchr(p, 0) -> p + strlen(p)
+  Value *StrLen = emitStrLen(SrcStr, B, DL, TLI);
+  return StrLen ? B.CreateGEP(B.getInt8Ty(), SrcStr, StrLen, "strchr")
+: nullptr;
+}
 return nullptr;
   }
 
Index: clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-replace-strchr-with-strlen.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-- -S -O1 -fno-builtin-strlen %s -o - | FileCheck %s
+char *strchr(const char *, int);
+char *b(char *a) {
+  return strchr(a, '\0');
+// CHECK: jmp	strchr
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70142: [OpenMP5.0] Fix a parsing issue for the extended defaultmap

2019-11-12 Thread Chi Chun Chen via Phabricator via cfe-commits

cchen created this revision.
cchen added a reviewer: ABataev.
Herald added subscribers: cfe-commits, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
cchen retitled this revision from "[OpenMP5.0] Fix a parsing issue for the 
extended defaultmap

This patch is for fixing a possible parsing issue in the OpenMP5.0 defaultmap 
patch.

In this case: "#pragma omp target defaultmap(scalar:", the implicit-behavior 
after the parsing will..." to "[OpenMP5.0] Fix a parsing issue for the extended 
defaultmap".
cchen edited the summary of this revision.
cchen removed a subscriber: guansong.

This patch is for fixing a possible parsing issue in the OpenMP5.0 defaultmap 
patch.
In this case:

  #pragma omp target defaultmap(scalar:

the implicit-behavior after the parsing will not be 
OMPC_DEFAULTMAP_MODIFIER_unknown.
So we need to either enforce the enum value of implicit behavior to be unknown 
or modify the logic inside getOpenMPSimpleClauseType.
And we now choose to just enforce the enum value of implicit behavior to be 
unknown.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70142

Files:
  clang/lib/Parse/ParseOpenMP.cpp


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2310,8 +2310,14 @@
   DelimLoc = ConsumeAnyToken();
   } else if (Kind == OMPC_defaultmap) {
 // Get a defaultmap modifier
-Arg.push_back(getOpenMPSimpleClauseType(
-Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok)));
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer (modifier should be either alloc, to, from, tofrom, 
firstprivate,
+// none, or default)
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);
 KLoc.push_back(Tok.getLocation());
 if (Tok.isNot(tok::r_paren) && Tok.isNot(tok::comma) &&
 Tok.isNot(tok::annot_pragma_openmp_end))


Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2310,8 +2310,14 @@
   DelimLoc = ConsumeAnyToken();
   } else if (Kind == OMPC_defaultmap) {
 // Get a defaultmap modifier
-Arg.push_back(getOpenMPSimpleClauseType(
-Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok)));
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer (modifier should be either alloc, to, from, tofrom, firstprivate,
+// none, or default)
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);
 KLoc.push_back(Tok.getLocation());
 if (Tok.isNot(tok::r_paren) && Tok.isNot(tok::comma) &&
 Tok.isNot(tok::annot_pragma_openmp_end))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

2019-11-12 Thread Johannes Doerfert via Phabricator via cfe-commits

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



Comment at: llvm/include/llvm/IR/OpenMPKinds.def:186
+///{
+
+#ifndef OMP_IDENT_FLAG

JonChesterfield wrote:
> Meinersbur wrote:
> > JonChesterfield wrote:
> > > jdoerfert wrote:
> > > > Meinersbur wrote:
> > > > > jdoerfert wrote:
> > > > > > JonChesterfield wrote:
> > > > > > > Sharing constants between the compiler and the runtime is an 
> > > > > > > interesting subproblem. I think the cleanest solution is the one 
> > > > > > > used by libc, where the compiler generates header files 
> > > > > > > containing the constants which the runtime library includes.
> > > > > > I'd prefer not to tackle this right now but get this done first and 
> > > > > > revisit the issue later. OK?
> > > > > I don't think this is a good solution. It means that libomp cannot 
> > > > > built built anymore without also building clang. Moreover, the values 
> > > > > cannot be changed anyway since it would break the ABI.
> > > > > 
> > > > > I'd go the other route: The libomp defines what it's ABI is, the 
> > > > > compiler generates code for it. 
> > > > This patch doesn't change what we do, just where. The numbers are hard 
> > > > coded in clang now. Let's start a discussion on the list and if we come 
> > > > up with a different scheme we do it after this landed.
> > > Revisit later sounds good.
> > > 
> > > @Meinersbur Do you know of an example of a non-llvm compiler using this 
> > > libomp?
> > > 
> > > The usual order is build a compiler, then use it to build the runtime 
> > > libraries, then the whole package can build other stuff. Provided the 
> > > compiler doesn't need any of the runtime libraries (compiler-rt, maths 
> > > libraries, libomp etc) itself the system bootstraps cleanly. Especially 
> > > important when cross compiling and I suspect the gpu targets in openmp 
> > > have similarly strict requirements on the first compiler.
> > > 
> > > Closely related to that, I tend to assume that the runtime libraries can 
> > > be rewritten to best serve their only client - the associated compiler - 
> > > so if libomp is used by out of tree compilers I'd like to know who we are 
> > > at risk of breaking.
> > > Do you know of an example of a non-llvm compiler using this libomp?
> > 
> > [[ 
> > https://github.com/llvm-project/llvm-project/blob/master/openmp/runtime/src/kmp_gsupport.cpp
> >  | gcc  ]] (using libomp's gomp compatibility layer), [[ 
> > https://www.openmprtl.org/ | icc  ]] (as libomp was initially donated by 
> > Intel).
> > 
> > I don't understand why it even matters if there are other compilers using 
> > libomp. Every LLVM runtime library can be built stand-alone. 
> > With constant values being determined during compiler bootstrapping, 
> > programs built on one computer would be potentially ABI-incompatible with a 
> > runtime library on another. Think about updating your 
> > compiler-rt/libomp/libc++ on you computer causing all existing binaries on 
> > the system to crash because constants changed in the updated compiler's 
> > bootstrapping process.
> > 
> > The only use case I know that does this is are operating system's syscall 
> > tables. Linux's reference is [[ 
> > https://github.com/torvalds/linux/blob/master/arch/sh/include/uapi/asm/unistd_64.h
> >  | unistd.h ]] which is platform-specific and Windows generates the table 
> > during its [[ https://j00ru.vexillium.org/syscalls/nt/64/ | build process 
> > ]]. Therefore on Windows, system calls can only be done through ntdll. Even 
> > on Linux one should use the system's libc instead of directly invoking a 
> > system call.
> Thanks. GCC and ICC would presumably be happier with the magic numbers stored 
> with openmp then (though with the move to a monorepo that's a little less 
> persuasive).
> 
> When constants that affect the ABI change, the result won't work with 
> existing software regardless of whether the compiler or the library contains 
> the change. Either the new compiler builds things that don't work with the 
> old library, or the new library doesn't work with things built by the old 
> compiler. The two have to agree on the ABI.
> 
> At present, openmp does the moral equivalent of #include OpenMPKinds.def from 
> clang. Moving the constants to libomp means clang will do the equivalent of 
> #include OpenMPKinds.def from openmp. Breaking that dependency means making a 
> new subproject that just holds/generates the constants, that both depend on, 
> which seems more hassle than it's worth. 
> 
> I'd like to generate this header as part of the clang build (though 
> ultimately don't care that much if it's generated as part of the openmp 
> build) because it's going to become increasingly challenging to read as 
> non-nvptx architectures are introduced. Likewise it would be useful to 
> generate the interface.h for deviceRTL (or equivalently a set of unit tests 
> checking the function types) from the same

[clang] 335ac2e - Allow additional file suffixes/extensions considered as source in main include grouping

2019-11-12 Thread via cfe-commits


Author: mydeveloperday
Date: 2019-11-12T21:26:52Z
New Revision: 335ac2eb662ce5f1888e2a50310b01fba2d40d68

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

LOG: Allow additional file suffixes/extensions considered as source in main 
include grouping

Summary:
By additional regex match, grouping of main include can be enabled in files 
that are not normally considered as a C/C++ source code.
For example, this might be useful in templated code, where template 
implementations are being held in *Impl.hpp files.
On the occassion, 'assume-filename' option description was reworded as it was 
misleading. It has nothing to do with `style=file` option and it does not 
influence sourced style filename.

Reviewers: rsmith, ioeric, krasimir, sylvestre.ledru, MyDeveloperDay

Reviewed By: MyDeveloperDay

Subscribers: MyDeveloperDay, cfe-commits

Patch by:  furdyna

Tags: #clang

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

Added: 


Modified: 
clang/docs/ClangFormat.rst
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/include/clang/Tooling/Inclusions/IncludeStyle.h
clang/lib/Format/Format.cpp
clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
clang/tools/clang-format/ClangFormat.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortIncludesTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index 7fe050ec9e29..1138b2332670 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -31,9 +31,9 @@ to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.
   Clang-format options:
 
 --Werror   - If set, changes formatting warnings to errors
---assume-filename= - When reading from stdin, clang-format assumes 
this
- filename to look for a style config file (with
- -style=file) and to determine the language.
+--assume-filename= - Override filename used to determine the 
language.
+ When reading from stdin, clang-format assumes 
this
+ filename to determine the language.
 --cursor=- The position of the cursor when invoking
  clang-format from an editor integration
 --dry-run  - If set, do not actually make the formatting 
changes

diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index e0ad96532136..1a9c5b8395f4 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1581,6 +1581,26 @@ the configuration (without a prefix: ``Auto``).
   For example, if configured to "(_test)?$", then a header a.h would be seen
   as the "main" include in both a.cc and a_test.cc.
 
+**IncludeIsMainSourceRegex** (``std::string``)
+  Specify a regular expression for files being formatted
+  that are allowed to be considered "main" in the
+  file-to-main-include mapping.
+
+  By default, clang-format considers files as "main" only when they end
+  with: ``.c``, ``.cc``, ``.cpp``, ``.c++``, ``.cxx``, ``.m`` or ``.mm``
+  extensions.
+  For these files a guessing of "main" include takes place
+  (to assign category 0, see above). This config option allows for
+  additional suffixes and extensions for files to be considered as "main".
+
+  For example, if this option is configured to ``(Impl\.hpp)$``,
+  then a file ``ClassImpl.hpp`` is considered "main" (in addition to
+  ``Class.c``, ``Class.cc``, ``Class.cpp`` and so on) and "main
+  include file" logic will be executed (with *IncludeIsMainRegex* setting
+  also being respected in later phase). Without this option set,
+  ``ClassImpl.hpp`` would not have the main include file put on top
+  before any other include.
+
 **IndentCaseLabels** (``bool``)
   Indent case labels one level from the switch statement.
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3db1603e0631..1139116ed101 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -292,6 +292,24 @@ clang-format
 - clang-format gets a new option called ``--dry-run`` or ``-n`` to emit a
   warning.
 
+- Option *IncludeIsMainSourceRegex* added to allow for additional
+  suffixes and file extensions to be considered as a source file
+  for execution of logic that looks for "main *include* file" to put
+  it on top.
+
+  By default, clang-format considers *source* files as "main" only when
+  they end with: ``.c``, ``.cc``, ``.cpp``, ``.c++``, ``.cxx``,
+  ``.m`` or ``.mm`` extensions. This config option allows to
+  extend this set of source files considered as "main".
+

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

2019-11-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

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



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

In D68185#1742809 , @xbolva00 wrote:

> Ah, Clang has it under -Wdeprecated. 
> https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated.
>
> But combo -Wall -Wextra does not enable this warning, sadly.
>
> This should be extracted to -Wdeprecated-copy and put into -Wextra. Seems 
> fine for you?


Sounds plausible to me - though I'd expect @rsmith should probably have a final 
say before that's submitted.


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

https://reviews.llvm.org/D68185



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

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

2019-11-12 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 228948.
hokein marked 4 inline comments as done.
hokein added a comment.

- make rename only work when the position is on the name range of the decl
- add more testsj


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -38,20 +38,20 @@
   return Result;
 }
 
-TEST(RenameTest, SingleFile) {
-  // "^" points to the position of the rename, and "[[]]" ranges point to the
+TEST(RenameTest, WithinFileRename) {
+  // rename is runnning on all "^" points, and "[[]]" ranges point to the
   // identifier that is being renamed.
   llvm::StringRef Tests[] = {
   // Rename function.
   R"cpp(
-void [[foo]]() {
+void [[foo^]]() {
   [[fo^o]]();
 }
   )cpp",
 
   // Rename type.
   R"cpp(
-struct [[foo]]{};
+struct [[foo^]]{};
 [[foo]] test() {
[[f^oo]] x;
return x;
@@ -66,18 +66,313 @@
   }
 }
   )cpp",
+
+  R"cpp(
+class [[F^oo]] {};
+template  void func() {}
+template  class Baz {};
+int main() {
+  func<[[F^oo]]>();
+  Baz<[[F^oo]]> obj;
+  return 0;
+}
+  )cpp",
+
+  // class simple rename
+  R"cpp(
+class [[F^oo]] {
+  void foo(int x);
+};
+void [[Foo]]::foo(int x) {}
+  )cpp",
+
+  // class constructor/destructor.
+  R"cpp(
+class [[^Foo]] {
+  [[F^oo]]();
+  ~[[Foo]]();
+};
+  )cpp",
+
+  // class overrides
+  R"cpp(
+struct A {
+ virtual void [[f^oo]]() {}
+};
+struct B : A {
+  void [[f^oo]]() override {}
+};
+struct C : B {
+  void [[f^oo]]() override {}
+};
+
+void func() {
+  A().[[f^oo]]();
+  B().[[f^oo]]();
+  C().[[f^oo]]();
+}
+  )cpp",
+
+  // complicated class type
+  R"cpp(
+ // Forward declaration.
+class [[Fo^o]];
+class Baz {
+  virtual int getValue() const = 0;
+};
+
+class [[F^oo]] : public Baz  {
+public:
+  [[Foo]](int value = 0) : x(value) {}
+
+  [[Foo]] ++(int);
+
+  bool operator<([[Foo]] const );
+  int getValue() const;
+private:
+  int x;
+};
+
+void func() {
+  [[Foo]] *Pointer = 0;
+  [[Foo]] Variable = [[Foo]](10);
+  for ([[Foo]] it; it < Variable; it++);
+  const [[Foo]] *C = new [[Foo]]();
+  const_cast<[[Foo]] *>(C)->getValue();
+  [[Foo]] foo;
+  const Baz  = foo;
+  const Baz *BazPointer = 
+  dynamic_cast(BazReference).getValue();
+  dynamic_cast(BazPointer)->getValue();
+  reinterpret_cast(BazPointer)->getValue();
+  static_cast(BazReference).getValue();
+  static_cast(BazPointer)->getValue();
+}
+  )cpp",
+
+  // class constructors
+  R"cpp(
+class [[^Foo]] {
+public:
+  [[Foo]]();
+};
+
+[[Foo]]::[[Fo^o]]() {}
+  )cpp",
+
+  // constructor initializer list
+  R"cpp(
+class Baz {};
+class Qux {
+  Baz [[F^oo]];
+public:
+  Qux();
+};
+// FIXME: selection tree on initializer list Foo below will returns
+// CXXConstructExpr node, which ends up with renaming class "Baze"
+// instead of "Foo".
+Qux::Qux() : [[Foo]]() {}
+  )cpp",
+
+  // DeclRefExpr
+  R"cpp(
+class C {
+public:
+  static int [[F^oo]];
+};
+
+int foo(int x) { return 0; }
+#define MACRO(a) foo(a)
+
+void func() {
+  C::[[F^oo]] = 1;
+  MACRO(C::[[Foo]]);
+  int y = C::[[F^oo]];
+}
+  )cpp",
+
+  // Forward declaration
+  R"cpp(
+class [[F^oo]];
+[[Foo]] *f();
+  )cpp",
+
+  // function marco
+  R"cpp(
+#define moo [[foo]]
+int [[fo^o]]() { return 42; }
+void boo(int value) {}
+
+void qoo() {
+  [[foo]]();
+  boo([[foo]]());
+  moo();
+  boo(moo());
+}
+  )cpp",
+
+  // MemberExpr in macro
+  R"cpp(
+class Baz {
+public:
+  int [[F^oo]];
+};
+int qux(int x) { return 0; }
+#define MACRO(a) qux(a)
+
+int main() {
+  Baz baz;
+  baz.[[Foo]] = 1;
+  MACRO(baz.[[Foo]]);
+  int

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

2019-11-12 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:326
+// class Foo, but the token under the cursor is not corresponding to the
+// "Foo" range, though the final result is correct.
 SourceLocation Loc = getBeginningOfIdentifier(

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > I would argue rename should not work in that case.
> > > Could we check that the cursor is actually on the name token of the 
> > > `NamedDecl` and not rename if it isn't?
> > you are probably right, we only allow rename which is triggered on the name 
> > token. Will update the patch.
> Definitely think we should do it before landing the patch.
> Otherwise we'll introduce regressions.
Done. 



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:225
   tooling::Replacements FilteredChanges;
-  for (const tooling::SymbolOccurrence  :
-   findOccurrencesWithinFile(AST, RenameDecl)) {
-// Currently, we only support normal rename (one range) for C/C++.
-// FIXME: support multiple-range rename for objective-c methods.
-if (Rename.getNameRanges().size() > 1)
-  continue;
-// We shouldn't have conflicting replacements. If there are conflicts, it
-// means that we have bugs either in clangd or in Rename library, therefore
-// we refuse to perform the rename.
+  for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) {
+// We shouldn't have conflicting replacements or replacements from 
different

ilya-biryukov wrote:
> This seems to assume all occurrences are outside macros.
> Won't it break in presence of macros?
> Do we have tests when the renamed token is:
> - inside macro body
> - inside macro arguments
> ?
if Loc is a macro location, tooling::Replacement will use the spelling 
location, so if the spelling location is in the main file, the code is correct, 
we have test cases for it.

but we will fail on cases like below, we should fix this, note that this issue 
exists even before this patch. Updated the comment here.

```
// foo.h
#define MACRO foo

// foo.cc
void f() {
  int fo^o = 2;
  MACRO;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

2019-11-12 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

Ah, Clang has it under -Wdeprecated. 
https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated.

But combo -Wall -Wextra does not enable this warning, sadly.

This should be extracted to -Wdeprecated-copy and put into -Wextra. Seems fine 
for you?


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

https://reviews.llvm.org/D68185



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

[PATCH] D69935: [DeclCXX] Remove unknown external linkage specifications

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

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

Looks good - thanks!


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

https://reviews.llvm.org/D69935



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

[PATCH] D70139: [clangd] Add bool return type to Index::refs API.

2019-11-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits

merge_guards_bot added a comment.

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



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70139



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

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

2019-11-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

sorry searching through old issues, are you still interested in this patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54628



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

[PATCH] D70139: [clangd] Add bool return type to Index::refs API.

2019-11-12 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Similar to fuzzyFind, the bool indicates whether there are more xref
results.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70139

Files:
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/MemIndex.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Merge.h
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DexTests.cpp
  clang-tools-extra/clangd/unittests/IndexTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2161,9 +2161,10 @@
 TEST(FindReferences, NoQueryForLocalSymbols) {
   struct RecordingIndex : public MemIndex {
 mutable Optional> RefIDs;
-void refs(const RefsRequest ,
+bool refs(const RefsRequest ,
   llvm::function_ref) const override {
   RefIDs = Req.IDs;
+  return false;
 }
   };
 
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -389,7 +389,8 @@
   RefsRequest Request;
   Request.IDs = {Foo.ID};
   RefSlab::Builder Results;
-  Merge.refs(Request, [&](const Ref ) { Results.insert(Foo.ID, O); });
+  EXPECT_FALSE(
+  Merge.refs(Request, [&](const Ref ) { Results.insert(Foo.ID, O); }));
   EXPECT_THAT(
   std::move(Results).build(),
   ElementsAre(Pair(
@@ -400,7 +401,8 @@
 
   Request.Limit = 1;
   RefSlab::Builder Results2;
-  Merge.refs(Request, [&](const Ref ) { Results2.insert(Foo.ID, O); });
+  EXPECT_TRUE(
+  Merge.refs(Request, [&](const Ref ) { Results2.insert(Foo.ID, O); }));
   EXPECT_THAT(std::move(Results2).build(),
   ElementsAre(Pair(
   _, ElementsAre(AnyOf(FileURI("unittest:///test.cc"),
Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -687,14 +687,18 @@
   Req.Filter = RefKind::Declaration | RefKind::Definition;
 
   std::vector Files;
-  Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
-  .refs(Req, [&](const Ref ) { Files.push_back(R.Location.FileURI); });
+  EXPECT_FALSE(Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
+   .refs(Req, [&](const Ref ) {
+ Files.push_back(R.Location.FileURI);
+   }));
   EXPECT_THAT(Files, UnorderedElementsAre("foo.h", "foo.cc"));
 
   Req.Limit = 1;
   Files.clear();
-  Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
-  .refs(Req, [&](const Ref ) { Files.push_back(R.Location.FileURI); });
+  EXPECT_TRUE(Dex(std::vector{Foo, Bar}, Refs, RelationSlab())
+  .refs(Req, [&](const Ref ) {
+Files.push_back(R.Location.FileURI);
+  }));
   EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc")));
 }
 
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1193,8 +1193,10 @@
   void lookup(const LookupRequest &,
   llvm::function_ref) const override {}
 
-  void refs(const RefsRequest &,
-llvm::function_ref) const override {}
+  bool refs(const RefsRequest &,
+llvm::function_ref) const override {
+return false;
+  }
 
   void relations(const RelationsRequest &,
  llvm::function_ref)
Index: clang-tools-extra/clangd/index/dex/Dex.h
===
--- clang-tools-extra/clangd/index/dex/Dex.h
+++ clang-tools-extra/clangd/index/dex/Dex.h
@@ -77,7 +77,7 @@
   void lookup(const LookupRequest ,
   llvm::function_ref Callback) const override;
 
-  void refs(const RefsRequest ,
+  bool refs(const RefsRequest ,
 llvm::function_ref Callback) const override;
 
   void relations(const RelationsRequest ,
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -249,18 +249,26 @@
   }
 }
 
-void Dex::refs(const RefsRequest ,

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

2019-11-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

Yeah, I'm OK with warning on the deprecated cases (especially for symmetry with 
GCC's warning) - not sure why that didn't occur to me/why I didn't mention it 
in my previous comment...

Deprecation wording is, in C++17, 15.8.1 [class.copy.ctor] p6, and 15.8.2 
[class.copy.assign] p2.


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

https://reviews.llvm.org/D68185



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

[PATCH] D6920: [clang-format] Add SpaceBeforeBrackets

2019-11-12 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

I realize this is an old diff, and you and I have just spoke about it on 
twitter https://twitter.com/NIV_Anteru/status/1193792307386081281?s=20 would 
you consider rebasing as the /brief style isn't used any more

It would help the approval process if you could highlight a public project with 
a style guide that uses this style.

I think this is a simple enough addition to consider by getting it upto date 
will help




Comment at: include/clang/Format/Format.h:415
+  bool SpaceBeforeSquareBrackets;
+
   /// \brief Supported language standards.

when you rebase you'll see examples of /codeblocks with before and after I 
think that would really help


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6920



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

[PATCH] D69598: Work on cleaning up denormal mode handling

2019-11-12 Thread Andy Kaylor via Phabricator via cfe-commits

andrew.w.kaylor accepted this revision.
andrew.w.kaylor added a comment.

Thanks. I understand your direction for denormal handling now, and I'm OK with 
this patch apart from the remaining references to subnormal that Sanjay 
mentioned.

In D69598#1739723 , @arsenm wrote:

> I do think the floating point environment bits should be a considered a 
> property of the calling convention, with attributes that override them. A 
> function which calls a function with a different mode would be responsible 
> for switching the mode before the call. This would require people actually 
> caring about getting this right to really implement

Do you mean the compiler should insert code to restore the FP environment on 
function transitions? Or do you mean that the function itself (i.e. the user's 
code) is responsible for switching the mode? I have some reservations about 
this, but I think the C standard specification for FENV_ACCESS is clear that 
the it is the programmer's responsibility to manage the floating point 
environment correctly. Yes, that's a sure recipe for broken code, but that's 
what it says. Obviously LLVM IR is not bound by the C standard and we could 
take a different approach, but I have concerns about the performance 
implications because in general the compiler won't know when the environment 
needs to be restored so it would have to take a conservative approach.

I've been meaning to write some documentation on the expected behavior at 
function boundaries of the FP environment. Perhaps we can continue this 
discussion there.

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

https://reviews.llvm.org/D69598

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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

2019-11-12 Thread Alexandre Ganea via Phabricator via cfe-commits

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

In D69825#1742276 , @hans wrote:

> Instead of invoking main() (or a similar function like ClangDriverMain) as an 
> alternative to spinning up the cc1 process, would it be possible to call 
> ExecuteCC1Tool() directly? I guess it would be a little less general, but 
> maybe it would be more straight-forward?

Possibly, yes, I'll have to check. There might be a few edge cases where we 
re-enter `ClangDriverMain()` twice if I recall correctly - I'll check.

In D69825#1742621 , @zturner wrote:

> Does this change crash recovery semantics in any meaningful way?  Will we 
> still be able to get stack traces on all platforms when the compiler crashes?

Yes, the changes in `CrashRecoveryContext` allow the same side-effects as the 
global exception handler. You would see the callstack and minidump in the same 
way.

There's still a minimal risk of memory/heap/CRT corruption, even with 
`CrashRecoveryContext` (after a crash). But that would possibly affect 
`LLVMUnhandledExceptionFilter` as well, and right now (//without this patch//), 
that handler runs in the context of the **crashed process **(not the calling 
one) so it wouldn't be worse than what it is now.

I could write a follow-up patch to prepare //on startup// the cmd-line invoked 
by `Driver::generateCompilationDiagnostics()`, instead of preparing after a 
crash. We could also pre-allocate a few virtual pages on advance, and use that 
in a BumpAllocator, instead of allocating after the crash. We could also merge 
the feature of `llvm-symbolizer` into `clang.exe`, so that we could call-back 
into `clang.exe` to render the callstack on stdout, even if `llvm-symbolizer` 
is not present.

The only drawback I see now is that we don't get the coredump on Linux, because 
the program doesn't end through the kernel signal handler. However, given that 
@Meinersbur says he sees no improvement on his side, we could disable this 
optimization on non-win? (or hide it behind a disabled flag on non-win)

Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+  ClangDriverMain = Driver::Main;

Meinersbur wrote:
> Why is this check necessary? Why not assuming that if `Driver::Main` is set, 
> it will be the right one?
The driver builds phases that do not always call the cc1 process. Simply 
stating `clang a.cpp` would invoke `clang -cc1`, then the linker. In the later 
case, even if we have `Driver::Main` it doesn't mean we should use it. There 
are a number of other edge cases of the same kind, such as `/fallback` or build 
cuda files.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825

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

[PATCH] D70047: [Analyzer] Use a reference in a range-based for

2019-11-12 Thread Mark de Wever via Phabricator via cfe-commits

Mordante added a comment.

@aaron.ballman good catch, I've fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70047



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

[PATCH] D70047: [Analyzer] Use a reference in a range-based for

2019-11-12 Thread Mark de Wever via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG964842861c8a: [Analyzer] Use a reference in a range-based 
for (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D70047?vs=228582=228935#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70047

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -91,7 +91,7 @@
   }
 
   assert(checkers);
-  for (const auto checker : *checkers)
+  for (const auto  : *checkers)
 checker(D, mgr, BR);
 }
 
@@ -99,7 +99,7 @@
   BugReporter ) {
   assert(D && D->hasBody());
 
-  for (const auto BodyChecker : BodyCheckers)
+  for (const auto  : BodyCheckers)
 BodyChecker(D, mgr, BR);
 }
 
@@ -402,7 +402,7 @@
 void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph ,
BugReporter ,
ExprEngine ) {
-  for (const auto EndAnalysisChecker : EndAnalysisCheckers)
+  for (const auto  : EndAnalysisCheckers)
 EndAnalysisChecker(G, BR, Eng);
 }
 
@@ -455,7 +455,7 @@
   // creates a successor for Pred, we do not need to generate an
   // autotransition for it.
   NodeBuilder Bldr(Pred, Dst, BC);
-  for (const auto checkFn : EndFunctionCheckers) {
+  for (const auto  : EndFunctionCheckers) {
 const ProgramPoint  =
 FunctionExitPoint(RS, Pred->getLocationContext(), checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
@@ -542,7 +542,7 @@
 /// Run checkers for live symbols.
 void CheckerManager::runCheckersForLiveSymbols(ProgramStateRef state,
SymbolReaper ) {
-  for (const auto LiveSymbolsChecker : LiveSymbolsCheckers)
+  for (const auto  : LiveSymbolsCheckers)
 LiveSymbolsChecker(state, SymReaper);
 }
 
@@ -599,7 +599,7 @@
 ArrayRef Regions,
 const LocationContext *LCtx,
 const CallEvent *Call) {
-  for (const auto RegionChangesChecker : RegionChangesCheckers) {
+  for (const auto  : RegionChangesCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -621,7 +621,7 @@
   (Kind != PSK_DirectEscapeOnCall &&
Kind != PSK_IndirectEscapeOnCall)) &&
  "Call must not be NULL when escaping on call");
-  for (const auto PointerEscapeChecker : PointerEscapeCheckers) {
+  for (const auto  : PointerEscapeCheckers) {
 // If any checker declares the state infeasible (or if it starts that
 //  way), bail out.
 if (!State)
@@ -635,7 +635,7 @@
 ProgramStateRef
 CheckerManager::runCheckersForEvalAssume(ProgramStateRef state,
  SVal Cond, bool Assumption) {
-  for (const auto EvalAssumeChecker : EvalAssumeCheckers) {
+  for (const auto  : EvalAssumeCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -658,7 +658,7 @@
 NodeBuilder B(Pred, checkDst, Eng.getBuilderContext());
 
 // Check if any of the EvalCall callbacks can evaluate the call.
-for (const auto EvalCallChecker : EvalCallCheckers) {
+for (const auto  : EvalCallCheckers) {
   // TODO: Support the situation when the call doesn't correspond
   // to any Expr.
   ProgramPoint L = ProgramPoint::getProgramPoint(
@@ -697,7 +697,7 @@
   const TranslationUnitDecl *TU,
   AnalysisManager ,
   BugReporter ) {
-  for (const auto EndOfTranslationUnitChecker : EndOfTranslationUnitCheckers)
+  for (const auto  : EndOfTranslationUnitCheckers)
 EndOfTranslationUnitChecker(TU, mgr, BR);
 }
 
@@ -904,6 +904,6 @@
 }
 
 CheckerManager::~CheckerManager() {
-  for (const auto CheckerDtor : CheckerDtors)
+  for (const auto  : CheckerDtors)
 CheckerDtor();
 }
Index: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
===
--- clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -567,7 +567,7 @@
 if (I == Events.end())
   return;
 const EventInfo  = I->second;
-for (const auto Checker : info.Checkers)
+for (const auto  : info.Checkers)
   Checker();
   }
 
___
cfe-commits

[clang] 9648428 - [Analyzer] Use a reference in a range-based for

2019-11-12 Thread Mark de Wever via cfe-commits


Author: Mark de Wever
Date: 2019-11-12T20:53:08+01:00
New Revision: 964842861c8acd53b8df8799f7c3800c5528fb72

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

LOG: [Analyzer] Use a reference in a range-based for

Let the checkers use a reference instead of a copy in a range-based
for loop.

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 38a9aaf72c27..45ab652d9e02 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -567,7 +567,7 @@ class CheckerManager {
 if (I == Events.end())
   return;
 const EventInfo  = I->second;
-for (const auto Checker : info.Checkers)
+for (const auto  : info.Checkers)
   Checker();
   }
 

diff  --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp 
b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index f676bd895283..a9361837cf68 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -91,7 +91,7 @@ void CheckerManager::runCheckersOnASTDecl(const Decl *D, 
AnalysisManager& mgr,
   }
 
   assert(checkers);
-  for (const auto checker : *checkers)
+  for (const auto  : *checkers)
 checker(D, mgr, BR);
 }
 
@@ -99,7 +99,7 @@ void CheckerManager::runCheckersOnASTBody(const Decl *D, 
AnalysisManager& mgr,
   BugReporter ) {
   assert(D && D->hasBody());
 
-  for (const auto BodyChecker : BodyCheckers)
+  for (const auto  : BodyCheckers)
 BodyChecker(D, mgr, BR);
 }
 
@@ -402,7 +402,7 @@ void CheckerManager::runCheckersForBind(ExplodedNodeSet 
,
 void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph ,
BugReporter ,
ExprEngine ) {
-  for (const auto EndAnalysisChecker : EndAnalysisCheckers)
+  for (const auto  : EndAnalysisCheckers)
 EndAnalysisChecker(G, BR, Eng);
 }
 
@@ -455,7 +455,7 @@ void 
CheckerManager::runCheckersForEndFunction(NodeBuilderContext ,
   // creates a successor for Pred, we do not need to generate an
   // autotransition for it.
   NodeBuilder Bldr(Pred, Dst, BC);
-  for (const auto checkFn : EndFunctionCheckers) {
+  for (const auto  : EndFunctionCheckers) {
 const ProgramPoint  =
 FunctionExitPoint(RS, Pred->getLocationContext(), checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
@@ -542,7 +542,7 @@ void CheckerManager::runCheckersForNewAllocator(
 /// Run checkers for live symbols.
 void CheckerManager::runCheckersForLiveSymbols(ProgramStateRef state,
SymbolReaper ) {
-  for (const auto LiveSymbolsChecker : LiveSymbolsCheckers)
+  for (const auto  : LiveSymbolsCheckers)
 LiveSymbolsChecker(state, SymReaper);
 }
 
@@ -599,7 +599,7 @@ CheckerManager::runCheckersForRegionChanges(ProgramStateRef 
state,
 ArrayRef 
Regions,
 const LocationContext *LCtx,
 const CallEvent *Call) {
-  for (const auto RegionChangesChecker : RegionChangesCheckers) {
+  for (const auto  : RegionChangesCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -621,7 +621,7 @@ CheckerManager::runCheckersForPointerEscape(ProgramStateRef 
State,
   (Kind != PSK_DirectEscapeOnCall &&
Kind != PSK_IndirectEscapeOnCall)) &&
  "Call must not be NULL when escaping on call");
-  for (const auto PointerEscapeChecker : PointerEscapeCheckers) {
+  for (const auto  : PointerEscapeCheckers) {
 // If any checker declares the state infeasible (or if it starts that
 //  way), bail out.
 if (!State)
@@ -635,7 +635,7 @@ CheckerManager::runCheckersForPointerEscape(ProgramStateRef 
State,
 ProgramStateRef
 CheckerManager::runCheckersForEvalAssume(ProgramStateRef state,
  SVal Cond, bool Assumption) {
-  for (const auto EvalAssumeChecker : EvalAssumeCheckers) {
+  for (const auto  : EvalAssumeCheckers) {
 // If any checker declares the state infeasible (or if it starts that way),
 // bail out.
 if (!state)
@@ -658,7 +658,7 @@ void CheckerManager::runCheckersForEvalCall(ExplodedNodeSet 
,
 NodeBuilder B(Pred, checkDst,

[PATCH] D70045: [AST] Use an explicit copy in a range-based for

2019-11-12 Thread Mark de Wever via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2149028c49f8: [AST] Use an explicit copy in a range-based 
for (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70045

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtDataCollectors.td
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -9380,7 +9380,7 @@
 
   SmallVector AssocExprs;
   SmallVector AssocTypes;
-  for (const GenericSelectionExpr::Association  : E->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : E->associations()) {
 TypeSourceInfo *TSI = Assoc.getTypeSourceInfo();
 if (TSI) {
   TypeSourceInfo *AssocType = getDerived().TransformType(TSI);
Index: clang/lib/Sema/SemaPseudoObject.cpp
===
--- clang/lib/Sema/SemaPseudoObject.cpp
+++ clang/lib/Sema/SemaPseudoObject.cpp
@@ -145,7 +145,7 @@
 assocExprs.reserve(numAssocs);
 assocTypes.reserve(numAssocs);
 
-for (const GenericSelectionExpr::Association  :
+for (const GenericSelectionExpr::Association assoc :
  gse->associations()) {
   Expr *assocExpr = assoc.getAssociationExpr();
   if (assoc.isSelected())
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -4353,7 +4353,7 @@
 SmallVector subTypes;
 subExprs.reserve(n);
 subTypes.reserve(n);
-for (const GenericSelectionExpr::Association  : gse->associations()) {
+for (const GenericSelectionExpr::Association assoc : gse->associations()) {
   subTypes.push_back(assoc.getTypeSourceInfo());
   Expr *sub = assoc.getAssociationExpr();
   if (assoc.isSelected())
Index: clang/lib/AST/StmtProfile.cpp
===
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1296,7 +1296,7 @@
 
 void StmtProfiler::VisitGenericSelectionExpr(const GenericSelectionExpr *S) {
   VisitExpr(S);
-  for (const GenericSelectionExpr::ConstAssociation  :
+  for (const GenericSelectionExpr::ConstAssociation Assoc :
S->associations()) {
 QualType T = Assoc.getType();
 if (T.isNull())
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1304,7 +1304,7 @@
 void StmtPrinter::VisitGenericSelectionExpr(GenericSelectionExpr *Node) {
   OS << "_Generic(";
   PrintExpr(Node->getControllingExpr());
-  for (const GenericSelectionExpr::Association  : Node->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : Node->associations()) {
 OS << ", ";
 QualType T = Assoc.getType();
 if (T.isNull())
Index: clang/include/clang/AST/StmtDataCollectors.td
===
--- clang/include/clang/AST/StmtDataCollectors.td
+++ clang/include/clang/AST/StmtDataCollectors.td
@@ -189,7 +189,7 @@
 }
 class GenericSelectionExpr {
   code Code = [{
-for (const GenericSelectionExpr::ConstAssociation  : S->associations()) {
+for (const GenericSelectionExpr::ConstAssociation Assoc : S->associations()) {
   addData(Assoc.getType());
 }
   }];
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2351,7 +2351,7 @@
 // generic associations).
 DEF_TRAVERSE_STMT(GenericSelectionExpr, {
   TRY_TO(TraverseStmt(S->getControllingExpr()));
-  for (const GenericSelectionExpr::Association  : S->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : S->associations()) {
 if (TypeSourceInfo *TSI = Assoc.getTypeSourceInfo())
   TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
 TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(Assoc.getAssociationExpr());
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -620,7 +620,7 @@
 Visit(E->getControllingExpr());
 Visit(E->getControllingExpr()->getType()); // FIXME: remove
 
-for (const auto  : E->associations()) {
+for (const auto Assoc : E->associations()) {
   Visit(Assoc);
 }
   }

[PATCH] D70046: [OpenMP] Use an explicit copy in a range-based for

2019-11-12 Thread Mark de Wever via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG51abcebbb6e5: [OpenMP] Use an explicit copy in a range-based 
for (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70046

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp


Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7940,17 +7940,17 @@
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, C->getMapType(), C->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_to, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_from, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
@@ -7966,7 +7966,7 @@
 
 for (const auto *C :
  CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 assert(!L.second.empty() && "Not expecting empty list of components!");
 const ValueDecl *VD = L.second.back().getAssociatedDeclaration();
 VD = cast(VD->getCanonicalDecl());
@@ -8119,7 +8119,7 @@
 
 for (const auto *C : CurMapperDir->clauselists()) {
   const auto *MC = cast(C);
-  for (const auto  : MC->component_lists()) {
+  for (const auto L : MC->component_lists()) {
 InfoGen(L.first, L.second, MC->getMapType(), MC->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, MC->isImplicit());
   }
@@ -8288,7 +8288,7 @@
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->decl_component_lists(VD)) {
+  for (const auto L : C->decl_component_lists(VD)) {
 assert(L.first == VD &&
"We got information for the wrong declaration??");
 assert(!L.second.empty() &&
@@ -8441,7 +8441,7 @@
 // Map other list items in the map clause which are not captured variables
 // but "declare target link" global variables.
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 if (!L.first)
   continue;
 const auto *VD = dyn_cast(L.first);


Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7940,17 +7940,17 @@
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, C->getMapType(), C->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_to, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_from, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
@@ -7966,7 +7966,7 @@
 
 for (const auto *C :
  CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 assert(!L.second.empty() && "Not expecting empty list of components!");
 const ValueDecl *VD = L.second.back().getAssociatedDeclaration();
 VD = cast(VD->getCanonicalDecl());
@@ -8119,7 +8119,7 @@
 
 for (const auto *C : CurMapperDir->clauselists()) {
   const auto *MC = cast(C);
-  for (const auto  : MC->component_lists()) {
+  for (const auto L : MC->component_lists()) {

[clang] 51abceb - [OpenMP] Use an explicit copy in a range-based for

2019-11-12 Thread Mark de Wever via cfe-commits


Author: Mark de Wever
Date: 2019-11-12T20:50:38+01:00
New Revision: 51abcebbb6e5c8f8befaa523ae873adecf2d1012

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

LOG: [OpenMP] Use an explicit copy in a range-based for

The std::pair>
type will be copied in a range-based for loop. Make the copy explicit to
avoid the -Wrange-loop-analysis warning.

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index ae881c3e2d7a..cd20cb83afb9 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7940,17 +7940,17 @@ class MappableExprsHandler {
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, C->getMapType(), C->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_to, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
 for (const auto *C : CurExecDir->getClausesOfKind())
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 InfoGen(L.first, L.second, OMPC_MAP_from, llvm::None,
 /*ReturnDevicePointer=*/false, C->isImplicit());
   }
@@ -7966,7 +7966,7 @@ class MappableExprsHandler {
 
 for (const auto *C :
  CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 assert(!L.second.empty() && "Not expecting empty list of components!");
 const ValueDecl *VD = L.second.back().getAssociatedDeclaration();
 VD = cast(VD->getCanonicalDecl());
@@ -8119,7 +8119,7 @@ class MappableExprsHandler {
 
 for (const auto *C : CurMapperDir->clauselists()) {
   const auto *MC = cast(C);
-  for (const auto  : MC->component_lists()) {
+  for (const auto L : MC->component_lists()) {
 InfoGen(L.first, L.second, MC->getMapType(), MC->getMapTypeModifiers(),
 /*ReturnDevicePointer=*/false, MC->isImplicit());
   }
@@ -8288,7 +8288,7 @@ class MappableExprsHandler {
"Expect a executable directive");
 const auto *CurExecDir = CurDir.get();
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->decl_component_lists(VD)) {
+  for (const auto L : C->decl_component_lists(VD)) {
 assert(L.first == VD &&
"We got information for the wrong declaration??");
 assert(!L.second.empty() &&
@@ -8441,7 +8441,7 @@ class MappableExprsHandler {
 // Map other list items in the map clause which are not captured variables
 // but "declare target link" global variables.
 for (const auto *C : CurExecDir->getClausesOfKind()) {
-  for (const auto  : C->component_lists()) {
+  for (const auto L : C->component_lists()) {
 if (!L.first)
   continue;
 const auto *VD = dyn_cast(L.first);



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

[clang] 2149028 - [AST] Use an explicit copy in a range-based for

2019-11-12 Thread Mark de Wever via cfe-commits


Author: Mark de Wever
Date: 2019-11-12T20:47:46+01:00
New Revision: 2149028c49f8af1f3d8a9d81b2081a2b302b2d9a

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

LOG: [AST] Use an explicit copy in a range-based for

The AssociationIteratorTy type will be copied in a range-based for loop.
Make the copy explicit to avoid the -Wrange-loop-analysis warning.

This avoids new warnings due to D68912 adds -Wrange-loop-analysis to -Wall.

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

Added: 


Modified: 
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/StmtDataCollectors.td
clang/lib/AST/StmtPrinter.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaPseudoObject.cpp
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index 0bb2aad553fb..ed9fc14aba42 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -620,7 +620,7 @@ class ASTNodeTraverser
 Visit(E->getControllingExpr());
 Visit(E->getControllingExpr()->getType()); // FIXME: remove
 
-for (const auto  : E->associations()) {
+for (const auto Assoc : E->associations()) {
   Visit(Assoc);
 }
   }

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 86059842da65..9fea4980c89a 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2351,7 +2351,7 @@ bool RecursiveASTVisitor::TraverseInitListExpr(
 // generic associations).
 DEF_TRAVERSE_STMT(GenericSelectionExpr, {
   TRY_TO(TraverseStmt(S->getControllingExpr()));
-  for (const GenericSelectionExpr::Association  : S->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : S->associations()) {
 if (TypeSourceInfo *TSI = Assoc.getTypeSourceInfo())
   TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
 TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(Assoc.getAssociationExpr());

diff  --git a/clang/include/clang/AST/StmtDataCollectors.td 
b/clang/include/clang/AST/StmtDataCollectors.td
index a46d2714eb4b..7cb9f16fbce2 100644
--- a/clang/include/clang/AST/StmtDataCollectors.td
+++ b/clang/include/clang/AST/StmtDataCollectors.td
@@ -189,7 +189,7 @@ class CXXFoldExpr {
 }
 class GenericSelectionExpr {
   code Code = [{
-for (const GenericSelectionExpr::ConstAssociation  : 
S->associations()) {
+for (const GenericSelectionExpr::ConstAssociation Assoc : 
S->associations()) {
   addData(Assoc.getType());
 }
   }];

diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 0f92d4c367e9..603ae5f9c48d 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -1304,7 +1304,7 @@ void 
StmtPrinter::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *Node){
 void StmtPrinter::VisitGenericSelectionExpr(GenericSelectionExpr *Node) {
   OS << "_Generic(";
   PrintExpr(Node->getControllingExpr());
-  for (const GenericSelectionExpr::Association  : Node->associations()) {
+  for (const GenericSelectionExpr::Association Assoc : Node->associations()) {
 OS << ", ";
 QualType T = Assoc.getType();
 if (T.isNull())

diff  --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 6f266cf12949..9edd9e5d5856 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -1296,7 +1296,7 @@ void StmtProfiler::VisitBlockExpr(const BlockExpr *S) {
 
 void StmtProfiler::VisitGenericSelectionExpr(const GenericSelectionExpr *S) {
   VisitExpr(S);
-  for (const GenericSelectionExpr::ConstAssociation  :
+  for (const GenericSelectionExpr::ConstAssociation Assoc :
S->associations()) {
 QualType T = Assoc.getType();
 if (T.isNull())

diff  --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp
index 207812c8848b..fc27094c9a53 100644
--- a/clang/lib/Sema/SemaExprObjC.cpp
+++ b/clang/lib/Sema/SemaExprObjC.cpp
@@ -4353,7 +4353,7 @@ Expr *Sema::stripARCUnbridgedCast(Expr *e) {
 SmallVector subTypes;
 subExprs.reserve(n);
 subTypes.reserve(n);
-for (const GenericSelectionExpr::Association  : gse->associations()) 
{
+for (const GenericSelectionExpr::Association assoc : gse->associations()) {
   subTypes.push_back(assoc.getTypeSourceInfo());
   Expr *sub = assoc.getAssociationExpr();
   if (assoc.isSelected())

diff  --git a/clang/lib/Sema/SemaPseudoObject.cpp 
b/clang/lib/Sema/SemaPseudoObject.cpp
index 602806968ced..5587e0d24c7f 100644
--- a/clang/lib/Sema/SemaPseudoObject.cpp
+++ b/clang/lib/Sema/SemaPseudoObject.cpp
@@

[PATCH] D61458: [hip] Relax CUDA call restriction within `decltype` context.

2019-11-12 Thread Michael Liao via Phabricator via cfe-commits

hliao updated this revision to Diff 228924.
hliao added a comment.

This patch is revived with more changes addressing the previous concerns.

Back to Justin's example:

  __host__ float bar();
  __device__ int bar();
  __host__ __device__ auto foo() -> decltype(bar()) { return bar(); }

Even without this patch, that example already passed the compilation without
either errors or warnings. Says

  clang -std=c++11 -x cuda -nocudainc -nocudalib --cuda-gpu-arch=sm_60 
--cuda-device-only -S -emit-llvm -O3 foo.cu

In c++14, that example could be even simplified without `decltype` but the same 
ambiguity.

  __host__ float bar();
  __device__ int bar();
  __host__ __device__ auto foo() { return bar(); }

Without any change, clang also compiles the code as well and uses different 
return types between host-side and device-side compilation.[^1]

[^1]: The first example has the same return type between host-side and 
device-side but that seems incorrect or unreasonable to me.

The ambiguity issue is in fact not introduced by relaxing `decltype`. That's an 
inherent one as we allow overloading over target attributes. Issuing warnings 
instead of errors seems more reasonable to me for such cases.

In this patch, besides relaxing the CUDA call rule under `decltype`, it also 
generates warning during function overloading if there are more than candidates 
with different return types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61458

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCUDA/function-overload.cu
  clang/test/Misc/warning-flags.c
  clang/test/SemaCUDA/function-overload.cu

Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -3,6 +3,8 @@
 
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -DCHECK_DECLTYPE -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -DCHECK_DECLTYPE -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
 
 #include "Inputs/cuda.h"
 
@@ -419,3 +421,30 @@
 int test_constexpr_overload(C2 , C2 ) {
   return constexpr_overload(x, y);
 }
+
+#if defined(CHECK_DECLTYPE)
+#if defined(__CUDA_ARCH__)
+// expected-note@+6 {{other definition of 't0'}}
+// expected-note@+6 {{use this definition of 't0'}}
+#else
+// expected-note@+3 {{use this definition of 't0'}}
+// expected-note@+3 {{other definition of 't0'}}
+#endif
+__host__ float t0();
+__device__ int t0();
+
+__host__ __device__ void dt0() {
+  // expected-warning@+1 {{return type of 't0' in 'decltype' is ambiguous and may not be expected}}
+  decltype(t0()) ret;
+}
+
+__host__ float t1();
+
+__device__ void dt1() {
+  decltype(t1()) ret; // OK. `decltype` is relaxed.
+}
+
+__host__ __device__ void dt2() {
+  decltype(t1()) ret; // OK. `decltype` is relaxed.
+}
+#endif
Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (74):
+CHECK: Warnings without flags (75):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -47,6 +47,7 @@
 CHECK-NEXT:   warn_conv_to_base_not_used
 CHECK-NEXT:   warn_conv_to_self_not_used
 CHECK-NEXT:   warn_conv_to_void_not_used
+CHECK-NEXT:   warn_decltype_ambiguous_return_type
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_double_const_requires_fp64
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
Index: clang/test/CodeGenCUDA/function-overload.cu
===
--- clang/test/CodeGenCUDA/function-overload.cu
+++ clang/test/CodeGenCUDA/function-overload.cu
@@ -8,6 +8,8 @@
 // RUN: | FileCheck -check-prefix=CHECK-BOTH -check-prefix=CHECK-HOST %s
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -emit-llvm -o - %s \
 // RUN: | FileCheck -check-prefix=CHECK-BOTH -check-prefix=CHECK-DEVICE %s
+// RUN: %clang_cc1 -std=c++11 -DCHECK_DECLTYPE -triple amdgcn -fcuda-is-device -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefix=CHECK-DECLTYPE %s
 
 #include "Inputs/cuda.h"
 
@@ -53,3 +55,14 @@
 // CHECK-BOTH: define linkonce_odr void @_ZN7s_cd_hdD2Ev(
 // CHECK-BOTH: store i32 32,
 // CHECK-BOTH: ret void
+
+#if defined(CHECK_DECLTYPE)
+int foo(float);
+// CHECK-DECLTYPE-LABEL: @_Z3barf
+// CHECK-DECLTYPE: fptosi
+// CHECK-DECLTYPE: sitofp

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

2019-11-12 Thread Zachary Turner via Phabricator via cfe-commits

zturner added a comment.

Does this change crash recovery semantics in any meaningful way?  Will we still 
be able to get stack traces on all platforms when the compiler crashes?




Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:207
+  // FIXME error: cannot compile this 'this' captured by SEH yet
+  CrashRecoveryContext *This = this;
   __try {

You can fix this by writing:

```
static bool wrap_function_call(function_ref Fn, bool 
EnableExceptionHandler, unsigned )
{
   __try {
 Fn();
 return true;
  } __except (EnableExceptionHandler
  ? LLVMUnhandledExceptionFilter(GetExceptionInformation())
  : 1) {
RetCode = GetExceptionCode();
return false;
  }
}

bool CrashRecoveryContext::RunSafely(function_ref Fn) {
...
bool Result = wrap_function_call(EnableExceptionHandler, Fn, RetCode);
...
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D69841: Target Ivy bridge on macOS Mojave and later

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

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

Hi Dave, thanks for checking in on this, but unfortunately we're not ready for 
this yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69841



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

2019-11-12 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);

cchen wrote:
> ABataev wrote:
> > I believe this problem exists in the original code? If so, better to split 
> > this patch and commit this fix in the separate patch.
> Yes, this problem is from the original code, but `bool isDefaultmapModifier = 
> (M != OMPC_DEFAULTMAP_MODIFIER_unknown);` at line 16437 relies on this 
> change. Otherwise, I'll have to write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || 
> (M == OMPC_DEFAULTMAP_MODIFIER_from) ||...`.
> So do you think that I should just write `(M == OMPC_DEFAULTMAP_MODIFIER_to) 
> || (M == OMPC_DEFAULTMAP_MODIFIER_from) ||...` or `M > 
> OMPC_DEFAULTMAP_MODIFIER_unknown` for this patch and fix them in another 
> patch?
You will just commit the second patch at first and then just rebase this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69832: [WebAssembly] -fwasm-exceptions enables reference-types

2019-11-12 Thread Heejin Ahn via Phabricator via cfe-commits

aheejin added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69832



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

2019-11-12 Thread Michael Kruse via Phabricator via cfe-commits

Meinersbur added inline comments.



Comment at: clang/lib/Driver/Job.cpp:347
+StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable);
+if (CommandExe.equals_lower(DriverExe))
+  ClangDriverMain = Driver::Main;

Why is this check necessary? Why not assuming that if `Driver::Main` is set, it 
will be the right one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[clang] 3c676e3 - [OPENMP]Use copy constructors instead of assignment operators in declare

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


Author: Alexey Bataev
Date: 2019-11-12T13:13:37-05:00
New Revision: 3c676e3891b962b859e7613781419ee0dacce7dd

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

LOG: [OPENMP]Use copy constructors instead of assignment operators in declare
reduction initializers.

Better to use copy constructor at the initialization of the declare
reduction construct rather than assignment operator.

Added: 


Modified: 
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/AST/dump.cpp
clang/test/OpenMP/declare_reduction_messages.cpp
clang/test/OpenMP/for_reduction_codegen_UDR.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 8430e72d3c88..49ba52897fe6 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -368,14 +368,8 @@ 
Parser::ParseOpenMPDeclareReductionDirective(AccessSpecifier AS) {
 // Check if initializer is omp_priv  or something else.
 if (Tok.is(tok::identifier) &&
 Tok.getIdentifierInfo()->isStr("omp_priv")) {
-  if (Actions.getLangOpts().CPlusPlus) {
-InitializerResult = Actions.ActOnFinishFullExpr(
-ParseAssignmentExpression().get(), D->getLocation(),
-/*DiscardedValue*/ false);
-  } else {
-ConsumeToken();
-ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
-  }
+  ConsumeToken();
+  ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
 } else {
   InitializerResult = Actions.ActOnFinishFullExpr(
   ParseAssignmentExpression().get(), D->getLocation(),
@@ -419,7 +413,8 @@ void Parser::ParseOpenMPReductionInitializerForDecl(VarDecl 
*OmpPrivParm) {
   return;
 }
 
-ExprResult Init(ParseInitializer());
+PreferredType.enterVariableInit(Tok.getLocation(), OmpPrivParm);
+ExprResult Init = ParseAssignmentExpression();
 
 if (Init.isInvalid()) {
   SkipUntil(tok::r_paren, tok::annot_pragma_openmp_end, StopBeforeMatch);

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index cd8b95231aad..600265e2d852 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3118,7 +3118,12 @@ Decl 
*TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
 SubstInitializer =
 SemaRef.SubstExpr(D->getInitializer(), TemplateArgs).get();
   } else {
-IsCorrect = IsCorrect && OmpPrivParm->hasInit();
+auto *OldPrivParm =
+cast(cast(D->getInitPriv())->getDecl());
+IsCorrect = IsCorrect && OldPrivParm->hasInit();
+if (IsCorrect)
+  SemaRef.InstantiateVariableInitializer(OmpPrivParm, OldPrivParm,
+ TemplateArgs);
   }
   SemaRef.ActOnOpenMPDeclareReductionInitializerEnd(
   NewDRD, SubstInitializer, OmpPrivParm);

diff  --git a/clang/test/AST/dump.cpp b/clang/test/AST/dump.cpp
index 641abc5ea646..4d271514349f 100644
--- a/clang/test/AST/dump.cpp
+++ b/clang/test/AST/dump.cpp
@@ -33,13 +33,11 @@ int ga, gb;
 // CHECK-NEXT: | | |-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_out' 'float'
 // CHECK-NEXT: | | `-ImplicitCastExpr {{.+}}  'float' 
 // CHECK-NEXT: | |   `-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_in' 'float'
-// CHECK-NEXT: | |-BinaryOperator {{.+}}  'float' lvalue '='
-// CHECK-NEXT: | | |-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_priv' 'float'
-// CHECK-NEXT: | | `-BinaryOperator {{.+}}  'float' '+'
-// CHECK-NEXT: | |   |-ImplicitCastExpr {{.+}}  'float' 

-// CHECK-NEXT: | |   | `-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_orig' 'float'
-// CHECK-NEXT: | |   `-ImplicitCastExpr {{.+}}  'float' 

-// CHECK-NEXT: | | `-IntegerLiteral {{.+}}  'int' 15
+// CHECK-NEXT: | |-BinaryOperator {{.+}}  'float' '+'
+// CHECK-NEXT: | | |-ImplicitCastExpr {{.+}}  'float' 
+// CHECK-NEXT: | | | `-DeclRefExpr {{.+}}  'float' lvalue Var {{.+}} 
'omp_orig' 'float'
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.+}}  'float' 

+// CHECK-NEXT: | |   `-IntegerLiteral {{.+}}  'int' 15
 
 struct S {
   int a, b;

diff  --git a/clang/test/OpenMP/declare_reduction_messages.cpp 
b/clang/test/OpenMP/declare_reduction_messages.cpp
index a674af7151da..1fc5ec6e683f 100644
--- a/clang/test/OpenMP/declare_reduction_messages.cpp
+++ b/clang/test/OpenMP/declare_reduction_messages.cpp
@@ -142,7 +142,7 @@ int main() {
 #if __cplusplus == 201103L
 struct A {
   A() {}
-  A& operator=(A&&) = default;
+  A(const A &) = default;
 };
 
 int A_TEST() {

diff  --git a/clang/test/OpenMP/for_reduction_codegen_UDR.cpp

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

2019-11-12 Thread Chi Chun Chen via Phabricator via cfe-commits

cchen marked 2 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2339-2345
+unsigned Modifier = getOpenMPSimpleClauseType(
+Kind, Tok.isAnnotation() ? "" : PP.getSpelling(Tok));
+// Set defaultmap modifier to unknown if it is either scalar, aggregate, or
+// pointer
+if (Modifier < OMPC_DEFAULTMAP_MODIFIER_unknown)
+  Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+Arg.push_back(Modifier);

ABataev wrote:
> I believe this problem exists in the original code? If so, better to split 
> this patch and commit this fix in the separate patch.
Yes, this problem is from the original code, but `bool isDefaultmapModifier = 
(M != OMPC_DEFAULTMAP_MODIFIER_unknown);` at line 16437 relies on this change. 
Otherwise, I'll have to write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || (M == 
OMPC_DEFAULTMAP_MODIFIER_from) ||...`.
So do you think that I should just write `(M == OMPC_DEFAULTMAP_MODIFIER_to) || 
(M == OMPC_DEFAULTMAP_MODIFIER_from) ||...` or `M > 
OMPC_DEFAULTMAP_MODIFIER_unknown` for this patch and fix them in another patch?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16393-16416
+static DefaultMapImplicitBehavior
+getImplicitBehaviorFromModifier(OpenMPDefaultmapClauseModifier M) {
+  switch (M) {
+  case OMPC_DEFAULTMAP_MODIFIER_alloc:
+return DMIB_alloc;
+  case OMPC_DEFAULTMAP_MODIFIER_to:
+return DMIB_to;

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > Do we need types `DefaultMapImplicitBehavior` and 
> > > > `DefaultMapVariableCategory` if the map 1 to 1 to 
> > > > `OpenMPDefaultmapClauseModifier` and `OpenMPDefaultmapClauseKind`?
> > > What about this?
> > The value of OpenMPDefaultmapClauseModifier does not start from one (due to 
> > the design in parsing I guess) so I'm not able to do so.
> No, I meant that the enumerics are almost the same. Can we reuse the original 
> one rather than adding 2 new enums with almost the same members and the same 
> meanings?
Got it, I'll use OpenMPDefaultmapClauseModifier and OpenMPDefaultmapClauseKind 
instead. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

2019-11-12 Thread Vedant Kumar via Phabricator via cfe-commits

vsk added a comment.

@dblaikie do you need more time to investigate? fwiw I didn't uncover any new 
failures in a stage2 build with this patch applied.


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

https://reviews.llvm.org/D69970



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

[PATCH] D70110: [Driver][FreeBSD] Enable unwind tables on !amd64

2019-11-12 Thread Dimitry Andric via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70110



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

[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.

2019-11-12 Thread Simon Tatham via Phabricator via cfe-commits

simon_tatham created this revision.
simon_tatham added reviewers: ostannard, MarkMurrayARM, dmgreen.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
simon_tatham added a parent revision: D70088: [ARM,MVE] Add intrinsics for 
contiguous load/stores..

This batch of intrinsics includes lots of things that move vector data
around or change its type without really affecting its value very
much. It includes the `vreinterpretq` family (cast one vector type to
another); `vuninitializedq` (create a vector of a given type with
don't-care contents); `vcreateq` (make a 128-bit vector out of two
`uint64_t` halves); and the `vgetq_lane` and `vsetq_lane` families, to
read and write an individual lane of a vector.

These are all implemented using completely standard IR that's already
tested in existing LLVM unit tests, so I've just written a clang test
to check the IR is correct, and left it at that.

One of the new `vgetq_lane` intrinsics returns a `float16_t`, which
causes a compile error if `%clang_cc1` doesn't get the option
`-fallow-half-arguments-and-returns`. The driver passes that option to
cc1 already, but I've had to edit all the explicit cc1 command lines
in the existing MVE intrinsics tests.

I've also added some richer infrastructure to the MveEmitter Tablegen
backend, to make it specify the exact integer type of integer
arguments passed to IR construction functions, and wrap those
arguments in a `static_cast` in the autogenerated C++. That was
necessary to prevent an overloading ambiguity when passing the integer
literal `0` to `IRBuilder::CreateInsertElement`, because otherwise, it
could mean either a null pointer `llvm::Value *` or a zero `uint64_t`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70133

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/admin.c
  clang/test/CodeGen/arm-mve-intrinsics/load-store.c
  clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
  clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
  clang/test/CodeGen/arm-mve-intrinsics/vadc.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
  clang/test/CodeGen/arm-mve-intrinsics/vld24.c
  clang/test/CodeGen/arm-mve-intrinsics/vldr.c
  clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
  clang/test/Sema/arm-mve-immediates.c
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -632,10 +632,10 @@
   StringRef CallPrefix;
   std::vector Args;
   std::set AddressArgs;
-  std::set IntConstantArgs;
+  std::map IntConstantArgs;
   IRBuilderResult(StringRef CallPrefix, std::vector Args,
   std::set AddressArgs,
-  std::set IntConstantArgs)
+  std::map IntConstantArgs)
 : CallPrefix(CallPrefix), Args(Args), AddressArgs(AddressArgs),
 IntConstantArgs(IntConstantArgs) {}
   void genCode(raw_ostream ,
@@ -644,11 +644,13 @@
 const char *Sep = "";
 for (unsigned i = 0, e = Args.size(); i < e; ++i) {
   Ptr Arg = Args[i];
-  if (IntConstantArgs.find(i) != IntConstantArgs.end()) {
+  auto it = IntConstantArgs.find(i);
+  if (it != IntConstantArgs.end()) {
 assert(Arg->hasIntegerConstantValue());
-OS << Sep
+OS << Sep << "static_cast<" << it->second << ">("
<< ParamAlloc.allocParam("unsigned",
-utostr(Arg->integerConstantValue()));
+utostr(Arg->integerConstantValue()))
+   << ")";
   } else {
 OS << Sep << Arg->varname();
   }
@@ -763,6 +765,14 @@
   // shares with at least one other intrinsic.
   std::string ShortName, FullName;
 
+  // A very small number of intrinsics _only_ have a polymorphic
+  // variant (vuninitializedq taking an unevaluated argument).
+  bool PolymorphicOnly;
+
+  // Another rarely-used flag indicating that the builtin doesn't
+  // evaluate its argument(s) at all.
+  bool NonEvaluating;
+
   const Type *ReturnType;
   std::vector ArgTypes;
   std::map ImmediateArgs;
@@ -796,6 +806,8 @@
 return false;
   }
   bool polymorphic() const { return ShortName != FullName; }
+  bool polymorphicOnly() const { return PolymorphicOnly; }
+  bool nonEvaluating() const { return NonEvaluating; }
 
   // External entry point for code generation, called from MveEmitter.
   void genCode(raw_ostream , CodeGenParamAllocator ,
@@ -1126,11 +1138,15 @@
   Args.push_back(getCodeForDagArg(D, i, Scope, Param));
 if (Op->isSubClassOf("IRBuilderBase")) {
   std::set AddressArgs;
-  for (unsigned i : Op->getValueAsListOfInts("address_params"))
-AddressArgs.insert(i);
-  std::set IntConstantArgs;
-  for (unsigned i :

1 2 >

1 - 100 of 153 matches

Mail list logo