[PATCH] D91049: [clangd][remote] Check an index file correctly

2020-11-09 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
ArcsinX requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

There is not reason to check `std::make_unique<...>(..)` return value,
but `clangd::clang::loadIndex()` returns `nullptr` if an index file could not 
be loaded (e.g. incorrect version).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91049

Files:
  clang-tools-extra/clangd/index/remote/server/Server.cpp


Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -357,13 +357,12 @@
 return Status.getError().value();
   }
 
-  auto Index = std::make_unique(
-  clang::clangd::loadIndex(IndexPath));
-
-  if (!Index) {
+  auto SymIndex = clang::clangd::loadIndex(IndexPath);
+  if (!SymIndex) {
 llvm::errs() << "Failed to open the index.\n";
 return -1;
   }
+  auto Index = std::make_unique(std::move(SymIndex));
 
   std::thread HotReloadThread([&Index, &Status, &FS]() {
 llvm::vfs::Status LastStatus = *Status;


Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -357,13 +357,12 @@
 return Status.getError().value();
   }
 
-  auto Index = std::make_unique(
-  clang::clangd::loadIndex(IndexPath));
-
-  if (!Index) {
+  auto SymIndex = clang::clangd::loadIndex(IndexPath);
+  if (!SymIndex) {
 llvm::errs() << "Failed to open the index.\n";
 return -1;
   }
+  auto Index = std::make_unique(std::move(SymIndex));
 
   std::thread HotReloadThread([&Index, &Status, &FS]() {
 llvm::vfs::Status LastStatus = *Status;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D91051: [clangd] Improve clangd-indexer performance

2020-11-09 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
ArcsinX requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This is a try to improve clangd-indexer tool performance:

- avoid processing already processed files.
- use different mutexes for different entities (e.g. do not block insertion of 
references while symbols are inserted)

Results for LLVM project indexing:

- before: ~30 minutes
- after: ~10 minutes


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91051

Files:
  clang-tools-extra/clangd/indexer/IndexerMain.cpp


Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -43,6 +43,19 @@
   std::unique_ptr create() override {
 SymbolCollector::Options Opts;
 Opts.CountReferences = true;
+Opts.FileFilter = [&](const SourceManager &SM, FileID FID) {
+  const auto *F = SM.getFileEntryForID(FID);
+  if (!F)
+return false; // Skip invalid files.
+  auto AbsPath = getCanonicalPath(F, SM);
+  if (!AbsPath)
+return false; // Skip files without absolute path.
+  std::lock_guard Lock(FilesMu);
+  if (Files.count(*AbsPath) != 0)
+return false; // Skip already processed files.
+  Files.insert(*AbsPath);
+  return true;
+};
 return createStaticIndexingAction(
 Opts,
 [&](SymbolSlab S) {
@@ -56,7 +69,7 @@
   }
 },
 [&](RefSlab S) {
-  std::lock_guard Lock(SymbolsMu);
+  std::lock_guard Lock(RefsMu);
   for (const auto &Sym : S) {
 // Deduplication happens during insertion.
 for (const auto &Ref : Sym.second)
@@ -64,7 +77,7 @@
   }
 },
 [&](RelationSlab S) {
-  std::lock_guard Lock(SymbolsMu);
+  std::lock_guard Lock(RelsMu);
   for (const auto &R : S) {
 Relations.insert(R);
   }
@@ -82,9 +95,13 @@
 
 private:
   IndexFileIn &Result;
+  std::mutex FilesMu;
+  llvm::StringSet<> Files;
   std::mutex SymbolsMu;
   SymbolSlab::Builder Symbols;
+  std::mutex RefsMu;
   RefSlab::Builder Refs;
+  std::mutex RelsMu;
   RelationSlab::Builder Relations;
 };
 


Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -43,6 +43,19 @@
   std::unique_ptr create() override {
 SymbolCollector::Options Opts;
 Opts.CountReferences = true;
+Opts.FileFilter = [&](const SourceManager &SM, FileID FID) {
+  const auto *F = SM.getFileEntryForID(FID);
+  if (!F)
+return false; // Skip invalid files.
+  auto AbsPath = getCanonicalPath(F, SM);
+  if (!AbsPath)
+return false; // Skip files without absolute path.
+  std::lock_guard Lock(FilesMu);
+  if (Files.count(*AbsPath) != 0)
+return false; // Skip already processed files.
+  Files.insert(*AbsPath);
+  return true;
+};
 return createStaticIndexingAction(
 Opts,
 [&](SymbolSlab S) {
@@ -56,7 +69,7 @@
   }
 },
 [&](RefSlab S) {
-  std::lock_guard Lock(SymbolsMu);
+  std::lock_guard Lock(RefsMu);
   for (const auto &Sym : S) {
 // Deduplication happens during insertion.
 for (const auto &Ref : Sym.second)
@@ -64,7 +77,7 @@
   }
 },
 [&](RelationSlab S) {
-  std::lock_guard Lock(SymbolsMu);
+  std::lock_guard Lock(RelsMu);
   for (const auto &R : S) {
 Relations.insert(R);
   }
@@ -82,9 +95,13 @@
 
 private:
   IndexFileIn &Result;
+  std::mutex FilesMu;
+  llvm::StringSet<> Files;
   std::mutex SymbolsMu;
   SymbolSlab::Builder Symbols;
+  std::mutex RefsMu;
   RefSlab::Builder Refs;
+  std::mutex RelsMu;
   RelationSlab::Builder Relations;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D91049: [clangd][remote] Check an index file correctly

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:365
   }
+  auto Index = std::make_unique(std::move(SymIndex));
 

well actually we don't need to make unique at all. why not just create a 
`SwapIndex Idx(std::move(SymIndex))` and pass it around ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91049

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


[PATCH] D91049: [clangd][remote] Check an index file correctly

2020-11-09 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 303766.
ArcsinX added a comment.

Do not use unique pointer for Index


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91049

Files:
  clang-tools-extra/clangd/index/remote/server/Server.cpp


Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -357,24 +357,23 @@
 return Status.getError().value();
   }
 
-  auto Index = std::make_unique(
-  clang::clangd::loadIndex(IndexPath));
-
-  if (!Index) {
+  auto SymIndex = clang::clangd::loadIndex(IndexPath);
+  if (!SymIndex) {
 llvm::errs() << "Failed to open the index.\n";
 return -1;
   }
+  clang::clangd::SwapIndex Index(std::move(SymIndex));
 
   std::thread HotReloadThread([&Index, &Status, &FS]() {
 llvm::vfs::Status LastStatus = *Status;
 static constexpr auto RefreshFrequency = std::chrono::seconds(30);
 while (!clang::clangd::shutdownRequested()) {
-  hotReload(*Index, llvm::StringRef(IndexPath), LastStatus, FS);
+  hotReload(Index, llvm::StringRef(IndexPath), LastStatus, FS);
   std::this_thread::sleep_for(RefreshFrequency);
 }
   });
 
-  runServerAndWait(*Index, ServerAddress, IndexPath);
+  runServerAndWait(Index, ServerAddress, IndexPath);
 
   HotReloadThread.join();
 }


Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -357,24 +357,23 @@
 return Status.getError().value();
   }
 
-  auto Index = std::make_unique(
-  clang::clangd::loadIndex(IndexPath));
-
-  if (!Index) {
+  auto SymIndex = clang::clangd::loadIndex(IndexPath);
+  if (!SymIndex) {
 llvm::errs() << "Failed to open the index.\n";
 return -1;
   }
+  clang::clangd::SwapIndex Index(std::move(SymIndex));
 
   std::thread HotReloadThread([&Index, &Status, &FS]() {
 llvm::vfs::Status LastStatus = *Status;
 static constexpr auto RefreshFrequency = std::chrono::seconds(30);
 while (!clang::clangd::shutdownRequested()) {
-  hotReload(*Index, llvm::StringRef(IndexPath), LastStatus, FS);
+  hotReload(Index, llvm::StringRef(IndexPath), LastStatus, FS);
   std::this_thread::sleep_for(RefreshFrequency);
 }
   });
 
-  runServerAndWait(*Index, ServerAddress, IndexPath);
+  runServerAndWait(Index, ServerAddress, IndexPath);
 
   HotReloadThread.join();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D91051: [clangd] Improve clangd-indexer performance

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/indexer/IndexerMain.cpp:46
 Opts.CountReferences = true;
+Opts.FileFilter = [&](const SourceManager &SM, FileID FID) {
+  const auto *F = SM.getFileEntryForID(FID);

this changes the behavior of clangd-indexer slightly. it feels like an okay-ish 
trade-off considering the 3x speed up, but might have dire consequences.

Without this change, clangd-indexer can index headers in multiple 
configurations, e.g. if you have src1.cc that includes a.h with a `-DFOO` and 
src2.cc that includes a.h with `-DBAR`, a.h might end up producing different 
symbols. All of them are being indexed at the moment. After this change, the 
first one to index will win.

This is also what we do with background-index but we have different 
requirements for this behavior, as we store only a single shard per source 
file, even if we indexed sources in different configurations only one of them 
would prevail (unless we postpone shard writing to end of indexing and 
accumulate results in memory).

Also we are planning to make use of this binary in a server-like setup, and use 
the produced index to serve multiple clients. In some situations keeping 
symbols from multiple configurations might be really useful, but I am not so 
sure about it as they'll still be collapsed if USR generation produces same IDs 
for those symbols (and I think it will). So I am leaning towards making this 
change, but I would like to hear what others think too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91051

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


[PATCH] D89790: [clangd] Add basic conflict detection for the rename.

2020-11-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 303768.
hokein marked 3 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89790

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -630,6 +630,52 @@
  class Foo {};
)cpp",
"no symbol", !HeaderFile, nullptr},
+
+  {R"cpp(
+namespace {
+int Conflict;
+int Va^r;
+}
+  )cpp",
+   "conflict", !HeaderFile, nullptr, "Conflict"},
+
+  {R"cpp(
+int Conflict;
+int Va^r;
+  )cpp",
+   "conflict", !HeaderFile, nullptr, "Conflict"},
+
+  {R"cpp(
+class Foo {
+  int Conflict;
+  int Va^r;
+};
+  )cpp",
+   "conflict", !HeaderFile, nullptr, "Conflict"},
+
+  {R"cpp(
+enum E {
+  Conflict,
+  Fo^o,
+};
+  )cpp",
+   "conflict", !HeaderFile, nullptr, "Conflict"},
+
+  {R"cpp(
+int Conflict;
+enum E { // transparent context.
+  F^oo,
+};
+  )cpp",
+   "conflict", !HeaderFile, nullptr, "Conflict"},
+
+  {R"cpp(// FIXME: detecting local variables is not supported yet.
+void func() {
+  int Conflict;
+  int [[V^ar]];
+}
+  )cpp",
+   nullptr, !HeaderFile, nullptr, "Conflict"},
   };
 
   for (const auto& Case : Cases) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -254,6 +254,86 @@
   return Results;
 }
 
+// Lookup the declarations (if any) with the given Name in the context of
+// RenameDecl.
+NamedDecl *lookupSiblingWithName(const ASTContext &Ctx,
+ const NamedDecl &RenamedDecl,
+ llvm::StringRef Name) {
+  const auto &II = Ctx.Idents.get(Name);
+  DeclarationName LookupName(&II);
+  DeclContextLookupResult LookupResult;
+  const auto *DC = RenamedDecl.getDeclContext();
+  while (DC && DC->isTransparentContext())
+DC = DC->getParent();
+  switch (DC->getDeclKind()) {
+  // The enclosing DeclContext may not be the enclosing scope, it might have
+  // false positives and negatives, so we only choose "confident" DeclContexts
+  // that don't have any subscopes that are neither DeclContexts nor
+  // transparent.
+  //
+  // Notably, FunctionDecl is excluded -- because local variables are not scoped
+  // to the function, but rather to the CompoundStmt that is its body. Lookup
+  // will not find function-local variables.
+  case Decl::TranslationUnit:
+  case Decl::Namespace:
+  case Decl::Record:
+  case Decl::Enum:
+  case Decl::CXXRecord:
+LookupResult = DC->lookup(LookupName);
+break;
+  default:
+break;
+  }
+  // Lookup may contain the RenameDecl itself, exclude it.
+  auto It = llvm::find_if(LookupResult, [&RenamedDecl](const NamedDecl *D) {
+return D->getCanonicalDecl() != RenamedDecl.getCanonicalDecl();
+  });
+  if (It != LookupResult.end())
+return *It;
+  return nullptr;
+}
+
+struct InvalidName {
+  enum Kind {
+Keywords,
+Conflict,
+  };
+  Kind K;
+  std::string Details;
+};
+
+llvm::Error makeError(InvalidName Reason) {
+  auto Message = [](InvalidName Reason) {
+switch (Reason.K) {
+case InvalidName::Keywords:
+  return llvm::formatv("the chosen name \"{0}\" is a keyword",
+   Reason.Details);
+case InvalidName::Conflict:
+  return llvm::formatv("conflict with the symbol in {0}", Reason.Details);
+}
+llvm_unreachable("unhandled InvalidName kind");
+  };
+  return error("invalid name: {0}", Message(Reason));
+}
+
+// Check if we can rename the given RenameDecl into NewName.
+// Return details if the rename would produce a conflict.
+llvm::Optional checkName(const NamedDecl &RenameDecl,
+  llvm::StringRef NewName) {
+  auto &ASTCtx = RenameDecl.getASTContext();
+  if (isKeyword(NewName, ASTCtx.getLangOpts()))
+return InvalidName{InvalidName::Keywords, NewName.str()};
+  // Name conflict detection.
+  // Function conflicts are subtle (overloading), so ignore them.
+  if (RenameDecl.getKind() != Decl::Function) {
+if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName))
+  return InvalidName{
+  InvalidName::Conflict,
+  Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
+  }
+  return llvm::None;
+}
+
 // AST-based rename, it renames all occurrences in

[PATCH] D91054: [LLVM][OpenMP] Frontend work for sections - D89671

2020-11-09 Thread Chirag Khandelwal via Phabricator via cfe-commits
AMDChirag created this revision.
AMDChirag added reviewers: anchu-rajendran, fghanim, jdoerfert, 
kiranchandramohan, kiranktp, SouraVX, AlexisPerry, Meinersbur.
Herald added subscribers: cfe-commits, guansong, yaxunl.
Herald added a project: clang.
AMDChirag requested review of this revision.
Herald added a subscriber: sstefan1.

Primary Author: @anchu-rajendran
This patch is child of D89671 , contains the 
clang
implementation to use the OpenMP IRBuilder's section
construct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91054

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3546,12 +3546,68 @@
 }
 
 void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
-  {
-auto LPCRegion =
-CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
-OMPLexicalScope Scope(*this, S, OMPD_unknown);
-EmitSections(S);
+  if (CGM.getLangOpts().OpenMPIRBuilder) {
+llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+using BodyGenCallbackTy =
+llvm::function_ref;
+
+auto FiniCB = [this](InsertPointTy IP) {
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
+};
+
+const Stmt *CapturedStmt = S.getInnermostCapturedStmt()->getCapturedStmt();
+const auto *CS = dyn_cast(CapturedStmt);
+llvm::SmallVector SectionCBVector;
+if (CS) {
+  for (const Stmt *SubStmt : CS->children()) {
+auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
+ InsertPointTy CodeGenIP,
+ llvm::BasicBlock &FiniBB) {
+  OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP,
+ FiniBB);
+  OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SubStmt, CodeGenIP,
+ FiniBB);
+};
+SectionCBVector.push_back(SectionCB);
+  }
+} else {
+  auto SectionCB = [this, CS](InsertPointTy AllocaIP,
+  InsertPointTy CodeGenIP,
+  llvm::BasicBlock &FiniBB) {
+OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, 
FiniBB);
+OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CS, CodeGenIP, FiniBB);
+  };
+  SectionCBVector.push_back(SectionCB);
+}
+ArrayRef SectionCBs = makeArrayRef(SectionCBVector);
+
+// Privatization callback that performs appropriate action for
+// shared/private/firstprivate/lastprivate/copyin/... variables.
+//
+// TODO: This defaults to shared right now.
+auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+ llvm::Value &Val, llvm::Value *&ReplVal) {
+  // The next line is appropriate only for variables (Val) with the
+  // data-sharing attribute "shared".
+  ReplVal = &Val;
+
+  return CodeGenIP;
+};
+
+llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
+AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
+Builder.restoreIP(OMPBuilder.CreateSections(
+Builder, AllocaIP, SectionCBs, PrivCB, FiniCB, S.hasCancel(),
+S.getSingleClause()));
+
+return;
   }
+  auto LPCRegion =
+  CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
+  OMPLexicalScope Scope(*this, S, OMPD_unknown);
+  EmitSections(S);
   // Emit an implicit barrier at the end.
   if (!S.getSingleClause()) {
 CGM.getOpenMPRuntime().emitBarrierCall(*this, S.getBeginLoc(),


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3546,12 +3546,68 @@
 }
 
 void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
-  {
-auto LPCRegion =
-CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
-OMPLexicalScope Scope(*this, S, OMPD_unknown);
-EmitSections(S);
+  if (CGM.getLangOpts().OpenMPIRBuilder) {
+llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+using BodyGenCallbackTy =
+llvm::function_ref;
+
+auto FiniCB = [this](InsertPointTy IP) {
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
+};
+
+const Stmt *CapturedStmt = S.getInnermostCapturedStmt()->getCapturedStmt();
+const auto *CS = dyn_cast(CapturedStmt);
+llvm::SmallVector SectionCBVector;
+if (CS) {
+  for (const Stmt *SubStmt : CS->children()) {
+auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
+  

[PATCH] D91047: Add a call super attribute plugin example

2020-11-09 Thread Yafei Liu via Phabricator via cfe-commits
psionic12 updated this revision to Diff 303772.
psionic12 added a comment.

add final attribute checking


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -S %s -o - 2>&1 | FileCheck %s --check-prefix=CALLSUPER
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+struct Base1 { [[call_super]] virtual void Test() {} };
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[call_super]] virtual void Test() override final; };
+// CALLSUPER: warning: call_super attribute attached on a final method
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// CALLSUPER: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
+// CALLSUPER: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
+// CALLSUPER: @llvm.global.annotations = {{.*}}@{{.*}}fn1a{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1b{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn1c{{.*}}[[STR1_VAR]]{{.*}}@{{.*}}fn2{{.*}}[[STR2_VAR]]
+
+#ifdef BAD_CALLSUPER
+struct Base1 { [[call_super]] virtual void Test() {} };
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: Base2::Test is marked as call_super but override method Derive::Test does not call it
+// BADCALLSUPER: note: overridden method which is marked as call_super here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,170 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// clang plugin, checks every overridden virtual function whether called
+// this function or not.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// cached methods which are marked as call_super
+llvm::SmallPtrSet ForcingCalledMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // use this way to avoid add an annotation attr to the AST.
+  return ForcingCalledMethods.contains(D);
+}
+
+std::string fullName(const CXXMethodDecl *D) {
+  std::string FullName;
+  FullName += D->getParent()->getName();
+  FullName += "::";
+  FullName += D->getName();
+  return FullName;
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}
+  bool VisitCallExpr(clang::CallExpr *CallExpr) {
+if (CallExpr->getCalleeDecl() == MethodDecl) {
+  // super is called
+  IsOverriddenUsed = true;
+  return false;
+}
+return true;
+  }
+
+private:
+  const clang::CXXMethodDecl *MethodDecl;
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine &Diags) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"%0

[PATCH] D91029: [clangd] Implement clang-tidy options from config

2020-11-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 303774.
njames93 added a comment.

- Fix compile error for gcc 5.3.
- Add back logging configuration when creating CTContext.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91029

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/TidyProvider.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h
  clang-tools-extra/clangd/unittests/TestTidyProvider.cpp
  clang-tools-extra/clangd/unittests/TestTidyProvider.h

Index: clang-tools-extra/clangd/unittests/TestTidyProvider.h
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/TestTidyProvider.h
@@ -0,0 +1,34 @@
+//===-- TestTidyProvider.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../TidyProvider.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+namespace clang {
+namespace clangd {
+
+class TestClangTidyProvider : public ClangdTidyProvider {
+public:
+  /// Convienence method for creating a provider that just uses \p Checks and \p
+  /// WarningsAsErrors
+  TestClangTidyProvider(llvm::StringRef Checks,
+llvm::StringRef WarningsAsErrors = {});
+  TestClangTidyProvider(const tidy::ClangTidyGlobalOptions &GlobalOptions,
+const tidy::ClangTidyOptions &Options);
+  std::vector getRawOptions(llvm::vfs::FileSystem * /*unused*/,
+   llvm::StringRef FileName) override;
+  const tidy::ClangTidyGlobalOptions &
+  getGlobalOptions(llvm::vfs::FileSystem * /*unused*/) override;
+
+private:
+  tidy::ClangTidyGlobalOptions GlobalOpts;
+  tidy::ClangTidyOptions Opts;
+};
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/TestTidyProvider.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/TestTidyProvider.cpp
@@ -0,0 +1,50 @@
+//===-- TestTidyProvider.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestTidyProvider.h"
+
+namespace clang {
+
+using namespace tidy;
+
+namespace clangd {
+
+TestClangTidyProvider::TestClangTidyProvider(llvm::StringRef Checks,
+ llvm::StringRef WarningsAsErrors) {
+  if (Checks.empty())
+Opts.Checks.emplace(getDefaultTidyChecks());
+  else
+Opts.Checks.emplace((Checks + "," + getUnusableTidyChecks()).str());
+  if (!WarningsAsErrors.empty())
+Opts.WarningsAsErrors.emplace(WarningsAsErrors);
+}
+
+TestClangTidyProvider::TestClangTidyProvider(
+const ClangTidyGlobalOptions &GlobalOptions,
+const ClangTidyOptions &Options)
+: GlobalOpts(GlobalOptions), Opts(Options) {
+  if (!Opts.Checks.hasValue() || Opts.Checks->empty())
+Opts.Checks = getDefaultTidyChecks().str();
+  else
+Opts.Checks->append(("," + getUnusableTidyChecks().str()));
+}
+
+std::vector
+TestClangTidyProvider::getRawOptions(llvm::vfs::FileSystem * /*unused*/,
+ llvm::StringRef FileName) {
+  std::vector RawOpts;
+  RawOpts.emplace_back(Opts, "mock-options");
+  return RawOpts;
+}
+
+const ClangTidyGlobalOptions &
+TestClangTidyProvider::getGlobalOptions(llvm::vfs::FileSystem * /*unused*/) {
+  return GlobalOpts;
+}
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -17,6 +17,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
+#include "../TidyProvider.h"
 #include "Compiler.h"
 #include "ParsedAST.h"
 #include "TestFS.

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

2020-11-09 Thread Chirag Khandelwal via Phabricator via cfe-commits
AMDChirag added a comment.

The test case will also be added here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91054

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


[PATCH] D91047: Add a call super attribute plugin example

2020-11-09 Thread Yafei Liu via Phabricator via cfe-commits
psionic12 added inline comments.



Comment at: clang/test/Frontend/plugin-call-super.cpp:7
+struct Base2 { [[call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[call_super]] virtual void 
Test() override final; };
+// CALLSUPER: warning: call_super attribute attached on a final method

If I remove the keyword "virtual" here, the isVirtual() will return false.

To my knowledge "final" means this method cannot be overridden, but it is still 
a virtual method, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91047

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


[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision.
lebedev.ri added reviewers: aaron.ballman, njames93, JonasToth.
lebedev.ri added a project: LLVM.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.
lebedev.ri requested review of this revision.

While casting an (integral) pointer to an integer is obvious - you just get
the integral value of the pointer, casting an integer to an (integral) pointer
is deceivingly different. While you will get a pointer with that integral value,
if you got that integral value via a pointer-to-integer cast originally,
the new pointer will lack the provenance information from the original pointer.

So while (integral) pointer to integer casts are effectively no-ops,
and are transparent to the optimizer, integer to (integral) pointer casts
are *NOT* transparent, and may conceal information from optimizer.

While that may be the intention, it is not always so. For example,
let's take a look at a routine to align the pointer up to the multiple of 16:
The obvious, naive implementation for that is:

  char* src(char* maybe_underbiased_ptr) {
uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
return (char*)aligned_intptr; // warning: avoid integer to pointer casts 
[misc-no-inttoptr]
  }

The check will rightfully diagnose that cast.

But when provenance concealment is not the goal of the code, but an accident,
this example can be rewritten as follows, without using integer to pointer cast:

  char*
  tgt(char* maybe_underbiased_ptr) {
  uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
  uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
  uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
  uintptr_t bias = aligned_intptr - maybe_underbiased_intptr;
  return maybe_underbiased_ptr + bias;
  }

See also:

- D71499 
- Juneyoung Lee, Chung-Kil Hur, Ralf Jung, Zhengyang Liu, John Regehr, and Nuno 
P. Lopes. 2018. Reconciling High-Level Optimizations and Low-Level Code in 
LLVM. Proc. ACM Program. Lang. 2, OOPSLA, Article 125 (November 2018), 28 
pages. 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91055

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp
  clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.c
  clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s misc-no-inttoptr %t
+
+// can not implicitly cast int to a pointer.
+// can not use static_cast<>() to cast integer to a pointer.
+// can not use dynamic_cast<>() to cast integer to a pointer.
+// can not use const_cast<>() to cast integer to a pointer.
+
+void *t0(long long int x) {
+  return (void *)x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+
+void *t1(int x) {
+  return reinterpret_cast(x);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.c
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s misc-no-inttoptr %t
+
+void *t0(char x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+void *t1(signed char x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+void *t2(unsigned char x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+
+void *t3(short x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+void *t4(unsigned short x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+void *t5(signed short x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+
+void *t6(int x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr]
+}
+

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

2020-11-09 Thread Hans Wennborg via Phabricator via cfe-commits
hans marked 3 inline comments as done.
hans added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5898
 // encountered.
   } else if (!MD->isTrivial() || MD->isExplicitlyDefaulted() ||
  MD->isCopyAssignmentOperator() ||

rnk wrote:
> Part of me wants to handle explicitly defaulted things separately from 
> implicit special members, so we don't have to check for explicitly 
> defaulted-ness twice.
Thanks, that makes it a bit nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90849

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


[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

2020-11-09 Thread Hans Wennborg via Phabricator via cfe-commits
hans updated this revision to Diff 303778.
hans marked an inline comment as done.
hans added a comment.

Addressing comments.


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

https://reviews.llvm.org/D90849

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport.cpp


Index: clang/test/CodeGenCXX/dllexport.cpp
===
--- clang/test/CodeGenCXX/dllexport.cpp
+++ clang/test/CodeGenCXX/dllexport.cpp
@@ -915,6 +915,36 @@
 // M32-DAG: define dso_local x86_thiscallcc void 
@"??$baz@H@?$ExportedClassTemplate2@H@pr34849@@QAEXXZ"
 }
 
+namespace pr47683 {
+struct X { X() {} };
+
+template  struct S {
+  S() = default;
+  X x;
+};
+template struct __declspec(dllexport) S;
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc 
%"struct.pr47683::S"* @"??0?$S@H@pr47683@@QAE@XZ"
+
+template  struct T {
+  T() = default;
+  X x;
+};
+extern template struct T;
+template struct __declspec(dllexport) T;
+// Don't assert about multiple codegen for explicitly defaulted method in 
explicit instantiation def.
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc 
%"struct.pr47683::T"* @"??0?$T@H@pr47683@@QAE@XZ"
+
+template  struct U {
+  U();
+  X x;
+};
+template  U::U() = default;
+extern template struct U;
+template struct __declspec(dllexport) U;
+// Same as T, but with out-of-line ctor.
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc 
%"struct.pr47683::U"* @"??0?$U@H@pr47683@@QAE@XZ"
+}
+
 
//===--===//
 // Classes with template base classes
 
//===--===//
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5895,13 +5895,22 @@
 
 // The function will be passed to the consumer when its definition is
 // encountered.
-  } else if (!MD->isTrivial() || MD->isExplicitlyDefaulted() ||
+  } else if (MD->isExplicitlyDefaulted()) {
+// Synthesize and instantiate explicitly defaulted methods.
+S.MarkFunctionReferenced(Class->getLocation(), MD);
+
+if (TSK != TSK_ExplicitInstantiationDefinition) {
+  // Except for explicit instantiation defs, we will not see the
+  // definition again later, so pass it to the consumer now.
+  S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
+}
+  } else if (!MD->isTrivial() ||
  MD->isCopyAssignmentOperator() ||
  MD->isMoveAssignmentOperator()) {
-// Synthesize and instantiate non-trivial implicit methods, explicitly
-// defaulted methods, and the copy and move assignment operators. The
-// latter are exported even if they are trivial, because the address of
-// an operator can be taken and should compare equal across libraries.
+// Synthesize and instantiate non-trivial implicit methods, and the 
copy
+// and move assignment operators. The latter are exported even if they
+// are trivial, because the address of an operator can be taken and
+// should compare equal across libraries.
 S.MarkFunctionReferenced(Class->getLocation(), MD);
 
 // There is no later point when we will see the definition of this


Index: clang/test/CodeGenCXX/dllexport.cpp
===
--- clang/test/CodeGenCXX/dllexport.cpp
+++ clang/test/CodeGenCXX/dllexport.cpp
@@ -915,6 +915,36 @@
 // M32-DAG: define dso_local x86_thiscallcc void @"??$baz@H@?$ExportedClassTemplate2@H@pr34849@@QAEXXZ"
 }
 
+namespace pr47683 {
+struct X { X() {} };
+
+template  struct S {
+  S() = default;
+  X x;
+};
+template struct __declspec(dllexport) S;
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.pr47683::S"* @"??0?$S@H@pr47683@@QAE@XZ"
+
+template  struct T {
+  T() = default;
+  X x;
+};
+extern template struct T;
+template struct __declspec(dllexport) T;
+// Don't assert about multiple codegen for explicitly defaulted method in explicit instantiation def.
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.pr47683::T"* @"??0?$T@H@pr47683@@QAE@XZ"
+
+template  struct U {
+  U();
+  X x;
+};
+template  U::U() = default;
+extern template struct U;
+template struct __declspec(dllexport) U;
+// Same as T, but with out-of-line ctor.
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.pr47683::U"* @"??0?$U@H@pr47683@@QAE@XZ"
+}
+
 //===--===//
 // Classes with template base classes
 //===--===//
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib

[PATCH] D90928: [OpenCL] Add assertions to extension lookup

2020-11-09 Thread Erik Tomusk via Phabricator via cfe-commits
erik2020 added a comment.

In D90928#2379322 , @Anastasia wrote:

> Ok, it would still segfault but perhaps it is acceptable as this is an 
> internal frontend only option for now.

Would it be better if these functions returned `false` for unknown extensions? 
I think it would be consistent with the function names (e.g., `isEnabled()` 
returns `false` for an unknown extensions, because an unknown extension cannot 
be enabled).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90928

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


[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Nuno Lopes via Phabricator via cfe-commits
nlopes added a comment.

Nice!
BTW, another popular idiom is to store data in the last few bits of the pointer 
(e.g., LLVM's own PointerIntPair). I guess that one can also be implement by 
casting the ptr to char* and doing operations over that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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


[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment.

Out of curiosity: why is this in clang-tidy and not in clang?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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


[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D91055#2382112 , @nlopes wrote:

> Nice!

Why thank you.

> BTW, another popular idiom is to store data in the last few bits of the 
> pointer (e.g., LLVM's own PointerIntPair). I guess that one can also be 
> implement by casting the ptr to char* and doing operations over that.

Yep, there is a number of such patterns.
My basic idea behind this patch is that it is very much non-obvious whether

1. such a cast was actually intended to be there
2. the provenance obscurement is intentional

so let's just diagnose everything, and the user can either silence it, or 
rewrite it without the cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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


[PATCH] D90949: [clang-format] avoid introducing multiline comments

2020-11-09 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir updated this revision to Diff 303788.
krasimir marked 3 inline comments as done.
krasimir added a comment.

- address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90949

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -745,6 +745,24 @@
getLLVMStyleWithColumns(49)));
 }
 
+TEST_F(FormatTestComments, DontIntroduceMultilineComments) {
+  // Avoid introducing a multiline comment by breaking after `\`.
+  for (int ColumnLimit = 15; ColumnLimit <= 17; ++ColumnLimit) {
+EXPECT_EQ(
+"// aa\n"
+"// \\ bb",
+format("// aa \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  bb",
+format("// a \\  bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  \\ bb",
+format("// a \\  \\ bb", 
getLLVMStyleWithColumns(ColumnLimit)));
+  }
+}
+
 TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) {
   FormatStyle Pragmas = getLLVMStyleWithColumns(30);
   Pragmas.CommentPragmas = "^ IWYU pragma:";
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -101,17 +101,37 @@
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
   static const auto kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+  // Some spaces are unacceptable to break on, rewind past them.
   while (SpaceOffset != StringRef::npos) {
+// If a line-comment ends with `\`, the next line continues the comment,
+// whether or not it starts with `//`. This is confusing and triggers
+// -Wcomment.
+// Avoid introducing multiline comments by not allowing a break right
+// after '\'.
+if (Style.isCpp()) {
+  StringRef::size_type LastNonBlank =
+  Text.find_last_not_of(Blanks, SpaceOffset);
+  if (LastNonBlank != StringRef::npos && Text[LastNonBlank] == '\\') {
+SpaceOffset = Text.find_last_of(Blanks, LastNonBlank);
+continue;
+  }
+}
+
 // Do not split before a number followed by a dot: this would be 
interpreted
 // as a numbered list, which would prevent re-flowing in subsequent passes.
-if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks))) {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  continue;
+}
+
 // Avoid ever breaking before a { in JavaScript.
-else if (Style.Language == FormatStyle::LK_JavaScript &&
- SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+if (Style.Language == FormatStyle::LK_JavaScript &&
+SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{') {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-else
-  break;
+  continue;
+}
+
+break;
   }
 
   if (SpaceOffset == StringRef::npos ||


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -745,6 +745,24 @@
getLLVMStyleWithColumns(49)));
 }
 
+TEST_F(FormatTestComments, DontIntroduceMultilineComments) {
+  // Avoid introducing a multiline comment by breaking after `\`.
+  for (int ColumnLimit = 15; ColumnLimit <= 17; ++ColumnLimit) {
+EXPECT_EQ(
+"// aa\n"
+"// \\ bb",
+format("// aa \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  bb",
+format("// a \\  bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  \\ bb",
+format("// a \\  \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+  }
+}
+
 TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) {
   FormatStyle Pragmas = getLLVMStyleWithColumns(30);
   Pragmas.CommentPragmas = "^ IWYU pragma:";
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -101,17 +101,37 @@
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
   static const auto kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+  // Some spaces are unacceptable to break on, rewind past them.
   while (SpaceOffset != String

[PATCH] D90949: [clang-format] avoid introducing multiline comments

2020-11-09 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added inline comments.



Comment at: clang/lib/Format/BreakableToken.cpp:109
+// after '\'.
+if (Style.isCpp()) {
+  StringRef::size_type LastNonBlank =

sammccall wrote:
> Do we really want to predicate this on isCpp()? `//` comments are allowed by 
> C99.
> Even if the warning only applies to C++ for some reason, the reasons for 
> confusion do not.
I think in Java and other non-C++-y languages, an `\` at the end of a 
line-comment line does not have any special meaning, hence I didn't want it to 
trigger in those cases.



Comment at: clang/lib/Format/BreakableToken.cpp:125
 else
   break;
   }

sammccall wrote:
> doesn't this mean that we won't loop? if Text ends with "blah \ \" then 
> you'll split between "blah" and the first "\"?
> 
> I guess this could be structured:
> 
> ```
> while () {
>   if (special case 1) {
> // adjust pos
> continue;
>   }
>   if (special case 2) {
> // adjust pos
> continue;
>   }
>   break;
> }
> ```
> 
> (This is equivalent to the old if/elseif/break which is too hard to add 
> complex conditions to)
Thank you! Updated this block and added a test case for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90949

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


[PATCH] D60193: [OpenCL] Added addrspace_cast operator

2020-11-09 Thread Marco Antognini via Phabricator via cfe-commits
mantognini added inline comments.



Comment at: clang/include/clang-c/Index.h:2057
+   */
+  CXCursor_CXXAddrspaceCastExpr = 129,
+

hans wrote:
> Anastasia wrote:
> > hans wrote:
> > > akyrtzi wrote:
> > > > Hi Anastasia, apologies for not catching this earlier, but libclang is 
> > > > intended to keep a stable ABI and changing the enumerator values breaks 
> > > > libclang's ABI compatibility.
> > > > 
> > > > Would you be able to make a follow-up commit and restore the enumerator 
> > > > values to their original values? I would suggest to add 
> > > > `CXCursor_CXXAddrspaceCastExpr` at the end and assign to it the next 
> > > > available value that is not already taken.
> > > It also broke the Python bindings, as someone reported here: 
> > > https://stackoverflow.com/questions/64542827/there-appears-to-be-mismatch-in-clang-llvm-ver-11-0-0-python-bindings
> > I guess it's too late to fix 11.0.0, does it make sense to fix 11.0.1?
> That's @tstellar 's call.
@tstellar, does this require any particular action from my side to be included 
in 11.0.1?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60193

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


[PATCH] D90765: [ARM][AArch64] Adding Neoverse V1 CPU support

2020-11-09 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas updated this revision to Diff 303803.
pratlucas added a comment.

Removing extra includes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90765

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  llvm/include/llvm/MC/SubtargetFeature.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -280,6 +280,12 @@
 ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_FP16 |
 ARM::AEK_RAS | ARM::AEK_DOTPROD,
 "8.2-A"));
+  EXPECT_TRUE(testARMCPU("neoverse-v1", "armv8.4-a", "crypto-neon-fp-armv8",
+ ARM::AEK_SEC | ARM::AEK_MP | ARM::AEK_VIRT |
+ ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB |
+ ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
+ ARM::AEK_FP16 | ARM::AEK_BF16 | ARM::AEK_DOTPROD,
+ "8.4-A"));
   EXPECT_TRUE(testARMCPU("cyclone", "armv8-a", "crypto-neon-fp-armv8",
  ARM::AEK_CRC | ARM::AEK_SEC | ARM::AEK_MP |
  ARM::AEK_VIRT | ARM::AEK_HWDIVARM |
@@ -322,7 +328,7 @@
  "7-S"));
 }
 
-static constexpr unsigned NumARMCPUArchs = 89;
+static constexpr unsigned NumARMCPUArchs = 90;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
@@ -881,6 +887,14 @@
   AArch64::AEK_LSE | AArch64::AEK_FP16 | AArch64::AEK_DOTPROD |
   AArch64::AEK_RCPC | AArch64::AEK_SSBS,
   "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+  "neoverse-v1", "armv8.4-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_RAS | AArch64::AEK_SVE | AArch64::AEK_SSBS |
+  AArch64::AEK_RCPC | AArch64::AEK_CRC | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_RAS | AArch64::AEK_LSE |
+  AArch64::AEK_RDM | AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+  AArch64::AEK_CRYPTO | AArch64::AEK_FP16 | AArch64::AEK_BF16,
+  "8.4-A"));
   EXPECT_TRUE(testAArch64CPU(
  "cortex-r82", "armv8-r", "crypto-neon-fp-armv8",
   AArch64::AEK_CRC | AArch64::AEK_RDM  | AArch64::AEK_SSBS|
@@ -1034,7 +1048,7 @@
   "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 43;
+static constexpr unsigned NumAArch64CPUArchs = 44;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/AArch64/cpus.ll
===
--- llvm/test/CodeGen/AArch64/cpus.ll
+++ llvm/test/CodeGen/AArch64/cpus.ll
@@ -20,6 +20,7 @@
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-x1 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=neoverse-e1 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=neoverse-n1 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=neoverse-v1 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=exynos-m3 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=exynos-m4 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=exynos-m5 2>&1 | FileCheck %s
Index: llvm/lib/Target/ARM/ARMSubtarget.h
===
--- llvm/lib/Target/ARM/ARMSubtarget.h
+++ llvm/lib/Target/ARM/ARMSubtarget.h
@@ -76,6 +76,7 @@
 Krait,
 Kryo,
 NeoverseN1,
+NeoverseV1,
 Swift
   };
   enum ARMProcClassEnum {
Index: llvm/lib/Target/ARM/ARMSubtarget.cpp
===
--- llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -314,6 +314,7 @@
 PreISelOperandLatencyAdjustment = 1;
 break;
   case NeoverseN1:
+  case NeoverseV1:
 break;
   case Swift:
 MaxInterleaveFactor = 2;
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -601,6 +601,9 @@
 def ProcX1  : SubtargetFeature<"cortex-x1", "ARMProcFamily", "CortexX1",
"Cortex-X1 ARM processors", []>;
 
+def ProcV1  : SubtargetFeature<"neoverse-v1", "ARMProcFamily",
+   "NeoverseV1", "Neoverse-V1 ARM processors", []>;
+
 def ProcKrait   : SubtargetFeature<"krait", "

[PATCH] D90555: [clangd] Handle duplicate enum constants in PopulateSwitch tweak

2020-11-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.

Fantastic, thanks!

Still LG, comments optional.




Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:61
 private:
+  class EnumAndCovered {
+  public:

for this kind of purpose, we'd typically just use `struct EnumAndCovered { 
const ECD *Enumerator; bool Covered; }` without encapsulation. But this is fine.

I think a name like `ExpectedCase` might hint a little more at the purpose. Up 
to you.



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:165
+auto Iter = EnumConstants.find(Val);
+if (Iter == EnumConstants.end())
+  return false;

njames93 wrote:
> sammccall wrote:
> > this says if you ever have a case statement that doesn't match an enum 
> > value, then we don't offer the tweak. Why?
> If the user puts in a case statement that doesn't directly match a value, 
> chances are they are doing something funky. In which case the tweak is likely 
> not going to help. WDYT?
I agree it signals that exhaustively enumerating the cases *might* not be 
useful. (It still could be though, e.g. imagine an enum that can't easily be 
extended where someone's added a sentinel value).

Since this is a code action rather than a diagnostic, I'd lean towards still 
offering the action, but either seems reasonable.
If you're going to bail out in this case, it definitely deserves a comment.



Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:170
+// we encounter it.
+if (IsCovered)
+  return false;

njames93 wrote:
> sammccall wrote:
> > Do we really need to specially detect or handle this?
> > 
> > Seems like a blind
> > ```
> > if (Iter != EnumConstants.end())
> >   Iter->second.second = true; // mark as covered
> > ```
> > would be enough here
> What are the rules for clangd tweaks on ill-formed code.  I'll remove this 
> anyhow.
Basically, don't crash on ill-formed code, try not to produce wildly invalid 
syntax.
Beyond that, we only really care about problems that come up a lot in practice.

(That said, if there are duplicated cases, I'm not sure that should stop us 
applying this tweak)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90555

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


[PATCH] D90949: [clang-format] avoid introducing multiline comments

2020-11-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.

Still LG




Comment at: clang/lib/Format/BreakableToken.cpp:109
+// after '\'.
+if (Style.isCpp()) {
+  StringRef::size_type LastNonBlank =

krasimir wrote:
> sammccall wrote:
> > Do we really want to predicate this on isCpp()? `//` comments are allowed 
> > by C99.
> > Even if the warning only applies to C++ for some reason, the reasons for 
> > confusion do not.
> I think in Java and other non-C++-y languages, an `\` at the end of a 
> line-comment line does not have any special meaning, hence I didn't want it 
> to trigger in those cases.
Sigh, I guess I forgot what isCpp() means again... :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90949

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


[PATCH] D90956: [clang][SVE] Additional macros implied by `-msve-vector-bits=`.

2020-11-09 Thread Francesco Petrogalli via Phabricator via cfe-commits
fpetrogalli updated this revision to Diff 303805.
fpetrogalli added a comment.

NFC: update tests to use `%clang_cc1` instead of `%clang` to prevent errors for 
missing includes: https://reviews.llvm.org/B77907


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90956

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_PREDICATE_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_PREDICATE_OPERATORS.cpp
  
clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS-expected-error-on-address.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/Preprocessor/aarch64-target-features.c

Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -440,14 +440,11 @@
 // CHECK-BFLOAT: __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
 
 // == Check sve-vector-bits flag.
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=128 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-128 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=256 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-256 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-512 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-1024 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS 128
-// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS 256
-// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS 512
-// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS 1024
-// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS 2048
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=128  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=128  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=256  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=256  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=512  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=1024 %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=2048 %s
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_BITS [[#VBITS:]]
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_PREDICATE_OPERATORS 1
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_VECTOR_OPERATORS 1
Index: clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -x c++ -triple aarch64-none-linux-gnu -target-feature \
+// RUN: +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall \
+// RUN: -emit-llvm -o - %s -msve-vector-bits=256 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -triple aarch64-none-linux-gnu -target-feature \
+// RUN: +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall \
+// RUN: -emit-llvm -o - %s -msve-vector-bits=512 | FileCheck %s --check-prefix=CHECK512
+// REQUIRES: aarch64-registered-target
+
+// Examples taken from section 3.7.3.3 of the SVE ACLE (Version
+// 00bet6) that can be found at
+// https://developer.arm.com/documentation/100987/latest
+
+#include 
+
+// Page 27, item 1.
+#if __ARM_FEATURE_SVE_BITS == 512 && __ARM_FEATURE_SVE_VECTOR_OPERATORS
+// CHECK512-LABEL: define  @_Z1f9__SVE_VLSIu11__SVInt32_tLj512EES_( %x.coerce,  %y.coerce)
+typedef svint32_t vec __attribute__((arm_sve_vector_bits(512)));
+auto f(vec x, vec y) { return x + y; } // Returns a vec.
+#endif
+
+// Page 27, item 3.
+typedef int8_t vec1 __attribute__((vector_size(32)));
+void f(vec1);
+#if __ARM_FEATURE_SVE_BITS == 256 && __ARM_FEATURE_SV

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment.

Nice! I wonder if this could be an off-by-default (maybe enabled by -Wpedantic) 
warning in clang instead?

Diagnostics like this could be very useful for CHERI since our capabilities are 
in some way tracking provenance  in hardware: we have bounds, permissions and a 
validity bit for all pointers.
All pointers must be derived from another valid pointer (CHERI capability) 
otherwise the validity bit is cleared.
Casting from an integer to a pointer always results in a value without 
provenance (lacking a tag bit in hardware) and will result in traps when 
dereferenced.
We do however treat uintptr_t as carrying provenance (casts retain the same 
permissions+bounds+validity bit in hardware as the original pointer).

Out compiler has some diagnostics to flag int -> pointer casts, so we warn 
about the example code when using long as the intermediate value (since that 
will cause run-time crashes if the value is dereferenced), but we do not warn 
for uintptr_t:
CHERI compiler explorer 


I just checked whether we emit diagnotics for the testscases here and 
discovered that we only diagnose explicit casts from  (non-uintptr_t) int -> 
pointer as lacking provenance, but are missing warnings for the 
-Wint-conversion case. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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


[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a subscriber: rsmith.
lebedev.ri added a comment.

CC'ing @rsmith regarding the suggestion/question of also having a clang 
diagnostic for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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


[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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


[PATCH] D90549: [Driver] Switch CHECK-DEBIAN-SPARC tests to use debian_multiarch_tree

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90549

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


[PATCH] D87981: [X86] AMX programming model prototype.

2020-11-09 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke added inline comments.



Comment at: clang/test/CodeGen/X86/amx_api.c:13
+  //CHECK-LABEL: @test_api
+  //CHECK: call <256 x i32> @llvm.x86.tileloadd64.internal
+  //CHECK: call <256 x i32> @llvm.x86.tdpbssd.internal

pengfei wrote:
> Shoud it check for 3 and only 3 `llvm.x86.tileloadd64.internal`?
I may create another simple test case that test __tile_loadd, __tile_dpbsud, 
__tile_stored seperately.



Comment at: llvm/test/CodeGen/X86/ipra-reg-usage.ll:6
 define preserve_allcc void @foo()#0 {
-; CHECK: foo Clobbered Registers: $cs $df $ds $eflags $eip $eiz $es $fpcw 
$fpsw $fs $gs $hip $ip $mxcsr $rip $riz $ss $ssp $bnd0 $bnd1 $bnd2 $bnd3 $cr0 
$cr1 $cr2 $cr3 $cr4 $cr5 $cr6 $cr7 $cr8 $cr9 $cr10 $cr11 $cr12 $cr13 $cr14 
$cr15 $dr0 $dr1 $dr2 $dr3 $dr4 $dr5 $dr6 $dr7 $dr8 $dr9 $dr10 $dr11 $dr12 $dr13 
$dr14 $dr15 $fp0 $fp1 $fp2 $fp3 $fp4 $fp5 $fp6 $fp7 $k0 $k1 $k2 $k3 $k4 $k5 $k6 
$k7 $mm0 $mm1 $mm2 $mm3 $mm4 $mm5 $mm6 $mm7 $r11 $st0 $st1 $st2 $st3 $st4 $st5 
$st6 $st7 $tmm0 $tmm1 $tmm2 $tmm3 $tmm4 $tmm5 $tmm6 $tmm7 $xmm16 $xmm17 $xmm18 
$xmm19 $xmm20 $xmm21 $xmm22 $xmm23 $xmm24 $xmm25 $xmm26 $xmm27 $xmm28 $xmm29 
$xmm30 $xmm31 $ymm0 $ymm1 $ymm2 $ymm3 $ymm4 $ymm5 $ymm6 $ymm7 $ymm8 $ymm9 
$ymm10 $ymm11 $ymm12 $ymm13 $ymm14 $ymm15 $ymm16 $ymm17 $ymm18 $ymm19 $ymm20 
$ymm21 $ymm22 $ymm23 $ymm24 $ymm25 $ymm26 $ymm27 $ymm28 $ymm29 $ymm30 $ymm31 
$zmm0 $zmm1 $zmm2 $zmm3 $zmm4 $zmm5 $zmm6 $zmm7 $zmm8 $zmm9 $zmm10 $zmm11 
$zmm12 $zmm13 $zmm14 $zmm15 $zmm16 $zmm17 $zmm18 $zmm19 $zmm20 $zmm21 $zmm22 
$zmm23 $zmm24 $zmm25 $zmm26 $zmm27 $zmm28 $zmm29 $zmm30 $zmm31 $r11b $r11bh 
$r11d $r11w $r11wh
+; CHECK: foo Clobbered Registers: $cs $df $ds $eflags $eip $eiz $es $fpcw 
$fpsw $fs $gs $hip $ip $mxcsr $rip $riz $ss $ssp $tmmcfg $bnd0 $bnd1 $bnd2 
$bnd3 $cr0 $cr1 $cr2 $cr3 $cr4 $cr5 $cr6 $cr7 $cr8 $cr9 $cr10 $cr11 $cr12 $cr13 
$cr14 $cr15 $dr0 $dr1 $dr2 $dr3 $dr4 $dr5 $dr6 $dr7 $dr8 $dr9 $dr10 $dr11 $dr12 
$dr13 $dr14 $dr15 $fp0 $fp1 $fp2 $fp3 $fp4 $fp5 $fp6 $fp7 $k0 $k1 $k2 $k3 $k4 
$k5 $k6 $k7 $mm0 $mm1 $mm2 $mm3 $mm4 $mm5 $mm6 $mm7 $r11 $st0 $st1 $st2 $st3 
$st4 $st5 $st6 $st7 $tmm0 $tmm1 $tmm2 $tmm3 $tmm4 $tmm5 $tmm6 $tmm7 $xmm16 
$xmm17 $xmm18 $xmm19 $xmm20 $xmm21 $xmm22 $xmm23 $xmm24 $xmm25 $xmm26 $xmm27 
$xmm28 $xmm29 $xmm30 $xmm31 $ymm0 $ymm1 $ymm2 $ymm3 $ymm4 $ymm5 $ymm6 $ymm7 
$ymm8 $ymm9 $ymm10 $ymm11 $ymm12 $ymm13 $ymm14 $ymm15 $ymm16 $ymm17 $ymm18 
$ymm19 $ymm20 $ymm21 $ymm22 $ymm23 $ymm24 $ymm25 $ymm26 $ymm27 $ymm28 $ymm29 
$ymm30 $ymm31 $zmm0 $zmm1 $zmm2 $zmm3 $zmm4 $zmm5 $zmm6 $zmm7 $zmm8 $zmm9 
$zmm10 $zmm11 $zmm12 $zmm13 $zmm14 $zmm15 $zmm16 $zmm17 $zmm18 $zmm19 $zmm20 
$zmm21 $zmm22 $zmm23 $zmm24 $zmm25 $zmm26 $zmm27 $zmm28 $zmm29 $zmm30 $zmm31 
$r11b $r11bh $r11d $r11w $r11wh $k0_k1 $k2_k3 $k4_k5 $k6_k7
   call void @bar1()

pengfei wrote:
> LuoYuanke wrote:
> > LuoYuanke wrote:
> > > pengfei wrote:
> > > > Why this patch affects the k registers?
> > > This looks wired to me too.  The patch only add "tmmcfg". I'll look into 
> > > it later.
> > I check the test case without my patch the k pair registers are clobbered. 
> > But FileCheck only match the strings, so the test passes. I can also remove 
> > "$k0_k1 $k2_k3 $k4_k5 $k6_k7" from the checking.
> > 
> > $ llc -enable-ipra -print-regusage test/CodeGen/X86/ipra-reg-usage.ll
> > foo Clobbered Registers: $cs $df $ds $eflags $eip $eiz $es $fpcw $fpsw $fs 
> > $gs $hip $ip $mxcsr $rip $riz $ss $ssp $bnd0 $bnd1 $bnd2 $bnd3 $cr0 $cr1 
> > $cr2 $cr3 $cr4 $cr5 $cr6
> > $cr7 $cr8 $cr9 $cr10 $cr11 $cr12 $cr13 $cr14 $cr15 $dr0 $dr1 $dr2 $dr3 $dr4 
> > $dr5 $dr6 $dr7 $dr8 $dr9 $dr10 $dr11 $dr12 $dr13 $dr14 $dr15 $fp0 $fp1 $fp2 
> > $fp3 $fp4 $fp5 $fp6 $fp7 $k0 $k1 $k2 $k3 $k4 $k5 $k6 $k7 $mm0 $mm1 $mm2 
> > $mm3 $mm4 $mm5 $mm6 $mm7 $r11 $st0 $st1 $st2 $st3 $st4 $st5 $st6 $st7 $tmm0 
> > $tmm1 $tmm2 $tmm3 $tmm4 $tmm5 $tmm6 $tmm7 $xmm16 $xmm17 $xmm18 $xmm19 
> > $xmm20 $xmm21 $xmm22 $xmm23 $xmm24 $xmm25 $xmm26 $xmm27 $xmm28 $xmm29 
> > $xmm30 $xmm31 $ymm0 $ymm1 $ymm2 $ymm3 $ymm4 $ymm5 $ymm6 $ymm7 $ymm8 $ymm9 
> > $ymm10 $ymm11 $ymm12 $ymm13 $ymm14 $ymm15 $ymm16 $ymm17 $ymm18 $ymm19 
> > $ymm20 $ymm21 $ymm22 $ymm23 $ymm24 $ymm25 $ymm26 $ymm27 $ymm28 $ymm29 
> > $ymm30 $ymm31 $zmm0 $zmm1 $zmm2 $zmm3
> > $zmm4 $zmm5 $zmm6 $zmm7 $zmm8 $zmm9 $zmm10 $zmm11 $zmm12 $zmm13 $zmm14 
> > $zmm15 $zmm16 $zmm17 $zmm18 $zmm19 $zmm20 $zmm21 $zmm22 $zmm23 $zmm24 
> > $zmm25 $zmm26 $zmm27 $zmm28 $zmm29 $zmm30 $zmm31 $r11b $r11bh $r11d $r11w 
> > $r11wh $k0_k1 $k2_k3 $k4_k5 $k6_k7
> > 
> You can commit a patch to fix it directly.
Done at https://reviews.llvm.org/D90810.



Comment at: llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir:94
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def 
dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-  ; CHECK:   STATEPOINT 0, 0, 1, @some_call, $rdi, 2, 0, 2, 0, 2, 5, 2, 0, 2, 
-1, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8

[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types

2020-11-09 Thread Joe Ellis via Phabricator via cfe-commits
joechrisellis created this revision.
joechrisellis added reviewers: peterwaller-arm, fpetrogalli, DavidTruby.
Herald added subscribers: cfe-commits, psnobl, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a reviewer: efriedma.
Herald added a project: clang.
joechrisellis requested review of this revision.

Lax vector conversions was behaving incorrectly for implicit casts
between scalable and fixed-length vector types. For example, this:

  #include 
  
  #define N __ARM_FEATURE_SVE_BITS
  #define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
  
  typedef svfloat32_t fixed_float32_t FIXED_ATTR;
  typedef svfloat64_t fixed_float64_t FIXED_ATTR;
  
  void allowed_depending() {
fixed_float32_t fs32;
svfloat64_t s64;
  
fs32 = s64;
  }

... would fail because the vectors have differing lane sizes. This patch
implements the correct behaviour for
-flax-vector-conversions={none,all,integer}. Specifically:

- -flax-vector-conversions=none prevents all lax vector conversions between 
scalable and fixed-sized vectors.
- -flax-vector-conversions=integer allows lax vector conversions between 
scalable and fixed-size vectors whose element types are integers.
- -flax-vector-conversions=all allows all lax vector conversions between 
scalable and fixed-size vectors (including those with floating point element 
types).

The implicit conversions are implemented as bitcasts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91067

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp

Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=none -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-none %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=integer -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-integer %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=all -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-all %s
+
+// lax-vector-all-no-diagnostics
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svfloat64_t fixed_float64_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svint64_t fixed_int64_t FIXED_ATTR;
+
+void allowed_always() {
+  fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // This explicit cast is always allowed, irrespective of the value of -flax-vector-conversions.
+  ff32 = (fixed_float32_t)sf64;
+}
+
+void allowed_with_integer_lax_conversions() {
+  fixed_int32_t fi32;
+  svint64_t si64;
+
+  // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
+  // -flax-vector-conversions={integer,all}.
+  fi32 = si64;
+  // lax-vector-none-error@-1 {{assigning to 'fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+}
+
+void allowed_with_all_lax_conversions() {
+  fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
+  // -flax-vector-conversions=all.
+  ff32 = sf64;
+  // lax-vector-none-error@-1 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1644,11 +1644,12 @@
 }
   }
 
-  if ((ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType()) &&
-  S.Context.areCompatibleSveTypes(FromType, ToType)) {
-ICK = ICK_SVE_Vector_Conversion;
-return true;
-  }
+  if (ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType())
+if (S.Context.areCompatibleSveTypes(FromType, ToType) ||
+S.Context.areLaxCompatibleSveTypes(FromType, ToType)) {
+  ICK = ICK_SVE_Vector_Conversion;
+  return true;
+}
 
   // We can perform the conversion between vector types in the following cases:
   // 1)vector types are equivalent AltiVec and GCC vector types
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2219,6 +2219,12 @@
   bool destIsVec

[PATCH] D91051: [clangd] Improve clangd-indexer performance

2020-11-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/indexer/IndexerMain.cpp:46
 Opts.CountReferences = true;
+Opts.FileFilter = [&](const SourceManager &SM, FileID FID) {
+  const auto *F = SM.getFileEntryForID(FID);

kadircet wrote:
> this changes the behavior of clangd-indexer slightly. it feels like an 
> okay-ish trade-off considering the 3x speed up, but might have dire 
> consequences.
> 
> Without this change, clangd-indexer can index headers in multiple 
> configurations, e.g. if you have src1.cc that includes a.h with a `-DFOO` and 
> src2.cc that includes a.h with `-DBAR`, a.h might end up producing different 
> symbols. All of them are being indexed at the moment. After this change, the 
> first one to index will win.
> 
> This is also what we do with background-index but we have different 
> requirements for this behavior, as we store only a single shard per source 
> file, even if we indexed sources in different configurations only one of them 
> would prevail (unless we postpone shard writing to end of indexing and 
> accumulate results in memory).
> 
> Also we are planning to make use of this binary in a server-like setup, and 
> use the produced index to serve multiple clients. In some situations keeping 
> symbols from multiple configurations might be really useful, but I am not so 
> sure about it as they'll still be collapsed if USR generation produces same 
> IDs for those symbols (and I think it will). So I am leaning towards making 
> this change, but I would like to hear what others think too.
+1, this looks like a good trade-off to me. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91051

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


[PATCH] D91051: [clangd] Improve clangd-indexer performance

2020-11-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

This seems like an accuracy/latency tradeoff in an environment where... it's 
not clear why we care about latency very much. Do we?

OTOH, how useful is it to have a static index that's more accurate if it's 
going to be shadowed by a dynamic index for the files you care most about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91051

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


[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

2020-11-09 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision.
thakis added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5917
 // There is no later point when we will see the definition of this
 // function, so pass it to the consumer now.
 S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));

should this grow a `assert(TSK != TSK_ExplicitInstantiationDefinition)` for 
symmetry?


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

https://reviews.llvm.org/D90849

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


[clang-tools-extra] 5918ef8 - [clangd] Handle duplicate enum constants in PopulateSwitch tweak

2020-11-09 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-11-09T12:14:53Z
New Revision: 5918ef8b1aac00af2f8dd8b99f3de7b176463444

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

LOG: [clangd] Handle duplicate enum constants in PopulateSwitch tweak

If an enum has different names for the same constant, make sure only the first 
one declared gets added into the switch. Failing to do so results in a compiler 
error as 2 case labels can't represent the same value.

```
lang=c
enum Numbers{
One,
Un = One,
Two,
Deux = Two,
Three,
Trois = Three
};

// Old behaviour
switch () {
  case One:
  case Un:
  case Two:
  case Duex:
  case Three:
  case Trois: break;
}

// New behaviour
switch () {
  case One:
  case Two:
  case Three: break;
}
```

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
llvm/include/llvm/ADT/DenseMapInfo.h

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
index a8ad90596681..852888f6a043 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -40,7 +40,8 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include 
 #include 
 
@@ -57,11 +58,27 @@ class PopulateSwitch : public Tweak {
   }
 
 private:
+  class ExpectedCase {
+  public:
+ExpectedCase(const EnumConstantDecl *Decl) : Data(Decl, false) {}
+bool isCovered() const { return Data.getInt(); }
+void setCovered(bool Val = true) { Data.setInt(Val); }
+const EnumConstantDecl *getEnumConstant() const {
+  return Data.getPointer();
+}
+
+  private:
+llvm::PointerIntPair Data;
+  };
+
   const DeclContext *DeclCtx = nullptr;
   const SwitchStmt *Switch = nullptr;
   const CompoundStmt *Body = nullptr;
   const EnumType *EnumT = nullptr;
   const EnumDecl *EnumD = nullptr;
+  // Maps the Enum values to the EnumConstantDecl and a bool signifying if its
+  // covered in the switch.
+  llvm::MapVector ExpectedCases;
 };
 
 REGISTER_TWEAK(PopulateSwitch)
@@ -112,21 +129,34 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
   if (!EnumD)
 return false;
 
-  // We trigger if there are fewer cases than enum values (and no case covers
-  // multiple values). This guarantees we'll have at least one case to insert.
-  // We don't yet determine what the cases are, as that means evaluating
-  // expressions.
-  auto I = EnumD->enumerator_begin();
-  auto E = EnumD->enumerator_end();
+  // We trigger if there are any values in the enum that aren't covered by the
+  // switch.
+
+  ASTContext &Ctx = Sel.AST->getASTContext();
+
+  unsigned EnumIntWidth = Ctx.getIntWidth(QualType(EnumT, 0));
+  bool EnumIsSigned = EnumT->isSignedIntegerOrEnumerationType();
 
-  for (const SwitchCase *CaseList = Switch->getSwitchCaseList();
-   CaseList && I != E; CaseList = CaseList->getNextSwitchCase(), I++) {
+  auto Normalize = [&](llvm::APSInt Val) {
+Val = Val.extOrTrunc(EnumIntWidth);
+Val.setIsSigned(EnumIsSigned);
+return Val;
+  };
+
+  for (auto *EnumConstant : EnumD->enumerators()) {
+ExpectedCases.insert(
+std::make_pair(Normalize(EnumConstant->getInitVal()), EnumConstant));
+  }
+
+  for (const SwitchCase *CaseList = Switch->getSwitchCaseList(); CaseList;
+   CaseList = CaseList->getNextSwitchCase()) {
 // Default likely intends to cover cases we'd insert.
 if (isa(CaseList))
   return false;
 
 const CaseStmt *CS = cast(CaseList);
-// Case statement covers multiple values, so just counting doesn't work.
+
+// GNU range cases are rare, we don't support them.
 if (CS->caseStmtIsGNURange())
   return false;
 
@@ -135,48 +165,36 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
 const ConstantExpr *CE = dyn_cast(CS->getLHS());
 if (!CE || CE->isValueDependent())
   return false;
+
+// Unsure if this case could ever come up, but prevents an unreachable
+// executing in getResultAsAPSInt.
+if (CE->getResultStorageKind() == ConstantExpr::RSK_None)
+  return false;
+auto Iter = ExpectedCases.find(Normalize(CE->getResultAsAPSInt()));
+if (Iter != ExpectedCases.end())
+  Iter->second.setCovered();
   }
 
-  // Only suggest tweak if we have more enumerators than cases.
-  return I != E;
+  return !llvm::all_of(ExpectedCases,
+   [](auto &Pair) { return Pair.second.isCovered(); });

[PATCH] D90555: [clangd] Handle duplicate enum constants in PopulateSwitch tweak

2020-11-09 Thread Nathan James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5918ef8b1aac: [clangd] Handle duplicate enum constants in 
PopulateSwitch tweak (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D90555?vs=303580&id=303821#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90555

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  llvm/include/llvm/ADT/DenseMapInfo.h

Index: llvm/include/llvm/ADT/DenseMapInfo.h
===
--- llvm/include/llvm/ADT/DenseMapInfo.h
+++ llvm/include/llvm/ADT/DenseMapInfo.h
@@ -14,6 +14,7 @@
 #define LLVM_ADT_DENSEMAPINFO_H
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringRef.h"
@@ -371,6 +372,26 @@
   }
 };
 
+/// Provide DenseMapInfo for APSInt, using the DenseMapInfo for APInt.
+template <> struct DenseMapInfo {
+  static inline APSInt getEmptyKey() {
+return APSInt(DenseMapInfo::getEmptyKey());
+  }
+
+  static inline APSInt getTombstoneKey() {
+return APSInt(DenseMapInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const APSInt &Key) {
+return static_cast(hash_value(Key));
+  }
+
+  static bool isEqual(const APSInt &LHS, const APSInt &RHS) {
+return LHS.getBitWidth() == RHS.getBitWidth() &&
+   LHS.isUnsigned() == RHS.isUnsigned() && LHS == RHS;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_DENSEMAPINFO_H
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2980,6 +2980,18 @@
 void function() { switch (ns::A) {case ns::A:break;} }
   )"",
   },
+  {
+  // Duplicated constant names
+  Function,
+  R""(enum Enum {A,B,b=B}; ^switch (A) {})"",
+  R""(enum Enum {A,B,b=B}; switch (A) {case A:case B:break;})"",
+  },
+  {
+  // Duplicated constant names all in switch
+  Function,
+  R""(enum Enum {A,B,b=B}; ^switch (A) {case A:case B:break;})"",
+  "unavailable",
+  },
   };
 
   for (const auto &Case : Cases) {
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -40,7 +40,8 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include 
 #include 
 
@@ -57,11 +58,27 @@
   }
 
 private:
+  class ExpectedCase {
+  public:
+ExpectedCase(const EnumConstantDecl *Decl) : Data(Decl, false) {}
+bool isCovered() const { return Data.getInt(); }
+void setCovered(bool Val = true) { Data.setInt(Val); }
+const EnumConstantDecl *getEnumConstant() const {
+  return Data.getPointer();
+}
+
+  private:
+llvm::PointerIntPair Data;
+  };
+
   const DeclContext *DeclCtx = nullptr;
   const SwitchStmt *Switch = nullptr;
   const CompoundStmt *Body = nullptr;
   const EnumType *EnumT = nullptr;
   const EnumDecl *EnumD = nullptr;
+  // Maps the Enum values to the EnumConstantDecl and a bool signifying if its
+  // covered in the switch.
+  llvm::MapVector ExpectedCases;
 };
 
 REGISTER_TWEAK(PopulateSwitch)
@@ -112,21 +129,34 @@
   if (!EnumD)
 return false;
 
-  // We trigger if there are fewer cases than enum values (and no case covers
-  // multiple values). This guarantees we'll have at least one case to insert.
-  // We don't yet determine what the cases are, as that means evaluating
-  // expressions.
-  auto I = EnumD->enumerator_begin();
-  auto E = EnumD->enumerator_end();
+  // We trigger if there are any values in the enum that aren't covered by the
+  // switch.
+
+  ASTContext &Ctx = Sel.AST->getASTContext();
+
+  unsigned EnumIntWidth = Ctx.getIntWidth(QualType(EnumT, 0));
+  bool EnumIsSigned = EnumT->isSignedIntegerOrEnumerationType();
 
-  for (const SwitchCase *CaseList = Switch->getSwitchCaseList();
-   CaseList && I != E; CaseList = CaseList->getNextSwitchCase(), I++) {
+  auto Normalize = [&](llvm::APSInt Val) {
+Val = Val.extOrTrunc(EnumIntWidth);
+Val.setIsSigned(EnumIsSigned);
+return Val;
+  };
+
+  for (auto *EnumConstant : EnumD->enumerators()) {
+ExpectedCases.insert(
+std::make_pair(Normalize(EnumConstant->getInitVal()), EnumConstant));
+  }
+
+  for (const SwitchCase *CaseList = Switc

[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types

2020-11-09 Thread Peter Waller via Phabricator via cfe-commits
peterwaller-arm added a comment.

fixed_float64_t appears in the commit message but also is unused.




Comment at: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp:15
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svint64_t fixed_int64_t FIXED_ATTR;
+

I can't see any uses of fixed_float64_t and fixed_int64_t?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91067

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


[PATCH] D90956: [clang][SVE] Additional macros implied by `-msve-vector-bits=`.

2020-11-09 Thread Peter Waller via Phabricator via cfe-commits
peterwaller-arm added inline comments.



Comment at: 
clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS-expected-error-on-address.c:6
+
+// Examples taken from section 3.7.3.3 of the SVE ACLE (Version
+// 00bet6) that can be found at




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90956

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


[PATCH] D82860: Port ObjCMTAction to new option parsing system

2020-11-09 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 commandeered this revision.
jansvoboda11 added a reviewer: dang.
jansvoboda11 added a comment.

Taking control of this revision, as Daniel is no longer involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82860

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


[PATCH] D90956: [clang][SVE] Additional macros implied by `-msve-vector-bits=`.

2020-11-09 Thread Peter Waller via Phabricator via cfe-commits
peterwaller-arm added a comment.

Suggestion: Where referencing sections you could include the title "3.7.3.3. 
Behavior specific to SVE vectors" instead of "3.7.3.3" for example. I think 
this makes it easier to spot misreferences and gives future souls a better 
chance to understand the context without indirection.




Comment at: 
clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_PREDICATE_OPERATORS.cpp:19
+auto f4(pred x, pred y) { return x & y; } // Returns a pred
+#endif

Do we need tests for unary operators, assignment and comparison operators?
(Page 28, item 3, subitems, 1, 3, 4)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90956

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


[PATCH] D90748: [clangd] Introduce config parsing for External blocks

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 303824.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Update comments
- Drop the warning for specifying server on non-grpc builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90748

Files:
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp


Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,6 +10,7 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
@@ -124,6 +125,25 @@
   ASSERT_THAT(Results, IsEmpty());
 }
 
+TEST(ParseYAML, ExternalBlock) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Index:
+  External:
+File: "foo"
+Server: ^"bar"
+MountPoint: "baz"
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_EQ(Results.size(), 1u);
+  ASSERT_TRUE(Results[0].Index.External);
+  EXPECT_THAT(*Results[0].Index.External->File, Val("foo"));
+  EXPECT_THAT(*Results[0].Index.External->MountPoint, Val("baz"));
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(*Results[0].Index.External->Server, Val("bar"));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -5,7 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
-
 #include "ConfigFragment.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
@@ -86,6 +85,20 @@
 DictParser Dict("Index", this);
 Dict.handle("Background",
 [&](Node &N) { F.Background = scalarValue(N, "Background"); });
+Dict.handle("External", [&](Node &N) {
+  F.External.emplace();
+  parse(*F.External, N);
+});
+Dict.parse(N);
+  }
+
+  void parse(Fragment::IndexBlock::ExternalBlock &F, Node &N) {
+DictParser Dict("External", this);
+Dict.handle("File", [&](Node &N) { F.File = scalarValue(N, "File"); });
+Dict.handle("Server",
+[&](Node &N) { F.Server = scalarValue(N, "Server"); });
+Dict.handle("MountPoint",
+[&](Node &N) { F.MountPoint = scalarValue(N, "MountPoint"); });
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -162,6 +162,22 @@
 /// This is checked for translation units only, not headers they include.
 /// Legal values are "Build" or "Skip".
 llvm::Optional> Background;
+/// An external index uses data source outside of clangd itself. This is
+/// usually prepared using clangd-indexer.
+/// Exactly one source (File/Server) should be configured.
+struct ExternalBlock {
+  /// Path to an index file generated by clangd-indexer. Relative paths may
+  /// be used, if config fragment is associated with a directory.
+  llvm::Optional> File;
+  /// Address and port number for a clangd-index-server. e.g.
+  /// `123.1.1.1:13337`.
+  llvm::Optional> Server;
+  /// Source root governed by this index. Default is the directory
+  /// associated with the config fragment. Absolute in case of user config
+  /// and relative otherwise. Should always use forward-slashes.
+  llvm::Optional> MountPoint;
+};
+llvm::Optional External;
   };
   IndexBlock Index;
 


Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,6 +10,7 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
@@ -124,6 +125,25 @@
   ASSERT_THAT(Results, IsEmpty());
 }
 
+TEST(ParseYAML, ExternalBlock) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Index:
+  External:
+File: "foo"
+Server: ^"bar"
+MountPoint: "baz"
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_EQ(Results.size(), 1u);
+  ASSERT_TRUE(Resu

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

2020-11-09 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5917
 // There is no later point when we will see the definition of this
 // function, so pass it to the consumer now.
 S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));

thakis wrote:
> should this grow a `assert(TSK != TSK_ExplicitInstantiationDefinition)` for 
> symmetry?
Hmm no, we can still have TSK==TSK_ExplicitInstantiationDefinition here, it's 
just for explicitly defaulted methods where we don't want to pass it to the 
consumer.


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

https://reviews.llvm.org/D90849

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


[PATCH] D82860: Port ObjCMTAction to new option parsing system

2020-11-09 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 updated this revision to Diff 303828.
jansvoboda11 added a comment.

Rebase on top of D82756 .


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

https://reviews.llvm.org/D82860

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -59,74 +59,20 @@
   OS << "[" << PrefixLength << "]";
 }
 
-class MarshallingKindInfo {
+class MarshallingInfo {
 public:
+  static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
+
   const Record &R;
-  const char *MacroName;
   bool ShouldAlwaysEmit;
   StringRef KeyPath;
   StringRef DefaultValue;
   StringRef NormalizedValuesScope;
-
-  void emit(raw_ostream &OS) const {
-write_cstring(OS, StringRef(getOptionSpelling(R)));
-OS << ", ";
-OS << ShouldAlwaysEmit;
-OS << ", ";
-OS << KeyPath;
-OS << ", ";
-emitScopedNormalizedValue(OS, DefaultValue);
-OS << ", ";
-emitSpecific(OS);
-  }
-
-  virtual Optional emitValueTable(raw_ostream &OS) const {
-return None;
-  }
-
-  virtual ~MarshallingKindInfo() = default;
-
-  static std::unique_ptr create(const Record &R);
-
-protected:
-  void emitScopedNormalizedValue(raw_ostream &OS,
- StringRef NormalizedValue) const {
-if (!NormalizedValuesScope.empty())
-  OS << NormalizedValuesScope << "::";
-OS << NormalizedValue;
-  }
-
-  virtual void emitSpecific(raw_ostream &OS) const = 0;
-  MarshallingKindInfo(const Record &R, const char *MacroName)
-  : R(R), MacroName(MacroName) {}
-};
-
-class MarshallingFlagInfo final : public MarshallingKindInfo {
-public:
-  bool IsPositive;
-
-  void emitSpecific(raw_ostream &OS) const override { OS << IsPositive; }
-
-  static std::unique_ptr create(const Record &R) {
-std::unique_ptr Ret(new MarshallingFlagInfo(R));
-Ret->IsPositive = R.getValueAsBit("IsPositive");
-// FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
-//   The constructor that is supposed to allow for Derived to Base
-//   conversion does not work. Remove this if we drop support for such
-//   configurations.
-return std::unique_ptr(Ret.release());
-  }
-
-private:
-  MarshallingFlagInfo(const Record &R)
-  : MarshallingKindInfo(R, "OPTION_WITH_MARSHALLING_FLAG") {}
-};
-
-class MarshallingStringInfo final : public MarshallingKindInfo {
-public:
   StringRef NormalizerRetTy;
   StringRef Normalizer;
   StringRef Denormalizer;
+  StringRef ValueMerger;
+  StringRef ValueExtractor;
   int TableIndex = -1;
   std::vector Values;
   std::vector NormalizedValues;
@@ -147,17 +93,29 @@
   static constexpr const char *ValueTablesDecl =
   "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
 
-  void emitSpecific(raw_ostream &OS) const override {
+  void emit(raw_ostream &OS) const {
+write_cstring(OS, StringRef(getOptionSpelling(R)));
+OS << ", ";
+OS << ShouldAlwaysEmit;
+OS << ", ";
+OS << KeyPath;
+OS << ", ";
+emitScopedNormalizedValue(OS, DefaultValue);
+OS << ", ";
 emitScopedNormalizedValue(OS, NormalizerRetTy);
 OS << ", ";
 OS << Normalizer;
 OS << ", ";
 OS << Denormalizer;
 OS << ", ";
+OS << ValueMerger;
+OS << ", ";
+OS << ValueExtractor;
+OS << ", ";
 OS << TableIndex;
   }
 
-  Optional emitValueTable(raw_ostream &OS) const override {
+  Optional emitValueTable(raw_ostream &OS) const {
 if (TableIndex == -1)
   return {};
 OS << "static const SimpleEnumValue " << ValueTableName << "[] = {\n";
@@ -173,23 +131,32 @@
 return StringRef(ValueTableName);
   }
 
-  static std::unique_ptr create(const Record &R) {
-assert(!isa(R.getValueInit("NormalizerRetTy")) &&
-   "String options must have a type");
-
-std::unique_ptr Ret(new MarshallingStringInfo(R));
-Ret->NormalizerRetTy = R.getValueAsString("NormalizerRetTy");
-
-Ret->Normalizer = R.getValueAsString("Normalizer");
-Ret->Denormalizer = R.getValueAsString("Denormalizer");
+  static MarshallingInfo create(const Record &R) {
+assert(!isa(R.getValueInit("KeyPath")) &&
+   !isa(R.getValueInit("DefaultValue")) &&
+   !isa(R.getValueInit("NormalizerRetTy")) &&
+   !isa(R.getValueInit("ValueMerger")) &&
+   "MarshallingInfo must have a type");
+
+MarshallingInfo Ret(R);
+Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
+Ret.KeyPath = R.getValueAsString("KeyPath");
+Ret.DefaultValue = R.getValueAsString("DefaultValue");
+Ret.NormalizedValuesScope = R.getValueAsStr

[PATCH] D82860: Port ObjCMTAction to new option parsing system

2020-11-09 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 requested review of this revision.
jansvoboda11 added a comment.

WDYT @Bigcheese?


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

https://reviews.llvm.org/D82860

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


[PATCH] D87981: [X86] AMX programming model prototype.

2020-11-09 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke updated this revision to Diff 303830.
LuoYuanke added a comment.

1. Address Pengfei's comments.
2. Add tilerelease at the epilog.
3. Fix a bug of emitting instruction that use subregister.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm/CodeGen/LiveIntervalUnion.h
  llvm/include/llvm/CodeGen/LiveRegMatrix.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TileShapeInfo.h
  llvm/include/llvm/CodeGen/VirtRegMap.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/CodeGen/InlineSpiller.cpp
  llvm/lib/CodeGen/LiveIntervalUnion.cpp
  llvm/lib/CodeGen/LiveRegMatrix.cpp
  llvm/lib/CodeGen/VirtRegMap.cpp
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86PreTileConfig.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86RegisterInfo.h
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Target/X86/X86TileConfig.cpp
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/ipra-reg-usage.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
  llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir

Index: llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir
===
--- llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir
+++ llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir
@@ -108,7 +108,7 @@
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
   ; CHECK:   MOV64mr [[STACK0:%stack.[0-9]+]], 1, $noreg, 0, $noreg, killed $rbx :: (store 8 into [[STACK0]])
   ; CHECK:   MOV64mr [[STACK1:%stack.[0-9]+]], 1, $noreg, 0, $noreg, killed $r14 :: (store 8 into [[STACK1]])
-  ; CHECK:   STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, [[STACK0]], 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK0]]), (load store 8 on [[STACK1]])
+  ; CHECK:   STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, [[STACK0]], 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK1]]), (load store 8 on [[STACK0]])
   ; CHECK-DAG:   $rbx = MOV64rm [[STACK0]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK0]])
   ; CHECK-DAG:   $r14 = MOV64rm [[STACK1]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK1]])
   ; CHECK:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -121,7 +121,7 @@
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
   ; CHECK-DAG:   MOV64mr [[STACK0]], 1, $noreg, 0, $noreg, killed $rbx :: (store 8 into [[STACK0]])
   ; CHECK-DAG:   MOV64mr [[STACK1]], 1, $noreg, 0, $noreg, killed $r14 :: (store 8 into [[STACK1]])
-  ; CHECK:   STATEPOINT 0, 0, 0, @bar, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, %stack.0, 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK0]]), (load store 8 on [[STACK1]])
+  ; CHECK:   STATEPOINT 0, 0, 0, @bar, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, %stack.0, 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK1]]), (load store 8 on [[STACK0]])
   ; CHECK-DAG:   $rbx = MOV64rm [[STACK0]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK0]])
   ; CHECK-DAG:   $r14 = MOV64rm [[STACK1]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK1]])
   ; CHECK:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
Index: llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
===
--- llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
+++ llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
@@ -91,7 +91,7 @@
   ; CHECK-DAG:   MOV64mr %stack.1, 1, $noreg, 0, $noreg, $rdi :: (store 8 into %stack.1)
   ; CHECK:   EH_LABEL 
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead 

[PATCH] D91051: [clangd] Improve clangd-indexer performance

2020-11-09 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment.

Thanks you all for your comments.

I will try to describe my thoughts:

- the more data `SymbolCollector` will collect, the greater the difference (`N` 
will increase in `this patch improves performance in N times`). E.g. collection 
call/contain relations will affect `clangd-indexer` performance significantly 
while `clangd` background indexer will not have such huge affection.
- performance improvement will help to update the index file more often (for 
remote server scenario). I mean an index file will be up to date every `K` 
commits instead of every `3 x K` commits (assume constant time difference 
between commits)
- the behavior with this patch is basically the same as clangd background 
indexer behavior.

If this patch could not be approved as is, then maybe we could add command line 
option to switch old/new behavior. What do you think? (Сan't think of a 
suitable name for this option =))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91051

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


[PATCH] D87981: [X86] AMX programming model prototype.

2020-11-09 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke marked 5 inline comments as done.
LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86TileConfig.cpp:101
+  unsigned SubIdx = (BitSize == 8) ? X86::sub_8bit : X86::sub_16bit;
+  unsigned Opc = (BitSize == 8) ? X86::MOV8mr : X86::MOV16mr;
+  MachineInstr *NewMI =

LuoYuanke wrote:
> akashk4 wrote:
> > I do not understand why wasn't PSTTILECFG used to store config to memory.  
> > I guess it will be difficult to do that because we do not know the scope of 
> > a transaction since the TILE_RELEASE is not supported.
> PSTTILECFG is use to store the config from tile config register to memory. 
> Here to need to load the config from memory to tile config register, so that 
> each tile data register is configured.
> 
> The LDTIELCFG has been inserted in the X86PreTileConfig pass. Since at 
> X86PreTileConfig pass we don't know the config of the tile physical 
> registers, the shape data in stack slot is zero. At the pass which is after 
> RA, we know the shape of tile registers, so we just fill in the shape in the 
> stack slot.
> 
> Do you mean to tile release at the end of each function which use AMX? Since 
> we config tile registers at the beginning of each function that use AMX, it 
> doesn't break AMX operation without tile release. But it may reduce the 
> overhead of thread switch with tile release, if AMX operation in only used 
> for a while in a thread. 
I add tilerelease in function's epilog if the function define AMX registers. 



Comment at: llvm/lib/Target/X86/X86TileConfig.cpp:117
+   FrameIdx, Offset)
+  .addImm(Imm);
+}

pengfei wrote:
> The format looks strange, I wonder why Lint didn't report it.
I modified the code, but it is reformated by lint again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

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


[PATCH] D90957: Frontend: Skip namespace around createVFSFromCompilerInvocation definition, NFC

2020-11-09 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D90957

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


[PATCH] D87981: [X86] AMX programming model prototype.

2020-11-09 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke updated this revision to Diff 303833.
LuoYuanke added a comment.

Reformt the code with lint fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm/CodeGen/LiveIntervalUnion.h
  llvm/include/llvm/CodeGen/LiveRegMatrix.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TileShapeInfo.h
  llvm/include/llvm/CodeGen/VirtRegMap.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/CodeGen/InlineSpiller.cpp
  llvm/lib/CodeGen/LiveIntervalUnion.cpp
  llvm/lib/CodeGen/LiveRegMatrix.cpp
  llvm/lib/CodeGen/VirtRegMap.cpp
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86PreTileConfig.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86RegisterInfo.h
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Target/X86/X86TileConfig.cpp
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/ipra-reg-usage.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
  llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir

Index: llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir
===
--- llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir
+++ llvm/test/CodeGen/X86/statepoint-fixup-shared-ehpad.mir
@@ -108,7 +108,7 @@
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
   ; CHECK:   MOV64mr [[STACK0:%stack.[0-9]+]], 1, $noreg, 0, $noreg, killed $rbx :: (store 8 into [[STACK0]])
   ; CHECK:   MOV64mr [[STACK1:%stack.[0-9]+]], 1, $noreg, 0, $noreg, killed $r14 :: (store 8 into [[STACK1]])
-  ; CHECK:   STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, [[STACK0]], 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK0]]), (load store 8 on [[STACK1]])
+  ; CHECK:   STATEPOINT 0, 0, 0, @foo, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, [[STACK0]], 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK1]]), (load store 8 on [[STACK0]])
   ; CHECK-DAG:   $rbx = MOV64rm [[STACK0]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK0]])
   ; CHECK-DAG:   $r14 = MOV64rm [[STACK1]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK1]])
   ; CHECK:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -121,7 +121,7 @@
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
   ; CHECK-DAG:   MOV64mr [[STACK0]], 1, $noreg, 0, $noreg, killed $rbx :: (store 8 into [[STACK0]])
   ; CHECK-DAG:   MOV64mr [[STACK1]], 1, $noreg, 0, $noreg, killed $r14 :: (store 8 into [[STACK1]])
-  ; CHECK:   STATEPOINT 0, 0, 0, @bar, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, %stack.0, 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK0]]), (load store 8 on [[STACK1]])
+  ; CHECK:   STATEPOINT 0, 0, 0, @bar, 2, 0, 2, 0, 2, 0, 2, 2, 1, 8, %stack.0, 0, 1, 8, [[STACK1]], 0, 2, 0, 2, 2, 0, 0, 1, 1, csr_64, implicit-def $rsp, implicit-def $ssp :: (load store 8 on [[STACK1]]), (load store 8 on [[STACK0]])
   ; CHECK-DAG:   $rbx = MOV64rm [[STACK0]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK0]])
   ; CHECK-DAG:   $r14 = MOV64rm [[STACK1]], 1, $noreg, 0, $noreg :: (load 8 from [[STACK1]])
   ; CHECK:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
Index: llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
===
--- llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
+++ llvm/test/CodeGen/X86/statepoint-fixup-invoke.mir
@@ -91,7 +91,7 @@
   ; CHECK-DAG:   MOV64mr %stack.1, 1, $noreg, 0, $noreg, $rdi :: (store 8 into %stack.1)
   ; CHECK:   EH_LABEL 
   ; CHECK:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-  ; CHECK:   STATEPOINT 0, 0,

[PATCH] D90983: Change algorithms to return iterators

2020-11-09 Thread 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/D90983/new/

https://reviews.llvm.org/D90983

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


[PATCH] D90765: [ARM][AArch64] Adding Neoverse V1 CPU support

2020-11-09 Thread Lucas Prates via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc2c2cc136013: [ARM][AArch64] Adding Neoverse V1 CPU support 
(authored by pratlucas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90765

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  llvm/include/llvm/MC/SubtargetFeature.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.cpp
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -280,6 +280,12 @@
 ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_FP16 |
 ARM::AEK_RAS | ARM::AEK_DOTPROD,
 "8.2-A"));
+  EXPECT_TRUE(testARMCPU("neoverse-v1", "armv8.4-a", "crypto-neon-fp-armv8",
+ ARM::AEK_SEC | ARM::AEK_MP | ARM::AEK_VIRT |
+ ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB |
+ ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
+ ARM::AEK_FP16 | ARM::AEK_BF16 | ARM::AEK_DOTPROD,
+ "8.4-A"));
   EXPECT_TRUE(testARMCPU("cyclone", "armv8-a", "crypto-neon-fp-armv8",
  ARM::AEK_CRC | ARM::AEK_SEC | ARM::AEK_MP |
  ARM::AEK_VIRT | ARM::AEK_HWDIVARM |
@@ -322,7 +328,7 @@
  "7-S"));
 }
 
-static constexpr unsigned NumARMCPUArchs = 89;
+static constexpr unsigned NumARMCPUArchs = 90;
 
 TEST(TargetParserTest, testARMCPUArchList) {
   SmallVector List;
@@ -881,6 +887,14 @@
   AArch64::AEK_LSE | AArch64::AEK_FP16 | AArch64::AEK_DOTPROD |
   AArch64::AEK_RCPC | AArch64::AEK_SSBS,
   "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+  "neoverse-v1", "armv8.4-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_RAS | AArch64::AEK_SVE | AArch64::AEK_SSBS |
+  AArch64::AEK_RCPC | AArch64::AEK_CRC | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_RAS | AArch64::AEK_LSE |
+  AArch64::AEK_RDM | AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+  AArch64::AEK_CRYPTO | AArch64::AEK_FP16 | AArch64::AEK_BF16,
+  "8.4-A"));
   EXPECT_TRUE(testAArch64CPU(
  "cortex-r82", "armv8-r", "crypto-neon-fp-armv8",
   AArch64::AEK_CRC | AArch64::AEK_RDM  | AArch64::AEK_SSBS|
@@ -1034,7 +1048,7 @@
   "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 43;
+static constexpr unsigned NumAArch64CPUArchs = 44;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
Index: llvm/test/CodeGen/AArch64/cpus.ll
===
--- llvm/test/CodeGen/AArch64/cpus.ll
+++ llvm/test/CodeGen/AArch64/cpus.ll
@@ -20,6 +20,7 @@
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-x1 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=neoverse-e1 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=neoverse-n1 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=neoverse-v1 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=exynos-m3 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=exynos-m4 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=exynos-m5 2>&1 | FileCheck %s
Index: llvm/lib/Target/ARM/ARMSubtarget.h
===
--- llvm/lib/Target/ARM/ARMSubtarget.h
+++ llvm/lib/Target/ARM/ARMSubtarget.h
@@ -76,6 +76,7 @@
 Krait,
 Kryo,
 NeoverseN1,
+NeoverseV1,
 Swift
   };
   enum ARMProcClassEnum {
Index: llvm/lib/Target/ARM/ARMSubtarget.cpp
===
--- llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -314,6 +314,7 @@
 PreISelOperandLatencyAdjustment = 1;
 break;
   case NeoverseN1:
+  case NeoverseV1:
 break;
   case Swift:
 MaxInterleaveFactor = 2;
Index: llvm/lib/Target/ARM/ARM.td
===
--- llvm/lib/Target/ARM/ARM.td
+++ llvm/lib/Target/ARM/ARM.td
@@ -601,6 +601,9 @@
 def ProcX1  : SubtargetFeature<"cortex-x1", "ARMProcFamily", "CortexX1",
"Cortex-X1 ARM processors", []>;
 
+def ProcV1  : SubtargetFeature<"neoverse-v1", "ARMProcFamily",

[clang] c2c2cc1 - [ARM][AArch64] Adding Neoverse V1 CPU support

2020-11-09 Thread Lucas Prates via cfe-commits

Author: Lucas Prates
Date: 2020-11-09T13:15:40Z
New Revision: c2c2cc13601374f987cb03dfc8ef841c64b14024

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

LOG: [ARM][AArch64] Adding Neoverse V1 CPU support

Add support for the Neoverse V1 CPU to the ARM and AArch64 backends.

This is based on patches from Mark Murray and Victor Campos.

Reviewed By: dmgreen

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

Added: 


Modified: 
clang/test/Driver/aarch64-cpus.c
clang/test/Driver/arm-cortex-cpus.c
llvm/include/llvm/MC/SubtargetFeature.h
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/ARMTargetParser.def
llvm/lib/Target/AArch64/AArch64.td
llvm/lib/Target/AArch64/AArch64Subtarget.cpp
llvm/lib/Target/AArch64/AArch64Subtarget.h
llvm/lib/Target/ARM/ARM.td
llvm/lib/Target/ARM/ARMSubtarget.cpp
llvm/lib/Target/ARM/ARMSubtarget.h
llvm/test/CodeGen/AArch64/cpus.ll
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/test/Driver/aarch64-cpus.c 
b/clang/test/Driver/aarch64-cpus.c
index 9cdf346148c3..139746823660 100644
--- a/clang/test/Driver/aarch64-cpus.c
+++ b/clang/test/Driver/aarch64-cpus.c
@@ -177,6 +177,8 @@
 // CORTEXX1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-x1"
 // RUN: %clang -target aarch64 -mcpu=cortex-a78  -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEXA78 %s
 // CORTEXA78: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a78"
+// RUN: %clang -target aarch64 -mcpu=neoverse-v1  -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-V1 %s
+// NEOVERSE-V1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-v1"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-r82  -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEXR82 %s
 // CORTEXR82: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-r82"

diff  --git a/clang/test/Driver/arm-cortex-cpus.c 
b/clang/test/Driver/arm-cortex-cpus.c
index 4481ba58fa64..5df872358a7a 100644
--- a/clang/test/Driver/arm-cortex-cpus.c
+++ b/clang/test/Driver/arm-cortex-cpus.c
@@ -840,6 +840,22 @@
 // CHECK-CORTEX-A76AE-SOFT: "-target-feature" "+soft-float"
 // CHECK-CORTEX-A76AE-SOFT: "-target-feature" "+soft-float-abi"
 
+// RUN: %clang -target arm -mcpu=neoverse-v1 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV84A %s
+// RUN: %clang -target arm -mcpu=neoverse-v1 -mlittle-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CPUV84A %s
+// CHECK-CPUV84A: "-cc1"{{.*}} "-triple" "armv8.4a-{{.*}}
+
+// RUN: %clang -target armeb -mcpu=neoverse-v1 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-BE-CPUV84A %s
+// RUN: %clang -target arm -mcpu=neoverse-v1 -mbig-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-BE-CPUV84A %s
+// CHECK-BE-CPUV84A: "-cc1"{{.*}} "-triple" "armebv8.4a-{{.*}}
+
+// RUN: %clang -target arm -mcpu=neoverse-v1 -mthumb -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CPUV84A-THUMB %s
+// RUN: %clang -target arm -mcpu=neoverse-v1 -mlittle-endian -mthumb -### -c 
%s 2>&1 | FileCheck -check-prefix=CHECK-CPUV84A-THUMB %s
+// CHECK-CPUV84A-THUMB: "-cc1"{{.*}} "-triple" "thumbv8.4a-{{.*}}
+
+// RUN: %clang -target armeb -mcpu=neoverse-v1 -mthumb -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-BE-CPUV84A-THUMB %s
+// RUN: %clang -target arm -mcpu=neoverse-v1 -mbig-endian -mthumb -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-BE-CPUV84A-THUMB %s
+// CHECK-BE-CPUV84A-THUMB: "-cc1"{{.*}} "-triple" "thumbebv8.4a-{{.*}}
+
 // RUN: %clang -target armv8a-arm-none-eabi -mcpu=cortex-x1 -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CORTEX-X1 %s
 // RUN: %clang -target armv8a-arm-none-eabi -mcpu=cortex-x1 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-X1-MFPU %s
 // CHECK-CORTEX-X1: "-cc1"{{.*}} "-triple" "armv8.2a-{{.*}} "-target-cpu" 
"cortex-x1"

diff  --git a/llvm/include/llvm/MC/SubtargetFeature.h 
b/llvm/include/llvm/MC/SubtargetFeature.h
index 01ea794a4bc3..cc36b25a4965 100644
--- a/llvm/include/llvm/MC/SubtargetFeature.h
+++ b/llvm/include/llvm/MC/SubtargetFeature.h
@@ -30,7 +30,7 @@ namespace llvm {
 class raw_ostream;
 class Triple;
 
-const unsigned MAX_SUBTARGET_WORDS = 3;
+const unsigned MAX_SUBTARGET_WORDS = 4;
 const unsigned MAX_SUBTARGET_FEATURES = MAX_SUBTARGET_WORDS * 64;
 
 /// Container class for subtarget features.

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.def 
b/llvm/include/llvm/Support/AArch64TargetParser.def
index e6bc1a2c5ff8..cbf0d5d079dd 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.def
+++ b/llvm/include/llvm/Support/AArch64TargetParser.def
@@ -150,6 +150,10 @@ AARCH64_CPU_NAME("neoverse-n1", ARMV8_2A, 
FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_DOTPROD | AArch64:

[clang-tools-extra] f0922ef - [clang-tidy] Remove bad assert after 3b9b90a1

2020-11-09 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-11-09T13:21:55Z
New Revision: f0922efddef49b7f9057f5c3d5b6b8968111aadf

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

LOG: [clang-tidy] Remove bad assert after 3b9b90a1

Forgot to remove it on push, just there to help debugging

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 5d63066490a6..d7160d52750f 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -754,7 +754,6 @@ IdentifierNamingCheck::getStyleForFile(StringRef FileName) 
const {
 assert(It.second);
 return It.first->getValue();
   }
-  assert(false);
   // Default construction gives an empty style.
   auto It = NamingStylesCache.try_emplace(Parent);
   assert(It.second);



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


[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types

2020-11-09 Thread Joe Ellis via Phabricator via cfe-commits
joechrisellis updated this revision to Diff 303838.
joechrisellis marked an inline comment as done.
joechrisellis added a comment.

Address @peterwaller-arm's comment regarding unused types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91067

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp

Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=none -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-none %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=integer -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-integer %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=all -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-all %s
+
+// lax-vector-all-no-diagnostics
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_always() {
+  fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // This explicit cast is always allowed, irrespective of the value of -flax-vector-conversions.
+  ff32 = (fixed_float32_t)sf64;
+}
+
+void allowed_with_integer_lax_conversions() {
+  fixed_int32_t fi32;
+  svint64_t si64;
+
+  // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
+  // -flax-vector-conversions={integer,all}.
+  fi32 = si64;
+  // lax-vector-none-error@-1 {{assigning to 'fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+}
+
+void allowed_with_all_lax_conversions() {
+  fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
+  // -flax-vector-conversions=all.
+  ff32 = sf64;
+  // lax-vector-none-error@-1 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1644,11 +1644,12 @@
 }
   }
 
-  if ((ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType()) &&
-  S.Context.areCompatibleSveTypes(FromType, ToType)) {
-ICK = ICK_SVE_Vector_Conversion;
-return true;
-  }
+  if (ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType())
+if (S.Context.areCompatibleSveTypes(FromType, ToType) ||
+S.Context.areLaxCompatibleSveTypes(FromType, ToType)) {
+  ICK = ICK_SVE_Vector_Conversion;
+  return true;
+}
 
   // We can perform the conversion between vector types in the following cases:
   // 1)vector types are equivalent AltiVec and GCC vector types
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2219,6 +2219,12 @@
   bool destIsVector = DestType->isVectorType();
   bool srcIsVector = SrcType->isVectorType();
   if (srcIsVector || destIsVector) {
+// Scalable vectors can be cast to and from liberally.
+if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType()) {
+  Kind = CK_BitCast;
+  return TC_Success;
+}
+
 // The non-vector type, if any, must have integral type.  This is
 // the same rule that C vector casts use; note, however, that enum
 // types are not integral in C++.
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8553,6 +8553,39 @@
  IsValidCast(SecondType, FirstType);
 }
 
+bool ASTContext::areLaxCompatibleSveTypes(QualType FirstType,
+  QualType SecondType) {
+  assert(((FirstType->isSizelessBuiltinType() && SecondType->isVectorType()) ||
+  (FirstType->isVectorType() && SecondType->isSizelessBuiltinType())) &&
+ "Expected SVE builtin type and vector type!");
+
+  auto IsLaxCompatible = [this](QualType FirstType, QualType SecondType) {
+if (const auto *BT = FirstType->getAs()) {
+  if (const auto *VT = SecondTyp

[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types

2020-11-09 Thread Joe Ellis via Phabricator via cfe-commits
joechrisellis added inline comments.



Comment at: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp:15
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svint64_t fixed_int64_t FIXED_ATTR;
+

peterwaller-arm wrote:
> I can't see any uses of fixed_float64_t and fixed_int64_t?
Good spot! Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91067

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


[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

As it happens, I'd arrived at exactly the same patch when I tried a build on a 
Debian/sparc64 system in the GCC compile farm (gcc202), submitted as D85582 
.

A couple of questions/caveats:

- I wouldn't call this patch a workaround, as you do in the summary: if `clang` 
cannot find the 32-bit libs, it's broken and needs to be fixed.
- Please reformat `Linux.cpp` as the pre-merge check requested.
- I'd be willing to accept this patch, but for one I'm not certain about LLVM 
policy about who may or may not do so.
- Besides, I believe it's a mistake to split the bug fix and the testsuite 
change into to different patches.  More about that in D90549 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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


[PATCH] D91000: [clang-tidy] CERT MSC24-C Obsolescent Functions check

2020-11-09 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

Quoting the revision summary:

> This checker guards against using some vulnerable C functions which are 
> mentioned in MSC24-C in obsolescent functions table.

Why don't we check the rest of the functions as well?
`asctime`, `atof`, `atoi`, `atol`, `atoll`, `ctime`, `fopen`, `freopen`, 
`rewind`, `setbuf`

Hm, I get that `cert-err34-c` will already diagnose the uses of `atof`, `atoi`, 
`atol`, `atoll`, but then why do we check `vfscanf`, `vscanf` then?
We should probably omit these, while documenting this.
On the other hand, I would recommend checking `asctime`, `ctime`, `fopen`, 
`freopen`, `rewind`, `setbuf` for the sake of completeness.
What do you think?

There is a mismatch between your code and docs too, regarding the function you 
check for.
In the code you match for `vsprintf`, `vsscanf`, `vswprintf`, `vswcanf`, 
`wcrtomb`, `wcscat`, but none of these are mentioned in the docs.

> The checker warns only if `STDC_LIB_EXT1` macro is defined and the value of 
> `STDC_WANT_LIB_EXT1` macro is `1` in this case it suggests the corresponding 
> functions from Annex K instead the vulnerable function.

I would suggest mentioning these macros and their **purpose** in the docs.
Eg. that the `STDC_WANT_LIB_EXT1` should be defined to `1` but the other is 
left to the implementation.

That being said, I would request more tests, demonstrating that this macro 
detection works accordingly.

This checker might be a bit noisy. Have you tried it on open-source projects?
If it is, we should probably note that in the docs as well.

In the tests, It is a good practice to demonstrate that the offered 
recommendation does not trigger yet another warning.
Don't forget to put a `no-warning` to highlight that.




Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:30
+  "::vfprintf", "::vfscanf", "vfwprintf", "vfwscanf", "vprintf", 
"::vscanf",
+  "::vsnprintf", "::vsprintf", "::vsscanf", "::vswprintf", "::vswcanf",
+  "::wcrtomb", "::wcscat", "::wcscpy", "::wcsncat", "::wcsncpy",

Hm, it must be a typo.
`vswcanf` -> `vswscanf`

By the same token, I miss these functions from this enumeration.
`vfwprintf`, `vfwscanf`, `vprintf`, `vwprintf`, `vwscanf`
However, they are present in the cert table you mentioned.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:33
+  "::wcsrtombs", "::wcstok", "::wcstombs", "::wctomb", "::wmemcpy",
+  "::wmemmove", "::wprintf", "::wscanf", "::strlen"));
+  Finder->addMatcher(

Why is `strlen` an //obsolescent// function? I don't feel it justified, even if 
there was a [[ 
https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions?focusedCommentId=88019519#comment-88019519
 | comment ]] about it on the cert page.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:69
+AreSafeFunctionsWanted = IntValue.getZExtValue();
+if (AreSafeFunctionsWanted.hasValue()) {
+  AnnexKIsWanted = AreSafeFunctionsWanted.getValue();

Eugene.Zelenko wrote:
> Please elide braces.
Please also check if the value is `1`, as your summary and the standard 
requires.



Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:77-79
+  std::pair AnnexKUsage = UsingAnnexK();
+  bool AnnexKIsAvailable = std::get<0>(AnnexKUsage);
+  bool AnnexKIsWanted = std::get<1>(AnnexKUsage);





Comment at: 
clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:87-89
+diag(Function->getExprLoc(), "Unsafe function " + FunctionName + " used. " 
+
+ "Safe " + FunctionName +
+ "_s can be used instead.");

We should probably use `llvm::Twine` instead of multiple `std::string` 
constructions.
I might be wrong though.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:89
+ "Safe " + FunctionName +
+ "_s can be used instead.");
+  }

`can` -> `should`
The same at the other diagnostic message.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:91
+  }
+  if (const auto *FunctionPointer = Result.Nodes.getNodeAs("fp")) {
+if (const FunctionDecl *FD = Result.Nodes.getNodeAs("fnp")) {

This and the previous `if` statement is exclusive, am I right?
If we have a match, that is **either** a callexpr **or** a vardecl.

I would prefer a `return` at the end of the previous `if` and an assert here 
that the `FunctionPointer` must be non-null.



Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h:27
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  const std:

[PATCH] D90549: [Driver] Switch CHECK-DEBIAN-SPARC tests to use debian_multiarch_tree

2020-11-09 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

As I'd mentioned in D90524 , I believe both 
patches should be merged into one.  Otherwise, no matter in which order you 
commit them, the buildbots might show failures otherwise.

Another point I'm not certain about is wether the checks for the old directory 
layout should be removed: shouldn't `clang` still work correctly on both?  If 
so, both should still be tested.  However, this is something I'm not familiar 
with, so I'll leave approval to someone who is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90549

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


[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

Hi Rainer!

Thanks for your review.

In D90524#2382555 , @ro wrote:

> As it happens, I'd arrived at exactly the same patch when I tried a build on 
> a Debian/sparc64 system in the GCC compile farm (gcc202), submitted as D85582 
> .
>
> A couple of questions/caveats:
>
> - I wouldn't call this patch a workaround, as you do in the summary: if 
> `clang` cannot find the 32-bit libs, it's broken and needs to be fixed.

I'm calling it a workaround because it's already called like that in the 
existing code comment (well, they call it "hack"):

> // FIXME: This is a bit of a hack. We should really unify this code for
> // reasoning about oslibdir spellings with the lib dir spellings in the
> // GCCInstallationDetector, but that is a more significant refactoring.



> - Please reformat `Linux.cpp` as the pre-merge check requested.

OK.

> - I'd be willing to accept this patch, but for one I'm not certain about LLVM 
> policy about who may or may not do so.
> - Besides, I believe it's a mistake to split the bug fix and the testsuite 
> change into to different patches.  More about that in D90549 
> .

OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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


[PATCH] D90984: Update matchers to be traverse-aware

2020-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I think this change should come with a warning in the release notes because it 
silently changes the behavior of existing matchers in a way that may be 
surprising to users. I think the behavior is correct in that these constructs 
are all implicit nodes that aren't really spelled in source, but users may have 
come to rely on the existing behavior. I will note that the default argument 
case is somewhat interesting to me because there is a defensible argument to be 
made that the expression is spelled in source (it's spelled at the location of 
the function declaration rather than at the call site) and that same logic 
applies to explicitly defaulted function declarations. However, given the goal 
of trying to model this on the syntactic form of what the user wrote, I think 
the proposed behavior is correct.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115
+
+  if (Finder->getASTContext().getParentMapContext().getTraversalKind() !=
+  TK_AsIs &&

Given how often this gets repeated (and what a mouthful it is), how about 
adding a static helper function `ClassifyTraversal()` (or some such) that 
returns the traversal kind given a `Finder`? (Alternatively, 
`isTraversalAsIs()` or some specific traversal behavior.)



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4056
   unsigned, N) {
-  return Node.getNumArgs() == N;
+  auto NumArgs = Node.getNumArgs();
+  if (Finder->getASTContext().getParentMapContext().getTraversalKind() ==

Please spell out the type.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4061
+  while (NumArgs) {
+const auto *Arg = Node.getArg(NumArgs - 1);
+if (!isa(Arg))

Likewise here.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4084
+return false;
+  const auto *Arg = Node.getArg(N);
+  if (Finder->getASTContext().getParentMapContext().getTraversalKind() !=

And here.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4311
   internal::Matcher, InnerMatcher) {
+  auto TK = Finder->getASTContext().getParentMapContext().getTraversalKind();
   for (const Expr *Arg : Node.arguments()) {

Spell the type out?



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5466
+TK_AsIs &&
+FD->isDefaulted())
+  return false;

In general (not specific to this matcher), I'd like to see some tests for 
explicitly defaulted functions vs implicitly defaulted ones. e.g., I think we 
should handle these differently, shouldn't we?
```
struct S {}; // Has implicitly defaulted constructor
struct T {
  T() = default; // Has explicitly defaulted constructor
};
```
Specific to this matcher, I am a bit concerned about the behavior of answering 
"isDefinition" based on whether a function is defaulted or not. The user has 
already gotten some `FunctionDecl` object, so why would the implicit vs 
explicit nature of the declaration matter for deciding whether the node is a 
definition? The behavior with `hasBody` makes sense to me because the body may 
be implicitly provided, but whether something is or is not a definition is a 
different kind of question.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7078
+TK_AsIs &&
+FD->isDefaulted())
+  return false;

Same question here about why defaulted matters as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90984

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


[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz updated this revision to Diff 303846.
glaubitz edited the summary of this revision.
glaubitz added a comment.
Herald added subscribers: steven.zhang, jrtc27.

Update as requested in previous review, also merge with D90549 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/debian_8_sparc64_tree/lib/sparc64-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_8_sparc64_tree/lib64/.keep
  clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/include/c++/4.9/.keep
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/include/sparc64-linux-gnu/c++/4.9/.keep
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/gcc/sparc64-linux-gnu/4.9/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/gcc/sparc64-linux-gnu/4.9/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crtn.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/lib64/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/include/c++/4.9/backward/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/include/sparc-linux-gnu/c++/4.9/64/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/64/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/64/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crtn.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crt1.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crti.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crtn.o
  clang/test/Driver/Inputs/debian_multiarch_tree/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_multiarch_tree/lib/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/c++/4.5/sparc-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/c++/4.5/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/sparc-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/gcc/sparc-linux-gnu/4.5/crtbegin.o
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/gcc/sparc64-linux-gnu/4.5/crtbegin.o
  clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/sparc64-linux-gnu/.keep
  clang/test/Driver/linux-header-search.cpp
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1282,67 +1282,32 @@
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib/gcc/mipsel-linux-gnu/4.5/../../.."
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/lib"
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib"
-//
-// Check linker paths on Debian 8 / Sparc
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=sparc-linux-gnu -rtlib=platform \
 // RUN: --gcc-toolchain="" \
-// RUN: --sysroot=%S/Inputs/debian_8_sparc_multilib_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-DEBIAN-SPARC32 %s
-// CHECK-DEBIAN-SPARC32: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu{{/|}}crt1.o"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu{{/|}}crti.o"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9{{/|}}crtbegin.o"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../../lib"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/lib/sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/lib"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr

[PATCH] D90549: [Driver] Switch CHECK-DEBIAN-SPARC tests to use debian_multiarch_tree

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment.

Merged into D90524 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90549

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


[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

2020-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:1837
   /// Actually a FunctionDefinitionKind.
-  unsigned FunctionDefinition : 2;
+  FunctionDefinitionKind FunctionDefinition : 2;
 

I think we need to keep this as `unsigned` because some compilers struggle with 
bit-fields of enumeration types (even when the enumeration underlying type is 
fixed): https://godbolt.org/z/P8x8Kz


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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


[PATCH] D91011: [NFC, Refactor] Rename the (scoped) enum DeclaratorContext's enumerators to avoid redundancy

2020-11-09 Thread 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/D91011/new/

https://reviews.llvm.org/D91011

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


[clang-tools-extra] 9ca6fc4 - Add a new altera kernel name restriction check to clang-tidy.

2020-11-09 Thread Aaron Ballman via cfe-commits

Author: Frank Derry Wanye
Date: 2020-11-09T09:26:50-05:00
New Revision: 9ca6fc4e095f9aacd70e406e640472ad2d370553

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

LOG: Add a new altera kernel name restriction check to clang-tidy.

The altera kernel name restriction check finds kernel files and include
directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl".
Such kernel file names cause the Altera Offline Compiler to generate
intermediate design files that have the same names as certain internal
files, which leads to a compilation error.

As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA
SDK for OpenCL Pro Edition: Programming Guide."

This reverts the reversion from 43a38a65233039b5e71797a644d41a890f8d7f2b.

Added: 
clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/uppercase/KERNEL.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/uppercase/VHDL.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/uppercase/vERILOG.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl

clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp

Modified: 
clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
clang-tools-extra/clang-tidy/altera/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp 
b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
index d91f67ac1485..d3e906b673ce 100644
--- a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "KernelNameRestrictionCheck.h"
 #include "StructPackAlignCheck.h"
 
 using namespace clang::ast_matchers;
@@ -20,6 +21,8 @@ namespace altera {
 class AlteraModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"altera-kernel-name-restriction");
 CheckFactories.registerCheck(
 "altera-struct-pack-align");
   }

diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
index ed28d9f4892d..8ab5cc1aa4ad 100644
--- a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_library(clangTidyAlteraModule
   AlteraTidyModule.cpp
+  KernelNameRestrictionCheck.cpp
   StructPackAlignCheck.cpp
 
   LINK_LIBS

diff  --git 
a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp 
b/clang-tools-extra/clang-tidy/altera/Kerne

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

2020-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D72218#2381149 , @ffrankies wrote:

> Moved test files `KERNEL.cl`, `VHDL.cl` and `vERILOG.cl` to the `uppercase` 
> subdirectory to prevent filename clashes in some environments.
>
> This is in response to the buildbot failure where `Verilog.cl`, `KERNEL.cl`, 
> and `VHDL.cl` were not present in the buildbot output despite showing up as 
> present in Differential.
>
> @aaron.ballman Can you please try committing this again?

I've recommit in 9ca6fc4e095f9aacd70e406e640472ad2d370553 
, thanks!


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

https://reviews.llvm.org/D72218

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


[PATCH] D91075: [clang] Fix ForStmt mustprogress handling

2020-11-09 Thread Atmn Patel via Phabricator via cfe-commits
atmnpatel created this revision.
atmnpatel added reviewers: jdoerfert, aqjune, aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
atmnpatel requested review of this revision.

D86841  had an error where for statements with 
no conditional were
required to make progress. This is not true, this patch removes that
line, and adds regression tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91075

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/attr-mustprogress-1.c
  clang/test/CodeGen/attr-mustprogress-1.cpp


Index: clang/test/CodeGen/attr-mustprogress-1.cpp
===
--- clang/test/CodeGen/attr-mustprogress-1.cpp
+++ clang/test/CodeGen/attr-mustprogress-1.cpp
@@ -7,6 +7,16 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f0v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @_Z2f1v(
 // CHECK-NEXT:  entry:
Index: clang/test/CodeGen/attr-mustprogress-1.c
===
--- clang/test/CodeGen/attr-mustprogress-1.c
+++ clang/test/CodeGen/attr-mustprogress-1.c
@@ -7,6 +7,17 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @f0(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+//
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @f1(
 // CHECK-NEXT:  entry:
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -947,7 +947,6 @@
   Expr::EvalResult Result;
   if (LanguageRequiresProgress()) {
 if (!S.getCond()) {
-  LoopMustProgress = true;
   FnIsMustProgress = false;
 } else if (!S.getCond()->EvaluateAsInt(Result, getContext())) {
   LoopMustProgress = true;


Index: clang/test/CodeGen/attr-mustprogress-1.cpp
===
--- clang/test/CodeGen/attr-mustprogress-1.cpp
+++ clang/test/CodeGen/attr-mustprogress-1.cpp
@@ -7,6 +7,16 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f0v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @_Z2f1v(
 // CHECK-NEXT:  entry:
Index: clang/test/CodeGen/attr-mustprogress-1.c
===
--- clang/test/CodeGen/attr-mustprogress-1.c
+++ clang/test/CodeGen/attr-mustprogress-1.c
@@ -7,6 +7,17 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @f0(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+//
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @f1(
 // CHECK-NEXT:  entry:
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -947,7 +947,6 @@
   Expr::EvalResult Result;
   if (LanguageRequiresProgress()) {
 if (!S.getCond()) {
-  LoopMustProgress = true;
   FnIsMustProgress = false;
 } else if (!S.getCond()->EvaluateAsInt(Result, getContext())) {
   LoopMustProgress = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 7b7170f - [clang-format] avoid introducing multiline comments

2020-11-09 Thread Krasimir Georgiev via cfe-commits

Author: Krasimir Georgiev
Date: 2020-11-09T15:29:09+01:00
New Revision: 7b7170fa5791a8fcf28418e6daa1ab7dd2126a25

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

LOG: [clang-format] avoid introducing multiline comments

In C++ with -Werror=comment, multiline comments are not allowed.
clang-format could accidentally introduce multiline comments when reflowing.
This adapts clang-format to not introduce multiline comments by not allowing a
break after `\`. Note that this does not apply to comment lines that already are
multiline comments, such as comments in macros.

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp
clang/unittests/Format/FormatTestComments.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index 2ef1540a7f82..c3853687c228 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -101,17 +101,37 @@ getCommentSplit(StringRef Text, unsigned 
ContentStartColumn,
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
   static const auto kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+  // Some spaces are unacceptable to break on, rewind past them.
   while (SpaceOffset != StringRef::npos) {
+// If a line-comment ends with `\`, the next line continues the comment,
+// whether or not it starts with `//`. This is confusing and triggers
+// -Wcomment.
+// Avoid introducing multiline comments by not allowing a break right
+// after '\'.
+if (Style.isCpp()) {
+  StringRef::size_type LastNonBlank =
+  Text.find_last_not_of(Blanks, SpaceOffset);
+  if (LastNonBlank != StringRef::npos && Text[LastNonBlank] == '\\') {
+SpaceOffset = Text.find_last_of(Blanks, LastNonBlank);
+continue;
+  }
+}
+
 // Do not split before a number followed by a dot: this would be 
interpreted
 // as a numbered list, which would prevent re-flowing in subsequent passes.
-if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks))) {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  continue;
+}
+
 // Avoid ever breaking before a { in JavaScript.
-else if (Style.Language == FormatStyle::LK_JavaScript &&
- SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+if (Style.Language == FormatStyle::LK_JavaScript &&
+SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{') {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-else
-  break;
+  continue;
+}
+
+break;
   }
 
   if (SpaceOffset == StringRef::npos ||

diff  --git a/clang/unittests/Format/FormatTestComments.cpp 
b/clang/unittests/Format/FormatTestComments.cpp
index 95afb17114a2..27dfe71367b3 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -745,6 +745,24 @@ TEST_F(FormatTestComments, 
DontSplitLineCommentsWithEscapedNewlines) {
getLLVMStyleWithColumns(49)));
 }
 
+TEST_F(FormatTestComments, DontIntroduceMultilineComments) {
+  // Avoid introducing a multiline comment by breaking after `\`.
+  for (int ColumnLimit = 15; ColumnLimit <= 17; ++ColumnLimit) {
+EXPECT_EQ(
+"// aa\n"
+"// \\ bb",
+format("// aa \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  bb",
+format("// a \\  bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  \\ bb",
+format("// a \\  \\ bb", 
getLLVMStyleWithColumns(ColumnLimit)));
+  }
+}
+
 TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) {
   FormatStyle Pragmas = getLLVMStyleWithColumns(30);
   Pragmas.CommentPragmas = "^ IWYU pragma:";



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


[PATCH] D90949: [clang-format] avoid introducing multiline comments

2020-11-09 Thread Krasimir Georgiev via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b7170fa5791: [clang-format] avoid introducing multiline 
comments (authored by krasimir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90949

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -745,6 +745,24 @@
getLLVMStyleWithColumns(49)));
 }
 
+TEST_F(FormatTestComments, DontIntroduceMultilineComments) {
+  // Avoid introducing a multiline comment by breaking after `\`.
+  for (int ColumnLimit = 15; ColumnLimit <= 17; ++ColumnLimit) {
+EXPECT_EQ(
+"// aa\n"
+"// \\ bb",
+format("// aa \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  bb",
+format("// a \\  bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  \\ bb",
+format("// a \\  \\ bb", 
getLLVMStyleWithColumns(ColumnLimit)));
+  }
+}
+
 TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) {
   FormatStyle Pragmas = getLLVMStyleWithColumns(30);
   Pragmas.CommentPragmas = "^ IWYU pragma:";
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -101,17 +101,37 @@
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
   static const auto kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+  // Some spaces are unacceptable to break on, rewind past them.
   while (SpaceOffset != StringRef::npos) {
+// If a line-comment ends with `\`, the next line continues the comment,
+// whether or not it starts with `//`. This is confusing and triggers
+// -Wcomment.
+// Avoid introducing multiline comments by not allowing a break right
+// after '\'.
+if (Style.isCpp()) {
+  StringRef::size_type LastNonBlank =
+  Text.find_last_not_of(Blanks, SpaceOffset);
+  if (LastNonBlank != StringRef::npos && Text[LastNonBlank] == '\\') {
+SpaceOffset = Text.find_last_of(Blanks, LastNonBlank);
+continue;
+  }
+}
+
 // Do not split before a number followed by a dot: this would be 
interpreted
 // as a numbered list, which would prevent re-flowing in subsequent passes.
-if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks))) {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+  continue;
+}
+
 // Avoid ever breaking before a { in JavaScript.
-else if (Style.Language == FormatStyle::LK_JavaScript &&
- SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{')
+if (Style.Language == FormatStyle::LK_JavaScript &&
+SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{') {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
-else
-  break;
+  continue;
+}
+
+break;
   }
 
   if (SpaceOffset == StringRef::npos ||


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -745,6 +745,24 @@
getLLVMStyleWithColumns(49)));
 }
 
+TEST_F(FormatTestComments, DontIntroduceMultilineComments) {
+  // Avoid introducing a multiline comment by breaking after `\`.
+  for (int ColumnLimit = 15; ColumnLimit <= 17; ++ColumnLimit) {
+EXPECT_EQ(
+"// aa\n"
+"// \\ bb",
+format("// aa \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  bb",
+format("// a \\  bb", getLLVMStyleWithColumns(ColumnLimit)));
+EXPECT_EQ(
+"// a\n"
+"// \\  \\ bb",
+format("// a \\  \\ bb", getLLVMStyleWithColumns(ColumnLimit)));
+  }
+}
+
 TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) {
   FormatStyle Pragmas = getLLVMStyleWithColumns(30);
   Pragmas.CommentPragmas = "^ IWYU pragma:";
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -101,17 +101,37 @@
   StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
 
   static const auto kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-09 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment.

I did not consider false positives, but it provides better visibility. It is 
always on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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


[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

2020-11-09 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Thanks for the test!

I think moving TextDiagnosticsPrinter to Basic is much better than making 
Format depend on Frontend. If you just want to fix your crash and be done with 
it, then doing that (and adding your test) is fine.

I think the best fix would be to change Basic to never emit diagnostics though. 
That would also fix this crash, and it'd completely remove clang-format's 
dependency on the huge array with all of clang's diag, which would 
significantly reduce the size of clang-format (iirc it'd cut it in half or 
something). This would need some reorganizing -- Basic would have to call some 
callback on errors instead, and in clang these callbacks would emit normal 
diags but in clang-format they'd do something else. If you wanted to look into 
that, that'd be amazing :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90121

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


[PATCH] D90799: [PowerPC] Add paired vector load and store builtins and intrinsics

2020-11-09 Thread Lei Huang via Phabricator via cfe-commits
lei added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:2658
+  return false;
+}
+

There's alot of nested `if`s, would it be possible to refactor to have some 
early exits instead?




Comment at: llvm/test/CodeGen/PowerPC/dform-pair-load-store.ll:2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 < %s | FileCheck %s
+

BE test?
Can we add `-ppc-asm-full-reg-names` and update the checks to also ensure the 
reg info is accurate?



Comment at: llvm/test/CodeGen/PowerPC/dform-pair-load-store.ll:4
+
+target datalayout = "e-m:e-i64:64-p:64:64-n32:64-v256:256:256-v512:512:512"
+

is this needed since we have the triple on the run line?



Comment at: llvm/test/CodeGen/PowerPC/loop-p10-pair-prepare.ll:3
+; RUN: llc -ppc-asm-full-reg-names -verify-machineinstrs -disable-lsr \
+; RUN: -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr10 < %s | FileCheck %s
+

BE test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90799

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


[PATCH] D91078: [clang-format] do not break before @tags in JS comments

2020-11-09 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
krasimir requested review of this revision.

In JavaScript breaking before a `@tag` in a comment puts it on a new line, and
machinery that parses these comments will fail to understand such comments.

This adapts clang-format to not break before `@`. Similar functionality exists
for not breaking before `{`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91078

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -187,22 +187,11 @@
 format("/** @returns j */", getGoogleJSStyleWithColumns(20)));
 
   // Break a single line long jsdoc comment pragma.
-  EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
-" */",
-format("/** @returns {string} jsdoc line 12 */",
-   getGoogleJSStyleWithColumns(20)));
   EXPECT_EQ("/**\n"
 " * @returns {string}\n"
 " * jsdoc line 12\n"
 " */",
 format("/** @returns {string} jsdoc line 12 */",
-   getGoogleJSStyleWithColumns(25)));
-
-  EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
-" */",
-format("/** @returns {string} jsdoc line 12  */",
getGoogleJSStyleWithColumns(20)));
 
   // FIXME: this overcounts the */ as a continuation of the 12 when breaking.
@@ -2194,6 +2183,16 @@
  " */",
  getGoogleJSStyleWithColumns(ColumnLimit));
   }
+  // don't break before @tags
+  verifyFormat("/**\n"
+   " * This\n"
+   " * tag @param\n"
+   " * stays.\n"
+   " */",
+   "/**\n"
+   " * This tag @param stays.\n"
+   " */",
+   getGoogleJSStyleWithColumns(13));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -124,9 +124,10 @@
   continue;
 }
 
-// Avoid ever breaking before a { in JavaScript.
+// Avoid ever breaking before a @tag or a { in JavaScript.
 if (Style.Language == FormatStyle::LK_JavaScript &&
-SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{') {
+SpaceOffset + 1 < Text.size() &&
+(Text[SpaceOffset + 1] == '{' || Text[SpaceOffset + 1] == '@')) {
   SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
   continue;
 }


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -187,22 +187,11 @@
 format("/** @returns j */", getGoogleJSStyleWithColumns(20)));
 
   // Break a single line long jsdoc comment pragma.
-  EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
-" */",
-format("/** @returns {string} jsdoc line 12 */",
-   getGoogleJSStyleWithColumns(20)));
   EXPECT_EQ("/**\n"
 " * @returns {string}\n"
 " * jsdoc line 12\n"
 " */",
 format("/** @returns {string} jsdoc line 12 */",
-   getGoogleJSStyleWithColumns(25)));
-
-  EXPECT_EQ("/**\n"
-" * @returns {string} jsdoc line 12\n"
-" */",
-format("/** @returns {string} jsdoc line 12  */",
getGoogleJSStyleWithColumns(20)));
 
   // FIXME: this overcounts the */ as a continuation of the 12 when breaking.
@@ -2194,6 +2183,16 @@
  " */",
  getGoogleJSStyleWithColumns(ColumnLimit));
   }
+  // don't break before @tags
+  verifyFormat("/**\n"
+   " * This\n"
+   " * tag @param\n"
+   " * stays.\n"
+   " */",
+   "/**\n"
+   " * This tag @param stays.\n"
+   " */",
+   getGoogleJSStyleWithColumns(13));
   verifyFormat("/**\n"
" * @see http://very/very/long/url/is/long\n";
" */",
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -124,9 +124,10 @@
   continue;
 }
 
-// Avoid ever breaking before a { in JavaScript.
+// Avoid ever breaking before a @tag or a { in JavaScript.
 if (Style.Language == FormatStyle::LK_JavaScript &&
-SpaceOffset + 1 < Text.size() && Text[SpaceOffset

[PATCH] D91075: [clang] Fix ForStmt mustprogress handling

2020-11-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision.
jdoerfert 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/D91075/new/

https://reviews.llvm.org/D91075

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


[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

2020-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:64
 void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
+  const auto hasStringCtorName =
+  hasAnyNameStdString(removeNamespaces(StringNames));

You should fix the lint warning.



Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:65
+  const auto hasStringCtorName =
+  hasAnyNameStdString(removeNamespaces(StringNames));
+

It took me a while to realize that this functionality is really about trying to 
get to the name of the constructor for a given type -- perhaps this should be a 
single function called `getCtorNameFromType()` or something?

Actually, given that this is a narrowing matcher from a `CXXConstructExpr`, 
can't you look at `CXXConstructExpr::getConstructor()` to get the 
`CXXConstructorDecl` and check the name from there? That seems cleaner than 
trying to identify the constructor name through string matching.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91015

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


[PATCH] D91078: [clang-format] do not break before @tags in JS comments

2020-11-09 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added inline comments.



Comment at: clang/unittests/Format/FormatTestJS.cpp:194
-format("/** @returns {string} jsdoc line 12 */",
-   getGoogleJSStyleWithColumns(20)));
   EXPECT_EQ("/**\n"

this actually demonstrates a bug fixed by this patch: with 20 column limit, the 
comment should be split. The new behavior makes the test with a column limit of 
25 redundant, so folded the two of them.



Comment at: clang/unittests/Format/FormatTestJS.cpp:205
-" */",
-format("/** @returns {string} jsdoc line 12  */",
getGoogleJSStyleWithColumns(20)));

this is an exact duplicate as the check on old line 190; removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91078

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


[PATCH] D90670: Simplifying memory globalization from the front end to move optimizations to the middle end.

2020-11-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

Let's start by adding an updated test to this so we can see how the result 
looks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90670

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


[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

2020-11-09 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1242
+def int_ubsantrap : Intrinsic<[], [llvm_i8_ty],
+  [IntrNoReturn, IntrCold, ImmArg>]>;
 

jdoerfert wrote:
> should this be readonly and inaccesiblememonly?
For `readonly` I'd say no (LangRef says "if a readonly function [...] has other 
side-effects, the behavior is undefined"), `inaccessiblememonly` maybe, but I 
doubt the definitions were written with something like a trap in mind so the 
whole topic seems pretty fuzzy to me.


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

https://reviews.llvm.org/D89959

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


[PATCH] D91051: [clangd] Improve clangd-indexer performance

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

Looks like everyone thinks that this sounds reasonable. So LGTM. Thanks for the 
patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91051

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


[clang] d093401 - [NFC] Remove string parameter of annotation attribute from AST childs.

2020-11-09 Thread via cfe-commits

Author: Tyker
Date: 2020-11-09T16:39:59+01:00
New Revision: d093401a2617d3c46aaed9eeaecf877e3ae1a9f1

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

LOG: [NFC] Remove string parameter of annotation attribute from AST childs.
this simplifies using annotation attributes when using clang as library

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/AST/ast-dump-attr.cpp
clang/test/Misc/pragma-attribute-cxx.cpp
clang/test/Misc/pragma-attribute-objc.m

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 687c03f55841..7ae5803f1c30 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -740,6 +740,16 @@ def Annotate : InheritableParamAttr {
   let Args = [StringArgument<"Annotation">, VariadicExprArgument<"Args">];
   // Ensure that the annotate attribute can be used with
   // '#pragma clang attribute' even though it has no subject list.
+  let AdditionalMembers = [{
+  static AnnotateAttr *Create(ASTContext &Ctx, llvm::StringRef Annotation, \
+  const AttributeCommonInfo &CommonInfo = {SourceRange{}}) {
+return AnnotateAttr::Create(Ctx, Annotation, nullptr, 0, CommonInfo);
+  }
+  static AnnotateAttr *CreateImplicit(ASTContext &Ctx, llvm::StringRef 
Annotation, \
+  const AttributeCommonInfo &CommonInfo = {SourceRange{}}) {
+return AnnotateAttr::CreateImplicit(Ctx, Annotation, nullptr, 0, 
CommonInfo);
+  }
+  }];
   let PragmaAttributeSupport = 1;
   let Documentation = [Undocumented];
 }

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index d8b819cf5bee..92b2b5c0dc43 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2391,7 +2391,6 @@ llvm::Constant 
*CodeGenModule::EmitAnnotationLineNo(SourceLocation L) {
 
 llvm::Constant *CodeGenModule::EmitAnnotationArgs(const AnnotateAttr *Attr) {
   ArrayRef Exprs = {Attr->args_begin(), Attr->args_size()};
-  Exprs = Exprs.drop_front();
   if (Exprs.empty())
 return llvm::ConstantPointerNull::get(Int8PtrTy);
 

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 71fccc4c68e7..ec3ea9ebd85e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3686,7 +3686,7 @@ void Sema::AddAnnotationAttr(Decl *D, const 
AttributeCommonInfo &CI,
  StringRef Str, MutableArrayRef Args) {
   auto *Attr = AnnotateAttr::Create(Context, Str, Args.data(), Args.size(), 
CI);
   llvm::SmallVector Notes;
-  for (unsigned Idx = 1; Idx < Attr->args_size(); Idx++) {
+  for (unsigned Idx = 0; Idx < Attr->args_size(); Idx++) {
 Expr *&E = Attr->args_begin()[Idx];
 assert(E && "error are handled before");
 if (E->isValueDependent() || E->isTypeDependent())
@@ -3718,7 +3718,7 @@ void Sema::AddAnnotationAttr(Decl *D, const 
AttributeCommonInfo &CI,
 /// current language mode.
 if (!Result || !Notes.empty()) {
   Diag(E->getBeginLoc(), diag::err_attribute_argument_n_type)
-  << CI << Idx << AANT_ArgumentConstantExpr;
+  << CI << (Idx + 1) << AANT_ArgumentConstantExpr;
   for (auto &Note : Notes)
 Diag(Note.first, Note.second);
   return;
@@ -3737,8 +3737,8 @@ static void handleAnnotateAttr(Sema &S, Decl *D, const 
ParsedAttr &AL) {
 return;
 
   llvm::SmallVector Args;
-  Args.reserve(AL.getNumArgs());
-  for (unsigned Idx = 0; Idx < AL.getNumArgs(); Idx++) {
+  Args.reserve(AL.getNumArgs() - 1);
+  for (unsigned Idx = 1; Idx < AL.getNumArgs(); Idx++) {
 assert(!AL.isArgIdent(Idx));
 Args.push_back(AL.getArgAsExpr(Idx));
   }

diff  --git a/clang/test/AST/ast-dump-attr.cpp 
b/clang/test/AST/ast-dump-attr.cpp
index 3207f692fc23..c2bd768dc2ad 100644
--- a/clang/test/AST/ast-dump-attr.cpp
+++ b/clang/test/AST/ast-dump-attr.cpp
@@ -252,12 +252,10 @@ int mergeAttrTest() 
__attribute__((unused,no_thread_safety_analysis));
 // CHECK-NEXT: DeprecatedAttr{{.*}} Inherited
 // CHECK-NEXT: WarnUnusedResultAttr{{.*}} Inherited
 // CHECK-NEXT: AnnotateAttr{{.*}}
-// CHECK-NEXT: StringLiteral
 
 // CHECK: FunctionDecl{{.*}} mergeAttrTest
 // CHECK-NEXT: DeprecatedAttr{{.*}} Inherited
 // CHECK-NEXT: WarnUnusedResultAttr{{.*}} Inherited
 // CHECK-NEXT: AnnotateAttr{{.*}} Inherited
-// CHECK-NEXT: StringLiteral
 // CHECK-NEXT: UnusedAttr
 // CHECK-NEXT: NoThreadSafetyAnalysisAttr

diff  --git a/clang/test/Misc/pragma-attribute-cxx.cpp 
b/clang/test/Misc/pragma-attribute-cxx.cpp
index fc80127dfbec..38b025e47691 100644
--- a/clang/test/Misc/pragma-attribute-cxx.cpp
+++ b/clang/test/Misc/pragma-attribute-cxx.cpp
@@ -21,15 +21,1

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-11-09 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

@dexonsmith, could you please commit this one for me? I don't have the rights 
to do so, as this is my first patch.


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

https://reviews.llvm.org/D82756

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


[PATCH] D90221: Include more attribute details when dumping AST in JSON

2020-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a subscriber: Tyker.
aaron.ballman added a comment.

In D90221#2381325 , @aronsky wrote:

> In D90221#2379793 , @aaron.ballman 
> wrote:
>
>> I think the issue is that ASTNodeTraverser is visiting all of the attribute 
>> arguments (around ASTNodeTraverse.h:726) and your patch also visits them (as 
>> part of what's emitted from ClangAttrEmitter.cpp).
>
> Thanks! I will take a look at that code.
>
>> Unfortunately, I don't think we can accept the patch as-is -- it's 
>> incomplete (misses some kinds of attribute arguments), has correctness 
>> issues (duplicates some kinds of attribute arguments), and would make it 
>> harder to maintain the code moving forward (with the layering issue). I can 
>> definitely understand not having a lot of time to investigate the right way 
>> to do this, so I spent a bit of time this afternoon working on a patch that 
>> gets partway to what I think needs to happen. I put up a pastebin for it 
>> here: https://pastebin.com/6ybrPVu6. It does not solve the issue of 
>> duplicate traversal, however, and I'm not certain I have more time to put 
>> into the patch right now. Perhaps one of us will have the time to take this 
>> the last mile -- I think the functionality is needed for JSON dumping.
>
> Thank you! I appreciate your feedback and your work on the issue. I'll take a 
> better look at your patch (from a quick look, it definitely looks more 
> organized and correct than mine), and see if I can understand - and fix - the 
> duplicate traversal issue.

You're welcome! FWIW, it looks like @Tyker just committed some changes in 
d093401a2617d3c46aaed9eeaecf877e3ae1a9f1 
 that will 
probably impact this work.


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

https://reviews.llvm.org/D90221

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


[PATCH] D90174: [HIP] Fix regressions due to fp contract change

2020-11-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

How about fast-constrained, fast-limited, fast-restricted, or fast-restrained?


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

https://reviews.llvm.org/D90174

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


[PATCH] D91078: [clang-format] do not break before @tags in JS comments

2020-11-09 Thread Martin Probst via Phabricator via cfe-commits
mprobst added a comment.

See my comment on the upstream bug, let's chat a bit about whether we need this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91078

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


[PATCH] D91075: [clang] Fix ForStmt mustprogress handling

2020-11-09 Thread Atmn Patel via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd3cad7a6016: [clang] Fix ForStmt mustprogress handling 
(authored by atmnpatel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91075

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/attr-mustprogress-1.c
  clang/test/CodeGen/attr-mustprogress-1.cpp


Index: clang/test/CodeGen/attr-mustprogress-1.cpp
===
--- clang/test/CodeGen/attr-mustprogress-1.cpp
+++ clang/test/CodeGen/attr-mustprogress-1.cpp
@@ -7,6 +7,16 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f0v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @_Z2f1v(
 // CHECK-NEXT:  entry:
Index: clang/test/CodeGen/attr-mustprogress-1.c
===
--- clang/test/CodeGen/attr-mustprogress-1.c
+++ clang/test/CodeGen/attr-mustprogress-1.c
@@ -7,6 +7,17 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @f0(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+//
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @f1(
 // CHECK-NEXT:  entry:
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -947,7 +947,6 @@
   Expr::EvalResult Result;
   if (LanguageRequiresProgress()) {
 if (!S.getCond()) {
-  LoopMustProgress = true;
   FnIsMustProgress = false;
 } else if (!S.getCond()->EvaluateAsInt(Result, getContext())) {
   LoopMustProgress = true;


Index: clang/test/CodeGen/attr-mustprogress-1.cpp
===
--- clang/test/CodeGen/attr-mustprogress-1.cpp
+++ clang/test/CodeGen/attr-mustprogress-1.cpp
@@ -7,6 +7,16 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f0v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @_Z2f1v(
 // CHECK-NEXT:  entry:
Index: clang/test/CodeGen/attr-mustprogress-1.c
===
--- clang/test/CodeGen/attr-mustprogress-1.c
+++ clang/test/CodeGen/attr-mustprogress-1.c
@@ -7,6 +7,17 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @f0(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+//
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @f1(
 // CHECK-NEXT:  entry:
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -947,7 +947,6 @@
   Expr::EvalResult Result;
   if (LanguageRequiresProgress()) {
 if (!S.getCond()) {
-  LoopMustProgress = true;
   FnIsMustProgress = false;
 } else if (!S.getCond()->EvaluateAsInt(Result, getContext())) {
   LoopMustProgress = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] fd3cad7 - [clang] Fix ForStmt mustprogress handling

2020-11-09 Thread Atmn Patel via cfe-commits

Author: Atmn Patel
Date: 2020-11-09T11:38:06-05:00
New Revision: fd3cad7a60168b49f7bc160354655d7605847d41

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

LOG: [clang] Fix ForStmt mustprogress handling

D86841 had an error where for statements with no conditional were
required to make progress. This is not true, this patch removes that
line, and adds regression tests.

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmt.cpp
clang/test/CodeGen/attr-mustprogress-1.c
clang/test/CodeGen/attr-mustprogress-1.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 972da3d4ea2d..d8be0ef4e525 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -947,7 +947,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
   Expr::EvalResult Result;
   if (LanguageRequiresProgress()) {
 if (!S.getCond()) {
-  LoopMustProgress = true;
   FnIsMustProgress = false;
 } else if (!S.getCond()->EvaluateAsInt(Result, getContext())) {
   LoopMustProgress = true;

diff  --git a/clang/test/CodeGen/attr-mustprogress-1.c 
b/clang/test/CodeGen/attr-mustprogress-1.c
index a5a8595218a1..2ff068b8b90a 100644
--- a/clang/test/CodeGen/attr-mustprogress-1.c
+++ b/clang/test/CodeGen/attr-mustprogress-1.c
@@ -7,6 +7,17 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @f0(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+//
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @f1(
 // CHECK-NEXT:  entry:

diff  --git a/clang/test/CodeGen/attr-mustprogress-1.cpp 
b/clang/test/CodeGen/attr-mustprogress-1.cpp
index 6d53d2d1148d..945d74663c6d 100644
--- a/clang/test/CodeGen/attr-mustprogress-1.cpp
+++ b/clang/test/CodeGen/attr-mustprogress-1.cpp
@@ -7,6 +7,16 @@
 int a = 0;
 int b = 0;
 
+// CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: @_Z2f0v(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NOT:br label [[FOR_COND]], !llvm.loop !{{.*}}
+void f0() {
+  for (; ;) ;
+}
+
 // CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: @_Z2f1v(
 // CHECK-NEXT:  entry:



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


[PATCH] D91011: [NFC, Refactor] Rename the (scoped) enum DeclaratorContext's enumerators to avoid redundancy

2020-11-09 Thread Wyatt Childers via Phabricator via cfe-commits
wchilders accepted this revision.
wchilders added a comment.

+1 Nice QOL change :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91011

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


[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-11-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

In D82756#2382885 , @jansvoboda11 
wrote:

> @dexonsmith, could you please commit this one for me? I don't have the rights 
> to do so, as this is my first patch.

Sure; what do you want for `GIT_AUTHOR_NAME` and `GIT_AUTHOR_EMAIL`?


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

https://reviews.llvm.org/D82756

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


[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

2020-11-09 Thread Wyatt Childers via Phabricator via cfe-commits
wchilders added a comment.

Generally agree with this direction; Are there plans for migrating 
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/Specifiers.h
 in a similar fashion, for consistency?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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


[clang] e625f9c - -fbasic-block-sections=list=: Suppress output if failed to open the file

2020-11-09 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-11-09T09:26:37-08:00
New Revision: e625f9c5d1e2e69d18febae0e8d3f808df35c4c8

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

LOG: -fbasic-block-sections=list=: Suppress output if failed to open the file

Reviewed By: tmsriram

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
clang/test/CodeGen/basic-block-sections.c

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 81ae79482d90..a5ca113b9e77 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -446,7 +446,7 @@ static CodeGenFileType getCodeGenFileType(BackendAction 
Action) {
   }
 }
 
-static void initTargetOptions(DiagnosticsEngine &Diags,
+static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
   const clang::TargetOptions &TargetOpts,
@@ -517,11 +517,12 @@ static void initTargetOptions(DiagnosticsEngine &Diags,
   if (Options.BBSections == llvm::BasicBlockSection::List) {
 ErrorOr> MBOrErr =
 MemoryBuffer::getFile(CodeGenOpts.BBSections.substr(5));
-if (!MBOrErr)
+if (!MBOrErr) {
   Diags.Report(diag::err_fe_unable_to_load_basic_block_sections_file)
   << MBOrErr.getError().message();
-else
-  Options.BBSectionsFuncListBuf = std::move(*MBOrErr);
+  return false;
+}
+Options.BBSectionsFuncListBuf = std::move(*MBOrErr);
   }
 
   Options.EnableMachineFunctionSplitter = CodeGenOpts.SplitMachineFunctions;
@@ -572,6 +573,8 @@ static void initTargetOptions(DiagnosticsEngine &Diags,
   Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + Entry.Path);
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
+
+  return true;
 }
 
 static Optional getGCOVOptions(const CodeGenOptions &CodeGenOpts,
@@ -858,7 +861,9 @@ void EmitAssemblyHelper::CreateTargetMachine(bool 
MustCreateTM) {
   CodeGenOpt::Level OptLevel = getCGOptLevel(CodeGenOpts);
 
   llvm::TargetOptions Options;
-  initTargetOptions(Diags, Options, CodeGenOpts, TargetOpts, LangOpts, HSOpts);
+  if (!initTargetOptions(Diags, Options, CodeGenOpts, TargetOpts, LangOpts,
+ HSOpts))
+return;
   TM.reset(TheTarget->createTargetMachine(Triple, TargetOpts.CPU, FeaturesStr,
   Options, RM, CM, OptLevel));
 }

diff  --git a/clang/test/CodeGen/basic-block-sections.c 
b/clang/test/CodeGen/basic-block-sections.c
index 0381bc78c6a9..805539f06f08 100644
--- a/clang/test/CodeGen/basic-block-sections.c
+++ b/clang/test/CodeGen/basic-block-sections.c
@@ -6,7 +6,9 @@
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=all -o - < %s | 
FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_ALL
 // RUN: %clang_cc1 -triple x86_64 -S 
-fbasic-block-sections=list=%S/Inputs/basic-block-sections.funcnames -o - < %s 
| FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_LIST
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=all 
-funique-basic-block-section-names -o - < %s | FileCheck %s 
--check-prefix=UNIQUE
-// RUN: not %clang_cc1 -fbasic-block-sections=list= -emit-llvm -o - %s 2>&1 | 
FileCheck %s --check-prefix=ERROR
+// RUN: rm -f %t
+// RUN: not %clang_cc1 -fbasic-block-sections=list= -emit-obj -o %t %s 2>&1 | 
FileCheck %s --check-prefix=ERROR
+// RUN: not ls %t
 
 int world(int a) {
   if (a > 10)



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


[PATCH] D90815: -fbasic-block-sections=list=: Suppress output if failed to open the file

2020-11-09 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe625f9c5d1e2: -fbasic-block-sections=list=: Suppress output 
if failed to open the file (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90815

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/basic-block-sections.c


Index: clang/test/CodeGen/basic-block-sections.c
===
--- clang/test/CodeGen/basic-block-sections.c
+++ clang/test/CodeGen/basic-block-sections.c
@@ -6,7 +6,9 @@
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=all -o - < %s | 
FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_ALL
 // RUN: %clang_cc1 -triple x86_64 -S 
-fbasic-block-sections=list=%S/Inputs/basic-block-sections.funcnames -o - < %s 
| FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_LIST
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=all 
-funique-basic-block-section-names -o - < %s | FileCheck %s 
--check-prefix=UNIQUE
-// RUN: not %clang_cc1 -fbasic-block-sections=list= -emit-llvm -o - %s 2>&1 | 
FileCheck %s --check-prefix=ERROR
+// RUN: rm -f %t
+// RUN: not %clang_cc1 -fbasic-block-sections=list= -emit-obj -o %t %s 2>&1 | 
FileCheck %s --check-prefix=ERROR
+// RUN: not ls %t
 
 int world(int a) {
   if (a > 10)
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -446,7 +446,7 @@
   }
 }
 
-static void initTargetOptions(DiagnosticsEngine &Diags,
+static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
   const clang::TargetOptions &TargetOpts,
@@ -517,11 +517,12 @@
   if (Options.BBSections == llvm::BasicBlockSection::List) {
 ErrorOr> MBOrErr =
 MemoryBuffer::getFile(CodeGenOpts.BBSections.substr(5));
-if (!MBOrErr)
+if (!MBOrErr) {
   Diags.Report(diag::err_fe_unable_to_load_basic_block_sections_file)
   << MBOrErr.getError().message();
-else
-  Options.BBSectionsFuncListBuf = std::move(*MBOrErr);
+  return false;
+}
+Options.BBSectionsFuncListBuf = std::move(*MBOrErr);
   }
 
   Options.EnableMachineFunctionSplitter = CodeGenOpts.SplitMachineFunctions;
@@ -572,6 +573,8 @@
   Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + Entry.Path);
   Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
+
+  return true;
 }
 
 static Optional getGCOVOptions(const CodeGenOptions &CodeGenOpts,
@@ -858,7 +861,9 @@
   CodeGenOpt::Level OptLevel = getCGOptLevel(CodeGenOpts);
 
   llvm::TargetOptions Options;
-  initTargetOptions(Diags, Options, CodeGenOpts, TargetOpts, LangOpts, HSOpts);
+  if (!initTargetOptions(Diags, Options, CodeGenOpts, TargetOpts, LangOpts,
+ HSOpts))
+return;
   TM.reset(TheTarget->createTargetMachine(Triple, TargetOpts.CPU, FeaturesStr,
   Options, RM, CM, OptLevel));
 }


Index: clang/test/CodeGen/basic-block-sections.c
===
--- clang/test/CodeGen/basic-block-sections.c
+++ clang/test/CodeGen/basic-block-sections.c
@@ -6,7 +6,9 @@
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=all -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_ALL
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=list=%S/Inputs/basic-block-sections.funcnames -o - < %s | FileCheck %s --check-prefix=BB_WORLD --check-prefix=BB_LIST
 // RUN: %clang_cc1 -triple x86_64 -S -fbasic-block-sections=all -funique-basic-block-section-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
-// RUN: not %clang_cc1 -fbasic-block-sections=list= -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=ERROR
+// RUN: rm -f %t
+// RUN: not %clang_cc1 -fbasic-block-sections=list= -emit-obj -o %t %s 2>&1 | FileCheck %s --check-prefix=ERROR
+// RUN: not ls %t
 
 int world(int a) {
   if (a > 10)
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -446,7 +446,7 @@
   }
 }
 
-static void initTargetOptions(DiagnosticsEngine &Diags,
+static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
   const clang::TargetOptions &TargetOpts,
@@ -517,11 +517,12 @@
   if (Options.BBSections == llvm::BasicBlockSection::List) {
 ErrorOr> MBOrErr =
 Memory

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

2020-11-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.
yaxunl requested review of this revision.

In C++ when a reference variable is captured by copy, the lambda
is supposed to make a copy of the referenced variable in the captures
and refer to the copy in the lambda. Therefore, it is valid to capture
a reference to a host global variable in a device lambda since the
device lambda will refer to the copy of the host global variable instead
of access the host global variable directly.

However, clang tries to avoid capturing of reference to a host global variable
if it determines the use of the reference variable in the lambda function is
not odr-use. Clang also tries to emit load of the reference to a global variable
as load of the global variable if it determines that the reference variable is
a compile-time constant.

For a device lambda to capture a reference variable to host global variable
and use the captured value, clang needs to be taught that in such cases the use 
of the reference
variable is odr-use and the reference variable is not compile-time constant.

This patch fixes that.


https://reviews.llvm.org/D91088

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/lambda-reference-var.cu

Index: clang/test/CodeGenCUDA/lambda-reference-var.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/lambda-reference-var.cu
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -x hip -emit-llvm -std=c++11 %s -o - \
+// RUN:   -triple x86_64-linux-gnu \
+// RUN:   | FileCheck -check-prefix=HOST %s
+// RUN: %clang_cc1 -x hip -emit-llvm -std=c++11 %s -o - \
+// RUN:   -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   | FileCheck -check-prefix=DEV %s
+
+#include "Inputs/cuda.h"
+
+// HOST: %[[T1:.*]] = type <{ i32*, i32, [4 x i8] }>
+// HOST: %[[T2:.*]] = type { i32*, i32** }
+// HOST: %[[T3:.*]] = type <{ i32*, i32, [4 x i8] }>
+// DEV: %[[T1:.*]] = type { i32* }
+// DEV: %[[T2:.*]] = type { i32** }
+// DEV: %[[T3:.*]] = type <{ i32*, i32, [4 x i8] }>
+int global_host_var;
+__device__ int global_device_var;
+
+template
+__global__ void kern(F f) { f(); }
+
+// DEV-LABEL: @_ZZ27dev_capture_dev_ref_by_copyPiENKUlvE_clEv(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_capture_dev_ref_by_copy(int *out) {
+  int &ref = global_device_var;
+  [=](){ *out = ref;}();
+}
+
+// DEV-LABEL: @_ZZ26dev_capture_dev_ref_by_refPiENKUlvE_clEv(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// DEV: store i32 %[[VAL2]], i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_capture_dev_ref_by_ref(int *out) {
+  int &ref = global_device_var;
+  [&](){ ref++; *out = ref;}();
+}
+
+// DEV-LABEL: define void @_Z7dev_refPi(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// DEV: store i32 %[[VAL2]], i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_ref(int *out) {
+  int &ref = global_device_var;
+  ref++;
+  *out = ref;
+}
+
+// DEV-LABEL: @_ZZ14dev_lambda_refPiENKUlvE_clEv(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// DEV: store i32 %[[VAL2]], i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_lambda_ref(int *out) {
+  [=](){
+int &ref = global_device_var;
+ref++;
+*out = ref;
+  }();
+}
+
+// HOST-LABEL: @_ZZ29host_capture_host_ref_by_copyPiENKUlvE_clEv(
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]]
+void host_capture_host_ref_by_copy(int *out) {
+  int &ref = global_host_var;
+  [=](){ *out = ref;}();
+}
+
+// HOST-LABEL: @_ZZ28host_capture_host_ref_by_refPiENKUlvE_clEv(
+// HOST: %[[CAP:.*]] = getelementptr inbounds %[[T2]], %[[T2]]* %this1, i32 0, i32 0
+// HOST: %[[REF:.*]] = load i32*, i32** %[[CAP]]
+// HOST: %[[VAL:.*]] = load i32, i32* %[[REF]]
+// HOST: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// HOST: store i32 %[[VAL2]], i32* %[[REF]]
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]]
+void host_capture_host_ref_by_ref(int *out) {
+  int &ref = global_host_var;
+  [&](){ ref++; *out = ref;}();
+}
+
+// HOST-LABEL: define void @_Z8host_refPi(
+// HOST: %[[VAL:

[PATCH] D90748: [clangd] Introduce config parsing for External blocks

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

(sorry looks like i forgot to hit submit :( )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90748

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


[PATCH] D90748: [clangd] Introduce config parsing for External blocks

2020-11-09 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

> (Meta-point: not sure how useful splitting this patch out from the compile 
> step is...)

I wanted to keep them separate especially to ensure testing isn't mixed up. As 
these things tend to get big easily :/




Comment at: clang-tools-extra/clangd/ConfigFragment.h:173
+  /// otherwise.
+  llvm::Optional> File;
+  /// Address and port number for a remote-index. e.g. `123.1.1.1:13337`.

sammccall wrote:
> Should we mention slashes here too? :-\
> 
> Maybe we should just lift "all paths use forward-slashes" to a top-level 
> comment.
This is literally used for pointing at a file on disk though. So it doesn't 
really matter if this is forward or back slashes?



Comment at: clang-tools-extra/clangd/ConfigFragment.h:176
+  llvm::Optional> Server;
+  /// Source root governed by this index. None implies current config file
+  /// location. Absolute in case of user config and relative otherwise.

sammccall wrote:
> Again: "Default is the directory associated with the config frament".
> 
> (Repeating this makes me wonder if we should have defined a more specific 
> name like "Home"...)
not sure where else we repeated this ?



Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:103
+#else
+  warning("Remote index support isn't enabled for this clangd.", N);
+#endif

sammccall wrote:
> nit: this sort of validation would normally live in ConfigCompile, since it's 
> not to do with serialization
moving into compile logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90748

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


[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

2020-11-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 303905.
yaxunl added a comment.

remove debug code


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

https://reviews.llvm.org/D91088

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/lambda-reference-var.cu

Index: clang/test/CodeGenCUDA/lambda-reference-var.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/lambda-reference-var.cu
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -x hip -emit-llvm -std=c++11 %s -o - \
+// RUN:   -triple x86_64-linux-gnu \
+// RUN:   | FileCheck -check-prefix=HOST %s
+// RUN: %clang_cc1 -x hip -emit-llvm -std=c++11 %s -o - \
+// RUN:   -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   | FileCheck -check-prefix=DEV %s
+
+#include "Inputs/cuda.h"
+
+// HOST: %[[T1:.*]] = type <{ i32*, i32, [4 x i8] }>
+// HOST: %[[T2:.*]] = type { i32*, i32** }
+// HOST: %[[T3:.*]] = type <{ i32*, i32, [4 x i8] }>
+// DEV: %[[T1:.*]] = type { i32* }
+// DEV: %[[T2:.*]] = type { i32** }
+// DEV: %[[T3:.*]] = type <{ i32*, i32, [4 x i8] }>
+int global_host_var;
+__device__ int global_device_var;
+
+template
+__global__ void kern(F f) { f(); }
+
+// DEV-LABEL: @_ZZ27dev_capture_dev_ref_by_copyPiENKUlvE_clEv(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_capture_dev_ref_by_copy(int *out) {
+  int &ref = global_device_var;
+  [=](){ *out = ref;}();
+}
+
+// DEV-LABEL: @_ZZ26dev_capture_dev_ref_by_refPiENKUlvE_clEv(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// DEV: store i32 %[[VAL2]], i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_capture_dev_ref_by_ref(int *out) {
+  int &ref = global_device_var;
+  [&](){ ref++; *out = ref;}();
+}
+
+// DEV-LABEL: define void @_Z7dev_refPi(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// DEV: store i32 %[[VAL2]], i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_ref(int *out) {
+  int &ref = global_device_var;
+  ref++;
+  *out = ref;
+}
+
+// DEV-LABEL: @_ZZ14dev_lambda_refPiENKUlvE_clEv(
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// DEV: store i32 %[[VAL2]], i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: %[[VAL:.*]] = load i32, i32* addrspacecast (i32 addrspace(1)* @global_device_var to i32*)
+// DEV: store i32 %[[VAL]]
+__device__ void dev_lambda_ref(int *out) {
+  [=](){
+int &ref = global_device_var;
+ref++;
+*out = ref;
+  }();
+}
+
+// HOST-LABEL: @_ZZ29host_capture_host_ref_by_copyPiENKUlvE_clEv(
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]]
+void host_capture_host_ref_by_copy(int *out) {
+  int &ref = global_host_var;
+  [=](){ *out = ref;}();
+}
+
+// HOST-LABEL: @_ZZ28host_capture_host_ref_by_refPiENKUlvE_clEv(
+// HOST: %[[CAP:.*]] = getelementptr inbounds %[[T2]], %[[T2]]* %this1, i32 0, i32 0
+// HOST: %[[REF:.*]] = load i32*, i32** %[[CAP]]
+// HOST: %[[VAL:.*]] = load i32, i32* %[[REF]]
+// HOST: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// HOST: store i32 %[[VAL2]], i32* %[[REF]]
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]]
+void host_capture_host_ref_by_ref(int *out) {
+  int &ref = global_host_var;
+  [&](){ ref++; *out = ref;}();
+}
+
+// HOST-LABEL: define void @_Z8host_refPi(
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// HOST: store i32 %[[VAL2]], i32* @global_host_var
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]]
+void host_ref(int *out) {
+  int &ref = global_host_var;
+  ref++;
+  *out = ref;
+}
+
+// HOST-LABEL: @_ZZ15host_lambda_refPiENKUlvE_clEv(
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: %[[VAL2:.*]] = add nsw i32 %[[VAL]], 1
+// HOST: store i32 %[[VAL2]], i32* @global_host_var
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]]
+void host_lambda_ref(int *out) {
+  [=](){
+int &ref = global_host_var;
+ref++;
+*out = ref;
+  }();
+}
+
+// HOST-LABEL: define void @_Z28dev_capture_host_ref_by_copyPi(
+// HOST: %[[CAP:.*]] = getelementptr inbounds %[[T3]], %[[T3]]* %{{.*}}, i32 0, i32 1
+// HOST: %[[VAL:.*]] = load i32, i32* @global_host_var
+// HOST: store i32 %[[VAL]], 

[PATCH] D91047: Add a call super attribute plugin example

2020-11-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

The number of underlines here looks off -- can you verify it's correct?



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

Should add backticks around `call_super` since it's syntax. Also, `sub-classes` 
should be `subclasses`.

"call it in overridden the method" -> "call it in the overridden method"



Comment at: clang/docs/ClangPlugins.rst:119
+
+Attribute plugin to mark a virtual method as call_super, sub-classes must call 
it in overridden the method.
+

aaron.ballman wrote:
> Should add backticks around `call_super` since it's syntax. Also, 
> `sub-classes` should be `subclasses`.
> 
> "call it in overridden the method" -> "call it in the overridden method"
There should be more explanation here about what concepts the example 
demonstrates. For example, one interesting bit to me is that it shows how you 
can add a custom attribute that's useful even if it does not generate an AST 
node for the attribute.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:25
+namespace {
+// cached methods which are marked as call_super
+llvm::SmallPtrSet ForcingCalledMethods;

Comments should be grammatical including starting with a capital letter and 
trailing punctuation. (Should be corrected elsewhere in the patch as well.)



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:32
+
+std::string fullName(const CXXMethodDecl *D) {
+  std::string FullName;

Is the functionality for getting names out of a `NamedDecl` insufficient?



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:43
+  bool IsOverriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}

ctor should probably be `explicit`



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:43
+  bool IsOverriddenUsed = false;
+  MethodUsageVisitor(const clang::CXXMethodDecl *MethodDecl)
+  : MethodDecl(MethodDecl) {}

aaron.ballman wrote:
> ctor should probably be `explicit`
Can drop `clang::` prefixed everywhere since you have a `using` declaration at 
the top of the file.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:63
+DiagnosticsEngine::Warning,
+"%0 is marked as call_super but override method %1 does not call it");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(

Should put single quotes around `call_super` as it's a syntactic element.

I'm not certain that %0 and %1 are necessary because the overridden methods 
will have the same names. I think you can drop the %1 and rely on the note to 
point out where the overridden method lives.




Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:66
+DiagnosticsEngine::Note,
+"overridden method which is marked as call_super here");
+  }

Single quotes around `call_super` here as well; we usually phrase this sort of 
thing as `"overridden method marked 'call_super' here`



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:73
+  DiagnosticsEngine::Warning,
+  "call_super attribute attached on a final method");
+  Diags.Report(MethodDecl->getLocation(), ID);

Single quotes around `call_super` and we should use consistent terminology for 
"attached" vs "marked".



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:76
+}
+for (const auto *Overridden : MethodDecl->overridden_methods()) {
+  if (isMarkedAsCallSuper(Overridden)) {

Won't this visit every method declaration multiple times? Once for the 
declaration itself (as we encounter it) and once for each time it's listed as 
an overridden method?



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:80
+// skip checking
+if (!MethodDecl->isThisDeclarationADefinition()) {
+  continue;

We typically elide the braces for single-line substatements.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:90
+Diags.Report(MethodDecl->getLocation(), WarningSuperNotCalled)
+<< fullName(Overridden) << fullName(MethodDecl);
+Diags.Report(Overridden->getLocation(),

FWIW, you can pass `Overridden` and `MethodDecl` in directly rather than trying 
to scrape the name out yourself -- the diagnostics engine knows how to properly 
turn a `NamedDecl` into a s

[PATCH] D90956: [clang][SVE] Additional macros implied by `-msve-vector-bits=`.

2020-11-09 Thread Francesco Petrogalli via Phabricator via cfe-commits
fpetrogalli updated this revision to Diff 303908.
fpetrogalli marked an inline comment as done.
fpetrogalli added a comment.

I removed the enablement of __ARM_FEATURE_SVE_PREDICATE_OPERATORS as the 
codegen is incorrect.

I have extended testing of the codegen for __ARM_FEATURE_SVE_VECTOR_OPERATORS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90956

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/Preprocessor/aarch64-target-features.c

Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -440,14 +440,11 @@
 // CHECK-BFLOAT: __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
 
 // == Check sve-vector-bits flag.
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=128 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-128 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=256 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-256 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-512 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-1024 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS 128
-// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS 256
-// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS 512
-// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS 1024
-// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS 2048
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=128  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=128  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=256  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=256  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=512  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=1024 %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=2048 %s
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_BITS [[#VBITS:]]
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_PREDICATE_OPERATORS 1
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_VECTOR_OPERATORS 1
Index: clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -x c++ -triple aarch64-none-linux-gnu -target-feature \
+// RUN: +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall \
+// RUN: -emit-llvm -o - %s -msve-vector-bits=256 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -triple aarch64-none-linux-gnu -target-feature \
+// RUN: +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall \
+// RUN: -emit-llvm -o - %s -msve-vector-bits=512 | FileCheck %s --check-prefix=CHECK512
+// REQUIRES: aarch64-registered-target
+
+// Examples taken from section 3.7.3.3 of the SVE ACLE (Version
+// 00bet6) that can be found at
+// https://developer.arm.com/documentation/100987/latest
+
+#include 
+
+// Page 27, item 1.
+#if __ARM_FEATURE_SVE_BITS == 512 && __ARM_FEATURE_SVE_VECTOR_OPERATORS
+// CHECK512-LABEL: define  @_Z1f9__SVE_VLSIu11__SVInt32_tLj512EES_( %x.coerce,  %y.coerce)
+// CHECK512-NEXT: entry:
+// CHECK512-NEXT:   %x = alloca <16 x i32>, align 16
+// CHECK512-NEXT:   %y = alloca <16 x i32>, align 16
+// CHECK512-NEXT:   %retval.coerce = alloca , align 16
+// CHECK512-NEXT:   %0 = bitcast <16 x i32>* %x to *
+// CHECK512-NEXT:   store  %x.coerce, * %0, align 16
+// CHECK512-NEXT:   %x1 = load <16 x i32>, <16 x i32>* %x, align 16
+// CHECK512-NEXT:   %1 = bitcast <16 x i32>* %y to *
+// CHECK512-NEXT:   store 

[PATCH] D90956: [clang][SVE] Activate macro `__ARM_FEATURE_SVE_VECTOR_OPERATORS`.

2020-11-09 Thread Francesco Petrogalli via Phabricator via cfe-commits
fpetrogalli updated this revision to Diff 303911.
fpetrogalli edited the summary of this revision.
fpetrogalli added a comment.

In the last update I removed the tests for 
`__ARM_FEATURE_SVE_PREDICATE_OPERATORS`but forgot to remove the code that 
generates and tests the macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90956

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/Preprocessor/aarch64-target-features.c

Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -440,14 +440,10 @@
 // CHECK-BFLOAT: __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
 
 // == Check sve-vector-bits flag.
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=128 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-128 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=256 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-256 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-512 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-1024 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS 128
-// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS 256
-// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS 512
-// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS 1024
-// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS 2048
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=128  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=128  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=256  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=256  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=512  -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=512  %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=1024 %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS -D#VBITS=2048 %s
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_BITS [[#VBITS:]]
+// CHECK-SVE-VECTOR-BITS: __ARM_FEATURE_SVE_VECTOR_OPERATORS 1
Index: clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -x c++ -triple aarch64-none-linux-gnu -target-feature \
+// RUN: +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall \
+// RUN: -emit-llvm -o - %s -msve-vector-bits=256 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -triple aarch64-none-linux-gnu -target-feature \
+// RUN: +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall \
+// RUN: -emit-llvm -o - %s -msve-vector-bits=512 | FileCheck %s --check-prefix=CHECK512
+// REQUIRES: aarch64-registered-target
+
+// Examples taken from section 3.7.3.3 of the SVE ACLE (Version
+// 00bet6) that can be found at
+// https://developer.arm.com/documentation/100987/latest
+
+#include 
+
+// Page 27, item 1.
+#if __ARM_FEATURE_SVE_BITS == 512 && __ARM_FEATURE_SVE_VECTOR_OPERATORS
+// CHECK512-LABEL: define  @_Z1f9__SVE_VLSIu11__SVInt32_tLj512EES_( %x.coerce,  %y.coerce)
+// CHECK512-NEXT: entry:
+// CHECK512-NEXT:   %x = alloca <16 x i32>, align 16
+// CHECK512-NEXT:   %y = alloca <16 x i32>, align 16
+// CHECK512-NEXT:   %retval.coerce = alloca , align 16
+// CHECK512-NEXT:   %0 = bitcast <16 x i32>* %x to *
+// CHECK512-NEXT:   store  %x.coerce, * %0, align 16
+// CHECK512-NEXT:   %x1 = load <16 x i32>, <16 x i32>* %x, align 16
+// CHECK512-NEXT:   %1 = bitcast <16 x i32>* %y to *
+// CHECK512-NEXT:   store  %y.coerce, * %1, align 16
+// CHECK512-NEXT:   %y2 = load <16 x i32>, <16 x i32>* %y, ali

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-11-09 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

Thanks, `Jan Svoboda` and `jan_svob...@apple.com` is fine. Is it possible to 
add @dang as a co-author? `git log` says he uses `Daniel Grumberg` and 
`dany.grumb...@gmail.com`.


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

https://reviews.llvm.org/D82756

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


[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

2020-11-09 Thread Artem Belevich via Phabricator via cfe-commits
tra added a subscriber: rsmith.
tra added a comment.

The use case in the tests makes sense, but I'd like to have a second opinion on 
whether it may have unintended consequences. Lambdas tend to bring interesting 
corner cases.

@rsmith Richard, do you see any issues with this approach?




Comment at: clang/test/CodeGenCUDA/lambda-reference-var.cu:61
+  [=](){
+int &ref = global_device_var;
+ref++;

Do we have current Sema tests that verify that we we would not allow accessing 
host vars here?


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

https://reviews.llvm.org/D91088

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


  1   2   >