[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:226
+// Add this function to global destructors.
+appendToGlobalDtors(M, Func, 0);
+  }

FYI, llvm.global_dtors does not work on Windows.  The symbol will be placed 
into .CRT$XC[A-Z] portion of .CRT in TargetLoweringObjectFileImpl.cpp, so, 
basically, __tgt_unregister_lib will be called right after __tgt_register_lib.  
We can either use a trick from ASAN, i.e. put llvm.global_dtors into 
.CRT$XT[A-Z] (I am not sure how solid this solution is) or call atexit() inside 
__tgt_register_lib to register __tgt_unregister_lib terminator.


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

https://reviews.llvm.org/D64943



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

[PATCH] D65846: Improve error message from FrontendAction

[JF Bastien via Phabricator via cfe-commits]

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

There's a failure in `clang/test/Index/pch-from-libclang.c`. @thakis disabled 
that test for all but Darwin in r352809, and with my change I now get:

  Internal compiler error: LFS error for 
"/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx"failed
 to create unique file 
/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx.lock-0ae18733:
 No such file or directory

From the test, it's the result of running:

  /Users/jfb/s/llvmm/debug/bin/clang -fsyntax-only -include 
/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.h
 /Users/jfb/s/llvmm/clang/test/Index/pch-from-libclang.c -Xclang -verify 
-fmodules 
-fmodules-cache-path=/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp
 -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang 
x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors

@akyrtzi do you know what's going on? I haven't investigate yet, and am hoping 
you'll just know :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65846



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

[PATCH] D65846: Improve error message from FrontendAction

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
jfb planned changes to this revision.
jfb added a comment.

There's a failure in `clang/test/Index/pch-from-libclang.c`. @thakis disabled 
that test for all but Darwin in r352809, and with my change I now get:

  Internal compiler error: LFS error for 
"/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx"failed
 to create unique file 
/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx.lock-0ae18733:
 No such file or directory

>From the test, it's the result of running:

  /Users/jfb/s/llvmm/debug/bin/clang -fsyntax-only -include 
/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.h
 /Users/jfb/s/llvmm/clang/test/Index/pch-from-libclang.c -Xclang -verify 
-fmodules 
-fmodules-cache-path=/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp
 -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang 
x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors

@akyrtzi do you know what's going on? I haven't investigate yet, and am hoping 
you'll just know :)


It's called in a few place, and for D63518  I 
initially set all their error propagation to ignore errors so we could 
propagate them one at a time. This change takes that second step and enables 
propagation, improving some module-related diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65846

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp


Index: clang/lib/Serialization/GlobalModuleIndex.cpp
===
--- clang/lib/Serialization/GlobalModuleIndex.cpp
+++ clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -860,7 +860,9 @@
   llvm::LockFileManager Locked(IndexPath);
   switch (Locked) {
   case llvm::LockFileManager::LFS_Error:
-return llvm::createStringError(std::errc::io_error, "LFS error");
+return llvm::createStringError(
+std::errc::io_error, "LFS error for \"%s\": %s", IndexPath.c_str(),
+Locked.getErrorMessage().c_str());
 
   case llvm::LockFileManager::LFS_Owned:
 // We're responsible for building the index ourselves. Do so below.
@@ -869,8 +871,10 @@
   case llvm::LockFileManager::LFS_Shared:
 // Someone else is responsible for building the index. We don't care
 // when they finish, so we're done.
-return llvm::createStringError(std::errc::device_or_resource_busy,
-   "someone else is building the index");
+return llvm::createStringError(
+std::errc::device_or_resource_busy,
+"someone else is building the index \"%s\": %s", IndexPath.c_str(),
+Locked.getErrorMessage().c_str());
   }
 
   // The module index builder.
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -940,15 +940,10 @@
   CI.hasPreprocessor()) {
 StringRef Cache =
 CI.getPreprocessor().getHeaderSearchInfo().getModuleCachePath();
-if (!Cache.empty()) {
+if (!Cache.empty())
   if (llvm::Error Err = GlobalModuleIndex::writeIndex(
-  CI.getFileManager(), CI.getPCHContainerReader(), Cache)) {
-// FIXME this drops the error on the floor, but
-// Index/pch-from-libclang.c seems to rely on dropping at least some of
-// the error conditions!
-consumeError(std::move(Err));
-  }
-}
+  CI.getFileManager(), CI.getPCHContainerReader(), Cache))
+return Err;
   }
 
   return llvm::Error::success();
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -943,9 +943,8 @@
   getSourceManager().clearIDTables();
 
 if (Act.BeginSourceFile(*this, FIF)) {
-  if (llvm::Error Err = Act.Execute()) {
-consumeError(std::move(Err)); // FIXME this drops errors on the floor.
-  }
+  if (llvm::Error Err = Act.Execute())
+OS << "Internal compiler error: " << toString(std::move(Err)) << "\n";
   Act.EndSourceFile();
 }
   }


Index: clang/lib/Serialization/GlobalModuleIndex.cpp
===
--- clang/lib/Serialization/GlobalModuleIndex.cpp
+++ clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -860,7 +860,9 @@
   llvm::LockFileManager Locked(IndexPath);
   switch (Locked) {
   case llvm::LockFileManager::LFS_Error:
-return llvm::createStringError(std::errc::io_error, "LFS error");
+return llvm::createStringError(
+

[PATCH] D65839: [Driver] Add verbatim dry run option

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/include/clang/Driver/Options.td:333
 HelpText<"Print (but do not run) the commands to run for this 
compilation">;
+def _HASH_HASH_HASH_VERBATIM : Flag<["-"], "###-verbatim">,
+Flags<[DriverOption, CoreOption]>,

What about `-print-raw-commands`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65839



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

Re: LLVM Types

[Richard Trieu via cfe-commits]

To avoid confusion, there's both a LLVM Type and a Clang Type.  The LLVM
Type is used in the LLVM IR while the Clang Type represents types in a
language, like C++.

The Clang Doxygen pages have this hierarchy, although it's truncated
because Type is so large, but you can click through to get the missing
nodes.  It's available at:
https://clang.llvm.org/doxygen/classclang_1_1Type.html

There's also "clang/AST/TypeNode.def" which has the Type classes available
as macro calls.  You define the macros, include the file, then the
preprocessor will do all the work of filling in hierarchy.  You can see
this in action by how Type fills up an enum:

class alignas(8) Type : public ExtQualsTypeCommonBase {
public:
  enum TypeClass {
#define TYPE(Class, Base) Class,
#define LAST_TYPE(Class) TypeLast = Class,
#define ABSTRACT_TYPE(Class, Base)
#include "clang/AST/TypeNodes.def"
TagFirst = Record, TagLast = Enum
  };

On Tue, Aug 6, 2019 at 8:15 AM Monalisa Rout via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hello,
> Can I dump LLVM Type hierarchies somehow ??
>
> Types which are declared in this file ("clang/AST/Type.h
> "  )
>
> Regards,
> Mona
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65838: [Driver] Use enumeration for quoting mode. NFC

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D65838/new/

https://reviews.llvm.org/D65838



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev marked 14 inline comments as done.
sdmitriev added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:69
+
+  using MemoryBuffersVector = SmallVectorImpl>;
+

ABataev wrote:
> Why not `ArrayRef`?
Changed to MemoryBufferRef which in some sense is similar to ArrayRef.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:72-75
+  IntegerType *getSizeTTy() {
+auto PtrSize = M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C));
+return PtrSize == 8 ? Type::getInt64Ty(C) : Type::getInt32Ty(C);
+  }

ABataev wrote:
> Not sure that this is the best solution, we may end up with incorrect 
> `size_t` type in some cases. 
 I have slightly revised this code to avoid unexpected results.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:148
+
+  auto *Image = new GlobalVariable(M, Data->getType(), true,
+  GlobalVariable::InternalLinkage,

ABataev wrote:
> Seems to me the code is not formatted
Right. Fixed.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:209
+  void createUnregisterFunction(GlobalVariable *BinDesc) {
+auto *FuncTy = FunctionType::get(Type::getVoidTy(C), {}, false);
+auto *Func = Function::Create(FuncTy, GlobalValue::InternalLinkage,

ABataev wrote:
> `llvm::None` instead of `{}`
Switched to a different constructor which does not have argument types.


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

https://reviews.llvm.org/D64943



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213776.
DiegoAstiazaran added a comment.

Remove unnecessary type casting.


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

https://reviews.llvm.org/D65483

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/docs/clang-doc.rst
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
 
   template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
 if (I.first)
   EmittedInfos.emplace_back(std::move(I.first));
 if (I.second)
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -22,9 +22,9 @@
 }
 
 ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
-  ClangDocContext CDCtx;
-  CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+   StringRef RepositoryUrl = "") {
+  ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
   CDCtx.UserStylesheets.insert(
   CDCtx.UserStylesheets.begin(),
   "../share/clang/clang-doc-default-stylesheet.css");
@@ -90,7 +90,7 @@
   I.Path = "X/Y/Z";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -110,7 +110,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx =
+  getClangDocContext({}, "http://www.repository.com;);
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -121,7 +122,12 @@
 
 
   class r
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+http://www.repository.com/dir/test.cpp#10;>10
+ of file 
+http://www.repository.com/dir/test.cpp;>test.cpp
+  
   
 Inherits from 
 F
@@ -159,7 +165,7 @@
   I.Name = "f";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -173,7 +179,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx =
+  getClangDocContext({}, "https://www.repository.com;);
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -190,7 +197,7 @@
 int
  P)
   
-  Defined at line 10 of test.cpp
+  Defined at line 10 of file dir/test.cpp
 
 )raw";
 
@@ -202,7 +209,7 @@
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
@@ -212,7 +219,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -226,7 +233,12 @@
   
 X
   
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+https://www.repository.com/test.cpp#10;>10
+ of file 
+https://www.repository.com/test.cpp;>test.cpp
+  
 
 )raw";
 
@@ -295,7 +307,7 @@
 
   f
   void f(int I, int J)
-  Defined at line 10 of test.cpp
+  Defined at line 10 of file test.cpp
   
 
Brief description.
Index: 

[PATCH] D65841: [Driver] Switch -stdlib++-isystem test to -###-verbatim

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: compnerd, phosek, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This allows the test to pass on Windows and avoid backslash quoting
issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65841

Files:
  clang/test/Driver/stdlibxx-isystem.cpp


Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- clang/test/Driver/stdlibxx-isystem.cpp
+++ clang/test/Driver/stdlibxx-isystem.cpp
@@ -1,53 +1,48 @@
-// Backslash escaping makes matching against the installation directory fail on
-// Windows. Temporarily disable the test there until we add an option to print
-// the installation directory unescaped.
-// UNSUPPORTED: system-windows
-
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
 // RUN: mkdir -p %t/include/c++/v1
 // RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
-// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -###-verbatim | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
-// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -###-verbatim | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // LIBCXX: InstalledDir: [[INSTALLDIR:.+$]]
-// LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
+// LIBCXX: -internal-isystem [[INSTALLDIR]]/../include/c++/v1
 
 // Passing -stdlib++-isystem should suppress the default search.
 // RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
-// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
+// RUN:   -fsyntax-only %s -###-verbatim | FileCheck -check-prefix=NODEFAULT %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
-// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
+// RUN:   -fsyntax-only %s -###-verbatim | FileCheck -check-prefix=NODEFAULT %s
 // NODEFAULT: InstalledDir: [[INSTALLDIR:.+$]]
-// NODEFAULT-NOT: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
+// NODEFAULT-NOT: -internal-isystem [[INSTALLDIR]]/../include/c++/v1
 
 // And we should add it as an -internal-isystem.
 // RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
-// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
+// RUN:   -fsyntax-only %s -###-verbatim | FileCheck -check-prefix=INCPATH %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ 
\
-// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
-// INCPATH: "-internal-isystem" "/tmp/foo" "-internal-isystem" "/tmp/bar"
+// RUN:   -fsyntax-only %s -###-verbatim | FileCheck -check-prefix=INCPATH %s
+// INCPATH: -internal-isystem /tmp/foo -internal-isystem /tmp/bar
 
 // We shouldn't pass the -stdlib++-isystem to cc1.
 // RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
-// RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
-// RUN:   FileCheck -check-prefix=NOCC1 %s
+// RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s \
+// RUN:   -###-verbatim | FileCheck -check-prefix=NOCC1 %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
-// RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
-// RUN:   FileCheck -check-prefix=NOCC1 %s
-// NOCC1-NOT: "-stdlib++-isystem" "/tmp"
+// RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s \
+// RUN:   -###-verbatim | FileCheck -check-prefix=NOCC1 %s
+// NOCC1-NOT: -stdlib++-isystem /tmp
 
 // It should respect -nostdinc++.
 // RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
-// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
+// RUN:   -fsyntax-only %s -###-verbatim | FileCheck -check-prefix=NOSTDINCXX 
%s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
-// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
-// NOSTDINCXX-NOT: "-internal-isystem" "/tmp/foo" "-internal-isystem" 
"/tmp/bar"
+// RUN:   -fsyntax-only %s -###-verbatim | FileCheck -check-prefix=NOSTDINCXX 
%s
+// NOSTDINCXX-NOT: -internal-isystem /tmp/foo -internal-isystem /tmp/bar


Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- 

[PATCH] D65839: [Driver] Add verbatim dry run option

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 213773.
smeenai edited the summary of this revision.
smeenai added a comment.

Output to stdout


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65839

Files:
  clang/include/clang/Driver/Job.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -61,6 +61,13 @@
 return nullptr;
   }
 
+  // Just print the cc1 options verbatim if -###-verbatim was present.
+  if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH_VERBATIM)) {
+C->getJobs().Print(llvm::errs(), "\n",
+   driver::Command::PrintingMode::Verbatim);
+return nullptr;
+  }
+
   // We expect to get back exactly one command job, if we didn't something
   // failed. Offload compilation is an exception as it creates multiple jobs. If
   // that's the case, we proceed with the first job. If caller needs a
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1345,7 +1345,8 @@
 llvm::errs() << LksBuffer;
 
   // If this is a dry run, do not create the linker script file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
+  C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM))
 return;
 
   // Open script file and write the contents.
@@ -1453,7 +1454,8 @@
 llvm::errs() << LksBuffer;
 
   // If this is a dry run, do not create the linker script file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
+  C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM))
 return;
 
   // Open script file and write the contents.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1994,7 +1994,8 @@
 StringRef Target, const InputInfo ,
 const InputInfo , const ArgList ) const {
   // If this is a dry run, do not create the compilation database file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
+  C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM))
 return;
 
   using llvm::yaml::escape;
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -101,7 +101,8 @@
 void Command::printArg(raw_ostream , StringRef Arg, PrintingMode Mode) {
   const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
-  if (Mode == PrintingMode::QuoteIfNeeded && !Escape) {
+  if (Mode == PrintingMode::Verbatim ||
+  (Mode == PrintingMode::QuoteIfNeeded && !Escape)) {
 OS << Arg;
 return;
   }
@@ -213,9 +214,11 @@
 
 void Command::Print(raw_ostream , const char *Terminator, PrintingMode Mode,
 CrashReportInfo *CrashInfo) const {
-  // Always quote the exe.
+  // Always quote the exe unless printing verbatim.
   OS << ' ';
-  printArg(OS, Executable, PrintingMode::AlwaysQuote);
+  printArg(OS, Executable,
+   Mode == PrintingMode::Verbatim ? PrintingMode::Verbatim
+  : PrintingMode::AlwaysQuote);
 
   ArrayRef Args = Arguments;
   SmallVector ArgsRespFile;
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1451,6 +1451,12 @@
 return 0;
   }
 
+  // Just print verbatim if -###-verbatim was present.
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM)) {
+C.getJobs().Print(llvm::outs(), "\n", Command::PrintingMode::Verbatim);
+return 0;
+  }
+
   // If there were errors building the compilation, quit now.
   if (Diags.hasErrorOccurred())
 return 1;
@@ -1682,6 +1688,11 @@
 SuppressMissingInputWarning = true;
   }
 
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM)) {
+PrintVersion(C, llvm::outs());
+SuppressMissingInputWarning = true;
+  }
+
   if (C.getArgs().hasArg(options::OPT_v)) {
 if (!SystemConfigDir.empty())
   llvm::errs() << "System configuration file directory: "
@@ -3565,8 +3576,9 @@
   

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/docs/clang-doc.rst:93
+--public- Document only public declarations.
+--repository=   -
+  URL of repository that hosts code.

juliehockett wrote:
> Formatting here is a bit weird
Do you mean the almost empty line that's only "-"? This is because the 
description of the flag starts with \n.
```R"(
URL of repository that hosts code.
Used for links to definition locations.)"```
I used the same format used by [[ https://clang.llvm.org/extra/clang-tidy/ | 
clang-tidy ]], it has this format for all the multi-line flag descriptions.


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

https://reviews.llvm.org/D65483



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213772.
DiegoAstiazaran marked 7 inline comments as done.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.

Add comments.
Change name of flag; `root-directory` -> `--source-root`
Moved fixing of repository link to ClangDocContext constructor and add test 
where the prefix is omitted.
RepositoryLink in ClangDocContext struct and writeFileDefinition function is 
now llvm::Optional<>


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

https://reviews.llvm.org/D65483

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/docs/clang-doc.rst
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
 
   template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
 if (I.first)
   EmittedInfos.emplace_back(std::move(I.first));
 if (I.second)
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -22,9 +22,9 @@
 }
 
 ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
-  ClangDocContext CDCtx;
-  CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+   StringRef RepositoryUrl = "") {
+  ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
   CDCtx.UserStylesheets.insert(
   CDCtx.UserStylesheets.begin(),
   "../share/clang/clang-doc-default-stylesheet.css");
@@ -90,7 +90,7 @@
   I.Path = "X/Y/Z";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -110,7 +110,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx =
+  getClangDocContext({}, StringRef{"http://www.repository.com"});
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -121,7 +122,12 @@
 
 
   class r
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+http://www.repository.com/dir/test.cpp#10;>10
+ of file 
+http://www.repository.com/dir/test.cpp;>test.cpp
+  
   
 Inherits from 
 F
@@ -159,7 +165,7 @@
   I.Name = "f";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -173,7 +179,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx =
+  getClangDocContext({}, StringRef{"https://www.repository.com"});
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -190,7 +197,7 @@
 int
  P)
   
-  Defined at line 10 of test.cpp
+  Defined at line 10 of file dir/test.cpp
 
 )raw";
 
@@ -202,7 +209,7 @@
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
@@ -212,7 +219,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -226,7 +233,12 @@
   
 X
   
-  Defined at line 10 

[PATCH] D65839: [Driver] Add verbatim dry run option

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I'm not tied to the name `-###-verbatim` and am open to suggestions if anyone 
can think of something better.

My troll suggestion was `-`, but @compnerd didn't like that for some 
reason...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65839



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

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai abandoned this revision.
smeenai added a comment.

Abandoning in favor of D65839 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64538



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

[PATCH] D65839: [Driver] Add verbatim dry run option

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: compnerd, phosek, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
smeenai added a parent revision: D65838: [Driver] Use enumeration for quoting 
mode. NFC.

When writing driver tests, it's useful to have a way to output arguments
verbatim (i.e. without any quoting and escaping). For example, on
Windows today, the installation directory is output without backslashes
being escaped, but any -internal-isystem arguments passed by the driver
to the frontend will have backslashes escaped in the -### output, making
it impossible to write a FileCheck match against the installation
directory. Add a new argument to avoid any quoting and escaping to ease
driver testing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65839

Files:
  clang/include/clang/Driver/Job.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -61,6 +61,13 @@
 return nullptr;
   }
 
+  // Just print the cc1 options verbatim if -###-verbatim was present.
+  if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH_VERBATIM)) {
+C->getJobs().Print(llvm::errs(), "\n",
+   driver::Command::PrintingMode::Verbatim);
+return nullptr;
+  }
+
   // We expect to get back exactly one command job, if we didn't something
   // failed. Offload compilation is an exception as it creates multiple jobs. If
   // that's the case, we proceed with the first job. If caller needs a
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1345,7 +1345,8 @@
 llvm::errs() << LksBuffer;
 
   // If this is a dry run, do not create the linker script file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
+  C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM))
 return;
 
   // Open script file and write the contents.
@@ -1453,7 +1454,8 @@
 llvm::errs() << LksBuffer;
 
   // If this is a dry run, do not create the linker script file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
+  C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM))
 return;
 
   // Open script file and write the contents.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1994,7 +1994,8 @@
 StringRef Target, const InputInfo ,
 const InputInfo , const ArgList ) const {
   // If this is a dry run, do not create the compilation database file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
+  C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM))
 return;
 
   using llvm::yaml::escape;
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -101,7 +101,8 @@
 void Command::printArg(raw_ostream , StringRef Arg, PrintingMode Mode) {
   const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
-  if (Mode == PrintingMode::QuoteIfNeeded && !Escape) {
+  if (Mode == PrintingMode::Verbatim ||
+  (Mode == PrintingMode::QuoteIfNeeded && !Escape)) {
 OS << Arg;
 return;
   }
@@ -213,9 +214,11 @@
 
 void Command::Print(raw_ostream , const char *Terminator, PrintingMode Mode,
 CrashReportInfo *CrashInfo) const {
-  // Always quote the exe.
+  // Always quote the exe unless printing verbatim.
   OS << ' ';
-  printArg(OS, Executable, PrintingMode::AlwaysQuote);
+  printArg(OS, Executable,
+   Mode == PrintingMode::Verbatim ? PrintingMode::Verbatim
+  : PrintingMode::AlwaysQuote);
 
   ArrayRef Args = Arguments;
   SmallVector ArgsRespFile;
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1451,6 +1451,12 @@
 return 0;
   }
 
+  // Just print verbatim if -###-verbatim was present.
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH_VERBATIM)) {
+C.getJobs().Print(llvm::errs(), "\n", 

[PATCH] D65838: [Driver] Use enumeration for quoting mode. NFC

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: compnerd, phosek, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Boolean parameters are generally hard to understand, especially when we
don't consistently have a comment for them. Change to an enumeration.

While I believe this change is worthwhile by itself, its main purpose is
to serve as cleanup for a follow-up which will add a third mode to this
enumeration, to allow verbatim printing of arguments (without any
quoting or escaping).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65838

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp

Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -94,7 +94,7 @@
   if (Jobs.size() != 1 || !isa(*Jobs.begin())) {
 SmallString<256> error_msg;
 llvm::raw_svector_ostream error_stream(error_msg);
-Jobs.Print(error_stream, "; ", true);
+Jobs.Print(error_stream, "; ", driver::Command::PrintingMode::AlwaysQuote);
 Diagnostics->Report(diag::err_fe_expected_compiler_job)
 << error_stream.str();
 return nullptr;
@@ -337,7 +337,8 @@
   // Show the invocation, with -v.
   if (Invocation->getHeaderSearchOpts().Verbose) {
 llvm::errs() << "clang Invocation:\n";
-Compilation->getJobs().Print(llvm::errs(), "\n", true);
+Compilation->getJobs().Print(llvm::errs(), "\n",
+ driver::Command::PrintingMode::AlwaysQuote);
 llvm::errs() << "\n";
   }
 
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -56,7 +56,8 @@
 
   // Just print the cc1 options if -### was present.
   if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH)) {
-C->getJobs().Print(llvm::errs(), "\n", true);
+C->getJobs().Print(llvm::errs(), "\n",
+   driver::Command::PrintingMode::AlwaysQuote);
 return nullptr;
   }
 
@@ -82,7 +83,7 @@
   (Jobs.size() > 1 && !OffloadCompilation)) {
 SmallString<256> Msg;
 llvm::raw_svector_ostream OS(Msg);
-Jobs.Print(OS, "; ", true);
+Jobs.Print(OS, "; ", driver::Command::PrintingMode::AlwaysQuote);
 Diags->Report(diag::err_fe_expected_compiler_job) << OS.str();
 return nullptr;
   }
Index: clang/lib/Driver/Job.cpp
===
--- clang/lib/Driver/Job.cpp
+++ clang/lib/Driver/Job.cpp
@@ -98,10 +98,10 @@
   return false;
 }
 
-void Command::printArg(raw_ostream , StringRef Arg, bool Quote) {
+void Command::printArg(raw_ostream , StringRef Arg, PrintingMode Mode) {
   const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
-  if (!Quote && !Escape) {
+  if (Mode == PrintingMode::QuoteIfNeeded && !Escape) {
 OS << Arg;
 return;
   }
@@ -211,11 +211,11 @@
   IncFlags.push_back(std::move(NewInc));
 }
 
-void Command::Print(raw_ostream , const char *Terminator, bool Quote,
+void Command::Print(raw_ostream , const char *Terminator, PrintingMode Mode,
 CrashReportInfo *CrashInfo) const {
   // Always quote the exe.
   OS << ' ';
-  printArg(OS, Executable, /*Quote=*/true);
+  printArg(OS, Executable, PrintingMode::AlwaysQuote);
 
   ArrayRef Args = Arguments;
   SmallVector ArgsRespFile;
@@ -243,7 +243,7 @@
 if (!NewIncFlags.empty()) {
   for (auto  : NewIncFlags) {
 OS << ' ';
-printArg(OS, F.c_str(), Quote);
+printArg(OS, F.c_str(), Mode);
   }
   i += NumArgs - 1;
   continue;
@@ -257,20 +257,20 @@
 // Replace the input file name with the crashinfo's file name.
 OS << ' ';
 StringRef ShortName = llvm::sys::path::filename(CrashInfo->Filename);
-printArg(OS, ShortName.str(), Quote);
+printArg(OS, ShortName.str(), Mode);
 continue;
   }
 }
 
 OS << ' ';
-printArg(OS, Arg, Quote);
+printArg(OS, Arg, Mode);
   }
 
   if (CrashInfo && HaveCrashVFS) {
 OS << ' ';
-printArg(OS, "-ivfsoverlay", Quote);
+printArg(OS, "-ivfsoverlay", Mode);
 OS << ' ';
-printArg(OS, CrashInfo->VFSPath.str(), Quote);
+printArg(OS, CrashInfo->VFSPath.str(), Mode);
 
 // The leftover modules from the crash are stored in
 //  .cache/vfs/modules
@@ -285,7 +285,7 @@
 ModCachePath.append(RelModCacheDir.c_str());
 
 OS << ' ';
-printArg(OS, ModCachePath, Quote);
+printArg(OS, ModCachePath, Mode);
   }
 
   if (ResponseFile != nullptr) {
@@ -379,10 +379,11 @@
   Fallback(std::move(Fallback_)) {}
 
 

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Jian Cai via Phabricator via cfe-commits]

jcai19 marked 4 inline comments as done.
jcai19 added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2418
+  bool PushLR = IntrinsicName &&
+  !memcmp(IntrinsicName, "\01__gnu_mcount_nc", 
sizeof("\01__gnu_mcount_nc"));
+  unsigned CallOpc = ARMSelectCallOp(UseReg, PushLR);

nickdesaulniers wrote:
> `memcmp` is a code smell in a C++ codebase, but I see that `IntrinsicName` is 
> a C style string.  Is there a reason why `strncmp` isn't used?
Good catch! I was thinking about one of str* functions and somehow ended up 
using memcpy. Guess I haven't written C code for a while :). Anyway, maybe 
strcmp is better here as the size of the two strings should match too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Artem Dergachev via Phabricator via cfe-commits]

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

Yay!


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

https://reviews.llvm.org/D65663



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

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Thanks for the review! It is much better: F9740817: 
report-Driver.cpp-operator()-6-2.html 




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420
+  if (!IsAssuming) {
+PathDiagnosticLocation Loc(BExpr->getLHS(), BRC.getSourceManager(), LCtx);
 return std::make_shared(Loc, Message);

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Just curious, can `BExpr->getLHS()` potentially still be a multi-line 
> > > expression? Or are we making sure it's always a 
> > > `DeclRefExpr`/`MemberExpr`?
> > > 
> > > In case of `MemberExpr` i'm pretty sure you can fit a newline 
> > > before/after `.` or `->`.
> > Previously I have focused on `DeclRefExpr` and `MemberExpr` 
> > value-evaluation, so that there the expression must be one of them. Because 
> > we have no `getField()` method for obtaining the field of the `MemberExpr`, 
> > I have picked `getBase()`.
> It's `MemberExpr::getMemberLoc()`. It's always a single token. Unlike the 
> base, which may still be arbitrarily complex.
I am thinking about the opposite when we `have->a->lot->of->stuff`. It is 
working fine, thanks for the ideas!


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

https://reviews.llvm.org/D65663



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 213762.
jcai19 added a comment.

Update based on comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019

Files:
  clang/lib/Basic/Targets/ARM.cpp
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.h
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
  llvm/test/CodeGen/ARM/gnu_mcount_nc.ll

Index: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
@@ -0,0 +1,21 @@
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-THUMB
+
+define dso_local void @callee() #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+  ret void
+}
+
+define dso_local void @caller() #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+  call void @callee()
+  ret void
+}
+
+attributes #0 = { nofree nounwind "instrument-function-entry-inlined"="llvm.arm.gnu.eabi.mcount" }
Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -24,7 +24,7 @@
 
   if (Func == "mcount" ||
   Func == ".mcount" ||
-  Func == "\01__gnu_mcount_nc" ||
+  Func == "llvm.arm.gnu.eabi.mcount" ||
   Func == "\01_mcount" ||
   Func == "\01mcount" ||
   Func == "__mcount" ||
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -565,6 +565,13 @@
   4, IIC_Br,
   [(ARMcall_nolink tGPR:$func)]>,
 Requires<[IsThumb, IsThumb1Only]>, Sched<[WriteBr]>;
+
+  // Also used for Thumb2
+  // push lr before the call
+  def tBL_PUSHLR : tPseudoInst<(outs), (ins pred:$p, thumb_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr tglobaladdr:$func)]>,
+ Requires<[IsThumb]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1, isBarrier = 1 in {
Index: llvm/lib/Target/ARM/ARMInstrInfo.td
===
--- llvm/lib/Target/ARM/ARMInstrInfo.td
+++ llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -148,6 +148,9 @@
 def ARMcall_nolink   : SDNode<"ARMISD::CALL_NOLINK", SDT_ARMcall,
   [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
SDNPVariadic]>;
+def ARMcall_pushlr : SDNode<"ARMISD::CALL_PUSHLR", SDT_ARMcall,
+  [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
+   SDNPVariadic]>;
 
 def ARMretflag   : SDNode<"ARMISD::RET_FLAG", SDTNone,
   [SDNPHasChain, SDNPOptInGlue, SDNPVariadic]>;
@@ -2350,6 +2353,12 @@
   def BMOVPCB_CALL : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
8, IIC_Br, [(ARMcall_nolink tglobaladdr:$func)]>,
   Requires<[IsARM]>, Sched<[WriteBr]>;
+
+  // push lr before the call
+  def BL_PUSHLR : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr tglobaladdr:$func)]>,
+ Requires<[IsARM]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1 in {
Index: llvm/lib/Target/ARM/ARMISelLowering.h
===
--- llvm/lib/Target/ARM/ARMISelLowering.h
+++ llvm/lib/Target/ARM/ARMISelLowering.h
@@ -68,6 +68,7 @@
   CALL, // Function call.
   CALL_PRED,// Function call that's predicable.
   CALL_NOLINK,  // Function call with branch not branch-and-link.
+  CALL_PUSHLR,  // Function call that pushes LR before the call.
   BRCOND,   // Conditional branch.
   BR_JT,// Jumptable branch.
   BR2_JT,   // Jumptable branch (2 level - jumptable entry is a jump).
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -1421,6 +1421,7 @@
   case ARMISD::CALL:  return "ARMISD::CALL";
   case 

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 213763.
Charusso marked 2 inline comments as done.
Charusso added a comment.

- Fix.


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

https://reviews.llvm.org/D65663

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
  clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -2513,7 +2513,7 @@
 
 
  line96
- col13
+ col8
  file0
 

@@ -2735,7 +2735,7 @@
 
 
  line96
- col13
+ col8
  file0
 

@@ -3554,7 +3554,7 @@
 
 
  line127
- col14
+ col9
  file0
 

@@ -3776,7 +3776,7 @@
 
 
  line127
- col14
+ col9
  file0
 

Index: clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-radar17039661.m.plist
@@ -836,7 +836,7 @@
 
 
  line38
- col37
+ col20
  file0
 

Index: clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
===
--- clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
+++ clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
@@ -548,7 +548,7 @@
 
 
  line45
- col12
+ col7
  file0
 

Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===
--- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -2727,7 +2727,7 @@
 
 
  line146
- col13
+ col8
  file0
 

@@ -2949,7 +2949,7 @@
 
 
  line146
- col13
+ col8
  file0
 

@@ -3929,7 +3929,7 @@
 
 
  line178
- col14
+ col9
  file0
 

@@ -4185,7 +4185,7 @@
 
 
  line178
- col14
+ col9
  file0
 

@@ -4281,7 +4281,7 @@
 
 
  line181
- col14
+ col9
  file0
 

@@ -8087,7 +8087,7 @@
  location
  
   line267
-  col18
+  col19
   file0
  
  ranges
@@ -8095,7 +8095,7 @@

 
  line267
- col18
+ col19
  file0
 
 
@@ -8119,12 +8119,12 @@
  
   
line267
-   col18
+   col19
file0
   
   
line267
-   col18
+   col22
file0
   
  
@@ -11983,12 +11983,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12000,7 +12000,7 @@
  location
  
   line457
-  col9
+  col10
   file0
  
  ranges
@@ -12008,7 +12008,7 @@

 
  line457
- col9
+ col10
  file0
 
 
@@ -12032,12 +12032,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12244,12 +12244,12 @@
  
   
line457
-   col9
+   col10
file0
   
   
line457
-   col9
+   col14
file0
   
  
@@ -12261,7 +12261,7 @@
  location
  
   line457
-  col9
+  col10
   file0
  
  ranges
@@ -12269,7 +12269,7 @@

 
  line457
- col9
+ col10
  file0
 
 
@@ -12293,12 +12293,12 @@
  
   
line457
-   col9
+   col10
file0
 

[PATCH] D65684: [WebAssembly] Lower ASan constructor priority on Emscripten

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:132
 static const uint64_t kAsanCtorAndDtorPriority = 1;
+static const uint64_t kAsanEmscriptenCtorAndDtorPriority = 50;
 static const char *const kAsanReportErrorTemplate = "__asan_report_";

quantum wrote:
> sbc100 wrote:
> > Any reason not to change this for all platforms?  In any case an explaining 
> > comment might be a good idea here.
> I suspect doing that will break things on other platforms. I shall add a 
> comment.
I think it might be a good idea to contact the original author to find out why 
1 was chosen here and if they think it would be OK to change to 50 across the 
board.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65684



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:211
 unsigned ARMMoveToIntReg(MVT VT, unsigned SrcReg);
-unsigned ARMSelectCallOp(bool UseReg);
+unsigned ARMSelectCallOp(bool UseReg, bool PushLR = false);
 unsigned ARMLowerPICELF(const GlobalValue *GV, unsigned Align, MVT VT);

jcai19 wrote:
> nickdesaulniers wrote:
> > How many other call sites would have to be updated if this was not a 
> > default parameter?
> There should be only 2 references to this function. But this argument is 
> likely to be false in most cases.
Then I'd just make it an explicit arg and update the 2 call sites.  If there 
were many call sites, then the default param would cut down on code churn, but 
I don't think 2 call sites is unreasonable to just be explicit about such 
arguments.



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2418
+  bool PushLR = IntrinsicName &&
+  !memcmp(IntrinsicName, "\01__gnu_mcount_nc", 
sizeof("\01__gnu_mcount_nc"));
+  unsigned CallOpc = ARMSelectCallOp(UseReg, PushLR);

`memcmp` is a code smell in a C++ codebase, but I see that `IntrinsicName` is a 
C style string.  Is there a reason why `strncmp` isn't used?



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2587
   }
+  case Intrinsic::arm_gnu_eabi_mcount: {
+return SelectCall(, "\01__gnu_mcount_nc");

`{}` are not needed here since you're not introducing a new scope for variables.



Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:11
+entry:
+  %cmp = icmp slt i32 %n, 2
+  br i1 %cmp, label %return, label %if.end

This test case can probably be simplified to just a call and ret void.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65663: [analyzer] ConditionBRVisitor: Fix HTML PathDiagnosticPopUpPieces

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2420
+  if (!IsAssuming) {
+PathDiagnosticLocation Loc(BExpr->getLHS(), BRC.getSourceManager(), LCtx);
 return std::make_shared(Loc, Message);

Charusso wrote:
> NoQ wrote:
> > Just curious, can `BExpr->getLHS()` potentially still be a multi-line 
> > expression? Or are we making sure it's always a `DeclRefExpr`/`MemberExpr`?
> > 
> > In case of `MemberExpr` i'm pretty sure you can fit a newline before/after 
> > `.` or `->`.
> Previously I have focused on `DeclRefExpr` and `MemberExpr` value-evaluation, 
> so that there the expression must be one of them. Because we have no 
> `getField()` method for obtaining the field of the `MemberExpr`, I have 
> picked `getBase()`.
It's `MemberExpr::getMemberLoc()`. It's always a single token. Unlike the base, 
which may still be arbitrarily complex.


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

https://reviews.llvm.org/D65663



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.
jdenny added a reviewer: ABataev.
Herald added a subscriber: guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

For `map`, the following restriction changed in OpenMP 5.0:

- OpenMP 4.5 [2.15.5.1, Restrictions]: "A list item cannot appear in both a map 
clause and a data-sharing attribute clause on the same construct.

- OpenMP 5.0 [2.19.7.1, Restrictions]: "A list item cannot appear in both a map 
clause and a data-sharing attribute clause on the same construct unless the 
construct is a combined construct."

For `is_device_ptr`, I don't see a restriction in either version, but 
perhaps I missed it.  However, Clang implements a similar restriction
for `is_device_ptr` as for `map`.

This patch removes this restriction in the case of combined
constructs, both for `map` and for `is_device_ptr`.  It does not 
remove the restriction for non-combined constructs even though I'm not 
sure why `is_device_ptr` has the restriction at all.

I haven't yet added any new tests for the `is_device_ptr` case.  I'm 
happy to do so if reviewers feel this patch is heading in the right
direction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65835

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_is_device_ptr_messages.cpp
  clang/test/OpenMP/target_parallel_is_device_ptr_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_is_device_ptr_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_lastprivate_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_is_device_ptr_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_firstprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_is_device_ptr_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_private_messages.cpp
  clang/test/OpenMP/target_teams_is_device_ptr_messages.cpp
  clang/test/OpenMP/target_teams_map_codegen.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp

Index: clang/test/OpenMP/target_teams_map_messages.cpp
===
--- clang/test/OpenMP/target_teams_map_messages.cpp
+++ clang/test/OpenMP/target_teams_map_messages.cpp
@@ -536,10 +536,10 @@
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
 
-#pragma omp target teams private(j) map(j) // expected-error {{private variable cannot be in a map clause in '#pragma omp target teams' directive}}  expected-note {{defined as private}}
+#pragma omp target teams private(j) map(j)
   {}
 
-#pragma omp target teams firstprivate(j) map(j)  // expected-error {{firstprivate variable cannot be in a map clause in '#pragma omp target teams' directive}} expected-note {{defined as firstprivate}}
+#pragma omp target teams firstprivate(j) map(j)
   {}
 
 #pragma omp target teams map(m)
Index: clang/test/OpenMP/target_teams_map_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_teams_map_codegen.cpp
@@ -0,0 +1,64 @@
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -triple i386-unknown-unknown -fopenmp-targets=i386-pc-linux-gnu -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// Test target codegen - host bc file has to be created first.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -triple 

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Jian Cai via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:211
 unsigned ARMMoveToIntReg(MVT VT, unsigned SrcReg);
-unsigned ARMSelectCallOp(bool UseReg);
+unsigned ARMSelectCallOp(bool UseReg, bool PushLR = false);
 unsigned ARMLowerPICELF(const GlobalValue *GV, unsigned Align, MVT VT);

nickdesaulniers wrote:
> How many other call sites would have to be updated if this was not a default 
> parameter?
There should be only 2 references to this function. But this argument is likely 
to be false in most cases.



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2417
   // Issue the call.
-  unsigned CallOpc = ARMSelectCallOp(UseReg);
+  unsigned CallOpc = ARMSelectCallOp(UseReg, Callee && Callee->getName() == 
"\01__gnu_mcount_nc");
   MachineInstrBuilder MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt,

nickdesaulniers wrote:
> efriedma wrote:
> > 80 cols
> also, what's up with `\01`?
I am not sure why the name is prefixed with \01 either but it was there in the 
original code.



Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:6
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NOT:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc

nickdesaulniers wrote:
> why does the `-NOT` case duplicate the non-`NOT` case?
They are unnecessary, I have removed them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Thanks Puyan!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65829



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 213748.
jcai19 marked 6 inline comments as done.
jcai19 added a comment.

Update based on comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019

Files:
  clang/lib/Basic/Targets/ARM.cpp
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.h
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
  llvm/test/CodeGen/ARM/gnu_mcount_nc.ll

Index: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
@@ -0,0 +1,38 @@
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-THUMB
+
+; Function Attrs: nounwind readnone
+define dso_local i32 @fib(i32 %n) local_unnamed_addr #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+entry:
+  %cmp = icmp slt i32 %n, 2
+  br i1 %cmp, label %return, label %if.end
+
+if.end:   ; preds = %entry
+  %sub = add nsw i32 %n, -1
+  %call = tail call i32 @fib(i32 %sub)
+  %sub1 = add nsw i32 %n, -2
+  %call2 = tail call i32 @fib(i32 %sub1)
+  %add = add nsw i32 %call2, %call
+  ret i32 %add
+
+return:   ; preds = %entry
+  ret i32 %n
+}
+
+; Function Attrs: nounwind readnone
+define dso_local i32 @foo(i32 %n) local_unnamed_addr #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+entry:
+  %call = tail call i32 @fib(i32 %n)
+  %mul = mul nsw i32 %call, %call
+  ret i32 %mul
+}
+
+attributes #0 = { nofree nounwind "instrument-function-entry-inlined"="llvm.arm.gnu.eabi.mcount" }
Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -24,7 +24,7 @@
 
   if (Func == "mcount" ||
   Func == ".mcount" ||
-  Func == "\01__gnu_mcount_nc" ||
+  Func == "llvm.arm.gnu.eabi.mcount" ||
   Func == "\01_mcount" ||
   Func == "\01mcount" ||
   Func == "__mcount" ||
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -565,6 +565,13 @@
   4, IIC_Br,
   [(ARMcall_nolink tGPR:$func)]>,
 Requires<[IsThumb, IsThumb1Only]>, Sched<[WriteBr]>;
+
+  // Also used for Thumb2
+  // push lr before the call
+  def tBL_PUSHLR : tPseudoInst<(outs), (ins pred:$p, thumb_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr tglobaladdr:$func)]>,
+ Requires<[IsThumb]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1, isBarrier = 1 in {
Index: llvm/lib/Target/ARM/ARMInstrInfo.td
===
--- llvm/lib/Target/ARM/ARMInstrInfo.td
+++ llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -148,6 +148,9 @@
 def ARMcall_nolink   : SDNode<"ARMISD::CALL_NOLINK", SDT_ARMcall,
   [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
SDNPVariadic]>;
+def ARMcall_pushlr : SDNode<"ARMISD::CALL_PUSHLR", SDT_ARMcall,
+  [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
+   SDNPVariadic]>;
 
 def ARMretflag   : SDNode<"ARMISD::RET_FLAG", SDTNone,
   [SDNPHasChain, SDNPOptInGlue, SDNPVariadic]>;
@@ -2350,6 +2353,12 @@
   def BMOVPCB_CALL : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
8, IIC_Br, [(ARMcall_nolink tglobaladdr:$func)]>,
   Requires<[IsARM]>, Sched<[WriteBr]>;
+
+  // push lr before the call
+  def BL_PUSHLR : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr tglobaladdr:$func)]>,
+ Requires<[IsARM]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1 in {
Index: llvm/lib/Target/ARM/ARMISelLowering.h
===
--- llvm/lib/Target/ARM/ARMISelLowering.h
+++ llvm/lib/Target/ARM/ARMISelLowering.h
@@ -68,6 +68,7 @@
   CALL, // Function call.
   CALL_PRED,// 

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368108: [clang][DirectoryWatcher][NFC] Swapping asserts for 
llvm fatal_error in create (authored by zer0, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65829?vs=213742=213746#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65829

Files:
  cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
  cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
  cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
  cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Index: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -277,14 +277,20 @@
{EventKind::Modified, "c"}}
   };
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -309,14 +315,20 @@
{EventKind::Modified, "c"}}
};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/false);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/false);
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -330,14 +342,20 @@
{EventKind::Modified, "b"},
{EventKind::Modified, "c"}}};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   fixture.addFile("a");
   fixture.addFile("b");
@@ -356,14 +374,20 @@
   {{EventKind::Modified, "a"}},
   {{EventKind::Modified, "a"}}};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   // modify the file
   {
@@ -387,14 +411,20 @@
   {{EventKind::Removed, "a"}},
   {{EventKind::Modified, "a"}, {EventKind::Removed, "a"}}};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+ 

r368108 - [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in create

[Puyan Lotfi via cfe-commits]

Author: zer0
Date: Tue Aug  6 16:25:34 2019
New Revision: 368108

URL: http://llvm.org/viewvc/llvm-project?rev=368108=rev
Log:
[clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in create

I also have replaced all the instances of
"auto DW = DirectoryWatcher::create" with
llvm::Expected> DW = DirectoryWatcher::create
to make it more clear that DirectoryWatcher::create is returning an Expected.

I've also allowed for logAllUnhandledErrors to consume errors in the case were
DirectoryWatcher::create produces them.

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



Modified:
cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Modified: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h?rev=368108=368107=368108=diff
==
--- cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h (original)
+++ cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h Tue Aug  6 
16:25:34 2019
@@ -99,10 +99,10 @@ public:
 : Kind(Kind), Filename(Filename) {}
   };
 
-  /// Asserts if \param Path doesn't exist or isn't a directory.
+  /// llvm fatal_error if \param Path doesn't exist or isn't a directory.
   /// Returns llvm::Expected Error if OS kernel API told us we can't start
   /// watching. In such case it's unclear whether just retrying has any chance
-  /// to succeeed.
+  /// to succeed.
   static llvm::Expected>
   create(llvm::StringRef Path,
  std::function Events,

Modified: cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp?rev=368108=368107=368108=diff
==
--- cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp (original)
+++ cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp Tue Aug  6 
16:25:34 2019
@@ -325,7 +325,9 @@ llvm::Expected, bool)> 
Receiver,
 bool WaitForInitialSync) {
-  assert(!Path.empty() && "Path.empty()");
+  if (Path.empty())
+llvm::report_fatal_error(
+"DirectoryWatcher::create can not accept an empty Path.");
 
   const int InotifyFD = inotify_init1(IN_CLOEXEC);
   if (InotifyFD == -1)

Modified: cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp?rev=368108=368107=368108=diff
==
--- cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp (original)
+++ cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp Tue Aug  6 
16:25:34 2019
@@ -150,7 +150,8 @@ FSEventStreamRef createFSEventStream(
 StringRef Path,
 std::function, bool)> 
Receiver,
 dispatch_queue_t Queue) {
-  assert(!Path.empty() && "Path.empty()");
+  if (Path.empty())
+return nullptr;
 
   CFMutableArrayRef PathsToWatch = [&]() {
 CFMutableArrayRef PathsToWatch =
@@ -208,7 +209,10 @@ llvm::Expectedhttp://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp?rev=368108=368107=368108=diff
==
--- cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp (original)
+++ cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp Tue Aug  6 
16:25:34 2019
@@ -277,14 +277,20 @@ TEST(DirectoryWatcherTest, InitialScanSy
{EventKind::Modified, "c"}}
   };
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -309,14 +315,20 @@ TEST(DirectoryWatcherTest, InitialScanAs
{EventKind::Modified, "c"}}
};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
- 

[PATCH] D65833: [Tooling] Expose ExecutorConcurrency option.

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added a reviewer: juliehockett.

https://reviews.llvm.org/D65833

Files:
  clang/include/clang/Tooling/AllTUsExecution.h
  clang/lib/Tooling/AllTUsExecution.cpp


Index: clang/lib/Tooling/AllTUsExecution.cpp
===
--- clang/lib/Tooling/AllTUsExecution.cpp
+++ clang/lib/Tooling/AllTUsExecution.cpp
@@ -147,7 +147,7 @@
   return llvm::Error::success();
 }
 
-static llvm::cl::opt ExecutorConcurrency(
+llvm::cl::opt ExecutorConcurrency(
 "execute-concurrency",
 llvm::cl::desc("The number of threads used to process all files in "
"parallel. Set to 0 for hardware concurrency. "
Index: clang/include/clang/Tooling/AllTUsExecution.h
===
--- clang/include/clang/Tooling/AllTUsExecution.h
+++ clang/include/clang/Tooling/AllTUsExecution.h
@@ -71,6 +71,7 @@
   unsigned ThreadCount;
 };
 
+extern llvm::cl::opt ExecutorConcurrency;
 extern llvm::cl::opt Filter;
 
 } // end namespace tooling


Index: clang/lib/Tooling/AllTUsExecution.cpp
===
--- clang/lib/Tooling/AllTUsExecution.cpp
+++ clang/lib/Tooling/AllTUsExecution.cpp
@@ -147,7 +147,7 @@
   return llvm::Error::success();
 }
 
-static llvm::cl::opt ExecutorConcurrency(
+llvm::cl::opt ExecutorConcurrency(
 "execute-concurrency",
 llvm::cl::desc("The number of threads used to process all files in "
"parallel. Set to 0 for hardware concurrency. "
Index: clang/include/clang/Tooling/AllTUsExecution.h
===
--- clang/include/clang/Tooling/AllTUsExecution.h
+++ clang/include/clang/Tooling/AllTUsExecution.h
@@ -71,6 +71,7 @@
   unsigned ThreadCount;
 };
 
+extern llvm::cl::opt ExecutorConcurrency;
 extern llvm::cl::opt Filter;
 
 } // end namespace tooling
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D65829#1617864 , @compnerd wrote:

> @lhames - I like the `logAllUnhandledErrors`!


@lhames @compnerd @jkorous

Only downside to using logAllUnhandledErrors over just letting the 
llvm::Expected's destructor handle the error print out is that 
logAllUnhandledErrors only logs the errors. It doesn't crash the rest of the 
deadlocking DirectoryWatcher test. So I had to include logAllUnhandledErrors 
along with exit(EXIT_FAILURE).

Anyhow, it seems we have consensus so I will land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 213742.
plotfi added a comment.

Updated to use logAllUnhandledErrors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829

Files:
  clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
  clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
  clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -277,14 +277,20 @@
{EventKind::Modified, "c"}}
   };
 
-  auto DW = DirectoryWatcher::create(
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
   fixture.TestWatchedDir,
   [](llvm::ArrayRef Events,
   bool IsInitial) {
 TestConsumer.consume(Events, IsInitial);
   },
   /*waitForInitialSync=*/true);
-  if (!DW) return;
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -309,14 +315,20 @@
{EventKind::Modified, "c"}}
};
 
-  auto DW = DirectoryWatcher::create(
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
   fixture.TestWatchedDir,
   [](llvm::ArrayRef Events,
   bool IsInitial) {
 TestConsumer.consume(Events, IsInitial);
   },
   /*waitForInitialSync=*/false);
-  if (!DW) return;
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -330,14 +342,20 @@
{EventKind::Modified, "b"},
{EventKind::Modified, "c"}}};
 
-  auto DW = DirectoryWatcher::create(
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
   fixture.TestWatchedDir,
   [](llvm::ArrayRef Events,
   bool IsInitial) {
 TestConsumer.consume(Events, IsInitial);
   },
   /*waitForInitialSync=*/true);
-  if (!DW) return;
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   fixture.addFile("a");
   fixture.addFile("b");
@@ -356,14 +374,20 @@
   {{EventKind::Modified, "a"}},
   {{EventKind::Modified, "a"}}};
 
-  auto DW = DirectoryWatcher::create(
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
   fixture.TestWatchedDir,
   [](llvm::ArrayRef Events,
   bool IsInitial) {
 TestConsumer.consume(Events, IsInitial);
   },
   /*waitForInitialSync=*/true);
-  if (!DW) return;
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   // modify the file
   {
@@ -387,14 +411,20 @@
   {{EventKind::Removed, "a"}},
   {{EventKind::Modified, "a"}, {EventKind::Removed, "a"}}};
 
-  auto DW = DirectoryWatcher::create(
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
   fixture.TestWatchedDir,
   [](llvm::ArrayRef Events,
   bool IsInitial) {
 TestConsumer.consume(Events, IsInitial);
   },
   /*waitForInitialSync=*/true);
-  if (!DW) return;
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   fixture.deleteFile("a");
 
@@ -409,14 +439,20 @@
   {{EventKind::WatchedDirRemoved, ""},
{EventKind::WatcherGotInvalidated, ""}}};
 
-  auto DW = DirectoryWatcher::create(
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
   fixture.TestWatchedDir,
   [](llvm::ArrayRef Events,
   bool IsInitial) {
 TestConsumer.consume(Events, IsInitial);
   },
   /*waitForInitialSync=*/true);
-  if (!DW) return;
+  if (!DW) {
+logAllUnhandledErrors(
+DW.takeError(), llvm::errs(),
+"DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+exit(EXIT_FAILURE);
+  }
 
   remove_directories(fixture.TestWatchedDir);
 
@@ -430,14 +466,20 @@
   {}, {{EventKind::WatcherGotInvalidated, ""}}};
 
   {
-auto DW = DirectoryWatcher::create(
+llvm::Expected> DW =
+DirectoryWatcher::create(
 fixture.TestWatchedDir,
   

r368104 - Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via cfe-commits]

Author: void
Date: Tue Aug  6 15:41:22 2019
New Revision: 368104

URL: http://llvm.org/viewvc/llvm-project?rev=368104=rev
Log:
Delay diagnosing asm constraints that require immediates until after inlining

Summary:
An inline asm call may result in an immediate input value after inlining.
Therefore, don't emit a diagnostic here if the input isn't an immediate.

Reviewers: joerg, eli.friedman, rsmith

Subscribers: asb, rbar, johnrusso, simoncook, apazos, sabuasal, niosHD, jrtc27, 
zzheng, edward-jones, rogfer01, MartinMosbeck, brucehoult, the_o, PkmX, 
jocewei, s.egerton, krytarowski, mgorny, riccibruno, eraman, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGen/pr41027.c
Removed:
cfe/trunk/test/Sema/pr41027.c
Modified:
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/Sema/inline-asm-validate-riscv.c
cfe/trunk/test/Sema/inline-asm-validate-x86.c

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=368104=368103=368104=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Tue Aug  6 15:41:22 2019
@@ -1846,11 +1846,9 @@ llvm::Value* CodeGenFunction::EmitAsmInp
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
-
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 }
 
 Expr::EvalResult Result;

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=368104=368103=368104=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Tue Aug  6 15:41:22 2019
@@ -383,25 +383,19 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceL
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-// For compatibility with GCC, we also allow pointers that would be
-// integral constant expressions if they were cast to int.
-llvm::APSInt IntResult;
-if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
- Context))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << IntResult.toString(10) << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
+if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
+  // For compatibility with GCC, we also allow pointers that would be
+  // integral constant expressions if they were cast to int.
+  llvm::APSInt IntResult;
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+   Context))
+if (!Info.isValidAsmImmediate(IntResult))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << IntResult.toString(10)
+   << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
+}
   }
 
 } else {

Added: cfe/trunk/test/CodeGen/pr41027.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pr41027.c?rev=368104=auto
==
--- cfe/trunk/test/CodeGen/pr41027.c (added)
+++ cfe/trunk/test/CodeGen/pr41027.c Tue Aug  6 15:41:22 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -O2 -o - %s | FileCheck %s
+
+// CHECK-LABEL: f:
+// CHECK: movl $1, %eax
+// CHECK-NEXT:#APP
+// CHECK-NEXT:outl %eax, $1
+// CHECK-NEXT:#NO_APP
+

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

Looking at the similar situation of `unroll(enable)`/`unroll_count(4)`, 
`unroll_count` also does not set `llvm.loop.unroll.enable`, but it is handled 
by the LoopUnroll pass itself:

  bool ExplicitUnroll = PragmaCount > 0 || PragmaFullUnroll ||
PragmaEnableUnroll || UserUnrollCount;

(LoopUnrollPass.cpp line 770f)

I do not know whether/how "setting a transformation option implicitly enables 
the transformation" should be implemented, maybe we should discuss this. It is 
currently inconsistent. Also consider that non-Clang frontends and .ll files in 
the wild might also expect a specific behavior.


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

https://reviews.llvm.org/D65776



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

[PATCH] D65549: [Sema] Prevent -Wunused-lambda-capture from generating false positive warnings on templated member function

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks for the patch @ziangwan !




Comment at: clang/lib/Sema/SemaExpr.cpp:5631
+  }
+}
+

LLVM style is not to use `{}` for single statement bodies.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65549



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

[PATCH] D65828: [clang-tidy] Add check to linuxkernel for unbalanced irq calls

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/IrqUnbalancedCheck.cpp:13
+
+std::string annotation = "ignore_irq_balancing";
+

Should be static and may be StringRef?



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:79
+
+  Checks Linux kernel for dangerous uses of ``local_irq_disable`` and 
``local_irq_enable``
+

Please add dot at the end. Probably// Linux kernel// is redundant because of 
check name.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87
 
 
+

Please remove two empty lines.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/linuxkernel-irq-unbalanced.rst:6
+
+Checks for calls to ``local_irq_disable`` without matching calls to 
``local_irq_enable``
+and vice-versa. In most cases these functions must be called in pairs to avoid 
indefinite

Please make first statement same as In Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65828



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

[PATCH] D65770: hwasan: Instrument globals.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368102: hwasan: Instrument globals. (authored by pcc, 
committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D65770?vs=213477=213732#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65770

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/Driver/fsanitize.c
  compiler-rt/trunk/lib/hwasan/hwasan.cpp
  compiler-rt/trunk/lib/hwasan/hwasan.h
  compiler-rt/trunk/lib/hwasan/hwasan_interface_internal.h
  compiler-rt/trunk/lib/hwasan/hwasan_report.cpp
  compiler-rt/trunk/test/hwasan/TestCases/global.c
  compiler-rt/trunk/test/hwasan/lit.cfg.py
  llvm/trunk/include/llvm/BinaryFormat/ELF.h
  llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/trunk/test/Instrumentation/HWAddressSanitizer/globals.ll

Index: llvm/trunk/test/Instrumentation/HWAddressSanitizer/globals.ll
===
--- llvm/trunk/test/Instrumentation/HWAddressSanitizer/globals.ll
+++ llvm/trunk/test/Instrumentation/HWAddressSanitizer/globals.ll
@@ -0,0 +1,37 @@
+; RUN: opt < %s -S -hwasan -mtriple=aarch64--linux-android29 | FileCheck --check-prefix=CHECK29 %s
+; RUN: opt < %s -S -hwasan -mtriple=aarch64--linux-android30 | FileCheck --check-prefix=CHECK30 %s
+
+; CHECK29-NOT: @hwasan.note
+; CHECK29: @four = global
+
+; CHECK30: @__start_hwasan_globals = external hidden constant [0 x i8]
+; CHECK30: @__stop_hwasan_globals = external hidden constant [0 x i8]
+
+; CHECK30: @hwasan.note = private constant { i32, i32, i32, [8 x i8], i32, i32 } { i32 8, i32 8, i32 3, [8 x i8] c"LLVM\00\00\00\00", i32 trunc (i64 sub (i64 ptrtoint ([0 x i8]* @__start_hwasan_globals to i64), i64 ptrtoint ({ i32, i32, i32, [8 x i8], i32, i32 }* @hwasan.note to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint ([0 x i8]* @__stop_hwasan_globals to i64), i64 ptrtoint ({ i32, i32, i32, [8 x i8], i32, i32 }* @hwasan.note to i64)) to i32) }, section ".note.hwasan.globals", comdat, align 4
+
+; CHECK30: @hwasan.dummy.global = private constant [0 x i8] zeroinitializer, section "hwasan_globals", comdat($hwasan.note), !associated [[NOTE:![0-9]+]]
+
+; CHECK30: @four.hwasan = private global { i32, [12 x i8] } { i32 1, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\AC" }, align 16
+; CHECK30: @four.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint ({ i32, [12 x i8] }* @four.hwasan to i64), i64 ptrtoint ({ i32, i32 }* @four.hwasan.descriptor to i64)) to i32), i32 -1409286140 }, section "hwasan_globals", !associated [[FOUR:![0-9]+]]
+
+; CHECK30: @sixteen.hwasan = private global [16 x i8] zeroinitializer, align 16
+; CHECK30: @sixteen.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint ([16 x i8]* @sixteen.hwasan to i64), i64 ptrtoint ({ i32, i32 }* @sixteen.hwasan.descriptor to i64)) to i32), i32 -1392508912 }, section "hwasan_globals", !associated [[SIXTEEN:![0-9]+]]
+
+; CHECK30: @huge.hwasan = private global [16777232 x i8] zeroinitializer, align 16
+; CHECK30: @huge.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint ([16777232 x i8]* @huge.hwasan to i64), i64 ptrtoint ({ i32, i32 }* @huge.hwasan.descriptor to i64)) to i32), i32 -1358954512 }, section "hwasan_globals", !associated [[HUGE:![0-9]+]]
+; CHECK30: @huge.hwasan.descriptor.1 = private constant { i32, i32 } { i32 trunc (i64 add (i64 sub (i64 ptrtoint ([16777232 x i8]* @huge.hwasan to i64), i64 ptrtoint ({ i32, i32 }* @huge.hwasan.descriptor.1 to i64)), i64 16777200) to i32), i32 -1375731680 }, section "hwasan_globals", !associated [[HUGE]]
+
+; CHECK30: @four = alias i32, inttoptr (i64 add (i64 ptrtoint ({ i32, [12 x i8] }* @four.hwasan to i64), i64 -6052837899185946624) to i32*)
+; CHECK30: @sixteen = alias [16 x i8], inttoptr (i64 add (i64 ptrtoint ([16 x i8]* @sixteen.hwasan to i64), i64 -5980780305148018688) to [16 x i8]*)
+; CHECK30: @huge = alias [16777232 x i8], inttoptr (i64 add (i64 ptrtoint ([16777232 x i8]* @huge.hwasan to i64), i64 -590872270090752) to [16777232 x i8]*)
+
+; CHECK30: [[NOTE]] = !}} i32, i32, i32, [8 x i8], i32, i32 }* @hwasan.note}
+; CHECK30: [[FOUR]] = !}} i32, [12 x i8] }* @four.hwasan}
+; CHECK30: [[SIXTEEN]] = !{[16 x i8]* @sixteen.hwasan}
+; CHECK30: [[HUGE]] = !{[16777232 x i8]* @huge.hwasan}
+
+source_filename = "foo"
+
+@four = global i32 1
+@sixteen = global [16 x i8] zeroinitializer
+@huge = global [16777232 x i8] zeroinitializer
Index: llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include 

r368102 - hwasan: Instrument globals.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Tue Aug  6 15:07:29 2019
New Revision: 368102

URL: http://llvm.org/viewvc/llvm-project?rev=368102=rev
Log:
hwasan: Instrument globals.

Globals are instrumented by adding a pointer tag to their symbol values
and emitting metadata into a special section that allows the runtime to tag
their memory when the library is loaded.

Due to order of initialization issues explained in more detail in the comments,
shadow initialization cannot happen during regular global initialization.
Instead, the location of the global section is marked using an ELF note,
and we require libc support for calling a function provided by the HWASAN
runtime when libraries are loaded and unloaded.

Based on ideas discussed with @evgeny777 in D56672.

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

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=368102=368101=368102=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Tue Aug  6 15:07:29 2019
@@ -1013,6 +1013,11 @@ void SanitizerArgs::addArgs(const ToolCh
 CmdArgs.push_back(Args.MakeArgString("hwasan-abi=" + HwasanAbi));
   }
 
+  if (Sanitizers.has(SanitizerKind::HWAddress)) {
+CmdArgs.push_back("-target-feature");
+CmdArgs.push_back("+tagged-globals");
+  }
+
   // MSan: Workaround for PR16386.
   // ASan: This is mainly to help LSan with cases such as
   // https://github.com/google/sanitizers/issues/373

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=368102=368101=368102=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Aug  6 15:07:29 2019
@@ -843,7 +843,9 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress 
-fsanitize-hwaddress-abi=platform %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-HWASAN-PLATFORM-ABI
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress 
-fsanitize-hwaddress-abi=foo %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-HWASAN-FOO-ABI
 // CHECK-HWASAN-INTERCEPTOR-ABI: "-default-function-attr" 
"hwasan-abi=interceptor"
+// CHECK-HWASAN-INTERCEPTOR-ABI: "-target-feature" "+tagged-globals"
 // CHECK-HWASAN-PLATFORM-ABI: "-default-function-attr" "hwasan-abi=platform"
+// CHECK-HWASAN-PLATFORM-ABI: "-target-feature" "+tagged-globals"
 // CHECK-HWASAN-FOO-ABI: error: invalid value 'foo' in 
'-fsanitize-hwaddress-abi=foo'
 
 // RUN: %clang -target x86_64-linux-gnu 
-fsanitize=address,pointer-compare,pointer-subtract %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-POINTER-ALL


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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

I was about to suggest something similar. SGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks so much for this patch! I look forward to support for `__gnu_mcount` for 
the arm32 Linux kernel.

> What a horrible function. AAPCS? Who cares about that?

haha




Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1944
+
+  // Replace the pseudo instruction with a call instruction
+  MachineInstrBuilder MIB = BuildMI(MBB, MBBI, MI.getDebugLoc(),

Here down seems to match the previous case? Maybe you could "Dont Repeat 
Yourself (DRY)" up the code by creating a shared function?



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1950
+MIB.add(MO);
+  }
+  MI.eraseFromParent();

No need for `{}` for single statement blocks.



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:211
 unsigned ARMMoveToIntReg(MVT VT, unsigned SrcReg);
-unsigned ARMSelectCallOp(bool UseReg);
+unsigned ARMSelectCallOp(bool UseReg, bool PushLR = false);
 unsigned ARMLowerPICELF(const GlobalValue *GV, unsigned Align, MVT VT);

How many other call sites would have to be updated if this was not a default 
parameter?



Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2417
   // Issue the call.
-  unsigned CallOpc = ARMSelectCallOp(UseReg);
+  unsigned CallOpc = ARMSelectCallOp(UseReg, Callee && Callee->getName() == 
"\01__gnu_mcount_nc");
   MachineInstrBuilder MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt,

efriedma wrote:
> 80 cols
also, what's up with `\01`?



Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:2298
+}
+  }
+

Ditto on single statement bodies. `{}`



Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:5
+define dso_local i32 @foo(i64) local_unnamed_addr #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NOT:stmdb   sp!, {lr}

Don't we want to check that these occur in a parent function before a call to a 
child function?



Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:6
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NOT:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc

why does the `-NOT` case duplicate the non-`NOT` case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

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

In D65829#1617850 , @lhames wrote:

> I think the right line is:
>
>   logAllUnhandledErrors(DW.takeError(), errs(), "");
>   
>
> It's a bit wordy, but will log a sensible error and works with and without 
> ENABLE_ABI_BREAKING_CHECKS enabled.
>
> I'm not against improving the output for cantFail, nor totally against 
> abusing cantFail in unit tests, but Puyan's right that it would be abuse: in 
> production code cantFail should only be used at call sites where you know 
> from context that the call //definitely// can't fail (e.g. subsequent calls 
> to an API that can only fail on the first call).


I am happy with this. Thanks @lhames !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@lhames - I like the `logAllUnhandledErrors`!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 213725.
paulkirth added a comment.

Use existing LLVM code for mapping case literals to their case arms.

This update refactors a great deal of the implementation and test code.

1. Removes the CaseMap data structure completely. LLVM already maintains a 
mapping of the constants to their case arm, so there is no reason to duplicate 
that logic. This also minimizes the changes impacting existing Clang/LLVM 
components.
2. Cleans up debug printing. Without the CaseMap printing debug output can be 
simplified.
3. Minimizes the code in the end-to-end tests & test profiles.
4. Improves formatting, white space, and comments.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65300

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/MisExpect.cpp
  clang/lib/CodeGen/MisExpect.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-no-warning-without-flag.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c

Index: clang/test/Profile/misexpect-switch.c
===
--- /dev/null
+++ clang/test/Profile/misexpect-switch.c
@@ -0,0 +1,41 @@
+// Test that misexpect detects mis-annotated switch statements
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+void init_arry();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+int main() {
+  init_arry();
+  int val = 0;
+
+  int j, k;
+  for (j = 0; j < outer_loop; ++j) {
+for (k = 0; k < inner_loop; ++k) {
+  unsigned condition = rand() % 1;
+  switch (__builtin_expect(condition, 0)) { // expected-warning-re {{Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.+}}% of profiled executions.}}
+  case 0:
+val += sum(arry, arry_size);
+break;
+  case 1:
+  case 2:
+  case 3:
+break;
+  default:
+val += random_sample(arry, arry_size);
+break;
+  } // end switch
+}   // end inner_loop
+  } // end outer_loop
+
+  return 0;
+}
Index: clang/test/Profile/misexpect-switch-only-default-case.c
===
--- /dev/null
+++ clang/test/Profile/misexpect-switch-only-default-case.c
@@ -0,0 +1,35 @@
+// Test that misexpect emits no warning when there is only one switch case
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch-default-only.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+// expected-no-diagnostics
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+void init_arry();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+int main() {
+  init_arry();
+  int val = 0;
+
+  int j, k;
+  for (j = 0; j < outer_loop; ++j) {
+for (k = 0; k < inner_loop; ++k) {
+  unsigned condition = rand() % 1;
+  switch (__builtin_expect(condition, 0)) {
+  default:
+val += random_sample(arry, arry_size);
+break;
+  }; // end switch
+}// end inner_loop
+  }  // end outer_loop
+
+  return 0;
+}
Index: clang/test/Profile/misexpect-switch-nonconst.c
===
--- /dev/null
+++ clang/test/Profile/misexpect-switch-nonconst.c
@@ -0,0 +1,43 @@
+// Test that misexpect emits no warning when switch condition is non-const
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+// expected-no-diagnostics
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+void init_arry();
+
+const int 

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Lang Hames via Phabricator via cfe-commits]

lhames added a comment.

I think the right line is:

  logAllUnhandledErrors(DW.takeError(), errs(), "");

It's a bit wordy, but will log a sensible error and works with and without 
ENABLE_ABI_BREAKING_CHECKS enabled.

I'm not against improving the output for cantFail, nor totally against abusing 
cantFail in unit tests, but Puyan's right that it would be abuse: in production 
code cantFail should only be used at call sites where you know from context 
that the call //definitely// can't fail (e.g. subsequent calls to an API that 
can only fail on the first call).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Jian Cai via Phabricator via cfe-commits]

jcai19 added a comment.

In D65019#1615898 , @efriedma wrote:

> > introduce quite some code to the target-independent part of SelectionDAG, 
> > specifically SelectionDAGBuilder and SelectionDAG classes
>
> Not sure why you think this would be necessary. Every target has 
> target-specific intrinsics, and we have infrastructure to ensure they don't 
> require any changes to target-independent code.


I guess what I was trying to say was that I could not find a way to handle the 
new ARM intrinsic in SelectionDAGBuilder::visitTargetIntrinsic easily with the 
existing code so I had to introduce a case just for the ARM intrinsic in 
SelectionDAGBuilder::visitIntrinsic, and I am not sure if that is acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65684: [WebAssembly] Lower ASan constructor priority on Emscripten

[Guanzhong Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368101: [WebAssembly] Lower ASan constructor priority on 
Emscripten (authored by quantum, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D65684?vs=213720=213730#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65684

Files:
  cfe/trunk/test/CodeGen/asan-constructor.c
  llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp


Index: cfe/trunk/test/CodeGen/asan-constructor.c
===
--- cfe/trunk/test/CodeGen/asan-constructor.c
+++ cfe/trunk/test/CodeGen/asan-constructor.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple wasm32-unknown-emscripten -fsanitize=address 
-emit-llvm -o - %s | FileCheck %s --check-prefix=EMSCRIPTEN
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 1, void ()* @asan.module_ctor, i8* null }]
+// EMSCRIPTEN: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* 
}] [{ i32, void ()*, i8* } { i32 50, void ()* @asan.module_ctor, i8* null }]
Index: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -129,6 +129,8 @@
 static const char *const kAsanModuleCtorName = "asan.module_ctor";
 static const char *const kAsanModuleDtorName = "asan.module_dtor";
 static const uint64_t kAsanCtorAndDtorPriority = 1;
+// On Emscripten, the system needs more than one priorities for constructors.
+static const uint64_t kAsanEmscriptenCtorAndDtorPriority = 50;
 static const char *const kAsanReportErrorTemplate = "__asan_report_";
 static const char *const kAsanRegisterGlobalsName = "__asan_register_globals";
 static const char *const kAsanUnregisterGlobalsName =
@@ -530,6 +532,14 @@
   return std::max(32U, 1U << MappingScale);
 }
 
+static uint64_t GetCtorAndDtorPriority(Triple ) {
+  if (TargetTriple.isOSEmscripten()) {
+return kAsanEmscriptenCtorAndDtorPriority;
+  } else {
+return kAsanCtorAndDtorPriority;
+  }
+}
+
 namespace {
 
 /// Module analysis for getting various metadata about the module.
@@ -1777,7 +1787,8 @@
   if (F->getName() == kAsanModuleCtorName) continue;
   ConstantInt *Priority = dyn_cast(CS->getOperand(0));
   // Don't instrument CTORs that will run before asan.module_ctor.
-  if (Priority->getLimitedValue() <= kAsanCtorAndDtorPriority) continue;
+  if (Priority->getLimitedValue() <= GetCtorAndDtorPriority(TargetTriple))
+continue;
   poisonOneInitializer(*F, ModuleName);
 }
   }
@@ -2429,22 +2440,22 @@
 Changed |= InstrumentGlobals(IRB, M, );
   }
 
+  const uint64_t Priority = GetCtorAndDtorPriority(TargetTriple);
+
   // Put the constructor and destructor in comdat if both
   // (1) global instrumentation is not TU-specific
   // (2) target is ELF.
   if (UseCtorComdat && TargetTriple.isOSBinFormatELF() && CtorComdat) {
 AsanCtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleCtorName));
-appendToGlobalCtors(M, AsanCtorFunction, kAsanCtorAndDtorPriority,
-AsanCtorFunction);
+appendToGlobalCtors(M, AsanCtorFunction, Priority, AsanCtorFunction);
 if (AsanDtorFunction) {
   AsanDtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleDtorName));
-  appendToGlobalDtors(M, AsanDtorFunction, kAsanCtorAndDtorPriority,
-  AsanDtorFunction);
+  appendToGlobalDtors(M, AsanDtorFunction, Priority, AsanDtorFunction);
 }
   } else {
-appendToGlobalCtors(M, AsanCtorFunction, kAsanCtorAndDtorPriority);
+appendToGlobalCtors(M, AsanCtorFunction, Priority);
 if (AsanDtorFunction)
-  appendToGlobalDtors(M, AsanDtorFunction, kAsanCtorAndDtorPriority);
+  appendToGlobalDtors(M, AsanDtorFunction, Priority);
   }
 
   return Changed;


Index: cfe/trunk/test/CodeGen/asan-constructor.c
===
--- cfe/trunk/test/CodeGen/asan-constructor.c
+++ cfe/trunk/test/CodeGen/asan-constructor.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple wasm32-unknown-emscripten -fsanitize=address -emit-llvm -o - %s | FileCheck %s --check-prefix=EMSCRIPTEN
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 1, void ()* @asan.module_ctor, i8* null }]
+// EMSCRIPTEN: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 50, void ()* @asan.module_ctor, i8* null }]
Index: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp

r368101 - [WebAssembly] Lower ASan constructor priority on Emscripten

[Guanzhong Chen via cfe-commits]

Author: quantum
Date: Tue Aug  6 14:52:58 2019
New Revision: 368101

URL: http://llvm.org/viewvc/llvm-project?rev=368101=rev
Log:
[WebAssembly] Lower ASan constructor priority on Emscripten

Summary:
This change gives Emscripten the ability to use more than one constructor
priorities that runs before ASan. By convention, constructor priorites 0-100
are reserved for use by the system. ASan on Emscripten now uses priority 50,
leaving plenty of room for use by Emscripten before and after ASan.

This change is done in response to:
https://github.com/emscripten-core/emscripten/pull/9076#discussion_r310323723

Reviewers: kripken, tlively, aheejin

Reviewed By: tlively

Subscribers: cfe-commits, dschuff, sbc100, jgravelle-google, hiraditya, 
sunfish, llvm-commits

Tags: #llvm, #clang

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

Added:
cfe/trunk/test/CodeGen/asan-constructor.c

Added: cfe/trunk/test/CodeGen/asan-constructor.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/asan-constructor.c?rev=368101=auto
==
--- cfe/trunk/test/CodeGen/asan-constructor.c (added)
+++ cfe/trunk/test/CodeGen/asan-constructor.c Tue Aug  6 14:52:58 2019
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple wasm32-unknown-emscripten -fsanitize=address 
-emit-llvm -o - %s | FileCheck %s --check-prefix=EMSCRIPTEN
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 1, void ()* @asan.module_ctor, i8* null }]
+// EMSCRIPTEN: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* 
}] [{ i32, void ()*, i8* } { i32 50, void ()* @asan.module_ctor, i8* null }]


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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 213728.
jcai19 added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Introduce a new ARM intrinsic for __gnu_mcount_nc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019

Files:
  clang/lib/Basic/Targets/ARM.cpp
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.h
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
  llvm/test/CodeGen/ARM/gnu_mcount_nc.ll

Index: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
@@ -0,0 +1,16 @@
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-THUMB
+
+define dso_local i32 @foo(i64) local_unnamed_addr #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NOT:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NOT:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+  %2 = mul nsw i64 %0, %0
+  %3 = trunc i64 %2 to i32
+  ret i32 %3
+}
+
+attributes #0 = { nofree nounwind "instrument-function-entry-inlined"="llvm.arm.gnu.eabi.mcount" }
Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -24,7 +24,7 @@
 
   if (Func == "mcount" ||
   Func == ".mcount" ||
-  Func == "\01__gnu_mcount_nc" ||
+  Func == "llvm.arm.gnu.eabi.mcount" ||
   Func == "\01_mcount" ||
   Func == "\01mcount" ||
   Func == "__mcount" ||
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -565,6 +565,13 @@
   4, IIC_Br,
   [(ARMcall_nolink tGPR:$func)]>,
 Requires<[IsThumb, IsThumb1Only]>, Sched<[WriteBr]>;
+
+  // Also used for Thumb2
+  // push lr before the call
+  def tBL_PUSHLR : tPseudoInst<(outs), (ins pred:$p, thumb_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr tglobaladdr:$func)]>,
+ Requires<[IsThumb]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1, isBarrier = 1 in {
Index: llvm/lib/Target/ARM/ARMInstrInfo.td
===
--- llvm/lib/Target/ARM/ARMInstrInfo.td
+++ llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -148,6 +148,9 @@
 def ARMcall_nolink   : SDNode<"ARMISD::CALL_NOLINK", SDT_ARMcall,
   [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
SDNPVariadic]>;
+def ARMcall_pushlr : SDNode<"ARMISD::CALL_PUSHLR", SDT_ARMcall,
+  [SDNPHasChain, SDNPOptInGlue, SDNPOutGlue,
+   SDNPVariadic]>;
 
 def ARMretflag   : SDNode<"ARMISD::RET_FLAG", SDTNone,
   [SDNPHasChain, SDNPOptInGlue, SDNPVariadic]>;
@@ -2350,6 +2353,12 @@
   def BMOVPCB_CALL : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
8, IIC_Br, [(ARMcall_nolink tglobaladdr:$func)]>,
   Requires<[IsARM]>, Sched<[WriteBr]>;
+
+  // push lr before the call
+  def BL_PUSHLR : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
+  4, IIC_Br,
+  [(ARMcall_pushlr tglobaladdr:$func)]>,
+ Requires<[IsARM]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1 in {
Index: llvm/lib/Target/ARM/ARMISelLowering.h
===
--- llvm/lib/Target/ARM/ARMISelLowering.h
+++ llvm/lib/Target/ARM/ARMISelLowering.h
@@ -68,6 +68,7 @@
   CALL, // Function call.
   CALL_PRED,// Function call that's predicable.
   CALL_NOLINK,  // Function call with branch not branch-and-link.
+  CALL_PUSHLR,  // Function call that pushes LR before the call.
   BRCOND,   // Conditional branch.
   BR_JT,// Jumptable branch.
   BR2_JT,   // Jumptable branch (2 level - jumptable entry is a jump).
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -1421,6 +1421,7 @@
   case ARMISD::CALL:  return "ARMISD::CALL";
   case 

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65203



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

@gribozavr I think that this usage here is actually useful because it
a) tests the actual behaviour
b) provides example code for other users

The check here ensures that the rest of the code is properly executed, *but* 
because the error is not actually consumed (you need to explicitly invoke 
`.takeError()`), `llvm::Expected` will actually tell you that a fatal error has 
occurred at this point and what the error was.  It provides a much clearer 
error message than the explicit `llvm_unreachable` IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

plotfi wrote:
> gribozavr wrote:
> > plotfi wrote:
> > > gribozavr wrote:
> > > > plotfi wrote:
> > > > > gribozavr wrote:
> > > > > > Call `llvm::cantFail` and there will be no need to explain anything.
> > > > > I explained this in the other patch: 
> > > > > 
> > > > > cantFail is arguably worse. I want this test to tell me the cause of 
> > > > > the crash (in this case, exceeding the inotify limit). What I want to 
> > > > > happen is:
> > > > > 
> > > > > ```
> > > > > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > > > > [==] Running 1 test from 1 test case.
> > > > > [--] Global test environment set-up.
> > > > > [--] 1 test from DirectoryWatcherTest
> > > > > [ RUN  ] DirectoryWatcherTest.AddFiles
> > > > > Expected must be checked before access or destruction.
> > > > > Unchecked Expected contained error:
> > > > > inotify_init1() error: out of inotify entries #0 0x00246b8f 
> > > > > llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> > > > >  #1 0x002450d2 llvm::sys::RunSignalHandlers() 
> > > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > > > >  #2 0x00247268 SignalHandler(int) 
> > > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> > > > >  #3 0x7f17924eb890 __restore_rt 
> > > > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> > > > >  #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
> > > > > ...
> > > > > ```
> > > > > 
> > > > > Wrapping the llvm::Expected in a cantFail hides this message, and 
> > > > > instead you get the much less useful:
> > > > > 
> > > > > ```
> > > > > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > > > > [==] Running 1 test from 1 test case. 
> > > > >   
> > > > >   
> > > > >
> > > > > [--] Global test environment set-up.
> > > > > [--] 1 test from DirectoryWatcherTest
> > > > > [ RUN  ] DirectoryWatcherTest.AddFiles
> > > > > Failure value returned from cantFail wrapped call
> > > > > UNREACHABLE executed at 
> > > > > /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
> > > > >  #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> > > > >  #1 0x00245352 llvm::sys::RunSignalHandlers() 
> > > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > > > >  #2 0x002474e8 SignalHandler(int) 
> > > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> > > > >  #3 0x7f20c64a8890 __restore_rt 
> > > > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> > > > >  #4 0x7f20c4f51e97 gsignal 
> > > > > /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
> > > > >  #5 0x7f20c4f53801 abort 
> > > > > /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
> > > > >  #6 0x00232b31 
> > > > > (build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
> > > > > ```
> > > > > 
> > > > > I specifically _want_ llvm::Expected to propagate the error up.
> > > > > I can drop the comments, because it should be fairly obvious here 
> > > > > that DW is an llvm::Expected since I got rid of the autos.
> > > > > 
> > > > > To be clear, wrapping llvm::Expected in cantFail goes against the 
> > > > > entire point of why I added this more robust error handling.
> > > > > If someone's machine is failing on this test because they've exceeded 
> > > > > their inotify limit, I would really like the message produced from 
> > > > > strerror(errno) to bubble up to the top so that it is obvious to them 
> > > > > that they need to either kill some tasks on their machine or up their 
> > > > > inotify limit. 
> > > > The automated checking in llvm::Expected is not guaranteed to happen 
> > > > (it is behind LLVM_ENABLE_ABI_BREAKING_CHECKS). If you are not happy 
> > > > with the error from `cantFail` I think everyone would appreciate an 
> > > > improvement there.
> > > > 
> > > > > I specifically _want_ llvm::Expected to propagate the error up.
> > > > 
> > > > I don't understand. It is not propagating anything up, it is just 
> > > > crashing in the destructor.
> > > Also, I have to point out here: cantFail is exactly the wrong tool here 
> > > because this code can in fact fail if your system has exceeded its 
> > > inotify limits. 
> > For the purposes of the test, the call can't fail. If the call fails, the 
> > test 

r368092 - fix clang-scan-deps test to match filepaths on Windows

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Aug  6 14:45:43 2019
New Revision: 368092

URL: http://llvm.org/viewvc/llvm-project?rev=368092=rev
Log:
fix clang-scan-deps test to match filepaths on Windows

Modified:
cfe/trunk/test/ClangScanDeps/header_stat_before_open.m

Modified: cfe/trunk/test/ClangScanDeps/header_stat_before_open.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/header_stat_before_open.m?rev=368092=368091=368092=diff
==
--- cfe/trunk/test/ClangScanDeps/header_stat_before_open.m (original)
+++ cfe/trunk/test/ClangScanDeps/header_stat_before_open.m Tue Aug  6 14:45:43 
2019
@@ -14,5 +14,5 @@
 
 // CHECK: clang-scan-deps dependency
 // CHECK-NEXT: header_stat_before_open.m
-// CHECK-NEXT: Inputs/frameworks/Framework.framework/Headers/Framework.h
-// CHECK-NEXT: 
Inputs/frameworks/Framework.framework/PrivateHeaders/PrivateHeader.h
+// CHECK-NEXT: 
Inputs{{/|\\}}frameworks{{/|\\}}Framework.framework{{/|\\}}Headers{{/|\\}}Framework.h
+// CHECK-NEXT: 
Inputs{{/|\\}}frameworks{{/|\\}}Framework.framework{{/|\\}}PrivateHeaders{{/|\\}}PrivateHeader.h


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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

gribozavr wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > plotfi wrote:
> > > > gribozavr wrote:
> > > > > Call `llvm::cantFail` and there will be no need to explain anything.
> > > > I explained this in the other patch: 
> > > > 
> > > > cantFail is arguably worse. I want this test to tell me the cause of 
> > > > the crash (in this case, exceeding the inotify limit). What I want to 
> > > > happen is:
> > > > 
> > > > ```
> > > > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > > > [==] Running 1 test from 1 test case.
> > > > [--] Global test environment set-up.
> > > > [--] 1 test from DirectoryWatcherTest
> > > > [ RUN  ] DirectoryWatcherTest.AddFiles
> > > > Expected must be checked before access or destruction.
> > > > Unchecked Expected contained error:
> > > > inotify_init1() error: out of inotify entries #0 0x00246b8f 
> > > > llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> > > >  #1 0x002450d2 llvm::sys::RunSignalHandlers() 
> > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > > >  #2 0x00247268 SignalHandler(int) 
> > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> > > >  #3 0x7f17924eb890 __restore_rt 
> > > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> > > >  #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
> > > > ...
> > > > ```
> > > > 
> > > > Wrapping the llvm::Expected in a cantFail hides this message, and 
> > > > instead you get the much less useful:
> > > > 
> > > > ```
> > > > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > > > [==] Running 1 test from 1 test case.   
> > > > 
> > > > 
> > > >  
> > > > [--] Global test environment set-up.
> > > > [--] 1 test from DirectoryWatcherTest
> > > > [ RUN  ] DirectoryWatcherTest.AddFiles
> > > > Failure value returned from cantFail wrapped call
> > > > UNREACHABLE executed at 
> > > > /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
> > > >  #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> > > >  #1 0x00245352 llvm::sys::RunSignalHandlers() 
> > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > > >  #2 0x002474e8 SignalHandler(int) 
> > > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> > > >  #3 0x7f20c64a8890 __restore_rt 
> > > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> > > >  #4 0x7f20c4f51e97 gsignal 
> > > > /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
> > > >  #5 0x7f20c4f53801 abort 
> > > > /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
> > > >  #6 0x00232b31 
> > > > (build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
> > > > ```
> > > > 
> > > > I specifically _want_ llvm::Expected to propagate the error up.
> > > > I can drop the comments, because it should be fairly obvious here that 
> > > > DW is an llvm::Expected since I got rid of the autos.
> > > > 
> > > > To be clear, wrapping llvm::Expected in cantFail goes against the 
> > > > entire point of why I added this more robust error handling.
> > > > If someone's machine is failing on this test because they've exceeded 
> > > > their inotify limit, I would really like the message produced from 
> > > > strerror(errno) to bubble up to the top so that it is obvious to them 
> > > > that they need to either kill some tasks on their machine or up their 
> > > > inotify limit. 
> > > The automated checking in llvm::Expected is not guaranteed to happen (it 
> > > is behind LLVM_ENABLE_ABI_BREAKING_CHECKS). If you are not happy with the 
> > > error from `cantFail` I think everyone would appreciate an improvement 
> > > there.
> > > 
> > > > I specifically _want_ llvm::Expected to propagate the error up.
> > > 
> > > I don't understand. It is not propagating anything up, it is just 
> > > crashing in the destructor.
> > Also, I have to point out here: cantFail is exactly the wrong tool here 
> > because this code can in fact fail if your system has exceeded its inotify 
> > limits. 
> For the purposes of the test, the call can't fail. If the call fails, the 
> test fails.
> 
> How about `ASSERT_TRUE(DW)`?
I meant, it forces the code to either have to takeError and propagate the error 
up or it will crash in the 

[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D65819#1617736 , @Hahnfeld wrote:

> Can you detail what "incorrect linking" means? AFAIK the additional sections 
> were just bloating the executable, but how do they affect correctness?
>
> Will this patch change the ability to consume a bundled object file without 
> calling the unbundler? Using known ELF tools on the produced object files was 
> an important design decision and IIRC was somewhat important for using build 
> systems that are unaware of the bundled format.


It might produce a lot of "junk" sections and I saw that in some cases it may 
affect the code linking. There is a report from Itaru Kitayama and I found out 
that this oartial linking leads to the incorrectly working application. Seems 
to me, there is a problem with partial linking in LD in some cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65819



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

plotfi wrote:
> gribozavr wrote:
> > plotfi wrote:
> > > gribozavr wrote:
> > > > Call `llvm::cantFail` and there will be no need to explain anything.
> > > I explained this in the other patch: 
> > > 
> > > cantFail is arguably worse. I want this test to tell me the cause of the 
> > > crash (in this case, exceeding the inotify limit). What I want to happen 
> > > is:
> > > 
> > > ```
> > > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > > [==] Running 1 test from 1 test case.
> > > [--] Global test environment set-up.
> > > [--] 1 test from DirectoryWatcherTest
> > > [ RUN  ] DirectoryWatcherTest.AddFiles
> > > Expected must be checked before access or destruction.
> > > Unchecked Expected contained error:
> > > inotify_init1() error: out of inotify entries #0 0x00246b8f 
> > > llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> > >  #1 0x002450d2 llvm::sys::RunSignalHandlers() 
> > > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > >  #2 0x00247268 SignalHandler(int) 
> > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> > >  #3 0x7f17924eb890 __restore_rt 
> > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> > >  #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
> > > ...
> > > ```
> > > 
> > > Wrapping the llvm::Expected in a cantFail hides this message, and instead 
> > > you get the much less useful:
> > > 
> > > ```
> > > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > > [==] Running 1 test from 1 test case. 
> > >   
> > >   
> > >
> > > [--] Global test environment set-up.
> > > [--] 1 test from DirectoryWatcherTest
> > > [ RUN  ] DirectoryWatcherTest.AddFiles
> > > Failure value returned from cantFail wrapped call
> > > UNREACHABLE executed at 
> > > /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
> > >  #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> > >  #1 0x00245352 llvm::sys::RunSignalHandlers() 
> > > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> > >  #2 0x002474e8 SignalHandler(int) 
> > > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> > >  #3 0x7f20c64a8890 __restore_rt 
> > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> > >  #4 0x7f20c4f51e97 gsignal 
> > > /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
> > >  #5 0x7f20c4f53801 abort 
> > > /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
> > >  #6 0x00232b31 
> > > (build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
> > > ```
> > > 
> > > I specifically _want_ llvm::Expected to propagate the error up.
> > > I can drop the comments, because it should be fairly obvious here that DW 
> > > is an llvm::Expected since I got rid of the autos.
> > > 
> > > To be clear, wrapping llvm::Expected in cantFail goes against the entire 
> > > point of why I added this more robust error handling.
> > > If someone's machine is failing on this test because they've exceeded 
> > > their inotify limit, I would really like the message produced from 
> > > strerror(errno) to bubble up to the top so that it is obvious to them 
> > > that they need to either kill some tasks on their machine or up their 
> > > inotify limit. 
> > The automated checking in llvm::Expected is not guaranteed to happen (it is 
> > behind LLVM_ENABLE_ABI_BREAKING_CHECKS). If you are not happy with the 
> > error from `cantFail` I think everyone would appreciate an improvement 
> > there.
> > 
> > > I specifically _want_ llvm::Expected to propagate the error up.
> > 
> > I don't understand. It is not propagating anything up, it is just crashing 
> > in the destructor.
> Also, I have to point out here: cantFail is exactly the wrong tool here 
> because this code can in fact fail if your system has exceeded its inotify 
> limits. 
For the purposes of the test, the call can't fail. If the call fails, the test 
fails.

How about `ASSERT_TRUE(DW)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

gribozavr wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > Call `llvm::cantFail` and there will be no need to explain anything.
> > I explained this in the other patch: 
> > 
> > cantFail is arguably worse. I want this test to tell me the cause of the 
> > crash (in this case, exceeding the inotify limit). What I want to happen is:
> > 
> > ```
> > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > [==] Running 1 test from 1 test case.
> > [--] Global test environment set-up.
> > [--] 1 test from DirectoryWatcherTest
> > [ RUN  ] DirectoryWatcherTest.AddFiles
> > Expected must be checked before access or destruction.
> > Unchecked Expected contained error:
> > inotify_init1() error: out of inotify entries #0 0x00246b8f 
> > llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> >  #1 0x002450d2 llvm::sys::RunSignalHandlers() 
> > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> >  #2 0x00247268 SignalHandler(int) 
> > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> >  #3 0x7f17924eb890 __restore_rt 
> > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> >  #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
> > ...
> > ```
> > 
> > Wrapping the llvm::Expected in a cantFail hides this message, and instead 
> > you get the much less useful:
> > 
> > ```
> > Note: Google Test filter = DirectoryWatcherTest.AddFiles
> > [==] Running 1 test from 1 test case.   
> > 
> > 
> >  
> > [--] Global test environment set-up.
> > [--] 1 test from DirectoryWatcherTest
> > [ RUN  ] DirectoryWatcherTest.AddFiles
> > Failure value returned from cantFail wrapped call
> > UNREACHABLE executed at 
> > /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
> >  #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
> >  #1 0x00245352 llvm::sys::RunSignalHandlers() 
> > /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
> >  #2 0x002474e8 SignalHandler(int) 
> > /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
> >  #3 0x7f20c64a8890 __restore_rt 
> > (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
> >  #4 0x7f20c4f51e97 gsignal 
> > /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
> >  #5 0x7f20c4f53801 abort 
> > /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
> >  #6 0x00232b31 
> > (build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
> > ```
> > 
> > I specifically _want_ llvm::Expected to propagate the error up.
> > I can drop the comments, because it should be fairly obvious here that DW 
> > is an llvm::Expected since I got rid of the autos.
> > 
> > To be clear, wrapping llvm::Expected in cantFail goes against the entire 
> > point of why I added this more robust error handling.
> > If someone's machine is failing on this test because they've exceeded their 
> > inotify limit, I would really like the message produced from 
> > strerror(errno) to bubble up to the top so that it is obvious to them that 
> > they need to either kill some tasks on their machine or up their inotify 
> > limit. 
> The automated checking in llvm::Expected is not guaranteed to happen (it is 
> behind LLVM_ENABLE_ABI_BREAKING_CHECKS). If you are not happy with the error 
> from `cantFail` I think everyone would appreciate an improvement there.
> 
> > I specifically _want_ llvm::Expected to propagate the error up.
> 
> I don't understand. It is not propagating anything up, it is just crashing in 
> the destructor.
Also, I have to point out here: cantFail is exactly the wrong tool here because 
this code can in fact fail if your system has exceeded its inotify limits. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

plotfi wrote:
> gribozavr wrote:
> > Call `llvm::cantFail` and there will be no need to explain anything.
> I explained this in the other patch: 
> 
> cantFail is arguably worse. I want this test to tell me the cause of the 
> crash (in this case, exceeding the inotify limit). What I want to happen is:
> 
> ```
> Note: Google Test filter = DirectoryWatcherTest.AddFiles
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DirectoryWatcherTest
> [ RUN  ] DirectoryWatcherTest.AddFiles
> Expected must be checked before access or destruction.
> Unchecked Expected contained error:
> inotify_init1() error: out of inotify entries #0 0x00246b8f 
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
>  #1 0x002450d2 llvm::sys::RunSignalHandlers() 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
>  #2 0x00247268 SignalHandler(int) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
>  #3 0x7f17924eb890 __restore_rt 
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
>  #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
> ...
> ```
> 
> Wrapping the llvm::Expected in a cantFail hides this message, and instead you 
> get the much less useful:
> 
> ```
> Note: Google Test filter = DirectoryWatcherTest.AddFiles
> [==] Running 1 test from 1 test case. 
>   
>   
>
> [--] Global test environment set-up.
> [--] 1 test from DirectoryWatcherTest
> [ RUN  ] DirectoryWatcherTest.AddFiles
> Failure value returned from cantFail wrapped call
> UNREACHABLE executed at 
> /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
>  #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
>  #1 0x00245352 llvm::sys::RunSignalHandlers() 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
>  #2 0x002474e8 SignalHandler(int) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
>  #3 0x7f20c64a8890 __restore_rt 
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
>  #4 0x7f20c4f51e97 gsignal 
> /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
>  #5 0x7f20c4f53801 abort 
> /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
>  #6 0x00232b31 
> (build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
> ```
> 
> I specifically _want_ llvm::Expected to propagate the error up.
> I can drop the comments, because it should be fairly obvious here that DW is 
> an llvm::Expected since I got rid of the autos.
> 
> To be clear, wrapping llvm::Expected in cantFail goes against the entire 
> point of why I added this more robust error handling.
> If someone's machine is failing on this test because they've exceeded their 
> inotify limit, I would really like the message produced from strerror(errno) 
> to bubble up to the top so that it is obvious to them that they need to 
> either kill some tasks on their machine or up their inotify limit. 
The automated checking in llvm::Expected is not guaranteed to happen (it is 
behind LLVM_ENABLE_ABI_BREAKING_CHECKS). If you are not happy with the error 
from `cantFail` I think everyone would appreciate an improvement there.

> I specifically _want_ llvm::Expected to propagate the error up.

I don't understand. It is not propagating anything up, it is just crashing in 
the destructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

gribozavr wrote:
> Call `llvm::cantFail` and there will be no need to explain anything.
I explained this in the other patch: 

cantFail is arguably worse. I want this test to tell me the cause of the crash 
(in this case, exceeding the inotify limit). What I want to happen is:

```
Note: Google Test filter = DirectoryWatcherTest.AddFiles
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from DirectoryWatcherTest
[ RUN  ] DirectoryWatcherTest.AddFiles
Expected must be checked before access or destruction.
Unchecked Expected contained error:
inotify_init1() error: out of inotify entries #0 0x00246b8f 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
 #1 0x002450d2 llvm::sys::RunSignalHandlers() 
/mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x00247268 SignalHandler(int) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
 #3 0x7f17924eb890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
...
```

Wrapping the llvm::Expected in a cantFail hides this message, and instead you 
get the much less useful:

```
Note: Google Test filter = DirectoryWatcherTest.AddFiles
[==] Running 1 test from 1 test case.   

 
[--] Global test environment set-up.
[--] 1 test from DirectoryWatcherTest
[ RUN  ] DirectoryWatcherTest.AddFiles
Failure value returned from cantFail wrapped call
UNREACHABLE executed at 
/mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
 #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
 #1 0x00245352 llvm::sys::RunSignalHandlers() 
/mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x002474e8 SignalHandler(int) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
 #3 0x7f20c64a8890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #4 0x7f20c4f51e97 gsignal 
/build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #5 0x7f20c4f53801 abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
 #6 0x00232b31 
(build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
```

I specifically _want_ llvm::Expected to propagate the error up.
I can drop the comments, because it should be fairly obvious here that DW is an 
llvm::Expected since I got rid of the autos.

To be clear, wrapping llvm::Expected in cantFail goes against the entire point 
of why I added this more robust error handling.
If someone's machine is failing on this test because they've exceeded their 
inotify limit, I would really like the message produced from strerror(errno) to 
bubble up to the top so that it is obvious to them that they need to either 
kill some tasks on their machine or up their inotify limit. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65684: [WebAssembly] Lower ASan constructor priority on Emscripten

[Guanzhong Chen via Phabricator via cfe-commits]

quantum updated this revision to Diff 213720.
quantum added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add constructor priority test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65684

Files:
  clang/test/CodeGen/asan-constructor.c
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -129,6 +129,8 @@
 static const char *const kAsanModuleCtorName = "asan.module_ctor";
 static const char *const kAsanModuleDtorName = "asan.module_dtor";
 static const uint64_t kAsanCtorAndDtorPriority = 1;
+// On Emscripten, the system needs more than one priorities for constructors.
+static const uint64_t kAsanEmscriptenCtorAndDtorPriority = 50;
 static const char *const kAsanReportErrorTemplate = "__asan_report_";
 static const char *const kAsanRegisterGlobalsName = "__asan_register_globals";
 static const char *const kAsanUnregisterGlobalsName =
@@ -530,6 +532,14 @@
   return std::max(32U, 1U << MappingScale);
 }
 
+static uint64_t GetCtorAndDtorPriority(Triple ) {
+  if (TargetTriple.isOSEmscripten()) {
+return kAsanEmscriptenCtorAndDtorPriority;
+  } else {
+return kAsanCtorAndDtorPriority;
+  }
+}
+
 namespace {
 
 /// Module analysis for getting various metadata about the module.
@@ -1777,7 +1787,8 @@
   if (F->getName() == kAsanModuleCtorName) continue;
   ConstantInt *Priority = dyn_cast(CS->getOperand(0));
   // Don't instrument CTORs that will run before asan.module_ctor.
-  if (Priority->getLimitedValue() <= kAsanCtorAndDtorPriority) continue;
+  if (Priority->getLimitedValue() <= GetCtorAndDtorPriority(TargetTriple))
+continue;
   poisonOneInitializer(*F, ModuleName);
 }
   }
@@ -2429,22 +2440,22 @@
 Changed |= InstrumentGlobals(IRB, M, );
   }
 
+  const uint64_t Priority = GetCtorAndDtorPriority(TargetTriple);
+
   // Put the constructor and destructor in comdat if both
   // (1) global instrumentation is not TU-specific
   // (2) target is ELF.
   if (UseCtorComdat && TargetTriple.isOSBinFormatELF() && CtorComdat) {
 AsanCtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleCtorName));
-appendToGlobalCtors(M, AsanCtorFunction, kAsanCtorAndDtorPriority,
-AsanCtorFunction);
+appendToGlobalCtors(M, AsanCtorFunction, Priority, AsanCtorFunction);
 if (AsanDtorFunction) {
   AsanDtorFunction->setComdat(M.getOrInsertComdat(kAsanModuleDtorName));
-  appendToGlobalDtors(M, AsanDtorFunction, kAsanCtorAndDtorPriority,
-  AsanDtorFunction);
+  appendToGlobalDtors(M, AsanDtorFunction, Priority, AsanDtorFunction);
 }
   } else {
-appendToGlobalCtors(M, AsanCtorFunction, kAsanCtorAndDtorPriority);
+appendToGlobalCtors(M, AsanCtorFunction, Priority);
 if (AsanDtorFunction)
-  appendToGlobalDtors(M, AsanDtorFunction, kAsanCtorAndDtorPriority);
+  appendToGlobalDtors(M, AsanDtorFunction, Priority);
   }
 
   return Changed;
Index: clang/test/CodeGen/asan-constructor.c
===
--- /dev/null
+++ clang/test/CodeGen/asan-constructor.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple wasm32-unknown-emscripten -fsanitize=address 
-emit-llvm -o - %s | FileCheck %s --check-prefix=EMSCRIPTEN
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 1, void ()* @asan.module_ctor, i8* null }]
+// EMSCRIPTEN: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* 
}] [{ i32, void ()*, i8* } { i32 50, void ()* @asan.module_ctor, i8* null }]


Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -129,6 +129,8 @@
 static const char *const kAsanModuleCtorName = "asan.module_ctor";
 static const char *const kAsanModuleDtorName = "asan.module_dtor";
 static const uint64_t kAsanCtorAndDtorPriority = 1;
+// On Emscripten, the system needs more than one priorities for constructors.
+static const uint64_t kAsanEmscriptenCtorAndDtorPriority = 50;
 static const char *const kAsanReportErrorTemplate = "__asan_report_";
 static const char *const kAsanRegisterGlobalsName = "__asan_register_globals";
 static const char *const kAsanUnregisterGlobalsName =
@@ -530,6 +532,14 @@
   return std::max(32U, 1U << MappingScale);
 }
 
+static uint64_t GetCtorAndDtorPriority(Triple ) {
+  if (TargetTriple.isOSEmscripten()) {
+return 

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)

Call `llvm::cantFail` and there will be no need to explain anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65829



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@jkorous Can you lemme know if you're happy with 
https://reviews.llvm.org/D65829.
@gribozavr lemme also know any other feedback in D65829 
, as this diff here is already closed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
plotfi added reviewers: jkorous, compnerd, gribozavr.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Also I went ahead and replaced all the instances of "auto DW = 
DirectoryWatcher::create" with 
"llvm::Expected> DW = 
DirectoryWatcher::create(" to make it more clear that DirectoryWatcher::create 
is returning an llvm::Expected (along wit a comment to make it extra clear).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65829

Files:
  clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
  clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
  clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -277,14 +277,17 @@
{EventKind::Modified, "c"}}
   };
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)
+return;
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -309,14 +312,17 @@
{EventKind::Modified, "c"}}
};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/false);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/false);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)
+return;
 
   checkEventualResultWithTimeout(TestConsumer);
 }
@@ -330,14 +336,17 @@
{EventKind::Modified, "b"},
{EventKind::Modified, "c"}}};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)
+return;
 
   fixture.addFile("a");
   fixture.addFile("b");
@@ -356,14 +365,17 @@
   {{EventKind::Modified, "a"}},
   {{EventKind::Modified, "a"}}};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)
+return;
 
   // modify the file
   {
@@ -387,14 +399,17 @@
   {{EventKind::Removed, "a"}},
   {{EventKind::Modified, "a"}, {EventKind::Removed, "a"}}};
 
-  auto DW = DirectoryWatcher::create(
-  fixture.TestWatchedDir,
-  [](llvm::ArrayRef Events,
-  bool IsInitial) {
-TestConsumer.consume(Events, IsInitial);
-  },
-  /*waitForInitialSync=*/true);
-  if (!DW) return;
+  llvm::Expected> DW =
+  DirectoryWatcher::create(
+  fixture.TestWatchedDir,
+  [](llvm::ArrayRef Events,
+  bool IsInitial) {
+TestConsumer.consume(Events, IsInitial);
+  },
+  /*waitForInitialSync=*/true);
+  // llvm::Expected will throw an error if DW is an Error.
+  if (!DW)
+return;
 
   fixture.deleteFile("a");
 
@@ -409,14 +424,17 @@
   {{EventKind::WatchedDirRemoved, ""},
{EventKind::WatcherGotInvalidated, ""}}};
 
-  auto DW = 

[PATCH] D65828: [clang-tidy] Add check to linuxkernel for unbalanced irq calls

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65828

Files:
  clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt
  clang-tools-extra/clang-tidy/linuxkernel/IrqUnbalancedCheck.cpp
  clang-tools-extra/clang-tidy/linuxkernel/IrqUnbalancedCheck.h
  clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/linuxkernel-irq-unbalanced.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/linuxkernel-irq-unbalanced.cpp

Index: clang-tools-extra/test/clang-tidy/linuxkernel-irq-unbalanced.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/linuxkernel-irq-unbalanced.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s linuxkernel-irq-unbalanced %t
+
+extern void arch_local_irq_disable();
+extern void arch_local_irq_enable();
+
+#define raw_local_irq_disable() arch_local_irq_disable()
+#define raw_local_irq_enable() arch_local_irq_enable()
+
+#define local_irq_enable()  \
+  do {  \
+raw_local_irq_enable(); \
+  } while (0)
+#define local_irq_disable()  \
+  do {   \
+raw_local_irq_disable(); \
+  } while (0)
+
+void foo() {
+  local_irq_disable();
+  local_irq_enable();
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: call to 'enable_local_irq' without 'disable_local_irq' in 'bar'  [linuxkernel-irq-unbalanced]
+void bar() {
+  local_irq_enable();
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: call to 'disable_local_irq' without 'enable_local_irq' in 'baz'  [linuxkernel-irq-unbalanced]
+void baz() {
+  local_irq_disable();
+}
+
+void f() {
+  if (1) {
+local_irq_disable();
+local_irq_enable();
+  }
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: call to 'disable_local_irq' without 'enable_local_irq' in 'f2'  [linuxkernel-irq-unbalanced]
+void f2() {
+  if (1) {
+local_irq_disable();
+  }
+}
+
+__attribute__((annotate("ignore_irq_balancing"))) void g() {
+  local_irq_enable();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -269,6 +269,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   linuxkernel-irq-unbalanced
linuxkernel-must-use-errs
llvm-header-guard
llvm-include-order
Index: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-irq-unbalanced.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/linuxkernel-irq-unbalanced.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - linuxkernel-irq-unbalanced
+
+linuxkernel-irq-unbalanced
+==
+
+Checks for calls to ``local_irq_disable`` without matching calls to ``local_irq_enable``
+and vice-versa. In most cases these functions must be called in pairs to avoid indefinite
+hanging in the kernel. Functions marked with ``__attribute__((annotate("ignore_irq_balancing")))``
+are ignored for analysis. This is common when the machine is powering down.
+
+Example:
+
+.. code-block:: c
+
+   void bar() {
+  local_irq_disable();
+  // No call to local_irq_enable();
+   }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,11 @@
   Checks Linux kernel code to see if it uses the results from the functions in
   ``linux/err.h``.
 
+- New :doc:`linuxkernel-irq-unbalanced
+  ` check.
+
+  Checks Linux kernel for dangerous uses of ``local_irq_disable`` and ``local_irq_enable``
+
 - New :doc:`google-upgrade-googletest-case
   ` check.
 
@@ -80,6 +85,7 @@
   replaces them with equivalent APIs with ``suite``.
 
 
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
+++ clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "IrqUnbalancedCheck.h"
 #include "MustCheckErrsCheck.h"
 
 namespace clang {
@@ -19,6 +20,8 @@
 class LinuxKernelModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"linuxkernel-irq-unbalanced");

[PATCH] D65827: [clang-doc] Fix paths of js in import tags

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368087: [clang-doc] Fix paths of js in import tags (authored 
by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65827?vs=213710=213714#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65827

Files:
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp


Index: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
@@ -256,6 +256,8 @@
 auto ScriptNode = llvm::make_unique(HTMLTag::TAG_SCRIPT);
 SmallString<128> ScriptPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(ScriptPath, llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(ScriptPath, llvm::sys::path::Style::posix);
 ScriptNode->Attributes.try_emplace("src", ScriptPath);
 Out.emplace_back(std::move(ScriptNode));
   }


Index: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
@@ -256,6 +256,8 @@
 auto ScriptNode = llvm::make_unique(HTMLTag::TAG_SCRIPT);
 SmallString<128> ScriptPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(ScriptPath, llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(ScriptPath, llvm::sys::path::Style::posix);
 ScriptNode->Attributes.try_emplace("src", ScriptPath);
 Out.emplace_back(std::move(ScriptNode));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65000: [ARM] Set default alignment to 64bits

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D65000#1616368 , @dnsampaio wrote:

> I have tested this in our MacOS and linux environments. @thakis @thegameg 
> @phosek, would it be possible for you to check if this works for you?


I just built trunk with this patch applied and `$ ninja check-clang-codegen` 
seems to pass. Thanks @dnsampaio!


Repository:
  rC Clang

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

https://reviews.llvm.org/D65000



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

[clang-tools-extra] r368087 - [clang-doc] Fix paths of js in import tags

[Diego Astiazaran via cfe-commits]

Author: diegoastiazaran
Date: Tue Aug  6 13:59:14 2019
New Revision: 368087

URL: http://llvm.org/viewvc/llvm-project?rev=368087=rev
Log:
[clang-doc] Fix paths of js in import tags

HTML requires posix-style paths.

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

Modified:
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp

Modified: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp?rev=368087=368086=368087=diff
==
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp Tue Aug  6 13:59:14 2019
@@ -256,6 +256,8 @@ genJsScriptsHTML(StringRef InfoPath, con
 auto ScriptNode = llvm::make_unique(HTMLTag::TAG_SCRIPT);
 SmallString<128> ScriptPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(ScriptPath, llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(ScriptPath, llvm::sys::path::Style::posix);
 ScriptNode->Attributes.try_emplace("src", ScriptPath);
 Out.emplace_back(std::move(ScriptNode));
   }


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

[PATCH] D65827: [clang-doc] Fix paths of js in import tags

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added a reviewer: juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.
juliehockett accepted this revision.
juliehockett added a comment.
This revision is now accepted and ready to land.

LGTM (let's make sure we double check this next time)


HTML requires posix-style paths.


https://reviews.llvm.org/D65827

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp


Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -256,6 +256,8 @@
 auto ScriptNode = llvm::make_unique(HTMLTag::TAG_SCRIPT);
 SmallString<128> ScriptPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(ScriptPath, llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(ScriptPath, llvm::sys::path::Style::posix);
 ScriptNode->Attributes.try_emplace("src", ScriptPath);
 Out.emplace_back(std::move(ScriptNode));
   }


Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -256,6 +256,8 @@
 auto ScriptNode = llvm::make_unique(HTMLTag::TAG_SCRIPT);
 SmallString<128> ScriptPath = computeRelativePath("", InfoPath);
 llvm::sys::path::append(ScriptPath, llvm::sys::path::filename(FilePath));
+// Paths in HTML must be in posix-style
+llvm::sys::path::native(ScriptPath, llvm::sys::path::Style::posix);
 ScriptNode->Attributes.try_emplace("src", ScriptPath);
 Out.emplace_back(std::move(ScriptNode));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
   /*waitForInitialSync=*/true);
+  if (!DW) return;
 

plotfi wrote:
> gribozavr wrote:
> > plotfi wrote:
> > > gribozavr wrote:
> > > > plotfi wrote:
> > > > > gribozavr wrote:
> > > > > > Why? This is a test -- ignoring errors would only hide them from 
> > > > > > developers.
> > > > > Please see how llvm::Expected works. This does not ignore or hide 
> > > > > anything. 
> > > > I think I know how llvm::Expected works.
> > > > 
> > > > The problem that this check and return hides is that if 
> > > > DirectoryWatcher::create returns an error, the rest of the test is 
> > > > silently slipped. The test should be using something like `cantFail` 
> > > > instead.
> > > If DirectoryWatcher returns an error and you hit the early return, 
> > > without taking the error the Expected destructor will print a callstack 
> > > and halt the program. I can check this again, but I am quite sure that 
> > > without invoking something like takeError you will hit the crash in the 
> > > case of an early return. 
> > That's good -- but I think a `cantFail` call would be more readable.
> Ah, you should have said so then. Except this is arguably worse. I want this 
> test to tell me that the cause of the crash (in this case, exceeding the 
> inotify limit). What I want to happen is:
> 
> ```
> Note: Google Test filter = DirectoryWatcherTest.AddFiles
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DirectoryWatcherTest
> [ RUN  ] DirectoryWatcherTest.AddFiles
> Expected must be checked before access or destruction.
> Unchecked Expected contained error:
> inotify_init1() error: out of inotify entries #0 0x00246b8f 
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
>  #1 0x002450d2 llvm::sys::RunSignalHandlers() 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
>  #2 0x00247268 SignalHandler(int) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
>  #3 0x7f17924eb890 __restore_rt 
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
>  #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
> ...
> 
> ```
> 
> Wrapping the llvm::Expected in a cantFail hides this message, and instead you 
> get the much less useful:
> 
> ```
> Note: Google Test filter = DirectoryWatcherTest.AddFiles
> [==] Running 1 test from 1 test case. 
>   
>   
>
> [--] Global test environment set-up.
> [--] 1 test from DirectoryWatcherTest
> [ RUN  ] DirectoryWatcherTest.AddFiles
> Failure value returned from cantFail wrapped call
> UNREACHABLE executed at 
> /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
>  #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
>  #1 0x00245352 llvm::sys::RunSignalHandlers() 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
>  #2 0x002474e8 SignalHandler(int) 
> /mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
>  #3 0x7f20c64a8890 __restore_rt 
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
>  #4 0x7f20c4f51e97 gsignal 
> /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
>  #5 0x7f20c4f53801 abort 
> /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
>  #6 0x00232b31 
> (build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)
> 
> ```
> 
> I can add comments assuring that llvm::Expected will dump an error prior to 
> halting, but I specifically _want_ llvm::Expected to propagate up what perror 
> would print to the console otherwise. 
> 
To be clear, I want the error to tell the programmer what is going on so that 
they can either figure out what is eating up their machine resources and either 
modify a sysctl entry or kill some process. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Can you detail what "incorrect linking" means? AFAIK the additional sections 
were just bloating the executable, but how do they affect correctness?

Will this patch change the ability to consume a bundled object file without 
calling the unbundler? Using known ELF tools on the produced object files was 
an important design decision and IIRC was somewhat important for using build 
systems that are unaware of the bundled format.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65819



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

[PATCH] D65827: [clang-doc] Fix paths of js in import tags

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM (let's make sure we double check this next time)


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

https://reviews.llvm.org/D65827



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
   /*waitForInitialSync=*/true);
+  if (!DW) return;
 

gribozavr wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > plotfi wrote:
> > > > gribozavr wrote:
> > > > > Why? This is a test -- ignoring errors would only hide them from 
> > > > > developers.
> > > > Please see how llvm::Expected works. This does not ignore or hide 
> > > > anything. 
> > > I think I know how llvm::Expected works.
> > > 
> > > The problem that this check and return hides is that if 
> > > DirectoryWatcher::create returns an error, the rest of the test is 
> > > silently slipped. The test should be using something like `cantFail` 
> > > instead.
> > If DirectoryWatcher returns an error and you hit the early return, without 
> > taking the error the Expected destructor will print a callstack and halt 
> > the program. I can check this again, but I am quite sure that without 
> > invoking something like takeError you will hit the crash in the case of an 
> > early return. 
> That's good -- but I think a `cantFail` call would be more readable.
Ah, you should have said so then. Except this is arguably worse. I want this 
test to tell me that the cause of the crash (in this case, exceeding the 
inotify limit). What I want to happen is:

```
Note: Google Test filter = DirectoryWatcherTest.AddFiles
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from DirectoryWatcherTest
[ RUN  ] DirectoryWatcherTest.AddFiles
Expected must be checked before access or destruction.
Unchecked Expected contained error:
inotify_init1() error: out of inotify entries #0 0x00246b8f 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
 #1 0x002450d2 llvm::sys::RunSignalHandlers() 
/mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x00247268 SignalHandler(int) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
 #3 0x7f17924eb890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #4 0x7f1790f94e97 gsignal /build/glibc-OTsEL5/glibc-
...

```

Wrapping the llvm::Expected in a cantFail hides this message, and instead you 
get the much less useful:

```
Note: Google Test filter = DirectoryWatcherTest.AddFiles
[==] Running 1 test from 1 test case.   

 
[--] Global test environment set-up.
[--] 1 test from DirectoryWatcherTest
[ RUN  ] DirectoryWatcherTest.AddFiles
Failure value returned from cantFail wrapped call
UNREACHABLE executed at 
/mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:707!
 #0 0x00246e0f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
 #1 0x00245352 llvm::sys::RunSignalHandlers() 
/mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x002474e8 SignalHandler(int) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
 #3 0x7f20c64a8890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #4 0x7f20c4f51e97 gsignal 
/build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #5 0x7f20c4f53801 abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
 #6 0x00232b31 
(build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x232b31)

```

I can add comments assuring that llvm::Expected will dump an error prior to 
halting, but I specifically _want_ llvm::Expected to propagate up what perror 
would print to the console otherwise. 



Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213706.
DiegoAstiazaran added a comment.

Changed `JumpToSection` in Index struct and `genHTML(const Index , ...)` 
function to `llvm::Optional`.


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

https://reviews.llvm.org/D65030

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -9,6 +9,7 @@
 #include "ClangDocTest.h"
 #include "Generators.h"
 #include "Representation.h"
+#include "Serialize.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -59,24 +60,60 @@
 
 
 
+
+  
+
+  Namespaces
+
+  
+  
+
+  Records
+
+  
+  
+
+  Functions
+
+
+  
+
+  OneFunction
+
+  
+
+  
+  
+
+  Enums
+
+
+  
+
+  OneEnum
+
+  
+
+  
+
 
   namespace Namespace
-  Namespaces
+  Namespaces
   
 ChildNamespace
   
-  Records
+  Records
   
 ChildStruct
   
-  Functions
+  Functions
   
-OneFunction
+OneFunction
 OneFunction()
   
-  Enums
+  Enums
   
-enum OneEnum
+enum OneEnum
   
 
 )raw";
@@ -119,6 +156,42 @@
 
 
 
+
+  
+
+  Members
+
+  
+  
+
+  Records
+
+  
+  
+
+  Functions
+
+
+  
+
+  OneFunction
+
+  
+
+  
+  
+
+  Enums
+
+
+  
+
+  OneEnum
+
+  
+
+  
+
 
   class r
   Defined at line 10 of test.cpp
@@ -127,7 +200,7 @@
 F
 , G
   
-  Members
+  Members
   
 
   private 
@@ -135,18 +208,18 @@
X
 
   
-  Records
+  Records
   
 ChildStruct
   
-  Functions
+  Functions
   
-OneFunction
+OneFunction
 OneFunction()
   
-  Enums
+  Enums
   
-enum OneEnum
+enum OneEnum
   
 
 )raw";
@@ -155,26 +228,39 @@
 }
 
 TEST(HTMLGeneratorTest, emitFunctionHTML) {
+  llvm::errs() << "1\n";
   FunctionInfo I;
+  llvm::errs() << "2\n";
   I.Name = "f";
+  llvm::errs() << "3\n";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
+  llvm::errs() << "4\n";
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  llvm::errs() << "5\n";
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
-
+  llvm::errs() << "6\n";
   SmallString<16> PathTo;
+  llvm::errs() << "7\n";
   llvm::sys::path::native("path/to", PathTo);
+  llvm::errs() << "8\n";
   I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, PathTo);
+  llvm::errs() << "9\n";
   I.Params.emplace_back("int", PathTo, "P");
+  llvm::errs() << "1\n";
   I.IsMethod = true;
+  llvm::errs() << "2\n";
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
+  llvm::errs() << "3\n";
 
   auto G = getHTMLGenerator();
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
   ClangDocContext CDCtx = getClangDocContext();
+  llvm::errs() << "4\n";
   auto Err = G->generateDocForInfo(, Actual, CDCtx);
+  llvm::errs() << "5\n";
   assert(!Err);
   std::string Expected = R"raw(
 
@@ -183,7 +269,7 @@
 
 
 
-  f
+  f
   
 float
  f(
@@ -193,8 +279,10 @@
   Defined at line 10 of test.cpp
 
 )raw";
+  llvm::errs() << "6\n";
 
   EXPECT_EQ(Expected, Actual.str());
+  llvm::errs() << "7\n";
 }
 
 TEST(HTMLGeneratorTest, emitEnumHTML) {
@@ -222,7 +310,7 @@
 
 
 
-  enum class e
+  enum class e
   
 X
   
@@ -293,7 +381,7 @@
 
 
 
-  f
+  f
   void f(int I, int J)
   Defined at line 10 of test.cpp
   
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -350,12 +350,15 @@
 
 struct Index : public Reference {
   Index() = default;
+  Index(StringRef Name, StringRef JumpToSection)
+  : Reference(Name), JumpToSection(JumpToSection) {}
   Index(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
   : Reference(USR, Name, IT, Path) {}
   // This is used to look for a USR in a vector of Indexes using std::find
   bool operator==(const SymbolID ) const { return USR == Other; }
   bool operator<(const Index ) const { return Name < Other.Name; }
 
+  llvm::Optional> JumpToSection;
   std::vector Children;
 
   void sort();
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -268,15 +268,22 @@
   return LinkNode;
 }
 
-static std::unique_ptr genTypeReference(const Reference ,
-  

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368086: [clang-scan-deps] Implementation of dependency 
scanner over minimized sources (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63907?vs=213664=213704#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63907

Files:
  cfe/trunk/include/clang/Basic/FileManager.h
  
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  cfe/trunk/lib/Tooling/DependencyScanning/CMakeLists.txt
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  
cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/Headers/Framework.h
  
cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/PrivateHeaders/PrivateHeader.h
  cfe/trunk/test/ClangScanDeps/Inputs/header_stat_before_open_cdb.json
  cfe/trunk/test/ClangScanDeps/Inputs/vfsoverlay.yaml
  cfe/trunk/test/ClangScanDeps/Inputs/vfsoverlay_cdb.json
  cfe/trunk/test/ClangScanDeps/header_stat_before_open.m
  cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
  cfe/trunk/test/ClangScanDeps/vfsoverlay.cpp
  cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Index: cfe/trunk/include/clang/Basic/FileManager.h
===
--- cfe/trunk/include/clang/Basic/FileManager.h
+++ cfe/trunk/include/clang/Basic/FileManager.h
@@ -231,6 +231,10 @@
 
   llvm::vfs::FileSystem () const { return *FS; }
 
+  void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
+this->FS = std::move(FS);
+  }
+
   /// Retrieve a file entry for a "virtual" file that acts as
   /// if there were a file with the given name on disk.
   ///
Index: cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
===
--- cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_WORKER_H
 
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -21,6 +22,9 @@
 namespace tooling {
 namespace dependencies {
 
+class DependencyScanningService;
+class DependencyScanningWorkerFilesystem;
+
 /// An individual dependency scanning worker that is able to run on its own
 /// thread.
 ///
@@ -29,7 +33,7 @@
 /// using the regular processing run.
 class DependencyScanningWorker {
 public:
-  DependencyScanningWorker();
+  DependencyScanningWorker(DependencyScanningService );
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
@@ -45,10 +49,11 @@
   IntrusiveRefCntPtr DiagOpts;
   std::shared_ptr PCHContainerOps;
 
+  llvm::IntrusiveRefCntPtr RealFS;
   /// The file system that is used by each worker when scanning for
   /// dependencies. This filesystem persists accross multiple compiler
   /// invocations.
-  llvm::IntrusiveRefCntPtr WorkerFS;
+  llvm::IntrusiveRefCntPtr DepFS;
 };
 
 } // end namespace dependencies
Index: cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -0,0 +1,168 @@
+//===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_FILESYSTEM_H
+#define LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_FILESYSTEM_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+namespace clang {
+namespace tooling {
+namespace dependencies {
+
+/// An in-memory representation of a file system entity that is of interest to
+/// the dependency 

r368086 - [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Aug  6 13:43:25 2019
New Revision: 368086

URL: http://llvm.org/viewvc/llvm-project?rev=368086=rev
Log:
[clang-scan-deps] Implementation of dependency scanner over minimized sources

This commit implements the fast dependency scanning mode in clang-scan-deps: the
preprocessing is done on files that are minimized using the dependency 
directives source minimizer.

A shared file system cache is used to ensure that the file system requests and 
source minimization
is performed only once. The cache assumes that the underlying filesystem won't 
change during the course
of the scan (or if it will, it will not affect the output), and it can't be 
evicted. This means that the
service and workers can be used for a single run of a dependency scanner, and 
can't be reused across multiple,
incremental runs. This is something that we'll most likely support in the 
future though.
Note that the driver still utilizes the underlying real filesystem.

This commit is also still missing the fast skipped PP block skipping 
optimization that I mentioned at EuroLLVM talk.
Additionally, the file manager is still not reused by the threads as well.

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

Added:

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
cfe/trunk/test/ClangScanDeps/Inputs/frameworks/
cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/
cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/Headers/

cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/Headers/Framework.h

cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/PrivateHeaders/

cfe/trunk/test/ClangScanDeps/Inputs/frameworks/Framework.framework/PrivateHeaders/PrivateHeader.h
cfe/trunk/test/ClangScanDeps/Inputs/header_stat_before_open_cdb.json
cfe/trunk/test/ClangScanDeps/Inputs/vfsoverlay.yaml
cfe/trunk/test/ClangScanDeps/Inputs/vfsoverlay_cdb.json
cfe/trunk/test/ClangScanDeps/header_stat_before_open.m
cfe/trunk/test/ClangScanDeps/vfsoverlay.cpp
Modified:
cfe/trunk/include/clang/Basic/FileManager.h

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
cfe/trunk/lib/Tooling/DependencyScanning/CMakeLists.txt
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=368086=368085=368086=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Tue Aug  6 13:43:25 2019
@@ -231,6 +231,10 @@ public:
 
   llvm::vfs::FileSystem () const { return *FS; }
 
+  void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
+this->FS = std::move(FS);
+  }
+
   /// Retrieve a file entry for a "virtual" file that acts as
   /// if there were a file with the given name on disk.
   ///

Added: 
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h?rev=368086=auto
==
--- 
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
 (added)
+++ 
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
 Tue Aug  6 13:43:25 2019
@@ -0,0 +1,168 @@
+//===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_FILESYSTEM_H
+#define LLVM_CLANG_TOOLING_DEPENDENCY_SCANNING_FILESYSTEM_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+namespace clang {
+namespace tooling {
+namespace dependencies {
+
+/// An in-memory representation of a file system entity that is of interest to
+/// the dependency scanning filesystem.
+///
+/// It represents one of the following:
+/// - an opened source file with minimized contents and a stat value.
+/// - an opened source 

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

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



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:76
+CachedFileSystemEntry
+CachedFileSystemEntry::createDirectoryEntry(llvm::vfs::Status Stat) {
+  assert(Stat.isDirectory() && "not a directory!");

aganea wrote:
> arphaman wrote:
> > aganea wrote:
> > > `llvm::vfs::Status &` to avoid a copy?
> > The copy should already be avoided, as I move the argument when passing in, 
> > and when it's assigned to the result.
> If `Stat` is not a rvalue reference, wouldn't the `std::move` in the call 
> site end up as a copy? See [[ https://godbolt.org/z/Rr7cdM | this ]]. If you 
> change `int test(A a)` to `int test(A &)` you can see the difference in the 
> asm output. However if the function is inlined, the extra copy will probably 
> be optimized out. Not a big deal - the OS calls like stat() will most likely 
> dominate here.
Isn't the difference in the output caused by the details of the calling 
convention (pass the structure on the stack by value)? The move constructor 
should still be called for the stat, it will not copy its members. We can 
optimize this though and pass by ref directly, I agree, so let me do that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63907



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
   /*waitForInitialSync=*/true);
+  if (!DW) return;
 

plotfi wrote:
> gribozavr wrote:
> > plotfi wrote:
> > > gribozavr wrote:
> > > > Why? This is a test -- ignoring errors would only hide them from 
> > > > developers.
> > > Please see how llvm::Expected works. This does not ignore or hide 
> > > anything. 
> > I think I know how llvm::Expected works.
> > 
> > The problem that this check and return hides is that if 
> > DirectoryWatcher::create returns an error, the rest of the test is silently 
> > slipped. The test should be using something like `cantFail` instead.
> If DirectoryWatcher returns an error and you hit the early return, without 
> taking the error the Expected destructor will print a callstack and halt the 
> program. I can check this again, but I am quite sure that without invoking 
> something like takeError you will hit the crash in the case of an early 
> return. 
That's good -- but I think a `cantFail` call would be more readable.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102
 
-  /// Returns nullptr if \param Path doesn't exist or isn't a directory.
-  /// Returns nullptr if OS kernel API told us we can't start watching. In such
-  /// case it's unclear whether just retrying has any chance to succeeed.
-  static std::unique_ptr
+  /// Asserts if \param Path doesn't exist or isn't a directory.
+  /// Returns llvm::Expected Error if OS kernel API told us we can't start

plotfi wrote:
> gribozavr wrote:
> > jkorous wrote:
> > > plotfi wrote:
> > > > gribozavr wrote:
> > > > > I don't see where this logic is implemented.
> > > > > 
> > > > > Also, LLVM is often built with assertions disabled, so "asserts" 
> > > > > can't be a part of the contract.
> > > > Please be more specific. What logic are you talking about? 
> > > We shouldn't assert. Just return an error and let client code handle it 
> > > in any way it wants.
> > I'm saying that it is not appropriate to document the API as `assert()`ing 
> > about something, because it is not a part of the contract.
> > 
> > You can say that the API *requires* the Path to be non-empty. Or you can 
> > call llvm_fatal_error explicitly if it is empty, and then you can document 
> > it. Or you can return an error like jkorous said. However, assertions are 
> > not a part of the API contract.
> > 
> > 
> That makes sense. Lemme think about which is best, and I'll amend this diff.
@jkorous I have another patch ready that will replace the asserts with llvm 
fatal_error. If this is good by you I will land that.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[clang-tools-extra] r368083 - [clangd] Unfold SourceLocation flattening from findNameLoc in preparation for adding more overloads. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Aug  6 13:25:59 2019
New Revision: 368083

URL: http://llvm.org/viewvc/llvm-project?rev=368083=rev
Log:
[clangd] Unfold SourceLocation flattening from findNameLoc in preparation for 
adding more overloads. NFC

Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp

Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=368083=368082=368083=diff
==
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Tue Aug  6 13:25:59 2019
@@ -8,6 +8,7 @@
 
 #include "AST.h"
 
+#include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
@@ -42,34 +43,13 @@ getTemplateSpecializationArgLocs(const N
 }
 } // namespace
 
-// Returns true if the complete name of decl \p D is spelled in the source 
code.
-// This is not the case for:
-//   * symbols formed via macro concatenation, the spelling location will
-// be ""
-//   * symbols controlled and defined by a compile command-line option
-// `-DName=foo`, the spelling location will be "".
-bool isSpelledInSourceCode(const Decl *D) {
-  const auto  = D->getASTContext().getSourceManager();
-  auto Loc = D->getLocation();
-  // FIXME: Revisit the strategy, the heuristic is limitted when handling
-  // macros, we should use the location where the whole definition occurs.
-  if (Loc.isMacroID()) {
-std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
-if (llvm::StringRef(PrintLoc).startswith(""))
-  return false;
-  }
-  return true;
+bool isImplementationDetail(const Decl *D) {
+  return !isSpelledInSource(D->getLocation(),
+D->getASTContext().getSourceManager());
 }
 
-bool isImplementationDetail(const Decl *D) { return !isSpelledInSourceCode(D); 
}
-
-SourceLocation findNameLoc(const clang::Decl *D) {
-  const auto  = D->getASTContext().getSourceManager();
-  if (!isSpelledInSourceCode(D))
-// Use the expansion location as spelling location is not interesting.
-return SM.getExpansionRange(D->getLocation()).getBegin();
-  return SM.getSpellingLoc(D->getLocation());
+SourceLocation findName(const clang::Decl *D) {
+  return D->getLocation();
 }
 
 std::string printQualifiedName(const NamedDecl ) {

Modified: clang-tools-extra/trunk/clangd/AST.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=368083=368082=368083=diff
==
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Tue Aug  6 13:25:59 2019
@@ -33,7 +33,7 @@ bool isImplementationDetail(const Decl *
 ///
 /// The returned location is usually the spelling location where the name of 
the
 /// decl occurs in the code.
-SourceLocation findNameLoc(const clang::Decl *D);
+SourceLocation findName(const clang::Decl *D);
 
 /// Returns the qualified name of ND. The scope doesn't contain unwritten 
scopes
 /// like inline namespaces.

Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=368083=368082=368083=diff
==
--- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Tue Aug  6 13:25:59 2019
@@ -138,7 +138,7 @@ namespace {
 llvm::Optional declToSym(ASTContext , const NamedDecl ) 
{
   auto  = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = findNameLoc();
+  SourceLocation NameLoc = spellingLocIfSpelled(findName(), SM);
   // getFileLoc is a good choice for us, but we also need to make sure
   // sourceLocToPosition won't switch files, so we call getSpellingLoc on top 
of
   // that to make sure it does not switch files.

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=368083=368082=368083=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Aug  6 13:25:59 2019
@@ -200,6 +200,24 @@ Position sourceLocToPosition(const Sourc
   return P;
 }
 
+bool isSpelledInSource(SourceLocation Loc, const SourceManager ) {
+  if (Loc.isMacroID()) {
+std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
+if (llvm::StringRef(PrintLoc).startswith(""))
+  return 

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:329
+  assert(!Path.empty() && "Path.empty()");
 
   const int InotifyFD = inotify_init1(IN_CLOEXEC);


@gribozavr As an example, let say we stuck the following right here:


```
return llvm::make_error("llvm::Expected will blow up", 
llvm::inconvertibleErrorCode());
```

When running:



```
DirectoryWatcherTests --gtest_filter=DirectoryWatcherTest.AddFiles
```

You will get:

```
Note: Google Test filter = DirectoryWatcherTest.AddFiles
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from DirectoryWatcherTest
[ RUN  ] DirectoryWatcherTest.AddFiles
Expected must be checked before access or destruction.
Unchecked Expected contained error:
llvm::Expected will blow up #0 0x00246b8f 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:533:13
 #1 0x002450d2 llvm::sys::RunSignalHandlers() 
/mnt/nvme0/llvm-project/llvm/lib/Support/Signals.cpp:69:18
 #2 0x00247268 SignalHandler(int) 
/mnt/nvme0/llvm-project/llvm/lib/Support/Unix/Signals.inc:385:1
 #3 0x7fa2a284d890 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #4 0x7fa2a12f6e97 gsignal 
/build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #5 0x7fa2a12f8801 abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
 #6 0x002229db llvm::raw_ostream::operator<<(llvm::StringRef) 
/mnt/nvme0/llvm-project/llvm/include/llvm/Support/raw_ostream.h:176:7
 #7 0x002229db llvm::raw_ostream::operator<<(char const*) 
/mnt/nvme0/llvm-project/llvm/include/llvm/Support/raw_ostream.h:186:0
 #8 0x002229db llvm::Expected > >::fatalUncheckedExpected() 
const /mnt/nvme0/llvm-project/llvm/include/llvm/Support/Error.h:661:0
 #9 0x0021e148 
(build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x21e148)
#10 0x0024cbaa testing::internal::UnitTestImpl::os_stack_trace_getter() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4919:7
#11 0x0024cbaa testing::Test::Run() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2481:0
#12 0x0024d840 testing::internal::UnitTestImpl::os_stack_trace_getter() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4919:7
#13 0x0024d840 testing::TestInfo::Run() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2660:0
#14 0x0024dee7 testing::TestCase::Run() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2773:44
#15 0x002557b7 testing::internal::UnitTestImpl::RunAllTests() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4648:24
#16 0x00255411 testing::UnitTest::Run() 
/mnt/nvme0/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4257:10
#17 0x0024859b main 
/mnt/nvme0/llvm-project/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50:3
#18 0x7fa2a12d9b97 __libc_start_main 
/build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:344:0
#19 0x0021b02a _start 
(build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests+0x21b02a)
Aborted (core dumped)
```

This is preferable to deadlocking. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65753: Builtins: Add some v2f16 variants

[Steve Canon via Phabricator via cfe-commits]

scanon added a comment.

(Ideally we would just call them e.g. __builtin_floor, but that would be 
source-breaking. __builtin_tgmath_floor seems like a good compromise.)


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

https://reviews.llvm.org/D65753



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

[PATCH] D65753: Builtins: Add some v2f16 variants

[Steve Canon via Phabricator via cfe-commits]

scanon added a comment.

Strongly agree with what @rjmccall said. If we can make these generic builtins 
instead of ending up with O(100) variants of each math operation, that would 
make life immensely nicer.


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

https://reviews.llvm.org/D65753



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

[PATCH] D63976: Allow clang -Os and -Oz to work with -flto and lld

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

> with a funny command line like `clang -Oz -flto -o foo a.o b.c`, where one 
> (or all) of the input arguments is a source file? Would it invoke the bitcode 
> compile of the source files with the requested `-Oz` and then invoke the LTO 
> linker plugin with `-O2`/`-O3`?

What you're invoking is the clang driver, it will drive the process and spawn 
multiple subprocess for each phase (this contributes to the UI challenge). Your 
"funny" command line will detect that it needs to spawn a clang instance for 
the compile phase of b.c to get an object file before invoking the linker on 
the temporary b.o and the a.o you provide on the input. To add some complexity, 
the exact behavior depends on the platform (Linux vs MacOS) and possible which 
linker is in use (-fuse-ld=lld).

But yes this patch changes the driver so that only the invocation of the linker 
is changed. Your previous "funny" command line would still yield Oz when 
compiling b.c to b.o before invoking the linker on b.o and a.o with O3 
.

(I don't know if this change is the best solution, I just provide my 
understanding of the situation :))


Repository:
  rC Clang

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

https://reviews.llvm.org/D63976



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

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Hi Florian, thanks for your input!

> IMO it would make sense to have the more specific pragmas imply 
> vectorize(enable) here (or update the docs accordingly).

Yep, fully agree with that, as I also wrote in my previous comment. And thanks 
for digging up that PR. I think that supports this case and that the proposed 
behaviour in this patch is what one would expect. If we agree on that, I 
wouldn't mind fixing that PR too as that seems low hanging-fruit to me.

And to be honest, at the moment I have no clue what the meaning is, or 
could/should be, when a more specific pragma like vector_width or 
vector_predicate is set, without also setting/implying vectorize(enable). I 
need to look into that...


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

https://reviews.llvm.org/D65776



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
   /*waitForInitialSync=*/true);
+  if (!DW) return;
 

gribozavr wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > Why? This is a test -- ignoring errors would only hide them from 
> > > developers.
> > Please see how llvm::Expected works. This does not ignore or hide anything. 
> I think I know how llvm::Expected works.
> 
> The problem that this check and return hides is that if 
> DirectoryWatcher::create returns an error, the rest of the test is silently 
> slipped. The test should be using something like `cantFail` instead.
If DirectoryWatcher returns an error and you hit the early return, without 
taking the error the Expected destructor will print a callstack and halt the 
program. I can check this again, but I am quite sure that without invoking 
something like takeError you will hit the crash in the case of an early return. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:7077
+//   someContainer.add(std::move(localOWner));
+//   return p;
+if (!IsTempGslOwner && pathOnlyInitializesGslPointer(Path) &&

xazax.hun wrote:
> gribozavr wrote:
> > Why is it a false positive? `std::move` left memory owned by `localOwner` 
> > in unspecified state.
> I saw user code relying on the semantics of certain classes. E.g. they assume 
> if a `std::unique_ptr` is moved the pointee is still in place, so it is safe 
> to return a reference to the pointee. Do you think those cases should be 
> diagnosed too?
It is... debatable. It is not obvious whether the lifetime of the pointed-to 
memory has ended or not without more detailed lifetime annotations. I think it 
is fair to silence it, however, I think the comment should be updated to 
explain the situation in a more detailed way, since without context it looks 
like a use-after-move.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64256



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102
 
-  /// Returns nullptr if \param Path doesn't exist or isn't a directory.
-  /// Returns nullptr if OS kernel API told us we can't start watching. In such
-  /// case it's unclear whether just retrying has any chance to succeeed.
-  static std::unique_ptr
+  /// Asserts if \param Path doesn't exist or isn't a directory.
+  /// Returns llvm::Expected Error if OS kernel API told us we can't start

gribozavr wrote:
> jkorous wrote:
> > plotfi wrote:
> > > gribozavr wrote:
> > > > I don't see where this logic is implemented.
> > > > 
> > > > Also, LLVM is often built with assertions disabled, so "asserts" can't 
> > > > be a part of the contract.
> > > Please be more specific. What logic are you talking about? 
> > We shouldn't assert. Just return an error and let client code handle it in 
> > any way it wants.
> I'm saying that it is not appropriate to document the API as `assert()`ing 
> about something, because it is not a part of the contract.
> 
> You can say that the API *requires* the Path to be non-empty. Or you can call 
> llvm_fatal_error explicitly if it is empty, and then you can document it. Or 
> you can return an error like jkorous said. However, assertions are not a part 
> of the API contract.
> 
> 
That makes sense. Lemme think about which is best, and I'll amend this diff.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102
 
-  /// Returns nullptr if \param Path doesn't exist or isn't a directory.
-  /// Returns nullptr if OS kernel API told us we can't start watching. In such
-  /// case it's unclear whether just retrying has any chance to succeeed.
-  static std::unique_ptr
+  /// Asserts if \param Path doesn't exist or isn't a directory.
+  /// Returns llvm::Expected Error if OS kernel API told us we can't start

jkorous wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > I don't see where this logic is implemented.
> > > 
> > > Also, LLVM is often built with assertions disabled, so "asserts" can't be 
> > > a part of the contract.
> > Please be more specific. What logic are you talking about? 
> We shouldn't assert. Just return an error and let client code handle it in 
> any way it wants.
I'm saying that it is not appropriate to document the API as `assert()`ing 
about something, because it is not a part of the contract.

You can say that the API *requires* the Path to be non-empty. Or you can call 
llvm_fatal_error explicitly if it is empty, and then you can document it. Or 
you can return an error like jkorous said. However, assertions are not a part 
of the API contract.





Comment at: cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp:153
 dispatch_queue_t Queue) {
-  if (Path.empty())
-return nullptr;
+  assert(!Path.empty() && "Path.empty()");
 

jkorous wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > No need to repeat the condition in the `&& "..."` part. assert does that 
> > > by default. Use an extra string to explain the assertion to the reader.
> > This is std assert. Are you referring to a different assert? If @compnerd 
> > is ok with changing the assert into an llvm::Expected I can do that. 
> I think the suggestion is just
> 
> ```
> assert(!Path.empty());
> ```
Right. The standard assert already prints the expression -- in fact, this is 
how the `&& "xyz"` part gets printed when assertion fails -- assert prints the 
expression that failed.



Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
   /*waitForInitialSync=*/true);
+  if (!DW) return;
 

plotfi wrote:
> gribozavr wrote:
> > Why? This is a test -- ignoring errors would only hide them from developers.
> Please see how llvm::Expected works. This does not ignore or hide anything. 
I think I know how llvm::Expected works.

The problem that this check and return hides is that if 
DirectoryWatcher::create returns an error, the rest of the test is silently 
slipped. The test should be using something like `cantFail` instead.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D65688: [AIX][test/Index] Set/propagate AIXTHREAD_STK for AIX

[David Tenty via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D65688



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

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

[Jan Korous via Phabricator via cfe-commits]

jkorous added inline comments.



Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102
 
-  /// Returns nullptr if \param Path doesn't exist or isn't a directory.
-  /// Returns nullptr if OS kernel API told us we can't start watching. In such
-  /// case it's unclear whether just retrying has any chance to succeeed.
-  static std::unique_ptr
+  /// Asserts if \param Path doesn't exist or isn't a directory.
+  /// Returns llvm::Expected Error if OS kernel API told us we can't start

plotfi wrote:
> gribozavr wrote:
> > I don't see where this logic is implemented.
> > 
> > Also, LLVM is often built with assertions disabled, so "asserts" can't be a 
> > part of the contract.
> Please be more specific. What logic are you talking about? 
We shouldn't assert. Just return an error and let client code handle it in any 
way it wants.



Comment at: cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp:153
 dispatch_queue_t Queue) {
-  if (Path.empty())
-return nullptr;
+  assert(!Path.empty() && "Path.empty()");
 

plotfi wrote:
> gribozavr wrote:
> > No need to repeat the condition in the `&& "..."` part. assert does that by 
> > default. Use an extra string to explain the assertion to the reader.
> This is std assert. Are you referring to a different assert? If @compnerd is 
> ok with changing the assert into an llvm::Expected I can do that. 
I think the suggestion is just

```
assert(!Path.empty());
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65704



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

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368072: Teach some warnings to respect gsl::Pointer and 
gsl::Owner attributes (authored by xazax, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64256?vs=212205=213682#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64256

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8107,6 +8107,10 @@
   "%select{binds to|is}2 a temporary object "
   "whose lifetime is shorter than the lifetime of the constructed object">,
   InGroup;
+def warn_dangling_lifetime_pointer_member : Warning<
+  "initializing pointer member %0 to point to a temporary object "
+  "whose lifetime is shorter than the lifetime of the constructed object">,
+  InGroup;
 def note_lifetime_extending_member_declared_here : Note<
   "%select{%select{reference|'std::initializer_list'}0 member|"
   "member with %select{reference|'std::initializer_list'}0 subobject}1 "
@@ -8125,6 +8129,10 @@
   "temporary bound to reference member of allocated object "
   "will be destroyed at the end of the full-expression">,
   InGroup;
+def warn_dangling_lifetime_pointer : Warning<
+  "object backing the pointer "
+  "will be destroyed at the end of the full-expression">,
+  InGroup;
 def warn_new_dangling_initializer_list : Warning<
   "array backing "
   "%select{initializer list subobject of the allocated object|"
Index: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -0,0 +1,135 @@
+// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s
+struct [[gsl::Owner(int)]] MyIntOwner {
+  MyIntOwner();
+  int *();
+  int *c_str() const;
+};
+
+struct [[gsl::Pointer(int)]] MyIntPointer {
+  MyIntPointer(int *p = nullptr);
+  // Conversion operator and constructor conversion will result in two
+  // different ASTs. The former is tested with another owner and 
+  // pointer type.
+  MyIntPointer(const MyIntOwner &);
+  int *();
+  MyIntOwner toOwner();
+};
+
+struct [[gsl::Pointer(long)]] MyLongPointerFromConversion {
+  MyLongPointerFromConversion(long *p = nullptr);
+  long *();
+};
+
+struct [[gsl::Owner(long)]] MyLongOwnerWithConversion {
+  MyLongOwnerWithConversion();
+  operator MyLongPointerFromConversion();
+  long *();
+  MyIntPointer releaseAsMyPointer();
+  long *releaseAsRawPointer();
+};
+
+void danglingHeapObject() {
+  new MyLongPointerFromConversion(MyLongOwnerWithConversion{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  new MyIntPointer(MyIntOwner{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+}
+
+void intentionalFalseNegative() {
+  int i;
+  MyIntPointer p{};
+  // In this case we do not have enough information in a statement local
+  // analysis to detect the problem.
+  new MyIntPointer(p);
+  new MyIntPointer(MyIntPointer{p});
+}
+
+MyIntPointer ownershipTransferToMyPointer() {
+  MyLongOwnerWithConversion t;
+  return t.releaseAsMyPointer(); // ok
+}
+
+long *ownershipTransferToRawPointer() {
+  MyLongOwnerWithConversion t;
+  return t.releaseAsRawPointer(); // ok
+}
+
+int *danglingRawPtrFromLocal() {
+  MyIntOwner t;
+  return t.c_str(); // TODO
+}
+
+int *danglingRawPtrFromTemp() {
+  MyIntPointer p;
+  return p.toOwner().c_str(); // TODO
+}
+
+struct Y {
+  int a[4];
+};
+
+void dangligGslPtrFromTemporary() {
+  MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+  (void)p;
+}
+
+struct DanglingGslPtrField {
+  MyIntPointer p; // expected-note 2{{pointer member declared here}}
+  MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}}
+  DanglingGslPtrField(int i) : p() {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}}
+  DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
+  DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of 

r368072 - Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Tue Aug  6 12:13:29 2019
New Revision: 368072

URL: http://llvm.org/viewvc/llvm-project?rev=368072=rev
Log:
Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

This patch extends some existing warnings to utilize the knowledge about the 
gsl::Pointer and gsl::Owner attributes.

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

Added:
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaInit.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=368072=368071=368072=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug  6 12:13:29 
2019
@@ -8107,6 +8107,10 @@ def warn_dangling_member : Warning<
   "%select{binds to|is}2 a temporary object "
   "whose lifetime is shorter than the lifetime of the constructed object">,
   InGroup;
+def warn_dangling_lifetime_pointer_member : Warning<
+  "initializing pointer member %0 to point to a temporary object "
+  "whose lifetime is shorter than the lifetime of the constructed object">,
+  InGroup;
 def note_lifetime_extending_member_declared_here : Note<
   "%select{%select{reference|'std::initializer_list'}0 member|"
   "member with %select{reference|'std::initializer_list'}0 subobject}1 "
@@ -8125,6 +8129,10 @@ def warn_new_dangling_reference : Warnin
   "temporary bound to reference member of allocated object "
   "will be destroyed at the end of the full-expression">,
   InGroup;
+def warn_dangling_lifetime_pointer : Warning<
+  "object backing the pointer "
+  "will be destroyed at the end of the full-expression">,
+  InGroup;
 def warn_new_dangling_initializer_list : Warning<
   "array backing "
   "%select{initializer list subobject of the allocated object|"

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368072=368071=368072=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Aug  6 12:13:29 2019
@@ -6513,6 +6513,7 @@ struct IndirectLocalPathEntry {
 VarInit,
 LValToRVal,
 LifetimeBoundCall,
+GslPointerInit
   } Kind;
   Expr *E;
   const Decl *D = nullptr;
@@ -6557,6 +6558,40 @@ static void visitLocalsRetainedByReferen
   Expr *Init, ReferenceKind RK,
   LocalVisitor Visit);
 
+template  static bool isRecordWithAttr(QualType Type) {
+  if (auto *RD = Type->getAsCXXRecordDecl())
+return RD->getCanonicalDecl()->hasAttr();
+  return false;
+}
+
+static void handleGslAnnotatedTypes(IndirectLocalPath , Expr *Call,
+LocalVisitor Visit) {
+  auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
+Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
+if (Arg->isGLValue())
+  visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
+Visit);
+else
+  visitLocalsRetainedByInitializer(Path, Arg, Visit, true);
+Path.pop_back();
+  };
+
+  if (auto *MCE = dyn_cast(Call)) {
+const FunctionDecl *Callee = MCE->getDirectCallee();
+if (auto *Conv = dyn_cast_or_null(Callee))
+  if (isRecordWithAttr(Conv->getConversionType()))
+VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+return;
+  }
+
+  if (auto *CCE = dyn_cast(Call)) {
+const auto *Ctor = CCE->getConstructor();
+const CXXRecordDecl *RD = Ctor->getParent()->getCanonicalDecl();
+if (CCE->getNumArgs() > 0 && RD->hasAttr())
+  VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0]);
+  }
+}
+
 static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
   const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
   if (!TSI)
@@ -6678,8 +6713,10 @@ static void visitLocalsRetainedByReferen
true);
   }
 
-  if (isa(Init))
+  if (isa(Init)) {
+handleGslAnnotatedTypes(Path, Init, Visit);
 return visitLifetimeBoundArguments(Path, Init, Visit);
+  }
 
   switch (Init->getStmtClass()) {
   case Stmt::DeclRefExprClass: {
@@ -6905,8 +6942,10 @@ static void visitLocalsRetainedByInitial
 }
   }
 
-  if (isa(Init) || isa(Init))
+  if (isa(Init) || isa(Init)) {
+handleGslAnnotatedTypes(Path, Init, Visit);
 return visitLifetimeBoundArguments(Path, Init, Visit);
+  }
 
   switch (Init->getStmtClass()) {
   case Stmt::UnaryOperatorClass: {
@@ -6988,6 +7027,7 @@ static SourceRange nextPathEntryRange(co
 case IndirectLocalPathEntry::AddressOf:
 case 

[PATCH] D65648: [clang-format] Add support to SpacesBeforeTrailingComments to add spaces before Block comments.

[Manikishan Ghantasala via Phabricator via cfe-commits]

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



Comment at: unittests/Format/FormatTest.cpp:3663
+  FormatStyle Style = getGoogleStyle();
+  Style.SpacesBeforeTrailingComments = 0;
   EXPECT_EQ("#define MACRO() \\\n"

MyDeveloperDay wrote:
> this feels odd, suggesting those using google style will get reformatted 
> changes?
Yes, so in that case shall I add a new style variable like 
spacesBeforeTrailingBlockComments. Using which they can enable the style. This 
also supports new styles which need. spaces before Block Comments and styles 
which doesn't need spaces.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65648



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

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alexandre Ganea via Phabricator via cfe-commits]

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

LGTM, thank you!

In D63907#1617417 , @arphaman wrote:

> Just for reference, this patch still doesn't reuse the FileManager across 
> invocations in a thread. We expect to get even better performance once we 
> reuse it, but I'm going have to improve its API first.


Can't wait! @SamChaps is already testing this patch. He found some minor 
edge-cases with the minimizer (most likely unrelated to this), I'll post a 
patch.




Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:76
+CachedFileSystemEntry
+CachedFileSystemEntry::createDirectoryEntry(llvm::vfs::Status Stat) {
+  assert(Stat.isDirectory() && "not a directory!");

arphaman wrote:
> aganea wrote:
> > `llvm::vfs::Status &` to avoid a copy?
> The copy should already be avoided, as I move the argument when passing in, 
> and when it's assigned to the result.
If `Stat` is not a rvalue reference, wouldn't the `std::move` in the call site 
end up as a copy? See [[ https://godbolt.org/z/Rr7cdM | this ]]. If you change 
`int test(A a)` to `int test(A &)` you can see the difference in the asm 
output. However if the function is inlined, the extra copy will probably be 
optimized out. Not a big deal - the OS calls like stat() will most likely 
dominate here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63907



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:69
+
+  using MemoryBuffersVector = SmallVectorImpl>;
+

Why not `ArrayRef`?



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:72-75
+  IntegerType *getSizeTTy() {
+auto PtrSize = M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C));
+return PtrSize == 8 ? Type::getInt64Ty(C) : Type::getInt32Ty(C);
+  }

Not sure that this is the best solution, we may end up with incorrect `size_t` 
type in some cases. 



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:73
+  IntegerType *getSizeTTy() {
+auto PtrSize = M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C));
+return PtrSize == 8 ? Type::getInt64Ty(C) : Type::getInt32Ty(C);

Use real type here, not `auto`



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:133
+  GlobalVariable *createBinDesc(const MemoryBuffersVector ) {
+auto *EntriesB = new GlobalVariable(M, getEntryTy(), true,
+GlobalValue::ExternalLinkage, nullptr,

Add comment for the `true` constant with the name of parameter.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:136
+"__omp_offloading_entries_begin");
+auto *EntriesE = new GlobalVariable(M, getEntryTy(), true,
+GlobalValue::ExternalLinkage, nullptr,

Same here



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:143
+
+SmallVector ImagesInits;
+for (const auto  : Bufs) {

Comments?



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:144
+SmallVector ImagesInits;
+for (const auto  : Bufs) {
+  auto *Data = ConstantDataArray::get(

Use real type, not `auto`



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:148
+
+  auto *Image = new GlobalVariable(M, Data->getType(), true,
+  GlobalVariable::InternalLinkage,

Seems to me the code is not formatted



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:196
+{ getBinDescPtrTy() }, false);
+auto RegFuncC = M.getOrInsertFunction("__tgt_register_lib",
+  RegFuncTy);

Use real type, not `auto`



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:209
+  void createUnregisterFunction(GlobalVariable *BinDesc) {
+auto *FuncTy = FunctionType::get(Type::getVoidTy(C), {}, false);
+auto *Func = Function::Create(FuncTy, GlobalValue::InternalLinkage,

`llvm::None` instead of `{}`



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:216
+auto *UnRegFuncTy = FunctionType::get(Type::getVoidTy(C),
+  { getBinDescPtrTy() }, false);
+auto UnRegFuncC = M.getOrInsertFunction("__tgt_unregister_lib",

Remove extra braces here, they are not needed.



Comment at: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp:222
+IRBuilder<> Builder(BasicBlock::Create(C, "entry", Func));
+Builder.CreateCall(UnRegFuncC, { BinDesc });
+Builder.CreateRetVoid();

Extra braces


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

https://reviews.llvm.org/D64943



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

[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: yaxunl, tra, jlebar, hfinkel.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Previously, object files were bundled using partial linking. It resulted
in the following structure of the bundled objects:

  clang-offload-bundle
  __CLANG_OFFLOAD_BUNDLE__
  
  

But when we tried to unbundle object files, it worked correctly only for
the target objects. The host object remain bundled. It produced a lot of
junk sections in the host object files and in some cases may caused
incorrect linking.

Patch improves bundling of the object files. After this patch the
bundled object looks like this:

  clang-offload-bundle
  __CLANG_OFFLOAD_BUNDLE__
  
  __CLANG_OFFLOAD_BUNDLE__
  

With this structure we are able to unbundle the host object files too so
that fter unbundling they are the same as were before.


Repository:
  rC Clang

https://reviews.llvm.org/D65819

Files:
  test/Driver/clang-offload-bundler.c
  test/Driver/clang-offload-bundler.c.o
  tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -468,10 +468,7 @@
   return;
 }
 
-if (Content->size() < 2)
-  OS.write(Input.getBufferStart(), Input.getBufferSize());
-else
-  OS.write(Content->data(), Content->size());
+OS.write(Content->data(), Content->size());
   }
 
   void WriteHeader(raw_fd_ostream ,
@@ -539,12 +536,11 @@
 OS.close();
 SmallString<128> TargetName = getTriple(TargetNames[HostInputIndex]);
 std::vector ClangArgs = {"clang",
-"-r",
+"-c",
 "-target",
 TargetName.c_str(),
 "-o",
 OutputFileNames.front().c_str(),
-InputFileNames[HostInputIndex].c_str(),
 BitcodeFileName.c_str(),
 "-nostdlib"};
 
@@ -593,15 +589,10 @@
 // bundling into (the host input), this is just a place-holder, so a single
 // byte is sufficient.
 assert(HostInputIndex != ~0u && "Host input index undefined??");
-Constant *Content;
-if (NumberOfProcessedInputs == HostInputIndex + 1) {
-  uint8_t Byte[] = {0};
-  Content = ConstantDataArray::get(VMContext, Byte);
-} else
-  Content = ConstantDataArray::get(
-  VMContext, ArrayRef(reinterpret_cast(
-   Input.getBufferStart()),
-   Input.getBufferSize()));
+auto *Content = ConstantDataArray::get(
+VMContext, ArrayRef(reinterpret_cast(
+ Input.getBufferStart()),
+ Input.getBufferSize()));
 
 // Create the global in the desired section. We don't want these globals in
 // the symbol table, so we mark them private.
Index: test/Driver/clang-offload-bundler.c
===
--- test/Driver/clang-offload-bundler.c
+++ test/Driver/clang-offload-bundler.c
@@ -229,17 +229,18 @@
 
 // RUN: clang-offload-bundler -type=o 
-targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o -### -dump-temporary-files 
2>&1 \
 // RUN: | FileCheck %s --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: private constant [1 x i8] zeroinitializer, section 
"__CLANG_OFFLOAD_BUNDLE__host-powerpc64le-ibm-linux-gnu"
+// CK-OBJ-CMD: private constant [{{[0-9]+}} x i8] c"{{.+}}", section 
"__CLANG_OFFLOAD_BUNDLE__host-powerpc64le-ibm-linux-gnu"
 // CK-OBJ-CMD: private constant [{{[0-9]+}} x i8] c"Content of device file 
1{{.+}}", section "__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu"
 // CK-OBJ-CMD: private constant [{{[0-9]+}} x i8] c"Content of device file 
2{{.+}}", section "__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu"
-// CK-OBJ-CMD: clang{{(.exe)?}}" "-r" "-target" "powerpc64le-ibm-linux-gnu" 
"-o" "{{.+}}.o" "{{.+}}.o" "{{.+}}.bc" "-nostdlib"
+// CK-OBJ-CMD: clang{{(.exe)?}}" "-c" "-target" "powerpc64le-ibm-linux-gnu" 
"-o" "{{.+}}.o" "{{.+}}.bc" "-nostdlib"
 
-// RUN: clang-offload-bundler -type=o 
-targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.o,%t.res.tgt1,%t.res.tgt2 -inputs=%s.o -unbundle
-// RUN: diff %s.o %t.res.o
+// RUN: clang-offload-bundler -type=o 
-targets=host-powerpc64le-ibm-linux-gnu,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o
+// RUN: 

[PATCH] D65690: [clang-doc] Add index in each info html file

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368070: [clang-doc] Add index in each info html file 
(authored by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65690?vs=213513=213669#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65690

Files:
  clang-tools-extra/trunk/clang-doc/Generators.cpp
  clang-tools-extra/trunk/clang-doc/Generators.h
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/assets/clang-doc-default-stylesheet.css
  clang-tools-extra/trunk/clang-doc/assets/index.js
  clang-tools-extra/trunk/clang-doc/stylesheets/clang-doc-default-stylesheet.css
  clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/trunk/unittests/clang-doc/GeneratorTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/trunk/clang-doc/assets/clang-doc-default-stylesheet.css
===
--- clang-tools-extra/trunk/clang-doc/assets/clang-doc-default-stylesheet.css
+++ clang-tools-extra/trunk/clang-doc/assets/clang-doc-default-stylesheet.css
@@ -0,0 +1,205 @@
+body,div {
+  margin: 0;
+  padding: 0;
+}
+
+body[no-overflow] {
+  overflow: hidden;
+}
+
+li>p:first-child {
+  margin-top: 0;
+}
+
+li>p:last-child {
+  margin-bottom: 0;
+}
+
+html {
+  -webkit-box-sizing: border-box;
+  box-sizing: border-box;
+}
+
+*,*::before,*::after {
+  -webkit-box-sizing: inherit;
+  box-sizing: inherit;
+}
+
+body,html {
+  color: #202124;
+  font: 400 16px/24px Roboto,sans-serif;
+  -moz-osx-font-smoothing: grayscale;
+  -webkit-font-smoothing: antialiased;
+  height: 100%;
+  margin: 36px;
+  -webkit-text-size-adjust: 100%;
+  -moz-text-size-adjust: 100%;
+  -ms-text-size-adjust: 100%;
+  text-size-adjust: 100%;
+}
+
+body[devsite-framebox] {
+  overflow: hidden;
+  padding: 20px;
+}
+
+body[sitemask--active] {
+  overflow: hidden;
+}
+
+p {
+  margin: 16px 0;
+  padding: 0;
+}
+
+:link,:visited {
+  color: #039be5;
+  outline: 0;
+  text-decoration: none;
+}
+
+ul {
+  margin: 0;
+  padding-left: 40px;
+}
+
+ul {
+  list-style: disc outside;
+}
+
+li,li p {
+  margin: 12px 0;
+  padding: 0;
+}
+
+*[visually-hidden] {
+  opacity: 0 !important;
+  pointer-events: none !important;
+  visibility: hidden !important;
+}
+
+*[hidden] {
+  display: none !important;
+}
+
+[render-hidden] {
+  display: inline !important;
+  position: absolute !important;
+  visibility: hidden !important;
+}
+
+*[no-scroll] {
+  overflow: hidden;
+}
+
+@supports (display: flex) {
+  body[ready] .devsite-wrapper {
+display: -webkit-box;
+display: -ms-flexbox;
+display: flex;
+-webkit-box-orient: vertical;
+-webkit-box-direction: normal;
+-ms-flex-direction: column;
+flex-direction: column;
+  }
+}
+
+@media screen and (max-width: 840px) {
+  body[devsite-book-nav--open] {
+overflow: hidden;
+  }
+}
+
+h1,h2,h3,h4,h5,h6 {
+  overflow: hidden;
+  padding: 0;
+  text-overflow: ellipsis;
+}
+
+h1 {
+  color: #80868b;
+  font: 300 34px/40px Roboto,sans-serif;
+  letter-spacing: -0.01em;
+  margin: 40px 0 20px;
+}
+
+[layout=docs] h2 {
+  border-bottom: 1px solid #e8eaed;
+  padding-bottom: 3px;
+}
+
+h2 {
+  font: 300 24px/32px Roboto,sans-serif;
+  letter-spacing: -0.01em;
+  margin: 40px 0 20px;
+}
+
+h3 {
+  font: 400 20px/32px Roboto,sans-serif;
+  margin: 32px 0 16px;
+}
+
+h4,h5,h6 {
+  margin: 32px 0 16px;
+}
+
+h4 {
+  font: 500 16px/24px Roboto,sans-serif;
+}
+
+h5 {
+  font: 700 14px/24px Roboto,sans-serif;
+}
+
+h6 {
+  font: 500 14px/24px Roboto,sans-serif;
+}
+
+h1+h1,h1+h2,h1+h3,h1+h4,h1+h5,h1+h6,h2+h1,h2+h2,h2+h3,h2+h4,h2+h5,h2+h6,h3+h1,h3+h2,h3+h3,h3+h4,h3+h5,h3+h6,h4+h1,h4+h2,h4+h3,h4+h4,h4+h5,h4+h6,h5+h1,h5+h2,h5+h3,h5+h4,h5+h5,h5+h6,h6+h1,h6+h2,h6+h3,h6+h4,h6+h5,h6+h6 {
+  margin-top: 0;
+}
+
+@media screen and (max-width: 600px) {
+  h1 {
+font: 300 24px/32px Roboto,sans-serif;
+  }
+}
+
+[scrollbars]::-webkit-scrollbar {
+  height: 8px;
+  width: 8px;
+}
+
+[scrollbars]::-webkit-scrollbar-thumb {
+  background: rgba(128,134,139,.26);
+  border-radius: 8px;
+}
+
+[no-horizontal-scrollbars]::-webkit-scrollbar {
+  height: 0;
+  width: 0;
+}
+
+[scrollbars]::-webkit-scrollbar-corner {
+  background: 0;
+}
+
+[background] h2 {
+  color: #fff;
+}
+
+@media print {
+  body,  html,  

[clang-tools-extra] r368070 - [clang-doc] Add index in each info html file

[Diego Astiazaran via cfe-commits]

Author: diegoastiazaran
Date: Tue Aug  6 11:31:46 2019
New Revision: 368070

URL: http://llvm.org/viewvc/llvm-project?rev=368070=rev
Log:
[clang-doc] Add index in each info html file

An index structure is created while generating the output file for each
info. This structure is parsed to JSON and written to a file in the
output directory. The html for the index is not rendered by clang-doc. A
Javascript file is included in the output directory, this will the JSON
file and insert HTML elements into the file.

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

Added:
clang-tools-extra/trunk/clang-doc/assets/
clang-tools-extra/trunk/clang-doc/assets/clang-doc-default-stylesheet.css
clang-tools-extra/trunk/clang-doc/assets/index.js
clang-tools-extra/trunk/unittests/clang-doc/GeneratorTest.cpp
Removed:
clang-tools-extra/trunk/clang-doc/stylesheets/
Modified:
clang-tools-extra/trunk/clang-doc/Generators.cpp
clang-tools-extra/trunk/clang-doc/Generators.h
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
clang-tools-extra/trunk/clang-doc/Representation.cpp
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/tool/CMakeLists.txt
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.h
clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/Generators.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Generators.cpp?rev=368070=368069=368070=diff
==
--- clang-tools-extra/trunk/clang-doc/Generators.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Generators.cpp Tue Aug  6 11:31:46 2019
@@ -57,6 +57,57 @@ std::string getTagType(TagTypeKind AS) {
   llvm_unreachable("Unknown TagTypeKind");
 }
 
+bool Generator::createResources(ClangDocContext ) { return true; }
+
+// A function to add a reference to Info in Idx.
+// Given an Info X with the following namespaces: [B,A]; a reference to X will
+// be added in the children of a reference to B, which should be also a child 
of
+// a reference to A, where A is a child of Idx.
+//   Idx
+//|-- A
+//|--B
+//   |--X
+// If the references to the namespaces do not exist, they will be created. If
+// the references already exist, the same one will be used.
+void Generator::addInfoToIndex(Index , const doc::Info *Info) {
+  // Index pointer that will be moving through Idx until the first parent
+  // namespace of Info (where the reference has to be inserted) is found.
+  Index *I = 
+  // The Namespace vector includes the upper-most namespace at the end so the
+  // loop will start from the end to find each of the namespaces.
+  for (const auto  : llvm::reverse(Info->Namespace)) {
+// Look for the current namespace in the children of the index I is
+// pointing.
+auto It = std::find(I->Children.begin(), I->Children.end(), R.USR);
+if (It != I->Children.end()) {
+  // If it is found, just change I to point the namespace refererence 
found.
+  I = &*It;
+} else {
+  // If it is not found a new reference is created
+  I->Children.emplace_back(R.USR, R.Name, R.RefType, R.Path);
+  // I is updated with the reference of the new namespace reference
+  I = >Children.back();
+}
+  }
+  // Look for Info in the vector where it is supposed to be; it could already
+  // exist if it is a parent namespace of an Info already passed to this
+  // function.
+  auto It = std::find(I->Children.begin(), I->Children.end(), Info->USR);
+  if (It == I->Children.end()) {
+// If it is not in the vector it is inserted
+I->Children.emplace_back(Info->USR, Info->extractName(), Info->IT,
+ Info->Path);
+  } else {
+// If it not in the vector we only check if Path and Name are not empty
+// because if the Info was included by a namespace it may not have those
+// values.
+if (It->Path.empty())
+  It->Path = Info->Path;
+if (It->Name.empty())
+  It->Name = Info->extractName();
+  }
+}
+
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the generators.
 extern volatile int YAMLGeneratorAnchorSource;

Modified: clang-tools-extra/trunk/clang-doc/Generators.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Generators.h?rev=368070=368069=368070=diff
==
--- clang-tools-extra/trunk/clang-doc/Generators.h (original)
+++ clang-tools-extra/trunk/clang-doc/Generators.h Tue Aug  6 11:31:46 2019

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Just for reference, this patch still doesn't reuse the FileManager across 
invocations in a thread. We expect to get even better performance once we reuse 
it, but I'm going have to improve its API first.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63907



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

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:117
+std::mutex CacheLock;
+llvm::StringMap> Cache;
+  };

aganea wrote:
> Why not use a bump allocator here? (it would be per-thread) On Windows the 
> heap is always thread-safe, which induce a lock in `malloc` for each new 
> entry. You could also avoid the usage of `unique_ptr` by the same occasion:
> 
> `llvm::StringMap SpecificBumpPtrAllocator> Cache;`
> 
> //(unless you're planning on removing entries from the cache later on?)//
Good idea, I switched to a bump pointer allocator (I don't think I can use a 
specific one, as that would cause the values to be destroyed twice). I'm not 
planning on removing entries from the cache, no.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:76
+CachedFileSystemEntry
+CachedFileSystemEntry::createDirectoryEntry(llvm::vfs::Status Stat) {
+  assert(Stat.isDirectory() && "not a directory!");

aganea wrote:
> `llvm::vfs::Status &` to avoid a copy?
The copy should already be avoided, as I move the argument when passing in, and 
when it's assigned to the result.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63907



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