[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

[Luís Marques via Phabricator via cfe-commits]

luismarques added a comment.

@simoncook: your commit doesn't include handling the case of TLS lowering when 
`-ffixed-x4` is used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67185



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: mehdi_amini, pcc, tejohnson.
Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, 
steven_wu, aheejin, hiraditya, inglorion.
Herald added projects: clang, LLVM.

Currently the link phase has a object file cache whereas the compile phase 
always
perform optimizations (most likely happen for large source files and O2 
 or above)
which could potentially waste time optimizing a file that finally hit the 
object file cache.
For example, with Intel W-2133 and 64GB memory, compile X86ISelLowering.cpp 
with -flto=thin -O3
takes about 40s (takes about 10s with caching implemented by this patch).
The patch makes sure bitcodes that hit LTO cache also skip IR optimizations.

Add a driver/cc1 flag (-fthinlto-cache-dir, default off) to cache the minimized 
or regular ThinLTO bitcode file.
The caching is only trigger if the input is large than 
`-fthinlto-cache-min-filesize=`. Default minimum is 1024 IR instructions.
Cache pruning (`-fthinlto-cache-policy=`) shares the implementation with `lld 
--thinlto-cache-policy`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69327

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/thin_link_bitcode.c
  llvm/include/llvm/LTO/Caching.h
  llvm/lib/LTO/Caching.cpp

Index: llvm/lib/LTO/Caching.cpp
===
--- llvm/lib/LTO/Caching.cpp
+++ llvm/lib/LTO/Caching.cpp
@@ -28,7 +28,8 @@
 using namespace llvm::lto;
 
 Expected lto::localCache(StringRef CacheDirectoryPath,
-AddBufferFn AddBuffer) {
+AddBufferFn AddBuffer,
+StringRef Prefix) {
   if (std::error_code EC = sys::fs::create_directories(CacheDirectoryPath))
 return errorCodeToError(EC);
 
@@ -36,7 +37,7 @@
 // This choice of file name allows the cache to be pruned (see pruneCache()
 // in include/llvm/Support/CachePruning.h).
 SmallString<64> EntryPath;
-sys::path::append(EntryPath, CacheDirectoryPath, "llvmcache-" + Key);
+sys::path::append(EntryPath, CacheDirectoryPath, Prefix + Key);
 // First, see if we have a cache hit.
 SmallString<64> ResultPath;
 Expected FDOrErr = sys::fs::openNativeFileForRead(
Index: llvm/include/llvm/LTO/Caching.h
===
--- llvm/include/llvm/LTO/Caching.h
+++ llvm/include/llvm/LTO/Caching.h
@@ -31,7 +31,8 @@
 /// file callback. This function also creates the cache directory if it does not
 /// already exist.
 Expected localCache(StringRef CacheDirectoryPath,
-   AddBufferFn AddBuffer);
+   AddBufferFn AddBuffer,
+   StringRef Prefix = "llvmcache-");
 
 } // namespace lto
 } // namespace llvm
Index: clang/test/CodeGen/thin_link_bitcode.c
===
--- clang/test/CodeGen/thin_link_bitcode.c
+++ clang/test/CodeGen/thin_link_bitcode.c
@@ -6,6 +6,17 @@
 // RUN: %clang_cc1 -o %t.newpm -flto=thin -fexperimental-new-pass-manager -fthin-link-bitcode=%t.newpm.nodebug -triple x86_64-unknown-linux-gnu -emit-llvm-bc -debug-info-kind=limited  %s
 // RUN: llvm-bcanalyzer -dump %t.newpm | FileCheck %s
 // RUN: llvm-bcanalyzer -dump %t.newpm.nodebug | FileCheck %s --check-prefix=NO_DEBUG
+
+// Test optimized bitcode files caching
+// RUN: rm -Rf %t.cache && mkdir %t.cache
+// RUN: %clang_cc1 -o %t -flto=thin -fthin-link-bitcode=%t.nodebug -fthinlto-cache-dir=%t.cache -fthinlto-cache-min-filesize=1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -debug-info-kind=limited %s
+// RUN: ls %t.cache | count 3
+// RUN: llvm-bcanalyzer -dump %t.cache/llvmcache-bc-* | FileCheck %s
+// RUN: llvm-bcanalyzer -dump %t.cache/llvmcache-thinlink-* | FileCheck %s --check-prefix=NO_DEBUG
+
+// RUN: rm -Rf %t.cache && mkdir %t.cache
+// RUN: %clang_cc1 -o %t -flto=thin -fthin-link-bitcode=%t.nodebug -fthinlto-cache-dir=%t.cache -fthinlto-cache-min-filesize=100 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -debug-info-kind=limited %s
+// RUN: ls %t.cache | count 0
 int main (void) {
   return 0;
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -991,6 +991,11 @@
 
   Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ);
 
+  Opts.ThinLTOCacheDir = Args.getLastArgValue(OPT_fthinlto_cache_dir_EQ);
+  Opts.ThinLTOCachePolicy = Args.getLastArgValue(OPT_fthinl

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
javed.absar, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69328

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


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -919,7 +919,9 @@
   llvm::unique_function Action) {
   if (!PreambleTasks)
 return Action();
-  PreambleTasks->runAsync(Name, std::move(Action));
+  auto ActionWithCtx = [Ctx = Context::current().clone(),
+Action = std::move(Action)]() mutable { Action(); };
+  PreambleTasks->runAsync(Name, std::move(ActionWithCtx));
 }
 
 void TUScheduler::runWithAST(


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -919,7 +919,9 @@
   llvm::unique_function Action) {
   if (!PreambleTasks)
 return Action();
-  PreambleTasks->runAsync(Name, std::move(Action));
+  auto ActionWithCtx = [Ctx = Context::current().clone(),
+Action = std::move(Action)]() mutable { Action(); };
+  PreambleTasks->runAsync(Name, std::move(ActionWithCtx));
 }
 
 void TUScheduler::runWithAST(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

[Owen Pan via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D68914



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

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Sam McCall via Phabricator via cfe-commits]

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

Can you modify  `TEST_F(TUSchedulerTests, Run)` to verify?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59544 tests passed, 1 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman, jkorous, 
MaskRay.
Herald added a project: clang.

A certain class of bug (e.g. infloop on an AST worker thread) currently means
clangd never terminates, even if the editor shuts down the protocol and closes
our stdin, and the main thread recognizes that.

Instead, let's wait 60 seconds for threads to finish cleanly, and then crash
if they haven't.

(Obviously, we should still fix these bugs).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69329

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : 
static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped, background index shutting down).
+  //
+  // These should terminate quickly, and ~ClangdLSPServer blocks on them.
+  // However if a bug causes them to run forever, we want to ensure the process
+  // eventually exits. As clangd isn't directly user-facing, an editor can
+  // "leak" clangd processes. Crashing in this case contains the damage.
+  //
+  // This is more portable than sys::WatchDog, and yields a stack trace.
+  std::thread([] {
+std::this_thread::sleep_for(std::chrono::seconds(6));
+std::abort();
+  }).detach();
+
+  return ExitCode;
 }
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -44,6 +44,7 @@
   llvm::Optional CompileCommandsDir, bool UseDirBasedCDB,
   llvm::Optional ForcedOffsetEncoding,
   const ClangdServer::Options &Opts);
+  /// The destructor blocks on any outstanding background tasks.
   ~ClangdLSPServer();
 
   /// Run LSP server loop, communicating with the Transport provided in the
@@ -211,11 +212,11 @@
   std::unique_ptr BaseCDB;
   // CDB is BaseCDB plus any comands overridden via LSP extensions.
   llvm::Optional CDB;
-  // The ClangdServer is created by the "initialize" LSP method.
-  // It is destroyed before run() returns, to ensure worker threads exit.
   ClangdServer::Options ClangdServerOpts;
-  llvm::Optional Server;
   llvm::Optional NegotiatedOffsetEncoding;
+  // The ClangdServer is created by the "initialize" LSP method. It should be
+  // destroyed first, to ensure worker threads don't access other members.
+  llvm::Optional Server;
 };
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1241,8 +1241,6 @@
 CleanExit = false;
   }
 
-  // Destroy ClangdServer to ensure all worker threads finish.
-  Server.reset();
   return CleanExit && ShutdownRequestReceived;
 }
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped, background index shutting down).
+  //
+  // These should terminate quickly, and ~ClangdLSPServer blocks on them

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Sam McCall via Phabricator via cfe-commits]

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

Oops, this doesn't actually work - you're not setting the cloned context 
anywhere...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun updated this revision to Diff 226101.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp

Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand with a binary bitwise operator
+  
+  unsigned URes2 = URes << 1; //Ok
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  IResult = ~0; //Ok
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  test1 = 1 << 12, //Ok
+  test2 = one << two,
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  test3 = 1u << 12, //Ok
+};
Index: clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
+++ clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
@@ -7,3 +7,11 @@
 undefined or implementation defined behaviour.
 
 The according rule is defined in the `High Integrity C++ Standard, Section 5.6.1 `_.
+
+Options
+---
+
+.. option:: IgnorePositiveIntegerLiterals
+
+   If this option is set to `true`, the check will not warn on bitwise operations with positive integer literals, e.g. `~0`, `2 << 1`, etc. 
+   Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,10 @@
   ` check.
 
   Rewrites function signatures to use a trailing return type.
+  
+- The :doc:`hicpp-signed-bitwise
+  ` now supports ``IgnorePositiveIntegerLiterals``
+  option.
 
 Improvements to include-fixer
 -
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
@@ -17,9 +17,24 @@
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(intege

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun marked an inline comment as done.
vladimir.plyashkun added a comment.

In D68694#1711035 , @aaron.ballman 
wrote:

> LGTM aside from a small nit.


Thanks, fixed it!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D69263#1717985 , @hokein wrote:

> Thinking more about this -- we have a dynamic index (for all opened files) 
> which is overlaid on a static index (which is a background index in 
> open-source world), so for all affected files, they are either in
>
> 1. an open state -- we can rely on the dynamic index, I think it is safe to 
> assume that index always returns up-to-date results;


Not sure that holds. What if the file in question is being rebuilt right now? 
We do not wait until all ASTs are built (and the dynamic index gets the new 
results).
Open files actually pose an interesting problem: their contents do not 
correspond to the file contents on disk.
We should choose which of those we update on rename: dirty buffers or file 
contents? (I believe we should do dirty buffers, but I believe @sammccall has 
the opposite opinion, so we should sync on this)

> 2. a non-open state -- rely on the background index, however background index 
> has more chance to be stale (especially we don't detect file-change events at 
> the moment), we could do a range patch heuristically to mitigate this stale 
> issue. Failing that, we surface the error to users.

To summarize, I think files can be stale in both cases and we should patch the 
ranges in both cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Not sure 60 is enough, e.g. building a preamble can often take more than a 
minute and the users will see clangd crashing for (seemingly) no reason on 
shutdown.
In the long run, we should probably attempt to cancel long-running operations 
within a much shorter timeframe and a timeout of a minute would make more 
sense, though.




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:689
+  std::thread([] {
+std::this_thread::sleep_for(std::chrono::seconds(6));
+std::abort();

Did you mean 60 seconds?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329



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

[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: rsmith.
Herald added subscribers: usaxena95, erik.pilkington, kadircet, arphaman.
Herald added a project: clang.

Normally clang avoids creating expressions when it encounters semantic
errors, even if the parser knows which expression to produce.

This works well for the compiler. However, this is not ideal for
source-level tools that have to deal with broken code, e.g. clangd is
not able to provide navigation features even for names that compiler
knows how to resolve.

The new RecoveryExpr aims to capture the minimal set of information
useful for the tools that need to deal with incorrect code:

- source range of the expression being dropped,
- subexpressions of the expression.

We aim to make constructing RecoveryExprs as simple as possible to
ensure writing code to avoid dropping expressions is easy.

Producing RecoveryExprs can result in new code paths being taken in the
frontend. In particular, clang can produce some new diagnostics now and
we aim to suppress bogus ones based on `Stmt::containsErrors`.

We deliberately produce RecoveryExprs only in the parser for now to
minimize the code affected by this patch. Producing RecoveryExprs in
Sema potentially allows to preserve more information (e.g. type of an
expression), but also results in more code being affected. E.g.
SFINAE checks will have to take presence of RecoveryExprs into account.

Initial implementation only works in C++ mode, as it relies on compiler
postponing diagnostics on dependent expressions. C and ObjC often do not
do this, so they require more work to make sure we do not produce too
many bogus diagnostics on the new expressions.

See documentation of RecoveryExpr for more details.

This change is based on https://reviews.llvm.org/D61722


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69330

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/AST/ast-dump-expr-errors.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/Index/getcursor-recovery.cpp
  clang/test/Parser/cxx-concepts-ambig-constraint-expr.cpp
  clang/test/Parser/objcxx0x-lambda-expressions.mm
  clang/test/Parser/objcxx11-invalid-lambda.cpp
  clang/test/SemaCXX/builtin-operator-new-delete.cpp
  clang/test/SemaCXX/builtins.cpp
  clang/test/SemaCXX/cast-conversion.cpp
  clang/test/SemaCXX/cxx1z-copy-omission.cpp
  clang/test/SemaCXX/decltype-crash.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/test/SemaOpenCLCXX/address-space-references.cl
  clang/test/SemaTemplate/instantiate-init.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -289,6 +289,7 @@
   case Stmt::ObjCDictionaryLiteralClass:
   case Stmt::ObjCBoxedExprClass:
   case Stmt::ObjCSubscriptRefExprClass:
+  case Stmt::RecoveryExprClass:
 K = CXCursor_UnexposedExpr;
 break;
 
Index: clang/test/SemaTemplate/instantiate-init.cpp
===
--- clang/test/SemaTemplate/instantiate-init.cpp
+++ clang/test/SemaTemplate/instantiate-init.cpp
@@ -99,7 +99,7 @@
   void test() {
 integral_c<1> ic1 = array_lengthof(Description::data);
 (void)sizeof(array_lengthof(Description::data));
-
+// expected-warning@+1{{expression result unused}}
 sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
   Description::data // expected-note{{in instantiation of static data member 'PR7985::Description::data' requested here}}
   ));
Index: clang/test/SemaOpenCLCXX/address-space-references.cl
===
--- clang/test/SemaOpenCLCXX/address-space-references.cl
+++ clang/test/SemaOpenCLCXX/address-space-references.cl
@@ -11,5 +11,6 @@
 int bar(const unsigned int &i);
 
 void foo() {
-  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes addr

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:924
+Action = std::move(Action)]() mutable { Action(); };
+  PreambleTasks->runAsync(Name, std::move(ActionWithCtx));
 }

Maybe inline `ActionWithCtx`? This should look fine as the last argument of the 
function call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 226106.
ilya-biryukov added a comment.

- Add the added bit to serialization
- Mention contains-errors in the AST dump. Still not tests in this revision, 
see D69330  for an expression that is actually 
preserved and has this bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591

Files:
  clang/include/clang/AST/ASTDumperUtils.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprObjC.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -426,6 +426,7 @@
   Record.push_back(E->isValueDependent());
   Record.push_back(E->isInstantiationDependent());
   Record.push_back(E->containsUnexpandedParameterPack());
+  Record.push_back(E->containsErrors());
   Record.push_back(E->getValueKind());
   Record.push_back(E->getObjectKind());
 }
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2228,6 +2228,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //DeclRefExpr
@@ -2252,6 +2253,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //Integer Literal
@@ -2271,6 +2273,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //Character Literal
@@ -2290,6 +2293,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   // CastExpr
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -115,7 +115,7 @@
 
 /// The number of record fields required for the Expr class
 /// itself.
-static const unsigned NumExprFields = NumStmtFields + 7;
+static const unsigned NumExprFields = NumStmtFields + 8;
 
 /// Read and initialize a ExplicitTemplateArgumentList structure.
 void ReadTemplateKWAndArgsInfo(ASTTemplateKWAndArgsInfo &Args,
@@ -525,6 +525,7 @@
   E->setValueDependent(Record.readInt());
   E->setInstantiationDependent(Record.readInt());
   E->ExprBits.ContainsUnexpandedParameterPack = Record.readInt();
+  E->ExprBits.ContainsErrors = Record.readInt();
   E->setValueKind(static_cast(Record.readInt()));
   E->setObjectKind(static_cast(Record.readInt()));
   assert(Record.getIdx() == NumExprFields &&
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -127,6 +127,11 @@
   if (const auto *E = dyn_cast(Node)) {
 dumpType(E->getType());
 
+if (E->containsErrors()) {
+  ColorScope Color(OS, ShowColors, ErrorsColor);
+  OS << " contains-errors";
+}
+
 {
  

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59551 tests passed, 1 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 226109.
serge-sans-paille added a comment.

Temporarily comment out call support as free probe, everything else passes 
validation but a call may have some stack effect I don't handle (yet).


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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D69328#1718549 , @kadircet wrote:

> In D69328#1718509 , @sammccall wrote:
>
> > Oops, this doesn't actually work - you're not setting the cloned context 
> > anywhere...
>
>
> I am moving it into the lambda, which should extend the lifespan of context 
> until destruction of it. Why would I need to set it in addition to that?


Because we don't just want the context to be alive, it should be the context 
associated with the current thread.
e.g. if a new span is created, it should be nested within the other context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59544 tests passed, 1 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59344 tests passed, 2 failed and 812 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test
  failed: LLVM.tools/llvm-objdump/X86/disassemble-functions.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:139
+// that constructor. FIXME(nridge): this should probably be handled in
+// targetDecl() itself.
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {

nridge wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > nridge wrote:
> > > > I would appreciate some tips on how we could handle this in 
> > > > `targetDecl()`. Please see more specific comments below.
> > > My thinking is basically:
> > >  - C++ syntax is vague and doesn't give us great ways of referring 
> > > directly to constructors.
> > >  - there's value to simplicity, and I've found go-to-definition 
> > > additionally finding implicit nodes to be confusing as often as useful. I 
> > > think on balance and given the constraints of LSP, we should consider 
> > > dropping this and having implicit references be returned by find-refs but 
> > > not targetable by go-to-def/hover/etc. It's OK for simplicity of 
> > > implementation to be a concern here.
> > >  - the node that targets the constructor is the CXXConstructExpr. 
> > > Treating the VarDecl conditionally as a reference to the constructor, 
> > > while clever, seems like a hack. Ideally the fix is to make SelectionTree 
> > > yield the CXXConstructExpr, not to change TargetDecl.
> > >  - CXXConstructExpr is only sometimes implicit. SelectionTree should 
> > > return it for the parens in `Foo()` or `Foo x()`. And ideally for the 
> > > equals in `Foo x = {}`, though I think there's no CXXConstructExpr in the 
> > > AST in that case :-(
> > >  - When the AST modelling is bad (as it seems to be for some aspects 
> > > CXXConstructExpr) we should consider accepting some glitches and trying 
> > > to improve things in clang if they're important. Hacking around them will 
> > > lead to inconsistencies between different clangd features.
> > > 
> > > The TL;DR is I think I'd be happy to drop this special case and try to 
> > > make SelectionTree surface CXXConstructExpr more often, even if it 
> > > changes some behavior.
> > +1 to the idea of landing this and changing behavior.
> > I don't think we're loosing much in terms of functionality and I personally 
> > like the new code much more.
> FWIW, I do find being able to get from a variable declaration to the invoked 
> constructor to be very useful.
> 
> If the class has several constructors, there's no other easy way to determine 
> which one is being invoked (only other thing I can think of is to perform 
> "find references" on each constructor and see which one includes the variable 
> declaration as a reference, which is a lot more tedious), and I think that's 
> an important thing to be able to do (just like for named functions).
> 
> So I'd definitely like to keep this case working. However, I'd be fine with 
> only having the parens or braces target the constructor. (That's still 
> slightly annoying, as you have to place  the cursor more precisely, and it 
> also may not be obvious to users, but at least there's a way to do it.) I'm 
> also fine with changing the behaviour for now, and deferring constructor 
> targeting to a follow-up, as there are other benefits this patch brings.
We discussed this offline: finding constructor references is useful, but 
current approach does not generalize very well, e.g. for implicit constructor 
calls:
```
struct Foo {
  Foo(int);
};

int func(Foo a, Foo b);
int test() {
  func(1, 2); // no way to find Foo(1) and Foo(2) calls.
}
```

So we'd probably want some other mechanism to expose *all* possible references.
In any case, changing behavior now and tweaking it with a follow-up patch is 
probably the best way to proceed with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237



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

[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

[Dave Green via Phabricator via cfe-commits]

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

I've read this through again and it looks good. If no one else has any issues, 
then LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67161



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

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:59
+// different location. Contains both function signature and body.
+llvm::Optional moveFunctionDef(const FunctionDecl *FD) {
+  auto &SM = FD->getASTContext().getSourceManager();

the function name doesn't reflect what it does, it doesn't move the function, 
it just returns the source code of the function, I'd call it some name like 
`getFunctionSourceCode`.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:117
+const SourceManager &SM = Sel.AST.getSourceManager();
+llvm::StringRef FileName = SM.getFilename(Sel.Cursor);
+

The FileName is not always absolute, `getCorrespondingHeaderOrSource` is 
expecting an absolute file path input.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:119
+
+auto SourceFile = getSourceFile(FileName, Sel);
+if (!SourceFile)

could we use a better name, `Source` in the context here has two different 
meaning: 1) the corresponding .cc file 2) the source of moving function 
declaration (we called it `Source`);

Maybe call it `CCFile`?




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:157
+SMFF.get().getComposedLoc(SMFF.get().getMainFileID(), 
*InsertionOffset);
+const tooling::Replacement InsertFunctionDef(SMFF.get(), InsertLoc, 0,
+ *FuncDef);

maybe just `const tooling::Replacement InsertFunctionDef(Contents, 
*InsertionOffset, 0, *FunDef);`, which would save us a `InsertLoc`.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:166
+Sel.AST.getSourceManager(),
+CharSourceRange(Source->getBody()->getSourceRange(), /*ITR=*/true),
+";");

nit: I think 
`CharSourceRange::getCharRange(Source->getBody()->getSourceRange())` is clearer.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1574
+  ExtraFiles["Test.cpp"] = "";
+  EXPECT_EQ(apply("void fo^o() { return; }", &EditedFiles), "void foo() ;");
+  EXPECT_THAT(EditedFiles,

thinking more about this,  if this function is inline, we will leave an 
unnecessary `inline` keyword after running the code action. No need to address 
it in the patch, just keep in mind.

can we add more tests? e.g. template functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298



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

[PATCH] D69266: [clangd] Define out-of-line availability checks

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1525
+TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
+  // Not available unless in a header file.
+  EXPECT_UNAVAILABLE(R"cpp(

nit: please explicitly spell out the FileName, even the default value is 
`TestTU.cpp`.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1548
+
+  ExtraFiles["Test.cpp"] = "";
+  // Basic check for function body and signature.

Is this needed? It is unclear to me why we need it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69266



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

[PATCH] D69250: [ARM][AArch64] Implement __cls and __clsl intrinsics from ACLE

[Victor Campos via Phabricator via cfe-commits]

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



Comment at: clang/lib/Headers/arm_acle.h:150
+__clsl(unsigned long __t) {
+#if __SIZEOF_LONG__ == 4
+  return __builtin_arm_cls(__t);

compnerd wrote:
> vhscampos wrote:
> > compnerd wrote:
> > > I don't see a pattern match for the `cls64` on ARM32, would that not fail 
> > > to lower?
> > Yes. However, for now, I am not enabling support for `cls64` on ARM32 as it 
> > is not done yet.
> Is the difference not just the parameter type?  I think that implementing it 
> should be a trivial change to the existing implementation.  Is there a reason 
> that you are not implementing that?
At clang's side, yes, but not in the backend: Arm32 does not have a `cls` 
instruction, thus the CLS operations need to be custom lowered. In the 
`llvm.arm.cls(i32)` case, lowering is quite simple, and it's been included in 
this patch. For `llvm.arm.cls64(i64)`, on the other hand, it is not as trivial 
since it's necessary to break its logic into 32-bit instructions.

So the reason not to implement that (yet) is just to split work in two 
different efforts.



Comment at: clang/lib/Headers/arm_acle.h:155
+#endif
+}
+

compnerd wrote:
> vhscampos wrote:
> > compnerd wrote:
> > > Should we have a `__clsll` extension, otherwise these two are the same in 
> > > LLP64?  I'm thinking about the LLP64 environments, where `long` and `long 
> > > long` are different (32-bit vs 64-bit).
> > ACLE does provide a `long long` version of `cls` called `__clsll`. But 
> > since the support for `cls64` on Arm32 is not done yet, I decided not to 
> > write support for `__clsll`. If I did, it would work for 64-bit but not for 
> > 32-bit.
> > 
> > Please let me know what you think.
> clang supports Windows where `long` is 4-bytes even on 64-bit targets, and 
> this means that this doesn't work for that target.  I think that we need to 
> add `__clsll` so that 64-bit ARM at least is covered.
I'm not sure if I am following you. On AArch64-Windows, `__clsl` will be 
lowered to `llvm.aarch64.cls(i32)` which will then be custom lowered correctly. 
Let me know if I am thinking this wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69250



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

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 226122.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -688,6 +688,15 @@
   S.run("add 2", [&] { Counter += 2; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(Counter.load(), 3);
+
+  Notification TaskRun;
+  Key TestKey;
+  WithContextValue CtxWithKey(TestKey, 10);
+  S.run("props context", [&] {
+EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+TaskRun.notify();
+  });
+  TaskRun.wait();
 }
 
 TEST_F(TUSchedulerTests, TUStatus) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -44,6 +44,7 @@
 #include "TUScheduler.h"
 #include "Cancellation.h"
 #include "Compiler.h"
+#include "Context.h"
 #include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
@@ -919,7 +920,11 @@
   llvm::unique_function Action) {
   if (!PreambleTasks)
 return Action();
-  PreambleTasks->runAsync(Name, std::move(Action));
+  PreambleTasks->runAsync(Name, [Ctx = Context::current().clone(),
+ Action = std::move(Action)]() mutable {
+WithContext WC(std::move(Ctx));
+Action();
+  });
 }
 
 void TUScheduler::runWithAST(


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -688,6 +688,15 @@
   S.run("add 2", [&] { Counter += 2; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(Counter.load(), 3);
+
+  Notification TaskRun;
+  Key TestKey;
+  WithContextValue CtxWithKey(TestKey, 10);
+  S.run("props context", [&] {
+EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+TaskRun.notify();
+  });
+  TaskRun.wait();
 }
 
 TEST_F(TUSchedulerTests, TUStatus) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -44,6 +44,7 @@
 #include "TUScheduler.h"
 #include "Cancellation.h"
 #include "Compiler.h"
+#include "Context.h"
 #include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
@@ -919,7 +920,11 @@
   llvm::unique_function Action) {
   if (!PreambleTasks)
 return Action();
-  PreambleTasks->runAsync(Name, std::move(Action));
+  PreambleTasks->runAsync(Name, [Ctx = Context::current().clone(),
+ Action = std::move(Action)]() mutable {
+WithContext WC(std::move(Ctx));
+Action();
+  });
 }
 
 void TUScheduler::runWithAST(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69338: [clangd] Collect name references in the index.

[Haojian Wu via Phabricator via cfe-commits]

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

This is used for cross-file rename. When renaming a class, we expect to
rename all related constructors/destructors.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69338

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -626,6 +626,23 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
 }
 
+TEST_F(SymbolCollectorTest, NameReferences) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+class $foo[[Foo]] {
+public:
+  $foo[[Foo]]() {}
+  ~$foo[[Foo]]() {}
+};
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  // When we find references for class Foo, we expect to see all
+  // constructor/destructor references.
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges("foo");
+}
 
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -263,10 +263,6 @@
Decl::FriendObjectKind::FOK_None) &&
   !(Roles & static_cast(index::SymbolRole::Definition)))
 return true;
-  // Skip non-semantic references, we should start processing these when we
-  // decide to implement renaming with index support.
-  if ((Roles & static_cast(index::SymbolRole::NameReference)))
-return true;
   // A declaration created for a friend declaration should not be used as the
   // canonical declaration in the index. Use OrigD instead, unless we've 
already
   // picked a replacement for D


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -626,6 +626,23 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
 }
 
+TEST_F(SymbolCollectorTest, NameReferences) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+class $foo[[Foo]] {
+public:
+  $foo[[Foo]]() {}
+  ~$foo[[Foo]]() {}
+};
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  // When we find references for class Foo, we expect to see all
+  // constructor/destructor references.
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges("foo");
+}
 
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -263,10 +263,6 @@
Decl::FriendObjectKind::FOK_None) &&
   !(Roles & static_cast(index::SymbolRole::Definition)))
 return true;
-  // Skip non-semantic references, we should start processing these when we
-  // decide to implement renaming with index support.
-  if ((Roles & static_cast(index::SymbolRole::NameReference)))
-return true;
   // A declaration created for a friend declaration should not be used as the
   // canonical declaration in the index. Use OrigD instead, unless we've already
   // picked a replacement for D
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

[Simon Cook via Phabricator via cfe-commits]

simoncook added a comment.

In D67185#1718459 , @luismarques wrote:

> @simoncook: your commit doesn't include handling the case of TLS lowering 
> when `-ffixed-x4` is used.


I looked at this, and did start writing the patch that covers the use of TP/X4 
for TLS lowering, but didn't push it because it doesn't match the rest of the 
the cases I error on here. As it stands, what landed produces errors whenever a 
reserved register is //modified//, but not when they are read (so in for 
example the argument case you can use incoming arguments, but not lower calls 
to functions that need arguments.

For TLS I believe we would only be consuming X4, so erroring here would be 
inconsistent. I don't necessarily think producing an error whenever the 
compiler wishes to read a particular register is useful, for inlineasm there's 
no way to verify the validity of reading a particular register, but I don't 
think we can express that to the backend well anyway. I can work on a follow-up 
error/warn on all the reads case if we think that's of value, but as far as the 
intended purpose of stopping the compiler clobbering reserved registers, I 
don't think there's anything needed in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67185



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

[PATCH] D69338: [clangd] Collect name references in the index.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:633
+  Annotations Header(R"(
+class $foo[[Foo]] {
+public:

maybe use unnamed ranges?
having `$foo[[]]` instead of `[[]]` only hurt readability


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69338



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

[PATCH] D68877: [AArch64][SVE] Implement masked load intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin updated this revision to Diff 226123.
kmclaughlin added a comment.

- Removed unnecessary pseudo from SVEInstrFormats.td


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

https://reviews.llvm.org/D68877

Files:
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-masked-ldst-nonext.ll
  llvm/test/CodeGen/AArch64/sve-masked-ldst-sext.ll
  llvm/test/CodeGen/AArch64/sve-masked-ldst-zext.ll

Index: llvm/test/CodeGen/AArch64/sve-masked-ldst-zext.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-masked-ldst-zext.ll
@@ -0,0 +1,72 @@
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; Masked Loads
+;
+
+define  @masked_zload_nxv2i8(* %src,  %mask) {
+; CHECK-LABEL: masked_zload_nxv2i8:
+; CHECK-NOT: ld1sb
+; CHECK: ld1b { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv2i8(* %src, i32 1,  %mask,  undef)
+  %ext = zext  %load to 
+  ret  %ext
+}
+
+define  @masked_zload_nxv2i16(* %src,  %mask) {
+; CHECK-LABEL: masked_zload_nxv2i16:
+; CHECK-NOT: ld1sh
+; CHECK: ld1h { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv2i16(* %src, i32 1,  %mask,  undef)
+  %ext = zext  %load to 
+  ret  %ext
+}
+
+define  @masked_zload_nxv2i32(* %src,  %mask) {
+; CHECK-LABEL: masked_zload_nxv2i32:
+; CHECK-NOT: ld1sw
+; CHECK: ld1w { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv2i32(* %src, i32 1,  %mask,  undef)
+  %ext = zext  %load to 
+  ret  %ext
+}
+
+define  @masked_zload_nxv4i8(* %src,  %mask) {
+; CHECK-LABEL: masked_zload_nxv4i8:
+; CHECK-NOT: ld1sb
+; CHECK: ld1b { [[IN:z[0-9]+]].s }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv4i8(* %src, i32 1,  %mask,  undef)
+  %ext = zext  %load to 
+  ret  %ext
+}
+
+define  @masked_zload_nxv4i16(* %src,  %mask) {
+; CHECK-LABEL: masked_zload_nxv4i16:
+; CHECK-NOT: ld1sh
+; CHECK: ld1h { [[IN:z[0-9]+]].s }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv4i16(* %src, i32 1,  %mask,  undef)
+  %ext = zext  %load to 
+  ret  %ext
+}
+
+define  @masked_zload_nxv8i8(* %src,  %mask) {
+; CHECK-LABEL: masked_zload_nxv8i8:
+; CHECK-NOT: ld1sb
+; CHECK: ld1b { [[IN:z[0-9]+]].h }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv8i8(* %src, i32 1,  %mask,  undef)
+  %ext = zext  %load to 
+  ret  %ext
+}
+
+declare  @llvm.masked.load.nxv2i8(*, i32, , )
+declare  @llvm.masked.load.nxv2i16(*, i32, , )
+declare  @llvm.masked.load.nxv2i32(*, i32, , )
+declare  @llvm.masked.load.nxv4i8(*, i32, , )
+declare  @llvm.masked.load.nxv4i16(*, i32, , )
+declare  @llvm.masked.load.nxv8i8(*, i32, , )
Index: llvm/test/CodeGen/AArch64/sve-masked-ldst-sext.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-masked-ldst-sext.ll
@@ -0,0 +1,66 @@
+; RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; Masked Loads
+;
+
+define  @masked_sload_nxv2i8( *%a,  %mask) {
+; CHECK-LABEL: masked_sload_nxv2i8:
+; CHECK: ld1sb { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv2i8( *%a, i32 1,  %mask,  undef)
+  %ext = sext  %load to 
+  ret  %ext
+}
+
+define  @masked_sload_nxv2i16( *%a,  %mask) {
+; CHECK-LABEL: masked_sload_nxv2i16:
+; CHECK: ld1sh { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv2i16( *%a, i32 1,  %mask,  undef)
+  %ext = sext  %load to 
+  ret  %ext
+}
+
+define  @masked_sload_nxv2i32( *%a,  %mask) {
+; CHECK-LABEL: masked_sload_nxv2i32:
+; CHECK: ld1sw { [[IN:z[0-9]+]].d }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv2i32( *%a, i32 1,  %mask,  undef)
+  %ext = sext  %load to 
+  ret  %ext
+}
+
+define  @masked_sload_nxv4i8( *%a,  %mask) {
+; CHECK-LABEL: masked_sload_nxv4i8:
+; CHECK: ld1sb { [[IN:z[0-9]+]].s }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv4i8( *%a, i32 1,  %mask,  undef)
+  %ext = sext  %load to 
+  ret  %ext
+}
+
+define  @masked_sload_nxv4i16( *%a,  %mask) {
+; CHECK-LABEL: masked_sload_nxv4i16:
+; CHECK: ld1sh { [[IN:z[0-9]+]].s }, [[PG:p[0-9]+]]/z, [x0]
+; CHECK-NEXT: ret
+  %load = call  @llvm.masked.load.nxv4i16( *%a, i32 1,  %mas

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59544 tests passed, 1 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328



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

[PATCH] D68877: [AArch64][SVE] Implement masked load intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

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

In D68877#1717820 , @dmgreen wrote:

> I'm not sure if there is support yet for vector selects in the SVE codegen?


There is not yet support for vector selects, so for this patch the intention 
was that any passthru which is not all zero or undef would result in a 
selection failure.
Do you think it would acceptable to handle different passthrus in a future 
patch which also implements vector selects for SVE?




Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:4753
+  let hasSideEffects = 1, hasNoSchedulingInfo = 1, mayLoad = 1 in {
+  def "" : Pseudo<(outs listty:$Zt), (ins PPR3bAny:$Pg, GPR64sp:$Rn, 
simm4s1:$imm4), []>,
+   PseudoInstExpansion<(!cast(NAME # _REAL) listty:$Zt, 
PPR3bAny:$Pg, GPR64sp:$Rn, simm4s1:$imm4)>;

dmgreen wrote:
> Can you explain why is this pseudo is needed, exactly? I feel that using 
> pseudos is often the wrong solution to a problem (it may be required here, 
> I'm just sure why exactly).
> 
> We currently seem to generate ld1b (for example), over ldnf1b. Is there ever 
> a time that we expect to generate a nf load?
The pseudo was a workaround that was added downstream for non-faulting loads, 
but it is not needed here.


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

https://reviews.llvm.org/D68877



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

[PATCH] D69184: [libTooling] Introduce general combinator for fixed-value `MatchConsumer`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D69184#1718427 , @ymandel wrote:

> In D69184#1715256 , @gribozavr wrote:
>
> > What are the use cases for non-text values?
> >
> > I like the name `text` much better... If the name conflict is the issue, I 
> > think you could rename it to `toText` -- it would even read more fluently 
> > `change(toText("abc"))`, also making it obvious that we are not changing 
> > *the text abc*, but we are changing the code to say "abc".
>
>
> Another example use is: 
> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/RangeSelector.h#L29-L32,
>  although I must admit that I hadn't thought of that before this patch.  It's 
> just the natural generalization (it's actually the K combinator form SKI 
> fame).  So, I haven't thought about other examples -- the primary motivation 
> was just the naming.
>
> That said, I'm not sure about `toText` because I consider the "to" as 
> implicit in `change`. In fact, it might be a good idea to rename `change` to 
> `changeTo` (WDYT?).  What about any of `asText`, `textMsg`, `textMessage`? (I 
> realize the unfortunate pun in the last two, but not sure that's worth caring 
> about).


An alternative, is not to introduce a new combinator at all, and just use 
`Stencil`s:

  change(cat("abc"))

Or, add `operator()` to `StencilPart` and use those directly:

  change(text("abc"))

Given that `Stencil` and `Transformer` are bundled in the same library, is 
there any strong reason for a client to avoid a dependency on `Stencil`?

  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69184



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

[PATCH] D69338: [clangd] Collect name references in the index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:268
-  // decide to implement renaming with index support.
-  if ((Roles & static_cast(index::SymbolRole::NameReference)))
-return true;

This also implies that find refs on a class type will also report locations for 
constructors and destructors. I believe that's the behavior we decided to have 
in our offline discussions, just statin in here for future reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69338



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

[PATCH] D66564: [clang-tidy] new performance struct pack align check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I sort of understand why this was asked to be put into `performance`, but I'm 
not convinced that's the right place to put it. Performance could be degraded 
by packing structures on some architectures depending on how the objects are 
accessed. I worry that people will start packing or aligning structures and 
having degraded performance due to architecture-specific details or access 
pattern issues.

I sort of wonder whether this is best put in its own module (opencl or altera, 
depending on how generic the functionality is)?




Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:21
+
+constexpr int MAX_CONFIGURED_ALIGNMENT = 128;
+constexpr float SIZEOF_BYTE = 8.0;

Is there a reason this isn't a user option?



Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:22
+constexpr int MAX_CONFIGURED_ALIGNMENT = 128;
+constexpr float SIZEOF_BYTE = 8.0;
+

This shouldn't be needed; we already track the size of a byte. If we do need 
it, why a float?



Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:37
+  CharUnits::fromQuantity(((int)NewAlign.getQuantity()) * 2));
+  // Abort if the computed alignment meets the maximum configured alignment
+  if (NewAlign.getQuantity() >= MAX_CONFIGURED_ALIGNMENT)

You should add a full stop at the end of the comment (here and in the rest of 
the files).



Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:66-68
+  // FIXME: Express this as CharUnit rather than a hardcoded 8-bits (Rshift3)i
+  // After computing the minimum size in bits, check for an existing alignment
+  // flag

`Result.Context->getCharWidth()`?



Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:83
+  if (MinByteSize < CurrSize &&
+  ((Struct->getMaxAlignment() / 8) != NewAlign.getQuantity()) &&
+  (CurrSize != NewAlign) && !IsPacked) {

Should this be dividing by the bit-width of a char, not hard-coded to 8?



Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:87
+ "accessing fields in struct %0 is inefficient due to padding; only "
+ "needs %1 bytes but is using %2 bytes, use \"__attribute((packed))\"")
+<< Struct << (int)MinByteSize.getQuantity()

The diagnostic is a bit wordy, but I like the words. ;-) I think a better 
approach may be to split this into two diagnostics. The first one is a warning 
and the second is a note with a fixit to add the attribute to the structure 
declaration in the correct place. Also, it probably should be `__attribute__` 
instead of `__attribute`.



Comment at: clang-tidy/performance/StructPackAlignCheck.cpp:98
+ "currently aligned to %1 bytes, but size %3 bytes is large enough to "
+ "benefit from \"__attribute((aligned(%2)))\"")
+<< Struct << (int)CurrAlign.getQuantity() << 
(int)NewAlign.getQuantity()

Similar suggestion here to split into a warning and a note/fixit.



Comment at: docs/clang-tidy/checks/performance-struct-pack-align.rst:4
+performance-struct-pack-align
+==
+

The underlining does not look correct here.


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

https://reviews.llvm.org/D66564



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

[PATCH] D69338: [clangd] Collect name references in the index.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59565 tests passed, 2 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test
  failed: LLVM.tools/llvm-objdump/X86/disassemble-functions.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69338



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

[PATCH] D69328: [clangd] Propogate context in TUScheduler::run

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10c8dbcb840c: [clangd] Propogate context in TUScheduler::run 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69328

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -688,6 +688,15 @@
   S.run("add 2", [&] { Counter += 2; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(Counter.load(), 3);
+
+  Notification TaskRun;
+  Key TestKey;
+  WithContextValue CtxWithKey(TestKey, 10);
+  S.run("props context", [&] {
+EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+TaskRun.notify();
+  });
+  TaskRun.wait();
 }
 
 TEST_F(TUSchedulerTests, TUStatus) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -44,6 +44,7 @@
 #include "TUScheduler.h"
 #include "Cancellation.h"
 #include "Compiler.h"
+#include "Context.h"
 #include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
@@ -919,7 +920,11 @@
   llvm::unique_function Action) {
   if (!PreambleTasks)
 return Action();
-  PreambleTasks->runAsync(Name, std::move(Action));
+  PreambleTasks->runAsync(Name, [Ctx = Context::current().clone(),
+ Action = std::move(Action)]() mutable {
+WithContext WC(std::move(Ctx));
+Action();
+  });
 }
 
 void TUScheduler::runWithAST(


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -688,6 +688,15 @@
   S.run("add 2", [&] { Counter += 2; });
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(Counter.load(), 3);
+
+  Notification TaskRun;
+  Key TestKey;
+  WithContextValue CtxWithKey(TestKey, 10);
+  S.run("props context", [&] {
+EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+TaskRun.notify();
+  });
+  TaskRun.wait();
 }
 
 TEST_F(TUSchedulerTests, TUStatus) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -44,6 +44,7 @@
 #include "TUScheduler.h"
 #include "Cancellation.h"
 #include "Compiler.h"
+#include "Context.h"
 #include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
@@ -919,7 +920,11 @@
   llvm::unique_function Action) {
   if (!PreambleTasks)
 return Action();
-  PreambleTasks->runAsync(Name, std::move(Action));
+  PreambleTasks->runAsync(Name, [Ctx = Context::current().clone(),
+ Action = std::move(Action)]() mutable {
+WithContext WC(std::move(Ctx));
+Action();
+  });
 }
 
 void TUScheduler::runWithAST(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a minor nit.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:869-873
+if (Ident->isKeyword(getLangOpts())) {
+  Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;
+} else if (Ident->hasMacroDefinition()) {
+  Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition;
+}

You can elide the curly braces.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:868
+  if (CheckNewIdentifier != Idents.end() &&
+  CheckNewIdentifier->second->isKeyword(getLangOpts())) {
+Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;

Daniel599 wrote:
> Daniel599 wrote:
> > aaron.ballman wrote:
> > > What if changing it would switch to using a macro instead of a keyword? 
> > > e.g.,
> > > ```
> > > #define foo 12
> > > 
> > > void func(int Foo); // Changing Foo to foo would be bad, no?
> > > ```
> > That's another type of bug, just like the one I found 
> > https://bugs.llvm.org/show_bug.cgi?id=43306
> > I don't aim on solving all of them in one patch, my patch just fixes 
> > keywords.
> > Also, I don't think my patch makes the above situation worse.
> Regarding your comment about macro name, I can fix it using 
> `IdentifierInfo::hasMacroDefinition`.
> Should I fix it in this patch? I`ll add another value to `ShouldFixStatus` 
> and another error message
Thanks for fixing this!



Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:88
+
+bool ShouldNotify() const {
+  return (FixStatus == ShouldFixStatus::ShouldFix ||

Daniel599 wrote:
> aaron.ballman wrote:
> > This seems a bit confusing to me. It seems like the reason to not generate 
> > a fixit is being used to determine whether to diagnose at all, which seems 
> > incorrect despite sort of being what the old code did.
> I`m sorry, I didn't understand you.
> I tried to keep the old behavior of "cannot fix inside macro" and that's why 
> I needed the method `ShouldNotify`.
> Do you suggest another idea or other code flow?
I see now that the reason we don't diagnose is because the construct is within 
a macro, which is a pretty reasonable thing to do, so I'm no longer confused. 
:-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68539



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

[PATCH] D68877: [AArch64][SVE] Implement masked load intrinsics

[Dave Green via Phabricator via cfe-commits]

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

In D68877#1718729 , @kmclaughlin wrote:

> There is not yet support for vector selects, so for this patch the intention 
> was that any passthru which is not all zero or undef would result in a 
> selection failure.
>  Do you think it would acceptable to handle different passthrus in a future 
> patch which also implements vector selects for SVE?


Yeah OK. That makes sense. I remember the same chicken and egg from MVE. We 
ended up doing them the other way around there. Can you try and add a FIXME 
comment somewhere?

Otherwise LGTM.




Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:4753
+  let hasSideEffects = 1, hasNoSchedulingInfo = 1, mayLoad = 1 in {
+  def "" : Pseudo<(outs listty:$Zt), (ins PPR3bAny:$Pg, GPR64sp:$Rn, 
simm4s1:$imm4), []>,
+   PseudoInstExpansion<(!cast(NAME # _REAL) listty:$Zt, 
PPR3bAny:$Pg, GPR64sp:$Rn, simm4s1:$imm4)>;

kmclaughlin wrote:
> dmgreen wrote:
> > Can you explain why is this pseudo is needed, exactly? I feel that using 
> > pseudos is often the wrong solution to a problem (it may be required here, 
> > I'm just sure why exactly).
> > 
> > We currently seem to generate ld1b (for example), over ldnf1b. Is there 
> > ever a time that we expect to generate a nf load?
> The pseudo was a workaround that was added downstream for non-faulting loads, 
> but it is not needed here.
Righteo. We know where to find it if it turns out we do need them.


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

https://reviews.llvm.org/D68877



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

[clang] a9c3c17 - Reland "[Support] Add a way to run a function on a detached thread""

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-10-23T15:51:44+02:00
New Revision: a9c3c176ad741b9c2b915abc59dd977d0299c53f

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

LOG: Reland "[Support] Add a way to run a function on a detached thread""

This reverts commit 7bc7fe6b789d25d48d6dc71d533a411e9e981237.
The immediate callers have been fixed to pass nullopt where appropriate.

Added: 


Modified: 
clang/tools/libclang/CIndex.cpp
llvm/include/llvm/Support/Threading.h
llvm/lib/Support/CrashRecoveryContext.cpp
llvm/lib/Support/Threading.cpp
llvm/lib/Support/Unix/Threading.inc
llvm/lib/Support/Unix/Unix.h
llvm/lib/Support/Windows/Process.inc
llvm/lib/Support/Windows/Threading.inc
llvm/lib/Support/Windows/WindowsSupport.h
llvm/unittests/Support/Threading.cpp

Removed: 




diff  --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index 586aca0091d1..5df02d51d036 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -6607,7 +6607,9 @@ void clang_enableStackTraces(void) {
 
 void clang_executeOnThread(void (*fn)(void*), void *user_data,
unsigned stack_size) {
-  llvm::llvm_execute_on_thread(fn, user_data, stack_size);
+  llvm::llvm_execute_on_thread(
+  fn, user_data,
+  stack_size == 0 ? llvm::None : llvm::Optional(stack_size));
 }
 
 
//===--===//

diff  --git a/llvm/include/llvm/Support/Threading.h 
b/llvm/include/llvm/Support/Threading.h
index 46d413dc487b..bacab8fa23b6 100644
--- a/llvm/include/llvm/Support/Threading.h
+++ b/llvm/include/llvm/Support/Threading.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_SUPPORT_THREADING_H
 #define LLVM_SUPPORT_THREADING_H
 
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
 #include "llvm/Support/Compiler.h"
@@ -52,9 +53,8 @@ class Twine;
 /// false otherwise.
 bool llvm_is_multithreaded();
 
-/// llvm_execute_on_thread - Execute the given \p UserFn on a separate
-/// thread, passing it the provided \p UserData and waits for thread
-/// completion.
+/// Execute the given \p UserFn on a separate thread, passing it the provided 
\p
+/// UserData and waits for thread completion.
 ///
 /// This function does not guarantee that the code will actually be executed
 /// on a separate thread or honoring the requested stack size, but tries to do
@@ -62,10 +62,26 @@ bool llvm_is_multithreaded();
 ///
 /// \param UserFn - The callback to execute.
 /// \param UserData - An argument to pass to the callback function.
-/// \param RequestedStackSize - If non-zero, a requested size (in bytes) for
-/// the thread stack.
-void llvm_execute_on_thread(void (*UserFn)(void *), void *UserData,
-unsigned RequestedStackSize = 0);
+/// \param StackSizeInBytes - A requested size (in bytes) for the thread stack
+/// (or None for default)
+void llvm_execute_on_thread(
+void (*UserFn)(void *), void *UserData,
+llvm::Optional StackSizeInBytes = llvm::None);
+
+/// Schedule the given \p Func for execution on a separate thread, then return
+/// to the caller immediately. Roughly equivalent to
+/// `std::thread(Func).detach()`, except it allows requesting a specific stack
+/// size, if supported for the platform.
+///
+/// This function would report a fatal error if it can't execute the code
+/// on a separate thread.
+///
+/// \param Func - The callback to execute.
+/// \param StackSizeInBytes - A requested size (in bytes) for the thread stack
+/// (or None for default)
+void llvm_execute_on_thread_async(
+llvm::unique_function Func,
+llvm::Optional StackSizeInBytes = llvm::None);
 
 #if LLVM_THREADING_USE_STD_CALL_ONCE
 

diff  --git a/llvm/lib/Support/CrashRecoveryContext.cpp 
b/llvm/lib/Support/CrashRecoveryContext.cpp
index 9d13fce9cc52..8d8529b474ff 100644
--- a/llvm/lib/Support/CrashRecoveryContext.cpp
+++ b/llvm/lib/Support/CrashRecoveryContext.cpp
@@ -404,7 +404,10 @@ bool 
CrashRecoveryContext::RunSafelyOnThread(function_ref Fn,
  unsigned RequestedStackSize) {
   bool UseBackgroundPriority = hasThreadBackgroundPriority();
   RunSafelyOnThreadInfo Info = { Fn, this, UseBackgroundPriority, false };
-  llvm_execute_on_thread(RunSafelyOnThread_Dispatch, &Info, 
RequestedStackSize);
+  llvm_execute_on_thread(RunSafelyOnThread_Dispatch, &Info,
+ RequestedStackSize == 0
+ ? llvm::None
+ : llvm::Optional(RequestedStackSize));
   if (CrashRecoveryContextImpl *CRC = (CrashRecoveryContextImpl *)Impl)
 CRC->setSwitchedThread();
   return Info.Result;

diff  --git a/

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a minor issue.




Comment at: 
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:30-31
+  // Check not applicable in C++17 or newer.
+  if (getLangOpts().CPlusPlus17 || getLangOpts().CPlusPlus2a)
+return;
+

This should be in the `registerMatchers()` function so that we don't even 
bother trying to match anything. You don't need to do the `||` either; if 2a is 
enabled, that automatically enables all previous modes as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545



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

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D68913



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

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 226135.
sammccall added a comment.

change timeout to 5 min


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : 
static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped, background index shutting down).
+  //
+  // These should terminate quickly, and ~ClangdLSPServer blocks on them.
+  // However if a bug causes them to run forever, we want to ensure the process
+  // eventually exits. As clangd isn't directly user-facing, an editor can
+  // "leak" clangd processes. Crashing in this case contains the damage.
+  //
+  // This is more portable than sys::WatchDog, and yields a stack trace.
+  std::thread([] {
+std::this_thread::sleep_for(std::chrono::minutes(5));
+std::abort();
+  }).detach();
+
+  return ExitCode;
 }
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -44,6 +44,7 @@
   llvm::Optional CompileCommandsDir, bool UseDirBasedCDB,
   llvm::Optional ForcedOffsetEncoding,
   const ClangdServer::Options &Opts);
+  /// The destructor blocks on any outstanding background tasks.
   ~ClangdLSPServer();
 
   /// Run LSP server loop, communicating with the Transport provided in the
@@ -211,11 +212,11 @@
   std::unique_ptr BaseCDB;
   // CDB is BaseCDB plus any comands overridden via LSP extensions.
   llvm::Optional CDB;
-  // The ClangdServer is created by the "initialize" LSP method.
-  // It is destroyed before run() returns, to ensure worker threads exit.
   ClangdServer::Options ClangdServerOpts;
-  llvm::Optional Server;
   llvm::Optional NegotiatedOffsetEncoding;
+  // The ClangdServer is created by the "initialize" LSP method. It should be
+  // destroyed first, to ensure worker threads don't access other members.
+  llvm::Optional Server;
 };
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1241,8 +1241,6 @@
 CleanExit = false;
   }
 
-  // Destroy ClangdServer to ensure all worker threads finish.
-  Server.reset();
   return CleanExit && ShutdownRequestReceived;
 }
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped, background index shutting down).
+  //
+  // These should terminate quickly, and ~ClangdLSPServer blocks on them.
+  // However if a bug causes them to run forever, we want to ensure the process
+  // eventually exits. As clangd isn't directly user-facing, an editor can
+  // "leak" clangd processes. Crashing in this case contains the damage.
+  //
+  // This is more portable than sys::WatchDog, and yields a stack trace.
+  std::thread([] {
+std::this_thread::sleep_for(s

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 226134.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments
- Handle template parameters when copying function and add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1562,6 +1562,31 @@
 })cpp");
 }
 
+TEST_F(DefineOutlineTest, ApplyTest) {
+  FileName = "Test.hpp";
+
+  // No implementation file.
+  EXPECT_EQ(apply("void fo^o() { return; }"),
+"fail: Couldn't find a suitable implementation file.");
+
+  llvm::StringMap EditedFiles;
+  ExtraFiles["Test.cpp"] = "";
+
+  EXPECT_EQ(apply("void fo^o() { return; }", &EditedFiles), "void foo() ;");
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("Test.cpp"),
+"void foo() { return; }")));
+
+  EditedFiles.clear();
+  EXPECT_EQ(apply("template  void fo^o(T, T x) { return; }",
+  &EditedFiles),
+"template  void foo(T, T x) ;");
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("Test.cpp"),
+  "template  void foo(T, T x) { return; }")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -9,12 +9,19 @@
 #include "HeaderSourceSwitch.h"
 #include "Path.h"
 #include "Selection.h"
+#include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Driver/Types.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/Support/Path.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 
 namespace clang {
 namespace clangd {
@@ -39,6 +46,41 @@
   return nullptr;
 }
 
+// Returns the begining location for a FunctionDecl. Returns location of
+// template keyword for templated functions.
+// FIXME: This is shared with define inline, move them to a common header once
+// we have a place for such.
+const SourceLocation getBeginLoc(const FunctionDecl *FD) {
+  // Include template parameter list.
+  if (auto *FTD = FD->getDescribedFunctionTemplate())
+return FTD->getBeginLoc();
+  return FD->getBeginLoc();
+}
+
+llvm::Optional getSourceFile(llvm::StringRef FileName,
+   const Tweak::Selection &Sel) {
+  if (auto Source = getCorrespondingHeaderOrSource(
+  FileName,
+  &Sel.AST.getSourceManager().getFileManager().getVirtualFileSystem()))
+return *Source;
+  return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
+}
+
+// Creates a modified version of function definition that can be inserted at a
+// different location. Contains both function signature and body.
+llvm::Optional getFunctionSourceCode(const FunctionDecl *FD) {
+  auto &SM = FD->getASTContext().getSourceManager();
+  auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
+   FD->getSourceRange());
+  if (!CharRange)
+return llvm::None;
+  CharRange->setBegin(getBeginLoc(FD));
+
+  // FIXME: Qualify return type.
+  // FIXME: Qualify function name depending on the target context.
+  return toSourceCode(SM, *CharRange);
+}
+
 /// Moves definition of a function/method to an appropriate implementation file.
 ///
 /// Before:
@@ -84,8 +126,66 @@
   }
 
   Expected apply(const Selection &Sel) override {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Not implemented yet");
+const SourceManager &SM = Sel.AST.getSourceManager();
+llvm::StringRef FileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+
+auto CCFile = getSourceFile(FileName, Sel);
+if (!CCFile)
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Couldn't find a suitable implementation file.");
+
+auto &FS =
+Sel.AST.getSourceManager().getFileManager().getVirtualFileSystem();
+auto Buffer = FS.getBufferForFile(*CCFile);
+// FIXME: Maybe we should consider creating the implementation file if it
+//

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 7 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1574
+  ExtraFiles["Test.cpp"] = "";
+  EXPECT_EQ(apply("void fo^o() { return; }", &EditedFiles), "void foo() ;");
+  EXPECT_THAT(EditedFiles,

hokein wrote:
> thinking more about this,  if this function is inline, we will leave an 
> unnecessary `inline` keyword after running the code action. No need to 
> address it in the patch, just keep in mind.
> 
> can we add more tests? e.g. template functions.
added a fixme for inline stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298



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

[PATCH] D69225: Sema: Fixes a crash with a templated destructor

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman 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/D69225/new/

https://reviews.llvm.org/D69225



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

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Mostly comments about tests, the implementation itself LG.




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:68
 
+llvm::Optional getSemiColonForDecl(const FunctionDecl *FD) {
+  const SourceManager &SM = FD->getASTContext().getSourceManager();

NIT: s/SemiColon/Semicolon



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:79
+  Token CurToken;
+  while (!CurToken.is(tok::semi)) {
+if (RawLexer.LexFromRawLexer(CurToken))

What are the tokens we expect to see before the semicolon here?
It feels safer to just skip comments and whitespace and check the next token is 
a semicolon.

Any reason to have an actual loop here?



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:181
+  if (ND->getDeclContext() != Ref.Targets.front()->getDeclContext()) {
+elog("Targets from multiple contexts: {0}, {1}",
+ printQualifiedName(*Ref.Targets.front()), 
printQualifiedName(*ND));

NIT: could we mention the tweak name here? to make sure it's easy to detect 
those lines in the logs:
`define inline: targets from multiple contexts`



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:208
+
+  if (HadErrors) {
+return llvm::createStringError(

NIT: braces are redundant



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:320
   Expected apply(const Selection &Sel) override {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Not implemented yet");
+const ASTContext &AST = Sel.AST.getASTContext();
+const SourceManager &SM = AST.getSourceManager();

NIT: use auto 



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:325
+if (!SemiColon) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),

NIT: braces are redundant



Comment at: clang-tools-extra/clangd/unittests/TweakTesting.cpp:85
+std::string TweakTest::apply(llvm::StringRef MarkedCode,
+ llvm::StringMap *EditedFiles) const {
   std::string WrappedCode = wrap(Context, MarkedCode);

Could you document `EditedFiles` contains all other edited files and the 
original `std::string` only contains results for the main file?
Would also be useful to document what is the key in the StringMap: 
absolute/relative paths or URI?



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:905
+  EXPECT_EQ(apply(R"cpp(
+  void foo()/*Comment -_-*/  ;
+

Why have a comment in this test?
If we need to test we handle comments before the semicolon, could we move to a 
separate focused test?
If we want to test something else, let's leave out the comment. The test is 
hard enough to read on its own



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:922
+
+template  class Bar {};
+Bar z;

Template declarations inside function bodies are not allowed in C++.
Are we getting compiler errors here and not actually testing anything?



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:961
+
+  void foo()/*Comment -_-*/  ;
+

Same here: we don't need the comment here.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:964
+  void f^oo() {
+using namespace a;
+bar>.bar();

Either remove `using namespace a` from the function body or put a FIXME this 
shouldn't actually qualify?
I believe the first option is what we want, if we're trying to check template 
arguments are getting transformed properly



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:989
+
+MATCHER_P2(FileWithContents, FileName, Contents, "") {
+  return arg.first() == FileName && arg.second == Contents;

Maybe move this matcher to the start of the file?



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1029
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(

We prefer to have `using ::testing::ElementsAre` at the start of the file and 
not qualify the usage everywhere in our tests.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1076
+  template <>
+  void foo(int p);
+

This is really uncommon, I'm not even sure we should test this.
The other case is much more common and interesting:

```
template 
void foo(T p);

template <>
void foo(int p) { /* do something */ }

```

Could we add a test that we don't suggest this action in that case?



Comment at: clang-tools

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D69263#1718525 , @ilya-biryukov 
wrote:

> In D69263#1717985 , @hokein wrote:
>
> > Thinking more about this -- we have a dynamic index (for all opened files) 
> > which is overlaid on a static index (which is a background index in 
> > open-source world), so for all affected files, they are either in
> >
> > 1. an open state -- we can rely on the dynamic index, I think it is safe to 
> > assume that index always returns up-to-date results;
>
>
> Not sure that holds. What if the file in question is being rebuilt right now? 
> We do not wait until all ASTs are built (and the dynamic index gets the new 
> results).


I'm not sure this would happen quite often in practice (probably depends on 
users' behavior). but yes, patching ranges would mitigate this issue.

> Open files actually pose an interesting problem: their contents do not 
> correspond to the file contents on disk.
>  We should choose which of those we update on rename: dirty buffers or file 
> contents? (I believe we should do dirty buffers, but I believe @sammccall has 
> the opposite opinion, so we should sync on this)

I have the same feeling of using dirty buffers for open files, let's discuss 
offline.

>> 2. a non-open state -- rely on the background index, however background 
>> index has more chance to be stale (especially we don't detect file-change 
>> events at the moment), we could do a range patch heuristically to mitigate 
>> this stale issue. Failing that, we surface the error to users.
> 
> To summarize, I think files can be stale in both cases and we should patch 
> the ranges in both cases.






Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:268
-  // decide to implement renaming with index support.
-  if ((Roles & static_cast(index::SymbolRole::NameReference)))
-return true;

ilya-biryukov wrote:
> Not sure what was filtered out here.
> I suggest moving this to a separate change, with unit-tests clearly 
> describing the new symbols we do not filter out anymore and an explanation 
> why it's necessary.
> 
> WDYT?
Agreed. split it out.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:119
 
+llvm::Optional
+renameableOutsideFile(const NamedDecl &RenameDecl, const SymbolIndex *Index) {

ilya-biryukov wrote:
> So `llvm::None` means we do not reject?
> Probably worth documenting that.
> 
> Maybe even add an enumerator `ReasonToReject::Accept`, indicating the symbol 
> should be accepted and get rid of `Optional` altogether.
> 
> Otherwise this leads to code like:
> ```
> auto R = renameableOutsideFile(N, I);
> if (!R) { // negative condition.
>  return doRename(N, I); // But we're running the rename anyway... WAT?
> }
> ``
yeah, returning `None` means that we accept the rename.

Adding `Accept` is not aligning with the mental model, `Accept` doesn't belong 
to `ReasonToReject` I think. I agree the above given code is misleading, but 
looking at the current code in this file, it doesn't seem too bad, I think?

```
 if (auto Reject = renameableOutsideFile(*RenameDecl, RInputs.Index))
return makeError(*Reject);
```



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:121
+renameableOutsideFile(const NamedDecl &RenameDecl, const SymbolIndex *Index) {
+  if (RenameDecl.getParentFunctionOrMethod())
+return None; // function-local symbols

ilya-biryukov wrote:
> This function resembles `renamableWithinFile`.
> We should probably share implementation and pass an extra flag. Something 
> like `isRenamable(..., bool IsCrossFile)`.
> 
> WDYT?
though they look similar,  the logic is quite different, for example, we query 
the index in within-file rename to detect a symbol is used outside of the file 
which is not needed for cross-file rename, and cross-file rename allows fewer 
symbols (as our index doesn't support them), so I don't think we can share a 
lot of implementation.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:317
+  std::string OldName = RenameDecl->getNameAsString();
+  for (const auto &FileAndOccurrences : AffectedFiles) {
+llvm::StringRef FilePath = FileAndOccurrences.first();

ilya-biryukov wrote:
> I feel this code is fundamental to the trade-offs we made for rename.
> Could we move this to a separate function and add unit-tests for it?
> 
> You probably want to have something that handles just a single file rather 
> than all edits in all files, to avoid mocking VFS in tests, etc.
> 
> 
Agree, especially when we start implementing complicated stuff, e.g. range 
patching heuristics.

 Moved it out, but note that I don't plan to add more stuff in this initial 
patch, so the logic is pretty straightforward, just assembling rename 
replacement from occurrences.




Comment at: clang-tools-extra/clangd/refactor/Renam

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59568 tests passed, 1 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 226137.
hokein marked 13 inline comments as done.
hokein added a comment.

- address comments;
- add more FIXMEs;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,10 +96,9 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer &Server,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
   Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
   return std::move(*Result);
 }
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -22,6 +22,19 @@
   return replacementToEdit(Code, arg).range == Range;
 }
 
+RenameInputs localRenameInputs(const Annotations &Code, llvm::StringRef NewName,
+   llvm::StringRef Path,
+   const SymbolIndex *Index = nullptr) {
+  RenameInputs Inputs;
+  Inputs.Pos = Code.point();
+  Inputs.MainFileCode = Code.code();
+  Inputs.MainFilePath = Path;
+  Inputs.NewName = NewName;
+  Inputs.Index = Index;
+  Inputs.AllowCrossFile = false;
+  return Inputs;
+}
+
 TEST(RenameTest, SingleFile) {
   struct Test {
 const char* Before;
@@ -80,10 +93,13 @@
 auto TU = TestTU::withCode(Code.code());
 auto AST = TU.build();
 auto RenameResult =
-renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde");
+rename(AST, localRenameInputs(Code, "abcde", testPath(TU.Filename)));
+
 ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
-auto ApplyResult =
-tooling::applyAllReplacements(Code.code(), *RenameResult);
+ASSERT_TRUE(RenameResult->size() == 1);
+EXPECT_EQ(Code.code(), RenameResult->begin()->second.InitialCode);
+auto ApplyResult = tooling::applyAllReplacements(
+Code.code(), RenameResult->begin()->second.Replacements);
 ASSERT_TRUE(bool(ApplyResult)) << ApplyResult.takeError();
 
 EXPECT_EQ(T.After, *ApplyResult) << T.Before;
@@ -177,24 +193,27 @@
 }
 auto AST = TU.build();
 
-auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(),
-"dummyNewName", Case.Index);
+auto Results =
+rename(AST, localRenameInputs(T, "dummyNewName", testPath(TU.Filename),
+  Case.Index));
 bool WantRename = true;
 if (T.ranges().empty())
   WantRename = false;
 if (!WantRename) {
   assert(Case.ErrorMessage && "Error message must be set!");
-  EXPECT_FALSE(Results) << "expected renameWithinFile returned an error: "
-<< T.code();
+  EXPECT_FALSE(Results)
+  << "expected rename returned an error: " << T.code();
   auto ActualMessage = llvm::toString(Results.takeError());
   EXPECT_THAT(ActualMessage, testing::HasSubstr(Case.ErrorMessage));
 } else {
-  EXPECT_TRUE(bool(Results)) << "renameWithinFile returned an error: "
+  EXPECT_TRUE(bool(Results)) << "rename returned an error: "
  << llvm::toString(Results.takeErr

[PATCH] D69338: [clangd] Collect name references in the index.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 226139.
hokein marked an inline comment as done.
hokein added a comment.

address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69338

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -626,6 +626,23 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
 }
 
+TEST_F(SymbolCollectorTest, NameReferences) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+class [[Foo]] {
+public:
+  [[Foo]]() {}
+  ~[[Foo]]() {}
+};
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  // When we find references for class Foo, we expect to see all
+  // constructor/destructor references.
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
 
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -263,10 +263,6 @@
Decl::FriendObjectKind::FOK_None) &&
   !(Roles & static_cast(index::SymbolRole::Definition)))
 return true;
-  // Skip non-semantic references, we should start processing these when we
-  // decide to implement renaming with index support.
-  if ((Roles & static_cast(index::SymbolRole::NameReference)))
-return true;
   // A declaration created for a friend declaration should not be used as the
   // canonical declaration in the index. Use OrigD instead, unless we've 
already
   // picked a replacement for D


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -626,6 +626,23 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
 }
 
+TEST_F(SymbolCollectorTest, NameReferences) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+class [[Foo]] {
+public:
+  [[Foo]]() {}
+  ~[[Foo]]() {}
+};
+  )");
+  CollectorOpts.RefFilter = RefKind::All;
+  runSymbolCollector(Header.code(), "");
+  // When we find references for class Foo, we expect to see all
+  // constructor/destructor references.
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
+  HaveRanges(Header.ranges();
+}
 
 TEST_F(SymbolCollectorTest, HeaderAsMainFile) {
   CollectorOpts.RefFilter = RefKind::All;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -263,10 +263,6 @@
Decl::FriendObjectKind::FOK_None) &&
   !(Roles & static_cast(index::SymbolRole::Definition)))
 return true;
-  // Skip non-semantic references, we should start processing these when we
-  // decide to implement renaming with index support.
-  if ((Roles & static_cast(index::SymbolRole::NameReference)))
-return true;
   // A declaration created for a friend declaration should not be used as the
   // canonical declaration in the index. Use OrigD instead, unless we've already
   // picked a replacement for D
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov added a comment.

Adding myself to the reviewers since I've already looked at it. Unless anyone 
objects.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:219
+  // destroyed first, to ensure worker threads don't access other members.
+  llvm::Optional Server;
 };

Maybe call `Server.reset()` in destructor?
This ensures `ClangdServer` is terminated first even if the members are 
re-ordered later


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329



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

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

[Sam Elliott via Phabricator via cfe-commits]

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

Sorry for approving, and then requesting changes. I've been investigating 
issues with this patch.

When I try to use `-print-multi-lib` (a clang option that is very 
under-documented, but I think it's there to match GCC's option), this often 
doesn't always print out available multilibs, which is confusing.

1. `clang --gcc-toolchain=<..> --target=riscv32-unknown-elf -print-multi-lib` 
does print out available multilibs (as gcc would)
2. `clang --gcc-toolchain=<..> --target=riscv64-unknown-elf -print-multi-lib` 
does not print out anything.
3. `clang --gcc-toolchain=<..> --target=riscv64-unknown-elf -march=rv64gc 
-mabi=lp64d -print-multi-lib` does not print anything.
4. `clang --gcc-toolchain=<..> --target=riscv64-unknown-elf -march=rv64imafdc 
-mabi=lp64d -print-multi-lib` does print out available multilibs (as gcc would).

In all cases, I'm using a crosstool-ng configured toolchain for 
`riscv64-unknown-elf`.

I get the correct output for `2.` if I change the line noted in the line 
comment below. Note that choosing a reasonable default here has to match other 
defaults chosen in clang.

I think the reason that `3.` is not working is because of MULTILIB_REUSE. I do 
not think that blocks this patch, as I've said, this patch does not need to 
include MULTILIB_REUSE support.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1547
+  else if (IsRV64)
+MArch = "rv64i";
+  else

I think this line is the issue: where someone doesn't specify `-march`, you 
choose a default `-march` that does not appear in the list of multilib arches 
in `RISCVMultilibSet` above.

I think this issue probably didn't arise when you had `rv64i/lp64` in the list, 
but given that's not in the riscv-gcc `t-elf-multilib`, so this code should 
choose a new default. I don't know how this new default will affect defaults 
chosen in other parts of the clang codebase.


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

https://reviews.llvm.org/D67508



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

[PATCH] D69338: [clangd] Collect name references in the index.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59566 tests passed, 1 failed and 805 were skipped.

  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69338



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

[clang] be86fdb - [analyzer] Fix off-by-one in operator call parameter binding.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2019-10-23T08:17:02-07:00
New Revision: be86fdb86e1efd6921c81f25ac0c0a78903c0a2d

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

LOG: [analyzer] Fix off-by-one in operator call parameter binding.

Member operator declarations and member operator expressions
have different numbering of parameters and arguments respectively:
one of them includes "this", the other does not.

Account for this inconsistency when figuring out whether
the parameter needs to be manually rebound from the Environment
to the Store when entering a stack frame of an operator call,
as opposed to being constructed with a constructor and as such
already having the necessary Store bindings.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/test/Analysis/temporaries.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp 
b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 5f04a59ba055..d95f809bec1a 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -519,7 +519,7 @@ static void addParameterValuesToBindings(const 
StackFrameContext *CalleeCtx,
 
 // TODO: Support allocator calls.
 if (Call.getKind() != CE_CXXAllocator)
-  if (Call.isArgumentConstructedDirectly(Idx))
+  if (Call.isArgumentConstructedDirectly(Call.getASTArgumentIndex(Idx)))
 continue;
 
 // TODO: Allocators should receive the correct size and possibly alignment,

diff  --git a/clang/test/Analysis/temporaries.cpp 
b/clang/test/Analysis/temporaries.cpp
index 012cef52f14e..325b689c0deb 100644
--- a/clang/test/Analysis/temporaries.cpp
+++ b/clang/test/Analysis/temporaries.cpp
@@ -1231,3 +1231,19 @@ S bar3(int coin) {
   return coin ? S() : foo(); // no-warning
 }
 } // namespace return_from_top_frame
+
+#if __cplusplus >= 201103L
+namespace arguments_of_operators {
+struct S {
+  S() {}
+  S(const S &) {}
+};
+
+void test() {
+  int x = 0;
+  auto foo = [](S s, int &y) { y = 1; };
+  foo(S(), x);
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+}
+} // namespace arguments_of_operators
+#endif // __cplusplus >= 201103L



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59475 tests passed, 2 failed and 805 were skipped.

  failed: Clangd.Clangd/rename.test
  failed: LLVM.tools/llvm-ar/mri-utf8.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe86fdb86e1e: [analyzer] Fix off-by-one in operator call 
parameter binding. (authored by dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69155

Files:
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/temporaries.cpp


Index: clang/test/Analysis/temporaries.cpp
===
--- clang/test/Analysis/temporaries.cpp
+++ clang/test/Analysis/temporaries.cpp
@@ -1231,3 +1231,19 @@
   return coin ? S() : foo(); // no-warning
 }
 } // namespace return_from_top_frame
+
+#if __cplusplus >= 201103L
+namespace arguments_of_operators {
+struct S {
+  S() {}
+  S(const S &) {}
+};
+
+void test() {
+  int x = 0;
+  auto foo = [](S s, int &y) { y = 1; };
+  foo(S(), x);
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+}
+} // namespace arguments_of_operators
+#endif // __cplusplus >= 201103L
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -519,7 +519,7 @@
 
 // TODO: Support allocator calls.
 if (Call.getKind() != CE_CXXAllocator)
-  if (Call.isArgumentConstructedDirectly(Idx))
+  if (Call.isArgumentConstructedDirectly(Call.getASTArgumentIndex(Idx)))
 continue;
 
 // TODO: Allocators should receive the correct size and possibly alignment,


Index: clang/test/Analysis/temporaries.cpp
===
--- clang/test/Analysis/temporaries.cpp
+++ clang/test/Analysis/temporaries.cpp
@@ -1231,3 +1231,19 @@
   return coin ? S() : foo(); // no-warning
 }
 } // namespace return_from_top_frame
+
+#if __cplusplus >= 201103L
+namespace arguments_of_operators {
+struct S {
+  S() {}
+  S(const S &) {}
+};
+
+void test() {
+  int x = 0;
+  auto foo = [](S s, int &y) { y = 1; };
+  foo(S(), x);
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+}
+} // namespace arguments_of_operators
+#endif // __cplusplus >= 201103L
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -519,7 +519,7 @@
 
 // TODO: Support allocator calls.
 if (Call.getKind() != CE_CXXAllocator)
-  if (Call.isArgumentConstructedDirectly(Idx))
+  if (Call.isArgumentConstructedDirectly(Call.getASTArgumentIndex(Idx)))
 continue;
 
 // TODO: Allocators should receive the correct size and possibly alignment,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Sorry I somehow missed the fact that I was a reviewer :D

LGTM




Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:219
+  // destroyed first, to ensure worker threads don't access other members.
+  llvm::Optional Server;
 };

ilya-biryukov wrote:
> Maybe call `Server.reset()` in destructor?
> This ensures `ClangdServer` is terminated first even if the members are 
> re-ordered later
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329



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

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

In D64742#1717054 , @mehdi_amini wrote:

> In D64742#1606244 , @glider wrote:
>
> > As a data point, Linus Torvalds suggested that we need a similar feature 
> > for GCC so that the "kernel C standard" mandates zero-initialization for 
> > locals: https://lkml.org/lkml/2019/7/28/206
>
>
> I'm wondering why they never pushed all the way for a `-std=linux-c` flag 
> instead, and produced a documentation with the behavior they want with 
> respect to the base C standard they align on.


Guess this was never necessary, as GCC used to be the only compiler that could 
build the Linux kernel, and the standard could be defined by a combination of 
GCC flags.
This is no more the case, but adding a separate "standard" (which may end up 
being vaguely defined and containing ad-hoc hacks) will require a lot of effort 
from the three communities (Linux, GCC and LLVM)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64742



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

[clang-tools-extra] 8bda5f2 - [clangd] abort if shutdown takes more than a minute.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-10-23T17:52:59+02:00
New Revision: 8bda5f20674df1765bce8f0866204dff93ed244c

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

LOG: [clangd] abort if shutdown takes more than a minute.

Summary:
A certain class of bug (e.g. infloop on an AST worker thread) currently means
clangd never terminates, even if the editor shuts down the protocol and closes
our stdin, and the main thread recognizes that.

Instead, let's wait 60 seconds for threads to finish cleanly, and then crash
if they haven't.

(Obviously, we should still fix these bugs).

Reviewers: kadircet

Subscribers: MaskRay, jkorous, arphaman, jfb, usaxena95, cfe-commits, 
ilya-biryukov

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 3f825a6febdf..9e24878dc8f6 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1231,7 +1231,11 @@ ClangdLSPServer::ClangdLSPServer(
   // clang-format on
 }
 
-ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true; }
+ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true;
+  // Explicitly destroy ClangdServer first, blocking on threads it owns.
+  // This ensures they don't access any other members.
+  Server.reset();
+}
 
 bool ClangdLSPServer::run() {
   // Run the Language Server loop.
@@ -1241,8 +1245,6 @@ bool ClangdLSPServer::run() {
 CleanExit = false;
   }
 
-  // Destroy ClangdServer to ensure all worker threads finish.
-  Server.reset();
   return CleanExit && ShutdownRequestReceived;
 }
 

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index dbd90064537e..f1ed317f6bad 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -44,6 +44,7 @@ class ClangdLSPServer : private DiagnosticsConsumer {
   llvm::Optional CompileCommandsDir, bool UseDirBasedCDB,
   llvm::Optional ForcedOffsetEncoding,
   const ClangdServer::Options &Opts);
+  /// The destructor blocks on any outstanding background tasks.
   ~ClangdLSPServer();
 
   /// Run LSP server loop, communicating with the Transport provided in the
@@ -211,11 +212,10 @@ class ClangdLSPServer : private DiagnosticsConsumer {
   std::unique_ptr BaseCDB;
   // CDB is BaseCDB plus any comands overridden via LSP extensions.
   llvm::Optional CDB;
-  // The ClangdServer is created by the "initialize" LSP method.
-  // It is destroyed before run() returns, to ensure worker threads exit.
   ClangdServer::Options ClangdServerOpts;
-  llvm::Optional Server;
   llvm::Optional NegotiatedOffsetEncoding;
+  // The ClangdServer is created by the "initialize" LSP method.
+  llvm::Optional Server;
 };
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 4cb6604072a3..2639df31dbe8 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : 
static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped, background index shutting down).
+  //
+  // These should terminate quickly, and ~ClangdLSPServer blocks on them.
+  // However if a bug causes them to run forever, we want to ensure the process
+  // eventually exits. As clangd isn't directly user-facing, an editor can
+  // "leak" clangd processes. Crashing in this case contains the damage.
+  //
+  // This is more portable than sys::WatchDog, and yields a stack trace.
+  std::thread([] {
+std::this_thread::sleep_for(std::chrono::minutes(5));
+std::abort();
+  }).detach();
+
+  return ExitCode;
 }



___

[PATCH] D67216: [cfi] Add flag to always generate .debug_frame

[David Candler via Phabricator via cfe-commits]

dcandler updated this revision to Diff 226147.
dcandler added a comment.

Updated with the new name for the option.


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

https://reviews.llvm.org/D67216

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/fforce-dwarf-frame.c
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
  llvm/lib/CodeGen/CFIInstrInserter.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
  llvm/lib/Target/ARC/ARCRegisterInfo.cpp
  llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
  llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
  llvm/test/CodeGen/ARM/dwarf-frame.ll

Index: llvm/test/CodeGen/ARM/dwarf-frame.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/dwarf-frame.ll
@@ -0,0 +1,38 @@
+; RUN: llc -mtriple armv7-unknown -frame-pointer=all -filetype=asm -o - %s | FileCheck %s --check-prefix=CHECK-NO-CFI
+; RUN: llc -mtriple armv7-unknown -frame-pointer=all -filetype=asm -force-dwarf-frame-section -o - %s | FileCheck %s --check-prefix=CHECK-ALWAYS-CFI
+
+declare void @dummy_use(i32*, i32)
+
+define void @test_basic() #0 {
+%mem = alloca i32, i32 10
+call void @dummy_use (i32* %mem, i32 10)
+  ret void
+}
+
+; CHECK-NO-CFI-LABEL: test_basic:
+; CHECK-NO-CFI:   .fnstart
+; CHECK-NO-CFI-NOT:   .cfi_sections .debug_frame
+; CHECK-NO-CFI-NOT:   .cfi_startproc
+; CHECK-NO-CFI:   @ %bb.0:
+; CHECK-NO-CFI:   push {r11, lr}
+; CHECK-NO-CFI-NOT:   .cfi_def_cfa_offset 8
+; CHECK-NO-CFI-NOT:   .cfi_offset lr, -4
+; CHECK-NO-CFI-NOT:   .cfi_offset r11, -8
+; CHECK-NO-CFI:   mov r11, sp
+; CHECK-NO-CFI-NOT:   .cfi_def_cfa_register r11
+; CHECK-NO-CFI-NOT:   .cfi_endproc
+; CHECK-NO-CFI:   .fnend
+
+; CHECK-ALWAYS-CFI-LABEL: test_basic:
+; CHECK-ALWAYS-CFI:   .fnstart
+; CHECK-ALWAYS-CFI:   .cfi_sections .debug_frame
+; CHECK-ALWAYS-CFI:   .cfi_startproc
+; CHECK-ALWAYS-CFI:   @ %bb.0:
+; CHECK-ALWAYS-CFI:   push {r11, lr}
+; CHECK-ALWAYS-CFI:   .cfi_def_cfa_offset 8
+; CHECK-ALWAYS-CFI:   .cfi_offset lr, -4
+; CHECK-ALWAYS-CFI:   .cfi_offset r11, -8
+; CHECK-ALWAYS-CFI:   mov r11, sp
+; CHECK-ALWAYS-CFI:   .cfi_def_cfa_register r11
+; CHECK-ALWAYS-CFI:   .cfi_endproc
+; CHECK-ALWAYS-CFI:   .fnend
Index: llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
===
--- llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
+++ llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
@@ -203,7 +203,7 @@
 }
 
 bool XCoreRegisterInfo::needsFrameMoves(const MachineFunction &MF) {
-  return MF.getMMI().hasDebugInfo() || MF.getFunction().needsUnwindTableEntry();
+  return MF.needsFrameMoves();
 }
 
 const MCPhysReg *
Index: llvm/lib/Target/X86/X86InstrInfo.cpp
===
--- llvm/lib/Target/X86/X86InstrInfo.cpp
+++ llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -3963,9 +3963,7 @@
   MachineFunction &MF = *MBB.getParent();
   const X86FrameLowering *TFL = Subtarget.getFrameLowering();
   bool IsWin64Prologue = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
-  bool NeedsDwarfCFI =
-  !IsWin64Prologue &&
-  (MF.getMMI().hasDebugInfo() || MF.getFunction().needsUnwindTableEntry());
+  bool NeedsDwarfCFI = !IsWin64Prologue && MF.needsFrameMoves();
   bool EmitCFI = !TFL->hasFP(MF) && NeedsDwarfCFI;
   if (EmitCFI) {
 TFL->BuildCFI(MBB, I, DL,
Index: llvm/lib/Target/X86/X86FrameLowering.cpp
===
--- llvm/lib/Target/X86/X86FrameLowering.cpp
+++ llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -993,8 +993,7 @@
   bool NeedsWinFPO =
   !IsFunclet && STI.isTargetWin32() && MMI.getModule()->getCodeViewFlag();
   bool NeedsWinCFI = NeedsWin64CFI || NeedsWinFPO;
-  bool NeedsDwarfCFI =
-  !IsWin64Prologue && (MMI.hasDebugInfo() || Fn.needsUnwindTableEntry());
+  bool NeedsDwarfCFI = !IsWin64Prologue && MF.needsFrameMoves();
   Register FramePtr = TRI->getFrameRegister(MF);
   const Register MachineFramePtr =
   STI.isTarget64BitILP32()
@@ -1614,10 +1613,9 @@
   bool HasFP = hasFP(MF);
   uint64_t NumBytes = 0;
 
-  bool NeedsDwarfCFI =
-  (!MF.getTarget().getTargetTriple().isOSDarwin() &&
-   !MF.getTarget().getTargetTriple().isOSWindows()) &&
-  (MF.getMMI().hasDebugInfo() || MF.getFunction().needsUnwindTableEntry());
+  bool NeedsDwarfCFI = (!MF.getTarget().getTargetTriple().isOSDarwin() &&
+!MF.get

[PATCH] D69329: [clangd] abort if shutdown takes more than a minute.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bda5f20674d: [clangd] abort if shutdown takes more than a 
minute. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D69329?vs=226135&id=226148#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69329

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : 
static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped, background index shutting down).
+  //
+  // These should terminate quickly, and ~ClangdLSPServer blocks on them.
+  // However if a bug causes them to run forever, we want to ensure the process
+  // eventually exits. As clangd isn't directly user-facing, an editor can
+  // "leak" clangd processes. Crashing in this case contains the damage.
+  //
+  // This is more portable than sys::WatchDog, and yields a stack trace.
+  std::thread([] {
+std::this_thread::sleep_for(std::chrono::minutes(5));
+std::abort();
+  }).detach();
+
+  return ExitCode;
 }
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -44,6 +44,7 @@
   llvm::Optional CompileCommandsDir, bool UseDirBasedCDB,
   llvm::Optional ForcedOffsetEncoding,
   const ClangdServer::Options &Opts);
+  /// The destructor blocks on any outstanding background tasks.
   ~ClangdLSPServer();
 
   /// Run LSP server loop, communicating with the Transport provided in the
@@ -211,11 +212,10 @@
   std::unique_ptr BaseCDB;
   // CDB is BaseCDB plus any comands overridden via LSP extensions.
   llvm::Optional CDB;
-  // The ClangdServer is created by the "initialize" LSP method.
-  // It is destroyed before run() returns, to ensure worker threads exit.
   ClangdServer::Options ClangdServerOpts;
-  llvm::Optional Server;
   llvm::Optional NegotiatedOffsetEncoding;
+  // The ClangdServer is created by the "initialize" LSP method.
+  llvm::Optional Server;
 };
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1231,7 +1231,11 @@
   // clang-format on
 }
 
-ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true; }
+ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true;
+  // Explicitly destroy ClangdServer first, blocking on threads it owns.
+  // This ensures they don't access any other members.
+  Server.reset();
+}
 
 bool ClangdLSPServer::run() {
   // Run the Language Server loop.
@@ -1241,8 +1245,6 @@
 CleanExit = false;
   }
 
-  // Destroy ClangdServer to ensure all worker threads finish.
-  Server.reset();
   return CleanExit && ShutdownRequestReceived;
 }
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -670,6 +671,24 @@
   /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
   OffsetEncodingFromFlag, Opts);
   llvm::set_thread_name("clangd.main");
-  return LSPServer.run() ? 0
- : static_cast(ErrorResultCode::NoShutdownRequest);
+  int ExitCode = LSPServer.run()
+ ? 0
+ : static_cast(ErrorResultCode::NoShutdownRequest);
+  log("LSP finished, exiting with status {0}", ExitCode);
+
+  // There may still be lingering background threads (e.g. slow requests
+  // whose results will be dropped

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 226163.
Typz added a comment.

Rebased on master, integrating required code from 
https://reviews.llvm.org/D32478


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5734,9 +5734,9 @@
getLLVMStyleWithColumns(60));
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
-   "  : bbb //\n"
-   "? ccc\n"
-   ": ddd;");
+   "  : bbb //\n"
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"
"  ? aaa\n"
"  : (bbb //\n"
@@ -5798,6 +5798,113 @@
" // comment\n"
" ? a = b\n"
" : a;");
+
+  // Chained conditionals
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 70;
+  Style.AlignOperands = true;
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return aa ? \n"
+   "   :  ? \n"
+   "  : ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? 22\n"
+   ": 33;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   : cc ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? (aaa ? bbb : ccc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   ": (aaa ? bbb : ccc);",
+   Style);
+  verifyFormat("return  ? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? (a ? bb\n"
+   " : cc)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a = (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? a + (a ? bb\n"
+   " : dd)\n"
+   "   : bb ? \n"
+   ": ;",
+   Style);
+  verifyFormat("return a? \n"
+   "   : bb ? \n"
+   ": a + (a ? bb\n"
+   " : dd)\n",
+   Style);
+  verifyFormat("return  ? \n"
+   "   : bb ? \n"
+   "   

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

[Andrew Paverd via Phabricator via cfe-commits]

ajpaverd updated this revision to Diff 226164.
ajpaverd added a comment.

Split cfguard_setjmp test into target-specific tests and add missing 
cfguard_longjmp pass for ARM and AArch64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-fallback.c
  clang/test/Driver/cl-options.c
  llvm/docs/LangRef.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/LLVMBuild.txt
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/LLVMBuild.txt
  llvm/lib/Target/X86/LLVMBuild.txt
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/CFGuard/CMakeLists.txt
  llvm/lib/Transforms/CFGuard/LLVMBuild.txt
  llvm/lib/Transforms/CMakeLists.txt
  llvm/lib/Transforms/LLVMBuild.txt
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/calling-conventions.3.2.ll.bc
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll
  llvm/test/CodeGen/X86/cfguard-x86-64-vectorcall.ll
  llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll

Index: llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
@@ -0,0 +1,43 @@
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; Control Flow Guard is currently only available on Windows
+
+
+; Test that Control Flow Guard checks are correctly added for x86 vector calls.
+define void @func_cf_vector_x86(void (%struct.HVA)* %0, %struct.HVA* %1) #0 {
+entry:
+  %2 = alloca %struct.HVA, align 8
+  %3 = bitcast %struct.HVA* %2 to i8*
+  %4 = bitcast %struct.HVA* %1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %3, i8* align 8 %4, i32 32, i1 false)
+  %5 = load %struct.HVA, %struct.HVA* %2, align 8
+  call x86_vectorcallcc void %0(%struct.HVA inreg %5)
+  ret void
+
+  ; X32-LABEL: func_cf_vector_x86
+  ; X32: 	 movl 12(%ebp), %eax
+  ; X32: 	 movl 8(%ebp), %ecx
+  ; X32: 	 movsd 24(%eax), %xmm4 # xmm4 = mem[0],zero
+  ; X32: 	 movsd %xmm4, 24(%esp)
+  ; X32: 	 movsd 16(%eax), %xmm5 # xmm5 = mem[0],zero
+  ; X32: 	 movsd %xmm5, 16(%esp)
+  ; X32: 	 movsd (%eax), %xmm6   # xmm6 = mem[0],zero
+  ; X32: 	 movsd 8(%eax), %xmm7  # xmm7 = mem[0],zero
+  ; X32: 	 movsd %xmm7, 8(%esp)
+  ; X32: 	 movsd %xmm6, (%esp)
+  ; X32: 	 calll *___guard_check_icall_fptr
+  ; X32: 	 movaps %xmm6, %xmm0
+  ; X32: 	 movaps %xmm

LLVM buildmaster will be updated and restarted later today

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted on or after 5:00 PM PDT.

Thanks

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

[PATCH] D68340: Add AIX toolchain and basic linker functionality

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.



Comment at: clang/lib/Driver/CMakeLists.txt:33
   ToolChains/Arch/X86.cpp
+  ToolChains/AIX.cpp
   ToolChains/Ananas.cpp

Looks like this list is following alphabetical order here, which means we 
should probably put "ToolChains/AIX.cpp" right after "ToolChain.cpp".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340



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

[PATCH] D68340: Add AIX toolchain and basic linker functionality

[Steven Wan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/CMakeLists.txt:33
   ToolChains/Arch/X86.cpp
+  ToolChains/AIX.cpp
   ToolChains/Ananas.cpp

jasonliu wrote:
> Looks like this list is following alphabetical order here, which means we 
> should probably put "ToolChains/AIX.cpp" right after "ToolChain.cpp".
I had the same doubt when I added it. There is definitely an alphabetical 
order, yet I found the distinction between Arch and OS took precedence. As we 
could see that the "n" in "Ananas" and "M" in "AMDGPU" would have both came 
before the "r" in "Arch" if they only followed alphabetical order. That said, 
we could still consider moving all three but that might be an unnecessary 
hassle, and having OS followed by Arch and back to OS seems a bit 
counter-intuitive after all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340



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

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

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

https://github.com/ClangBuiltLinux/linux/issues/488#issuecomment-545218125


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

https://reviews.llvm.org/D69292



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

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this! I don't think that the check should live in `misc`. It 
should have an alias in the `CERT` module, but maybe we want to have this live 
in a `posix` module? If not, this should probably live in `bugprone`.




Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:31
+void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) 
{
+  const auto IsSigterm = [](const auto &KeyValue) -> bool {
+return KeyValue.first->getName() == "SIGTERM";

You can drop the trailing return type on the lambda.



Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:35
+  const auto TryExpandAsInteger =
+  [PP = PP](Preprocessor::macro_iterator It) -> Optional {
+if (It == PP->macro_end())

This lambda capture looks suspicious -- why do you need to initialize the 
capture?



Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:59
+diag(MatchedExpr->getBeginLoc(),
+ "Thread should not be terminated by SIGTERM signal.");
+  }

Clang-tidy diagnostics are not supposed to be grammatically correct. I think a 
better way to phrase it might be something like: `thread should not be 
terminated by raising the 'SIGTERM' signal`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst:6
+
+Finds ``pthread_kill`` function calls when thread is terminated by 
+``SIGTERM`` signal and the signal kills the entire process, not just the

when thread is terminated by -> when a thread is terminated by raising the



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst:8
+``SIGTERM`` signal and the signal kills the entire process, not just the
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+

Why does the check not look for `SIGKILL` as well as `SIGTERM`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69181



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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

I agree with the changes and want to see this go in, but it needs a test case.


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

https://reviews.llvm.org/D69292



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

I haven't read through the patch in detail yet, but from the description it 
sounds like a cache is being added for the compile step outputs, e.g. the 
bitcode output object of the "clang -flto=thin -c" step? Typically the build 
system takes care of incremental build handling for these explicit outputs, 
just as it would for a non-LTO clang compile output. Can you clarify the 
motivation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69327



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

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Needs a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68912



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

You know that feeling when you are doing a review and you think... I'm just not 
a compiler... I feel given the previous discussion and the level of extra tests 
maybe this is just worth going ahead with as long as you are prepared to 
support it in the interim.




Comment at: clang/unittests/Format/FormatTest.cpp:5739
+   "  ? ccc\n"
+   "  : ddd;");
   verifyFormat("bool aa = a //\n"

Nit: I'm just slightly confused as to what is happening here.. I assume this is 
the case where they are not aligned in the style. 



Comment at: clang/unittests/Format/FormatTest.cpp:5998
+   "  ddd;",
Style);
   verifyFormat("bool aa = a ? //\n"

as above how does one get back to the original case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 226166.
lewis-revill added a comment.

Rebase on top of shrink wrapping patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-features.c
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.h
  llvm/lib/Target/RISCV/RISCVMachineFunctionInfo.h
  llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
  llvm/lib/Target/RISCV/RISCVRegisterInfo.h
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/saverestore.ll

Index: llvm/test/CodeGen/RISCV/saverestore.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/saverestore.ll
@@ -0,0 +1,299 @@
+; RUN: llc -mtriple=riscv32 < %s | FileCheck %s -check-prefix=RV32I
+; RUN: llc -mtriple=riscv64 < %s | FileCheck %s -check-prefix=RV64I
+; RUN: llc -mtriple=riscv32 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV32I-SR
+; RUN: llc -mtriple=riscv64 -mattr=+save-restore < %s | FileCheck %s -check-prefix=RV64I-SR
+; RUN: llc -mtriple=riscv32 -mattr=+f,+save-restore -target-abi=ilp32f < %s | FileCheck %s -check-prefix=RV32I-FP-SR
+; RUN: llc -mtriple=riscv64 -mattr=+f,+d,+save-restore -target-abi=lp64d < %s | FileCheck %s -check-prefix=RV64I-FP-SR
+
+; Check that the correct save/restore libcalls are generated.
+
+@var0 = global [18 x i32] zeroinitializer
+@var1 = global [24 x i32] zeroinitializer
+@var2 = global [30 x i32] zeroinitializer
+
+define void @callee_saved0() nounwind {
+; RV32I-LABEL: callee_saved0:
+; RV32I-NOT: call t0, __riscv_save
+; RV32I-NOT: tail __riscv_restore
+;
+; RV64I-LABEL: callee_saved0:
+; RV64I-NOT: call t0, __riscv_save
+; RV64I-NOT: tail __riscv_restore
+;
+; RV32I-SR-LABEL: callee_saved0:
+; RV32I-SR: call t0, __riscv_save_5
+; RV32I-SR: tail __riscv_restore_5
+;
+; RV64I-SR-LABEL: callee_saved0:
+; RV64I-SR: call t0, __riscv_save_5
+; RV64I-SR: tail __riscv_restore_5
+;
+; RV32I-FP-SR-LABEL: callee_saved0:
+; RV32I-FP-SR: call t0, __riscv_save_5
+; RV32I-FP-SR: tail __riscv_restore_5
+;
+; RV64I-FP-SR-LABEL: callee_saved0:
+; RV64I-FP-SR: call t0, __riscv_save_5
+; RV64I-FP-SR: tail __riscv_restore_5
+  %val = load [18 x i32], [18 x i32]* @var0
+  store volatile [18 x i32] %val, [18 x i32]* @var0
+  ret void
+}
+
+define void @callee_saved1() nounwind {
+; RV32I-LABEL: callee_saved1:
+; RV32I-NOT: call t0, __riscv_save
+; RV32I-NOT: tail __riscv_restore
+;
+; RV64I-LABEL: callee_saved1:
+; RV64I-NOT: call t0, __riscv_save
+; RV64I-NOT: tail __riscv_restore
+;
+; RV32I-SR-LABEL: callee_saved1:
+; RV32I-SR: call t0, __riscv_save_11
+; RV32I-SR: tail __riscv_restore_11
+;
+; RV64I-SR-LABEL: callee_saved1:
+; RV64I-SR: call t0, __riscv_save_11
+; RV64I-SR: tail __riscv_restore_11
+;
+; RV32I-FP-SR-LABEL: callee_saved1:
+; RV32I-FP-SR: call t0, __riscv_save_11
+; RV32I-FP-SR: tail __riscv_restore_11
+;
+; RV64I-FP-SR-LABEL: callee_saved1:
+; RV64I-FP-SR: call t0, __riscv_save_11
+; RV64I-FP-SR: tail __riscv_restore_11
+  %val = load [24 x i32], [24 x i32]* @var1
+  store volatile [24 x i32] %val, [24 x i32]* @var1
+  ret void
+}
+
+define void @callee_saved2() nounwind {
+; RV32I-LABEL: callee_saved2:
+; RV32I-NOT: call t0, __riscv_save
+; RV32I-NOT: tail __riscv_restore
+;
+; RV64I-LABEL: callee_saved2:
+; RV64I-NOT: call t0, __riscv_save
+; RV64I-NOT: tail __riscv_restore
+;
+; RV32I-SR-LABEL: callee_saved2:
+; RV32I-SR: call t0, __riscv_save_12
+; RV32I-SR: tail __riscv_restore_12
+;
+; RV64I-SR-LABEL: callee_saved2:
+; RV64I-SR: call t0, __riscv_save_12
+; RV64I-SR: tail __riscv_restore_12
+;
+; RV32I-FP-SR-LABEL: callee_saved2:
+; RV32I-FP-SR: call t0, __riscv_save_12
+; RV32I-FP-SR: tail __riscv_restore_12
+;
+; RV64I-FP-SR-LABEL: callee_saved2:
+; RV64I-FP-SR: call t0, __riscv_save_12
+; RV64I-FP-SR: tail __riscv_restore_12
+  %val = load [30 x i32], [30 x i32]* @var2
+  store volatile [30 x i32] %val, [30 x i32]* @var2
+  ret void
+}
+
+; Check that floating point callee saved registers are still manually saved and
+; restored.
+
+define void @callee_saved_fp() nounwind {
+; RV32I-LABEL: callee_saved_fp:
+; RV32I-NOT: call t0, __riscv_save
+; RV32I-NOT: tail __riscv_restore
+;
+; RV64I-LABEL: callee_saved_fp:
+; RV64I-NOT: call t0, __riscv_save
+; RV64I-NOT: tail __riscv_restore
+;
+; RV32I-SR-LABEL: callee_saved_fp:
+; RV32I-SR: call t0, __riscv_save_7
+; RV32I-SR: tail __riscv_restore_7
+;
+; RV64I-SR-LABEL: callee_saved_fp:
+; RV64I-SR: call t0, __riscv_save_7
+; RV64I-SR: tail __riscv_restore_7
+;
+; RV3

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc created this revision.
chandlerc added a reviewer: klimek.
Herald added a subscriber: mcrosier.
Herald added a project: clang.

Working with Meike and others to improve the wording in this document.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69351

Files:
  clang/www/get_involved.html


Index: clang/www/get_involved.html
===
--- clang/www/get_involved.html
+++ clang/www/get_involved.html
@@ -50,8 +50,8 @@
 as well.
 
 
-The best way to talk with other developers on the project is through the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
+The most common way to talk with other developers on the project is through
+the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
 list.  The clang mailing list is a very friendly place and we welcome
 newcomers.  In addition to the cfe-dev list, a significant amount of design
 discussion takes place on the Contributing Extensions to Clang
 
-Clang has always been designed as a platform for experimentation,
+Clang is designed to support experimentation,
 allowing programmers to easily extend the compiler to support great
 new language features and tools. At some point, the authors of these
 extensions may propose that the extensions become a part of Clang
-itself, to benefit the whole Clang community. But not every idea--not
-even every great idea--should become part of Clang. Extensions
-(particularly language extensions) pose a long-term maintenance burden
-on Clang, and therefore the benefits of the extension must outweigh
-those costs. Hence, these are the seven criteria used to evaluate the
-merits of a proposed extension:
+itself, to benefit the whole Clang community. However, extensions
+(particularly language extensions) have long-term maintenance costs
+for Clang. The benefits of the extension need to be evaluated against
+these costs. The Clang project uses the following criteria for this
+evaluation:
 
 
-  Evidence of a significant user community: This is based on a number of 
factors, including an actual, existing user community, the perceived likelihood 
that users would adopt such a feature if it were available, and any 
"trickle-down" effects that come from, e.g., a library adopting the feature and 
providing benefits to its users.
-
-  A specific need to reside within the Clang tree: There are some 
extensions that would be better expressed as a separate tool, and should remain 
as separate tools even if they end up being hosted as part of the LLVM umbrella 
project.
-
-  A complete specification: The specification must be sufficient to 
understand the design of the feature as well as interpret the meaning of 
specific examples. The specification should be detailed enough that another 
compiler vendor could conceivably implement the feature.
-
-  Representation within the appropriate governing organization: For 
extensions to a language governed by a standards committee (C, C++, OpenCL), 
the extension itself must have an active proposal and proponent within that 
committee and have a reasonable chance of acceptance. Clang should drive the 
standard, not diverge from it. This criterion does not apply to all extensions, 
since some extensions fall outside of the realm of the standards bodies.
-
-  A long-term support plan: Contributing a non-trivial extension to Clang 
implies a commitment to supporting that extension, improving the implementation 
and specification as Clang evolves. The capacity of the contributor to make 
that commitment is as important as the commitment itself.
-
-  A high-quality implementation: The implementation must fit well into 
Clang's architecture, follow LLVM's coding conventions, and meet Clang's 
quality standards, including high-quality diagnostics and rich AST 
representations. This is particularly important for language extensions, 
because users will learn how those extensions work through the behavior of the 
compiler.
-
-  A proper test suite: Extensive testing is crucial to ensure that the 
language extension is not broken by ongoing maintenance in Clang. The test 
suite should be complete enough that another compiler vendor could conceivably 
validate their implementation of the feature against it.
+  Evidence of a significant user community: This is based on a number of
+  factors, including an existing user community, the perceived likelihood that
+  users would adopt such a feature if it were available, and any secondary
+  effects that come from, e.g., a library adopting the feature and providing
+  benefits to its users.
+
+  A specific need to reside within the Clang tree: There are some 
extensions
+  that would be better expressed as a separate tool, and should remain as
+  separate tools even if they end up being hosted as part of the LLVM umbrella
+  project.
+
+  A complete specification: The specification must be sufficient to
+  understand the design of the feature as well as

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/www/get_involved.html:94
+
+  A complete specification: The specification must be sufficient to
+  understand the design of the feature as well as interpret the meaning of

Remove "complete" - the explanation afterwards seems sufficient?



Comment at: clang/www/get_involved.html:107
+
+  A long-term support plan: Contributing a non-trivial extension to Clang
+  implies a commitment to supporting that extension, improving the

s/non-trivial/substantial/?



Comment at: clang/www/get_involved.html:108-109
+  A long-term support plan: Contributing a non-trivial extension to Clang
+  implies a commitment to supporting that extension, improving the
+  implementation and specification as Clang evolves. The capacity of the
+  contributor to make that commitment is as important as the commitment

s/,/and/?



Comment at: clang/www/get_involved.html:113
+
+  A high-quality implementation: The implementation must fit well into
+  Clang's architecture, follow LLVM's coding conventions, and meet Clang's

Perhaps: a standard-conforming implementation?



Comment at: clang/www/get_involved.html:115-116
+  Clang's architecture, follow LLVM's coding conventions, and meet Clang's
+  quality standards, including high-quality diagnostics and rich AST
+  representations. This is particularly important for language extensions,
+  because users will learn how those extensions work through the behavior of 
the

"high-quality diagnostics" is a bit unclear
"rich AST" that seems to imply a lot of domain knowledge




Comment at: clang/www/get_involved.html:120
+
+  A proper test suite: Extensive testing is crucial to ensure that the
+  language extension is not broken by ongoing maintenance in Clang. The test

s/proper//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69351



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

[PATCH] D68340: Add AIX toolchain and basic linker functionality

[Steven Wan via Phabricator via cfe-commits]

stevewan updated this revision to Diff 226170.
stevewan added a comment.

Avoid blank else block when assertion is off


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68340

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h
  clang/test/Driver/Inputs/aix_ppc_tree/powerpc-ibm-aix7.1.0.0/dummy.a
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crt0.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crt0_64.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/crti_64.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/gcrt0.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/gcrt0_64.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/mcrt0.o
  clang/test/Driver/Inputs/aix_ppc_tree/usr/lib/mcrt0_64.o
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- /dev/null
+++ clang/test/Driver/aix-ld.c
@@ -0,0 +1,177 @@
+// General tests that ld invocations on AIX targets are sane. Note that we use
+// sysroot to make these tests independent of the host system.
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32 %s
+// CHECK-LD32-NOT: warning:
+// CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-NOT: "-bnso"
+// CHECK-LD32: "-b32" 
+// CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000" 
+// CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32: "-L[[SYSROOT]]/usr/lib" 
+// CHECK-LD32: "-lc"
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64 %s
+// CHECK-LD64-NOT: warning:
+// CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD64-NOT: "-bnso"
+// CHECK-LD64: "-b64" 
+// CHECK-LD64: "-bpT:0x1" "-bpD:0x11000" 
+// CHECK-LD64: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64: "-L[[SYSROOT]]/usr/lib" 
+// CHECK-LD64: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -pthread \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-PTHREAD %s
+// CHECK-LD32-PTHREAD-NOT: warning:
+// CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-PTHREAD-NOT: "-bnso"
+// CHECK-LD32-PTHREAD: "-b32" 
+// CHECK-LD32-PTHREAD: "-bpT:0x1000" "-bpD:0x2000" 
+// CHECK-LD32-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-PTHREAD: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32-PTHREAD: "-lpthreads"
+// CHECK-LD32-PTHREAD: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -pthreads \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64-PTHREAD %s
+// CHECK-LD64-PTHREAD-NOT: warning:
+// CHECK-LD64-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64-PTHREAD: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD64-PTHREAD-NOT: "-bnso"
+// CHECK-LD64-PTHREAD: "-b64" 
+// CHECK-LD64-PTHREAD: "-bpT:0x1" "-bpD:0x11000" 
+// CHECK-LD64-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64-PTHREAD: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD64-PTHREAD: "-lpthreads"
+// CHECK-LD64-PTHREAD: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable profiling.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -p \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-PROF %s
+// CHECK-LD32-PROF-NOT: warning:
+// CHECK-LD32-PROF: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PROF: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-PROF: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-PROF-NOT: "-bnso"
+// CHECK-LD32-PROF: "-b32" 
+// CHECK-LD32-PROF: "-bpT:0x1000" "-bpD:0x200

[PATCH] D67787: Add 8548 CPU definition and attributes

[Vitaly Cheptsov via Phabricator via cfe-commits]

vit9696 added a comment.

@jhibbits Should not the test in test/Preprocessor/init.c also ensure that 
`__SPE__` and `__NO_FPRS__` macros are defined with CPU 8548? The rest looks 
good to me, thank you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67787



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

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc updated this revision to Diff 226174.
chandlerc marked 3 inline comments as done.
chandlerc added a comment.

More fixes from Manuel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69351

Files:
  clang/www/get_involved.html


Index: clang/www/get_involved.html
===
--- clang/www/get_involved.html
+++ clang/www/get_involved.html
@@ -50,8 +50,8 @@
 as well.
 
 
-The best way to talk with other developers on the project is through the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
+The most common way to talk with other developers on the project is through
+the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
 list.  The clang mailing list is a very friendly place and we welcome
 newcomers.  In addition to the cfe-dev list, a significant amount of design
 discussion takes place on the Contributing Extensions to Clang
 
-Clang has always been designed as a platform for experimentation,
+Clang is designed to support experimentation,
 allowing programmers to easily extend the compiler to support great
 new language features and tools. At some point, the authors of these
 extensions may propose that the extensions become a part of Clang
-itself, to benefit the whole Clang community. But not every idea--not
-even every great idea--should become part of Clang. Extensions
-(particularly language extensions) pose a long-term maintenance burden
-on Clang, and therefore the benefits of the extension must outweigh
-those costs. Hence, these are the seven criteria used to evaluate the
-merits of a proposed extension:
+itself, to benefit the whole Clang community. However, extensions
+(particularly language extensions) have long-term maintenance costs
+for Clang. The benefits of the extension need to be evaluated against
+these costs. The Clang project uses the following criteria for this
+evaluation:
 
 
-  Evidence of a significant user community: This is based on a number of 
factors, including an actual, existing user community, the perceived likelihood 
that users would adopt such a feature if it were available, and any 
"trickle-down" effects that come from, e.g., a library adopting the feature and 
providing benefits to its users.
-
-  A specific need to reside within the Clang tree: There are some 
extensions that would be better expressed as a separate tool, and should remain 
as separate tools even if they end up being hosted as part of the LLVM umbrella 
project.
-
-  A complete specification: The specification must be sufficient to 
understand the design of the feature as well as interpret the meaning of 
specific examples. The specification should be detailed enough that another 
compiler vendor could conceivably implement the feature.
-
-  Representation within the appropriate governing organization: For 
extensions to a language governed by a standards committee (C, C++, OpenCL), 
the extension itself must have an active proposal and proponent within that 
committee and have a reasonable chance of acceptance. Clang should drive the 
standard, not diverge from it. This criterion does not apply to all extensions, 
since some extensions fall outside of the realm of the standards bodies.
-
-  A long-term support plan: Contributing a non-trivial extension to Clang 
implies a commitment to supporting that extension, improving the implementation 
and specification as Clang evolves. The capacity of the contributor to make 
that commitment is as important as the commitment itself.
-
-  A high-quality implementation: The implementation must fit well into 
Clang's architecture, follow LLVM's coding conventions, and meet Clang's 
quality standards, including high-quality diagnostics and rich AST 
representations. This is particularly important for language extensions, 
because users will learn how those extensions work through the behavior of the 
compiler.
-
-  A proper test suite: Extensive testing is crucial to ensure that the 
language extension is not broken by ongoing maintenance in Clang. The test 
suite should be complete enough that another compiler vendor could conceivably 
validate their implementation of the feature against it.
+  Evidence of a significant user community: This is based on a number of
+  factors, including an existing user community, the perceived likelihood that
+  users would adopt such a feature if it were available, and any secondary
+  effects that come from, e.g., a library adopting the feature and providing
+  benefits to its users.
+
+  A specific need to reside within the Clang tree: There are some 
extensions
+  that would be better expressed as a separate tool, and should remain as
+  separate tools even if they end up being hosted as part of the LLVM umbrella
+  project.
+
+  A specification: The specification must be sufficient to understand the
+  design of the feature as well as interpr

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc updated this revision to Diff 226171.
chandlerc marked an inline comment as done.
chandlerc added a comment.

Address feedback from Manuel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69351

Files:
  clang/www/get_involved.html


Index: clang/www/get_involved.html
===
--- clang/www/get_involved.html
+++ clang/www/get_involved.html
@@ -50,8 +50,8 @@
 as well.
 
 
-The best way to talk with other developers on the project is through the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
+The most common way to talk with other developers on the project is through
+the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
 list.  The clang mailing list is a very friendly place and we welcome
 newcomers.  In addition to the cfe-dev list, a significant amount of design
 discussion takes place on the Contributing Extensions to Clang
 
-Clang has always been designed as a platform for experimentation,
+Clang is designed to support experimentation,
 allowing programmers to easily extend the compiler to support great
 new language features and tools. At some point, the authors of these
 extensions may propose that the extensions become a part of Clang
-itself, to benefit the whole Clang community. But not every idea--not
-even every great idea--should become part of Clang. Extensions
-(particularly language extensions) pose a long-term maintenance burden
-on Clang, and therefore the benefits of the extension must outweigh
-those costs. Hence, these are the seven criteria used to evaluate the
-merits of a proposed extension:
+itself, to benefit the whole Clang community. However, extensions
+(particularly language extensions) have long-term maintenance costs
+for Clang. The benefits of the extension need to be evaluated against
+these costs. The Clang project uses the following criteria for this
+evaluation:
 
 
-  Evidence of a significant user community: This is based on a number of 
factors, including an actual, existing user community, the perceived likelihood 
that users would adopt such a feature if it were available, and any 
"trickle-down" effects that come from, e.g., a library adopting the feature and 
providing benefits to its users.
-
-  A specific need to reside within the Clang tree: There are some 
extensions that would be better expressed as a separate tool, and should remain 
as separate tools even if they end up being hosted as part of the LLVM umbrella 
project.
-
-  A complete specification: The specification must be sufficient to 
understand the design of the feature as well as interpret the meaning of 
specific examples. The specification should be detailed enough that another 
compiler vendor could conceivably implement the feature.
-
-  Representation within the appropriate governing organization: For 
extensions to a language governed by a standards committee (C, C++, OpenCL), 
the extension itself must have an active proposal and proponent within that 
committee and have a reasonable chance of acceptance. Clang should drive the 
standard, not diverge from it. This criterion does not apply to all extensions, 
since some extensions fall outside of the realm of the standards bodies.
-
-  A long-term support plan: Contributing a non-trivial extension to Clang 
implies a commitment to supporting that extension, improving the implementation 
and specification as Clang evolves. The capacity of the contributor to make 
that commitment is as important as the commitment itself.
-
-  A high-quality implementation: The implementation must fit well into 
Clang's architecture, follow LLVM's coding conventions, and meet Clang's 
quality standards, including high-quality diagnostics and rich AST 
representations. This is particularly important for language extensions, 
because users will learn how those extensions work through the behavior of the 
compiler.
-
-  A proper test suite: Extensive testing is crucial to ensure that the 
language extension is not broken by ongoing maintenance in Clang. The test 
suite should be complete enough that another compiler vendor could conceivably 
validate their implementation of the feature against it.
+  Evidence of a significant user community: This is based on a number of
+  factors, including an existing user community, the perceived likelihood that
+  users would adopt such a feature if it were available, and any secondary
+  effects that come from, e.g., a library adopting the feature and providing
+  benefits to its users.
+
+  A specific need to reside within the Clang tree: There are some 
extensions
+  that would be better expressed as a separate tool, and should remain as
+  separate tools even if they end up being hosted as part of the LLVM umbrella
+  project.
+
+  A specification: The specification must be sufficient to understand the
+  design of the feature as well as i

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Manuel Klimek via Phabricator via cfe-commits]

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

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69351



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

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc marked 2 inline comments as done.
chandlerc added a comment.

Thanks, updated.




Comment at: clang/www/get_involved.html:113
+
+  A high-quality implementation: The implementation must fit well into
+  Clang's architecture, follow LLVM's coding conventions, and meet Clang's

klimek wrote:
> Perhaps: a standard-conforming implementation?
Sadly, that goes further than this is trying to go.

While this may be a bit vague, we do at least clarify the particular things we 
are looking for afterward. Better words are welcome if anyone comes up with 
them of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69351



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

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

PING for review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322



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

[PATCH] D67787: Add 8548 CPU definition and attributes

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

@vit9696 The only thing GCC defines for mcpu=8548 is __NO_LWSYNC__, the 
SPE-specific #defines are from -mspe.  That said, since I explicitly do enable 
SPE for e500 CPU, I guess it makes sense to add it to the test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67787



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 59518 tests passed, 1 failed and 763 were skipped.

  failed: Clangd.Clangd/rename.test

Log files: cmake-log.txt 
, 
ninja_check_all-log.txt 
, 
CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D69327#1719109 , @tejohnson wrote:

> I haven't read through the patch in detail yet, but from the description it 
> sounds like a cache is being added for the compile step outputs, e.g. the 
> bitcode output object of the "clang -flto=thin -c" step? Typically the build 
> system takes care of incremental build handling for these explicit outputs, 
> just as it would for a non-LTO clang compile output. Can you clarify the 
> motivation?


Hi Teresa, thanks for the feedback. I think the motivation is to provide an 
option that reliably caches thinLTO compile output regardless of the external 
setup of compiler cache or the platform is used. I'm not sure if compiler cache 
such as ccache could save `-fthin-link-bitcode` output and if there is a ccache 
equivalence on windows? Even if such tool exists, having linking phase caching 
managed by toolchain/linker and the compile phase by an external tool feels 
awkward and fragile. This patch mostly shares the implementation with linke 
phase caching.

Additionally, this also has a small benefit of trigging a little bit more 
caching because it is hashing IR instead of preprocessor output or even literal 
source file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69327



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

[PATCH] D67787: Add 8548 CPU definition and attributes

[Vitaly Cheptsov via Phabricator via cfe-commits]

vit9696 added a comment.

Yes, that's the point, since 8548 is an alias.

A side note regarding SPE support. I am currently upgrading to LLVM 9.0 and I 
discovered that this patch was not committed anyhow at all:
https://reviews.llvm.org/D54583#1444288


Repository:
  rC Clang

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

https://reviews.llvm.org/D67787



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

[PATCH] D51899: Remove extraneous ".a" suffix from baremetal clang_rt.builtins when compiling for baremetal.

[Robert Widmann via Phabricator via cfe-commits]

CodaFi added a comment.

@MaskRay I have a refactoring in https://reviews.llvm.org/D59425 that covers 
that.


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

https://reviews.llvm.org/D51899



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Can you clarify what do you mean by 'waste time optimizing a file that finally 
hit the object file cache'?

No matter what build system to use, it should figure out during an incremental 
build that the input wasn't changed and not rerun the clang invocation on the 
same input. If you are looking to achieve what `ccache` is doing, I don't think 
implement in the compiler is a good option. That should really be done on the 
build system level. This solution is strictly worse than `ccache` because 
`ccache` is able to fetch the optimized bitcode without even running clang 
frontend and ir-gen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69327



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

I was just typing up a response similar to @steven_wu 's response... I don't 
think clang should be in the business of caching the outputs of a prior clang 
invocation, the build system should and usually does avoid re-executing if the 
inputs have not changed. Note that this is very different from the caching of 
objects LTO is doing - because those are not otherwise emitted at all, the 
cache has to be built in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69327



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D69327#1719295 , @tejohnson wrote:

> I was just typing up a response similar to @steven_wu 's response... I don't 
> think clang should be in the business of caching the outputs of a prior clang 
> invocation, the build system should and usually does avoid re-executing if 
> the inputs have not changed. Note that this is very different from the 
> caching of objects LTO is doing - because those are not otherwise emitted at 
> all, the cache has to be built in.


And also because even if some of the inputs of the link change, not all backend 
threads necessarily need to be re-executed and not all native (intermediate) 
objects will change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69327



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

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

[Tyker via Phabricator via cfe-commits]

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



Comment at: clang/lib/Serialization/ASTReader.cpp:9635
+if (IsExpr) {
+  Base = APValue::LValueBase(ReadExpr(F), CallIndex, Version);
+  ElemTy = Base.get()->getType();

Tyker wrote:
> rsmith wrote:
> > This is problematic.
> > 
> > `ReadExpr` will read a new copy of the expression, creating a distinct 
> > object. But in the case where we reach this when deserializing (for a 
> > `MaterializeTemporaryExpr`), we need to refer to the existing 
> > `MaterializeTemporaryExpr` in the initializer of its lifetime-extending 
> > declaration. We will also need to serialize the `ASTContext`'s 
> > `MaterializedTemporaryValues` collection so that the temporaries 
> > lifetime-extended in a constant initializer get properly handled.
> > 
> > That all sounds very messy, so I think we should reconsider the model that 
> > we use for lifetime-extended materialized temporaries. As a half-baked idea:
> > 
> >  * When we lifetime-extend a temporary, create a 
> > `MaterializedTemporaryDecl` to hold its value, and modify 
> > `MaterializeTemporaryExpr` to refer to the `MaterializedTemporaryDecl` 
> > rather than to just hold the subexpression for the temporary.
> >  * Change the `LValueBase` representation to denote the declaration rather 
> > than the expression.
> >  * Store the constant evaluated value for a materialized temporary on the 
> > `MaterializedTemporaryDecl` rather than on a side-table in the `ASTContext`.
> > 
> > With that done, we should verify that all remaining `Expr*`s used as 
> > `LValueBase`s are either only transiently used during evaluation or don't 
> > have these kinds of identity problems.
> Would it be possible to adapt serialization/deserialization so that they make 
> sure that `MaterializeTemporaryExpr` are unique.
> by:
> 
>   - When serializing `MaterializeTemporaryExpr` serialize a key obtained from 
> the pointer to the expression as it is unique.
>   - When deserializing `MaterializeTemporaryExpr` deserializing the key, and 
> than have a cache for previously deserialized expression that need to be 
> unique.
> 
> This would make easier adding new `Expr` that require uniqueness and seem 
> less complicated.
> What do you think ?
i added a review that does the refactoring https://reviews.llvm.org/D69360.


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

https://reviews.llvm.org/D63640



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

[PATCH] D69360: [NFC] Refactor representation of materialized temporaries

[Tyker via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a reviewer: martong.
Herald added subscribers: cfe-commits, arphaman.
Herald added a reviewer: shafik.
Herald added a project: clang.

this patch refactor representation of materialized temporaries to prevent an 
issue raised by rsmith in https://reviews.llvm.org/D63640#inline-612718


Repository:
  rC Clang

https://reviews.llvm.org/D69360

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6299,6 +6299,7 @@
   case Decl::PragmaDetectMismatch:
   case Decl::UsingPack:
   case Decl::Concept:
+  case Decl::MaterializeTemporary:
 return C;
 
   // Declaration kinds that don't make any sense here, but are
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1835,9 +1835,11 @@
 
 void ASTStmtWriter::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E) {
   VisitExpr(E);
-  Record.AddStmt(E->getTemporary());
-  Record.AddDeclRef(E->getExtendingDecl());
-  Record.push_back(E->getManglingNumber());
+  Record.push_back(static_cast(E->getMaterializeTemporaryDecl()));
+  if (E->getMaterializeTemporaryDecl())
+Record.AddDeclRef(E->getMaterializeTemporaryDecl());
+  else
+Record.AddStmt(E->getTemporary());
   Code = serialization::EXPR_MATERIALIZE_TEMPORARY;
 }
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -124,6 +124,7 @@
 void VisitBlockDecl(BlockDecl *D);
 void VisitCapturedDecl(CapturedDecl *D);
 void VisitEmptyDecl(EmptyDecl *D);
+void VisitMaterializeTemporaryDecl(MaterializeTemporaryDecl* D);
 
 void VisitDeclContext(DeclContext *DC);
 template  void VisitRedeclarable(Redeclarable *D);
@@ -1131,6 +1132,17 @@
   Code = serialization::DECL_EMPTY;
 }
 
+void ASTDeclWriter::VisitMaterializeTemporaryDecl(MaterializeTemporaryDecl *D) {
+  VisitDecl(D);
+  Record.AddDeclRef(D->getExtendingDecl());
+  Record.AddStmt(D->getStmtWithTemporary());
+  Record.push_back(static_cast(D->getValue()));
+  if (D->getValue())
+Record.AddAPValue(*D->getValue());
+  Record.push_back(D->getManglingNumber());
+  Code = serialization::DECL_MATERIALIZE_TEMPORARY;
+}
+
 void ASTDeclWriter::VisitBlockDecl(BlockDecl *D) {
   VisitDecl(D);
   Record.AddStmt(D->getBody());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1900,10 +1900,11 @@
 
 void ASTStmtReader::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E) {
   VisitExpr(E);
-  E->State = Record.readSubExpr();
-  auto *VD = ReadDeclAs();
-  unsigned ManglingNumber = Record.readInt();
-  E->setExtendingDecl(VD, ManglingNumber);
+  bool HasMaterialzedDecl = Record.readInt();
+  if (HasMaterialzedDecl)
+E->State = ReadDeclAs();
+  else
+E->State = Record.readSubExpr();
 }
 
 void ASTStmtReader::VisitCXXFoldExpr(CXXFoldExpr *E) {
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -405,6 +405,7 @@
 void VisitBlockDecl(BlockDecl *BD);
 void VisitCapturedDecl(CapturedDecl *CD);
 void VisitEmptyDecl(EmptyDecl *D);
+void VisitMaterializeTemporaryDecl(MaterializeTemporaryDecl* D);
 
 std::pair VisitDeclContext(DeclContext *DC);
 
@@ -2346,6 +2347,15 @@
   VisitDecl(D);
 }
 
+void ASTDeclReader::VisitMaterializeTemporaryDecl(MaterializeTemporaryDecl* D) {
+  VisitDecl(D);
+  D->ExtendingDecl = ReadDeclAs();
+  D->StmtWithTemporary = Record.readStmt();
+  if (Record.readInt())
+D->Value = new (D->getASTContext()) APValue(Record.readAPValue());
+  D->ManglingNumber = Record.readInt();
+}
+
 

[clang] dc1499b - Improve Clang's getting involved document and make it more inclusive in wording.

[Chandler Carruth via cfe-commits]

Author: Chandler Carruth
Date: 2019-10-23T16:11:24-07:00
New Revision: dc1499b90dc41838e1ee8c7052bbe9535b3609bb

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

LOG: Improve Clang's getting involved document and make it more inclusive in 
wording.

Summary: Working with Meike and others to improve the wording in this document.

Reviewers: klimek

Subscribers: mcrosier, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/www/get_involved.html

Removed: 




diff  --git a/clang/www/get_involved.html b/clang/www/get_involved.html
index ed961ca0b61c..fabaec46bda3 100755
--- a/clang/www/get_involved.html
+++ b/clang/www/get_involved.html
@@ -50,8 +50,8 @@ Follow what's going on
 as well.
 
 
-The best way to talk with other developers on the project is through the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
+The most common way to talk with other developers on the project is through
+the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
 list.  The clang mailing list is a very friendly place and we welcome
 newcomers.  In addition to the cfe-dev list, a significant amount of design
 discussion takes place on the Follow what's going on
 
 Contributing Extensions to Clang
 
-Clang has always been designed as a platform for experimentation,
+Clang is designed to support experimentation,
 allowing programmers to easily extend the compiler to support great
 new language features and tools. At some point, the authors of these
 extensions may propose that the extensions become a part of Clang
-itself, to benefit the whole Clang community. But not every idea--not
-even every great idea--should become part of Clang. Extensions
-(particularly language extensions) pose a long-term maintenance burden
-on Clang, and therefore the benefits of the extension must outweigh
-those costs. Hence, these are the seven criteria used to evaluate the
-merits of a proposed extension:
+itself, to benefit the whole Clang community. However, extensions
+(particularly language extensions) have long-term maintenance costs
+for Clang. The benefits of the extension need to be evaluated against
+these costs. The Clang project uses the following criteria for this
+evaluation:
 
 
-  Evidence of a significant user community: This is based on a number of 
factors, including an actual, existing user community, the perceived likelihood 
that users would adopt such a feature if it were available, and any 
"trickle-down" effects that come from, e.g., a library adopting the feature and 
providing benefits to its users.
-
-  A specific need to reside within the Clang tree: There are some 
extensions that would be better expressed as a separate tool, and should remain 
as separate tools even if they end up being hosted as part of the LLVM umbrella 
project.
-
-  A complete specification: The specification must be sufficient to 
understand the design of the feature as well as interpret the meaning of 
specific examples. The specification should be detailed enough that another 
compiler vendor could conceivably implement the feature.
-
-  Representation within the appropriate governing organization: For 
extensions to a language governed by a standards committee (C, C++, OpenCL), 
the extension itself must have an active proposal and proponent within that 
committee and have a reasonable chance of acceptance. Clang should drive the 
standard, not diverge from it. This criterion does not apply to all extensions, 
since some extensions fall outside of the realm of the standards bodies.
-
-  A long-term support plan: Contributing a non-trivial extension to Clang 
implies a commitment to supporting that extension, improving the implementation 
and specification as Clang evolves. The capacity of the contributor to make 
that commitment is as important as the commitment itself.
-
-  A high-quality implementation: The implementation must fit well into 
Clang's architecture, follow LLVM's coding conventions, and meet Clang's 
quality standards, including high-quality diagnostics and rich AST 
representations. This is particularly important for language extensions, 
because users will learn how those extensions work through the behavior of the 
compiler.
-
-  A proper test suite: Extensive testing is crucial to ensure that the 
language extension is not broken by ongoing maintenance in Clang. The test 
suite should be complete enough that another compiler vendor could conceivably 
validate their implementation of the feature against it.
+  Evidence of a significant user community: This is based on a number of
+  factors, including an existing user community, the perceived likelihood that
+  users would adopt such a feature if it were available

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

[Chandler Carruth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc1499b90dc4: Improve Clang's getting involved document 
and make it more inclusive in wording. (authored by chandlerc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69351

Files:
  clang/www/get_involved.html


Index: clang/www/get_involved.html
===
--- clang/www/get_involved.html
+++ clang/www/get_involved.html
@@ -50,8 +50,8 @@
 as well.
 
 
-The best way to talk with other developers on the project is through the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
+The most common way to talk with other developers on the project is through
+the http://lists.llvm.org/mailman/listinfo/cfe-dev";>cfe-dev mailing
 list.  The clang mailing list is a very friendly place and we welcome
 newcomers.  In addition to the cfe-dev list, a significant amount of design
 discussion takes place on the Contributing Extensions to Clang
 
-Clang has always been designed as a platform for experimentation,
+Clang is designed to support experimentation,
 allowing programmers to easily extend the compiler to support great
 new language features and tools. At some point, the authors of these
 extensions may propose that the extensions become a part of Clang
-itself, to benefit the whole Clang community. But not every idea--not
-even every great idea--should become part of Clang. Extensions
-(particularly language extensions) pose a long-term maintenance burden
-on Clang, and therefore the benefits of the extension must outweigh
-those costs. Hence, these are the seven criteria used to evaluate the
-merits of a proposed extension:
+itself, to benefit the whole Clang community. However, extensions
+(particularly language extensions) have long-term maintenance costs
+for Clang. The benefits of the extension need to be evaluated against
+these costs. The Clang project uses the following criteria for this
+evaluation:
 
 
-  Evidence of a significant user community: This is based on a number of 
factors, including an actual, existing user community, the perceived likelihood 
that users would adopt such a feature if it were available, and any 
"trickle-down" effects that come from, e.g., a library adopting the feature and 
providing benefits to its users.
-
-  A specific need to reside within the Clang tree: There are some 
extensions that would be better expressed as a separate tool, and should remain 
as separate tools even if they end up being hosted as part of the LLVM umbrella 
project.
-
-  A complete specification: The specification must be sufficient to 
understand the design of the feature as well as interpret the meaning of 
specific examples. The specification should be detailed enough that another 
compiler vendor could conceivably implement the feature.
-
-  Representation within the appropriate governing organization: For 
extensions to a language governed by a standards committee (C, C++, OpenCL), 
the extension itself must have an active proposal and proponent within that 
committee and have a reasonable chance of acceptance. Clang should drive the 
standard, not diverge from it. This criterion does not apply to all extensions, 
since some extensions fall outside of the realm of the standards bodies.
-
-  A long-term support plan: Contributing a non-trivial extension to Clang 
implies a commitment to supporting that extension, improving the implementation 
and specification as Clang evolves. The capacity of the contributor to make 
that commitment is as important as the commitment itself.
-
-  A high-quality implementation: The implementation must fit well into 
Clang's architecture, follow LLVM's coding conventions, and meet Clang's 
quality standards, including high-quality diagnostics and rich AST 
representations. This is particularly important for language extensions, 
because users will learn how those extensions work through the behavior of the 
compiler.
-
-  A proper test suite: Extensive testing is crucial to ensure that the 
language extension is not broken by ongoing maintenance in Clang. The test 
suite should be complete enough that another compiler vendor could conceivably 
validate their implementation of the feature against it.
+  Evidence of a significant user community: This is based on a number of
+  factors, including an existing user community, the perceived likelihood that
+  users would adopt such a feature if it were available, and any secondary
+  effects that come from, e.g., a library adopting the feature and providing
+  benefits to its users.
+
+  A specific need to reside within the Clang tree: There are some 
extensions
+  that would be better expressed as a separate tool, and should remain as
+  separate tools even if they end up being hosted as part of the LLVM umbrella
+  project.
+
+  A specification: The specification must be suf

[PATCH] D69250: [ARM][AArch64] Implement __cls and __clsl intrinsics from ACLE

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Headers/arm_acle.h:150
+__clsl(unsigned long __t) {
+#if __SIZEOF_LONG__ == 4
+  return __builtin_arm_cls(__t);

vhscampos wrote:
> compnerd wrote:
> > vhscampos wrote:
> > > compnerd wrote:
> > > > I don't see a pattern match for the `cls64` on ARM32, would that not 
> > > > fail to lower?
> > > Yes. However, for now, I am not enabling support for `cls64` on ARM32 as 
> > > it is not done yet.
> > Is the difference not just the parameter type?  I think that implementing 
> > it should be a trivial change to the existing implementation.  Is there a 
> > reason that you are not implementing that?
> At clang's side, yes, but not in the backend: Arm32 does not have a `cls` 
> instruction, thus the CLS operations need to be custom lowered. In the 
> `llvm.arm.cls(i32)` case, lowering is quite simple, and it's been included in 
> this patch. For `llvm.arm.cls64(i64)`, on the other hand, it is not as 
> trivial since it's necessary to break its logic into 32-bit instructions.
> 
> So the reason not to implement that (yet) is just to split work in two 
> different efforts.
Would it not be sufficient to do the top half (after a shift right of 32-bits), 
and if it is exactly 32, then do the bottom 32-bits, otherwise, you're done?



Comment at: clang/lib/Headers/arm_acle.h:155
+#endif
+}
+

vhscampos wrote:
> compnerd wrote:
> > vhscampos wrote:
> > > compnerd wrote:
> > > > Should we have a `__clsll` extension, otherwise these two are the same 
> > > > in LLP64?  I'm thinking about the LLP64 environments, where `long` and 
> > > > `long long` are different (32-bit vs 64-bit).
> > > ACLE does provide a `long long` version of `cls` called `__clsll`. But 
> > > since the support for `cls64` on Arm32 is not done yet, I decided not to 
> > > write support for `__clsll`. If I did, it would work for 64-bit but not 
> > > for 32-bit.
> > > 
> > > Please let me know what you think.
> > clang supports Windows where `long` is 4-bytes even on 64-bit targets, and 
> > this means that this doesn't work for that target.  I think that we need to 
> > add `__clsll` so that 64-bit ARM at least is covered.
> I'm not sure if I am following you. On AArch64-Windows, `__clsl` will be 
> lowered to `llvm.aarch64.cls(i32)` which will then be custom lowered 
> correctly. Let me know if I am thinking this wrong.
Windows is LLP64, `__cls` and `__clsl` are the same thing.  It needs a 
`__clsll` for the 64-bit value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69250



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

[PATCH] D69363: [www] Change URLs to HTTPS.

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, ldionne, mclow.lists, zoecarver.
Herald added a reviewer: bollu.
Herald added subscribers: llvm-commits, arphaman, dexonsmith, christof.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLVM.

This changes most URLs in llvm's html files to HTTPS. Most changes were 
search-and-replace with manual verification; some changes were manual. For a 
few URLs, the websites were performing redirects or had changed their anchors; 
I fixed those up manually. There are a very small number of dead links for 
which I don't know any replacements (they are equally dead as HTTP or HTTPS).

I have this structured as a series of git commits with mechanical and manual 
replacements separated, but I don't know how to present that through 
Differential (as GitHub PRs aren't available yet).


Repository:
  rC Clang

https://reviews.llvm.org/D69363

Files:
  clang/www/OpenProjects.html
  clang/www/UniversalDriver.html
  clang/www/analyzer/alpha_checks.html
  clang/www/analyzer/annotations.html
  clang/www/analyzer/available_checks.html
  clang/www/analyzer/checker_dev_manual.html
  clang/www/analyzer/faq.html
  clang/www/analyzer/filing_bugs.html
  clang/www/analyzer/implicit_checks.html
  clang/www/analyzer/index.html
  clang/www/analyzer/installation.html
  clang/www/analyzer/open_projects.html
  clang/www/analyzer/potential_checkers.html
  clang/www/analyzer/release_notes.html
  clang/www/analyzer/scan-build.html
  clang/www/analyzer/xcode.html
  clang/www/comparison.html
  clang/www/compatibility.html
  clang/www/cxx_compatibility.html
  clang/www/cxx_dr_status.html
  clang/www/cxx_status.html
  clang/www/demo/DemoInfo.html
  clang/www/features.html
  clang/www/get_involved.html
  clang/www/get_started.html
  clang/www/hacking.html
  clang/www/index.html
  clang/www/related.html
  compiler-rt/www/index.html
  libclc/www/index.html
  libcxx/www/index.html
  libcxxabi/www/index.html
  libcxxabi/www/spec.html
  openmp/www/index.html
  polly/www/changelog.html
  polly/www/documentation.html
  polly/www/get_started.html
  polly/www/index.html
  polly/www/performance.html
  polly/www/phonecall.html
  polly/www/projects.html
  polly/www/publications.html
  polly/www/todo.html



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

[PATCH] D69363: [www] Change URLs to HTTPS.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

GitHub PRs can't come soon enough :) There was a round table meeting about that 
earlier today which I am anxious to hear about.




Comment at: clang/www/cxx_status.html:78
   Rvalue references
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html";>N2118
+  https://wg21.link/n2118";>N2118
   Clang 2.9

These will all now redirect. Maybe that's OK/better because they are now more 
uniform, though. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D69363



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

[PATCH] D69363: [www] Change URLs to HTTPS.

[Louis Dionne via Phabricator via cfe-commits]

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

Nice!


Repository:
  rC Clang

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

https://reviews.llvm.org/D69363



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

[PATCH] D69363: [www] Change URLs to HTTPS.

[Stephan T. Lavavej via Phabricator via cfe-commits]

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



Comment at: clang/www/cxx_status.html:78
   Rvalue references
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html";>N2118
+  https://wg21.link/n2118";>N2118
   Clang 2.9

zoecarver wrote:
> These will all now redirect. Maybe that's OK/better because they are now more 
> uniform, though. 
Yep, that's intentional for consistency with the newer papers that were already 
using the official redirector. (It's my hope that open-std will eventually 
support https so the destination is secure; if and when that happens, these 
will automatically be enhanced.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D69363



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