date:20191107

[PATCH] D69928: [clangd] Set RetainCommentsFromSystemHeaders to true

2019-11-07 Thread Ilya Biryukov via Phabricator via cfe-commits

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

LGTM, thanks!
Do you need someone to land this?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69928



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

[PATCH] D69933: [ASTImporter] Limit imports of structs

2019-11-07 Thread Jaroslav Sevcik via Phabricator via cfe-commits

jarin created this revision.
jarin added a reviewer: martong.
jarin added projects: LLDB, clang.
Herald added subscribers: cfe-commits, teemperor, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
jarin edited the summary of this revision.

This is a work in progress patch for discussion. The goal is to improve 
performance of LLDB's expression evaluator.

During my investigations, I noticed that the AST importer is very eager to 
import classes/structs that were already completed on the 'From' side, even if 
they are not needed. This is best illustrated with an example:

  struct C0 { int x = 0; };
  struct C1 { int x = 1; C0* c0 = 0; };
  struct C2 { int x = 2; C1* c1 = 0; };
  
  int main() {
C0 c0;
C1 c1;
C2 c2;
  
return 0;  // break here
  }

When we evaluate “c2.x” in LLDB, AST importer completes and imports only class 
C2. This is working as intended. Similarly, evaluating “c1.x” imports just C1 
and “c0.x” imports C0. However, if we evaluate “c2.x” after evaluating “c1.x” 
and “c0.x”, the importer suddenly imports both C1 and C0 (in addition to C2). 
See a log from a lldb session at the end of this email for illustration.

I believe the culprit here is the following code at the end of the 
ASTNodeImporter::VisitRecordDecl method:

  if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2, IDK_Default))
  return std::move(Err);

This will import a definition of a class from LLDB if LLDB already happens to 
have a complete definition from before. For large programs, this can lead to 
importing very large chunks of ASTs even if they are not needed. I have tried 
to remove the code above from the AST importer and test performance on several 
expressions in an Unreal engine sample - preliminary results show this cuts 
down evaluation time by roughly 50%.

This is work in progress,  couple of lldb tests are failing (but hopefully 
fixable). What would the experts here think? Is this a plausible direction?

  —— lldb session illustrating the unnecessary imports —-
  This shows that evaluation of “c2.x” after evaluation “c1.x” and “c0.x” calls 
to LayoutRecordType for C2, C1 and C0.
  
  $ lldb a.out
  (lldb) b h.cc:10
  Breakpoint 1: where = a.out`main + 44 at h.cc:10:3, address = ...
  (lldb) r
  ... Process stopped ...
  (lldb) log enable lldb expr
  (lldb) p c2.x
  ...
  LayoutRecordType[6] ... for (RecordDecl*)0x... [name = 'C2']
  ...
  (lldb) p c1.x
  ...
  LayoutRecordType[7] ... for (RecordDecl*)0x... [name = 'C1']
  ...
  (lldb) p c0.x
  ...
  LayoutRecordType[8] ... for (RecordDecl*)0x... [name = 'C0']
  ...
  (lldb) p c2.x
  ...
  LayoutRecordType[9] ... for (RecordDecl*)0x... [name = 'C2']
  LayoutRecordType[10] ... for (RecordDecl*)0x... [name = 'C1']
  LayoutRecordType[11] ... for (RecordDecl*)0x... [name = 'C0']
  ...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69933

Files:
  clang/lib/AST/ASTImporter.cpp


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -2801,7 +2801,7 @@
   if (D->isAnonymousStructOrUnion())
 D2->setAnonymousStructOrUnion(true);
 
-  if (D->isCompleteDefinition())
+  if (!Importer.isMinimalImport() && D->isCompleteDefinition())
 if (Error Err = ImportDefinition(D, D2, IDK_Default))
   return std::move(Err);
 
@@ -3440,6 +3440,9 @@
   if (ToInitializer)
 ToField->setInClassInitializer(ToInitializer);
   ToField->setImplicit(D->isImplicit());
+  if (CXXRecordDecl *FieldType = D->getType()->getAsCXXRecordDecl())
+if (Error Err = ImportDefinitionIfNeeded(FieldType))
+  return std::move(Err);
   LexicalDC->addDeclInternal(ToField);
   return ToField;
 }
@@ -5323,7 +5326,7 @@
 
   D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
 
-  if (D->isCompleteDefinition())
+  if (!Importer.isMinimalImport() && D->isCompleteDefinition())
 if (Error Err = ImportDefinition(D, D2))
   return std::move(Err);
 


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -2801,7 +2801,7 @@
   if (D->isAnonymousStructOrUnion())
 D2->setAnonymousStructOrUnion(true);
 
-  if (D->isCompleteDefinition())
+  if (!Importer.isMinimalImport() && D->isCompleteDefinition())
 if (Error Err = ImportDefinition(D, D2, IDK_Default))
   return std::move(Err);
 
@@ -3440,6 +3440,9 @@
   if (ToInitializer)
 ToField->setInClassInitializer(ToInitializer);
   ToField->setImplicit(D->isImplicit());
+  if (CXXRecordDecl *FieldType = D->getType()->getAsCXXRecordDecl())
+if (Error Err = ImportDefinitionIfNeeded(FieldType))
+  return std::move(Err);
   LexicalDC->addDeclInternal(ToField);
   return ToField;
 }
@@ -5323,7 +5326,7 @@
 
   D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
 
-

[PATCH] D69928: [clangd] Set RetainCommentsFromSystemHeaders to true

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

lh123 added a comment.

In D69928#1736569 , @ilya-biryukov 
wrote:

> LGTM, thanks!
>  Do you need someone to land this?


Yes, I can‘t commit it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69928



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

[clang] 96065cf - [Syntax] Silence "unused function" warning in no-assert builds. NFC

2019-11-07 Thread Ilya Biryukov via cfe-commits


Author: Ilya Biryukov
Date: 2019-11-07T09:37:25+01:00
New Revision: 96065cf79ff76d5fd4fdaeb2fb2650074b3e0e51

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

LOG: [Syntax] Silence "unused function" warning in no-assert builds. NFC

A helper `isImpicitExpr` is only used inside assert.

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 1be23f7e7978..dddc265c8c41 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -21,12 +21,14 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
 using namespace clang;
 
+LLVM_ATTRIBUTE_UNUSED
 static bool isImplicitExpr(clang::Expr *E) { return E->IgnoreImplicit() != E; }
 
 /// A helper class for constructing the syntax tree while traversing a clang



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

Re: [PATCH] D69897: Add #pragma clang loop aligned

2019-11-07 Thread HAPPY Mahto via cfe-commits

On Wed, Nov 6, 2019 at 10:43 PM Hal Finkel via Phabricator <
revi...@reviews.llvm.org> wrote:

> hfinkel added inline comments.
>
>
> 
> Comment at: clang/docs/LanguageExtensions.rst:3135
> +
> +This predicates all the array references inside the loop to be aligned.
> The aligned access to them can increase fetch time and increase the
> performance.
> +
> 
> lebedev.ri wrote:
> > What does this actually mean, in the end?
> > Will it assume that whatever alignment is required
> > for whatever vectorization width chosen,
> > is the actual alignment?
>
Currently, it is using 32 bit.

> Also, just arrays, or also pointers?
>
> By pointers, if you mean pointers to arrays? Then Yes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

2019-11-07 Thread Balázs Kéri via Phabricator via cfe-commits

balazske added a comment.

I removed the previous comment because I realized that 
`StdCLibraryFunctionsChecker` does not use `evalCall` for fread (returns false 
because "non-pure" evaluation).

Anyway the checks that do not use BindExpr (all except the open functions) 
could be moved into a PreCall or PostCall callback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69662



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

[clang-tools-extra] 0019684 - [clangd] Set RetainCommentsFromSystemHeaders to true

2019-11-07 Thread Ilya Biryukov via cfe-commits


Author: Ilya Biryukov
Date: 2019-11-07T09:54:20+01:00
New Revision: 0019684900491f517f3b08b4fa92740b69a8cc0f

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

LOG: [clangd] Set RetainCommentsFromSystemHeaders to true

clangd should retain comments from system headers.

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

Patch by lh123!

Differential revision: https://reviews.llvm.org/D69928

Added: 


Modified: 
clang-tools-extra/clangd/Compiler.cpp
clang-tools-extra/clangd/index/IndexAction.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Compiler.cpp 
b/clang-tools-extra/clangd/Compiler.cpp
index e08014333190..795fd0082594 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -63,6 +63,7 @@ buildCompilerInvocation(const ParseInputs &Inputs,
   // createInvocationFromCommandLine sets DisableFree.
   CI->getFrontendOpts().DisableFree = false;
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  CI->getLangOpts()->RetainCommentsFromSystemHeaders = true;
   return CI;
 }
 

diff  --git a/clang-tools-extra/clangd/index/IndexAction.cpp 
b/clang-tools-extra/clangd/index/IndexAction.cpp
index 0814e7f09793..8fd2159932b4 100644
--- a/clang-tools-extra/clangd/index/IndexAction.cpp
+++ b/clang-tools-extra/clangd/index/IndexAction.cpp
@@ -160,6 +160,7 @@ class IndexAction : public ASTFrontendAction {
   bool BeginInvocation(CompilerInstance &CI) override {
 // We want all comments, not just the doxygen ones.
 CI.getLangOpts().CommentOpts.ParseAllComments = true;
+CI.getLangOpts().RetainCommentsFromSystemHeaders = true;
 // Index the whole file even if there are warnings and -Werror is set.
 // Avoids some analyses too. Set in two places as we're late to the party.
 CI.getDiagnosticOpts().IgnoreWarnings = true;



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

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D69813#1736045 , @Charusso wrote:

> In D69813#1735988 , @aaron.ballman 
> wrote:
>
> > I'm not @NoQ, but I do agree that there should be a separate check per rule 
> > in terms of the UI presented to the user. The name should follow the rule 
> > ID like they do in clang-tidy, for some consistency there.
> >  I think that the rule number should be in the name. I'd probably go with 
> > `cert.STR31-C` or `cert.str31-c` (so it's clear which CERT standard the 
> > rule came from).
>
>
> We warmly welcome not (@NoQ)s! I think Artem really wanted to start this 
> direction to make the two tool work together, but I have seen his project is 
> unbelievably difficult so that it is a little-bit far away, sadly. Even we 
> are far away to have multiple CERT rules in this package, if the Tidy users 
> like the code-names, I cannot say no to start the collaboration with Tidy. I 
> would pick `cert.str.31-c`, as @Szelethus pointed out we use lower-case words 
> for package names and then we can run every `cert.str` checker at once.


Would it make sense to use `cert.str.31.c` to remove the random dash? Would 
this also help the user to do something like `cert.str.*.cpp`? if they want 
just the CERT C++ STR rules checked? Or can they do that already even with the 
`-`?


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

https://reviews.llvm.org/D69813



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

[PATCH] D69928: [clangd] Set RetainCommentsFromSystemHeaders to true

2019-11-07 Thread Ilya Biryukov via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG001968490049: [clangd] Set RetainCommentsFromSystemHeaders 
to true (authored by ilya-biryukov).

Changed prior to commit:
  https://reviews.llvm.org/D69928?vs=228173&id=228186#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69928

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp


Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -160,6 +160,7 @@
   bool BeginInvocation(CompilerInstance &CI) override {
 // We want all comments, not just the doxygen ones.
 CI.getLangOpts().CommentOpts.ParseAllComments = true;
+CI.getLangOpts().RetainCommentsFromSystemHeaders = true;
 // Index the whole file even if there are warnings and -Werror is set.
 // Avoids some analyses too. Set in two places as we're late to the party.
 CI.getDiagnosticOpts().IgnoreWarnings = true;
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -63,6 +63,7 @@
   // createInvocationFromCommandLine sets DisableFree.
   CI->getFrontendOpts().DisableFree = false;
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  CI->getLangOpts()->RetainCommentsFromSystemHeaders = true;
   return CI;
 }
 


Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -160,6 +160,7 @@
   bool BeginInvocation(CompilerInstance &CI) override {
 // We want all comments, not just the doxygen ones.
 CI.getLangOpts().CommentOpts.ParseAllComments = true;
+CI.getLangOpts().RetainCommentsFromSystemHeaders = true;
 // Index the whole file even if there are warnings and -Werror is set.
 // Avoids some analyses too. Set in two places as we're late to the party.
 CI.getDiagnosticOpts().IgnoreWarnings = true;
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -63,6 +63,7 @@
   // createInvocationFromCommandLine sets DisableFree.
   CI->getFrontendOpts().DisableFree = false;
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  CI->getLangOpts()->RetainCommentsFromSystemHeaders = true;
   return CI;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68568: [clang-format] Make '.clang-format' variants finding a loop

2019-11-07 Thread Anders Waldenborg via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG86825dbe3306: [clang-format] Make '.clang-format' 
variants finding a loop (NFC) (authored by wanders).

Changed prior to commit:
  https://reviews.llvm.org/D68568?vs=223488&id=228187#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68568

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2600,6 +2600,10 @@
   if (std::error_code EC = FS->makeAbsolute(Path))
 return make_string_error(EC.message());
 
+  llvm::SmallVector FilesToLookFor;
+  FilesToLookFor.push_back(".clang-format");
+  FilesToLookFor.push_back("_clang-format");
+
   for (StringRef Directory = Path; !Directory.empty();
Directory = llvm::sys::path::parent_path(Directory)) {
 
@@ -2609,43 +2613,35 @@
   continue;
 }
 
-SmallString<128> ConfigFile(Directory);
-
-llvm::sys::path::append(ConfigFile, ".clang-format");
-LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
+for (const auto &F : FilesToLookFor) {
+  SmallString<128> ConfigFile(Directory);
 
-Status = FS->status(ConfigFile.str());
-bool FoundConfigFile =
-Status && (Status->getType() == 
llvm::sys::fs::file_type::regular_file);
-if (!FoundConfigFile) {
-  // Try _clang-format too, since dotfiles are not commonly used on 
Windows.
-  ConfigFile = Directory;
-  llvm::sys::path::append(ConfigFile, "_clang-format");
+  llvm::sys::path::append(ConfigFile, F);
   LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
+
   Status = FS->status(ConfigFile.str());
-  FoundConfigFile = Status && (Status->getType() ==
-   llvm::sys::fs::file_type::regular_file);
-}
 
-if (FoundConfigFile) {
-  llvm::ErrorOr> Text =
-  FS->getBufferForFile(ConfigFile.str());
-  if (std::error_code EC = Text.getError())
-return make_string_error(EC.message());
-  if (std::error_code ec =
-  parseConfiguration(Text.get()->getBuffer(), &Style)) {
-if (ec == ParseError::Unsuitable) {
-  if (!UnsuitableConfigFiles.empty())
-UnsuitableConfigFiles.append(", ");
-  UnsuitableConfigFiles.append(ConfigFile);
-  continue;
+  if (Status &&
+  (Status->getType() == llvm::sys::fs::file_type::regular_file)) {
+llvm::ErrorOr> Text =
+FS->getBufferForFile(ConfigFile.str());
+if (std::error_code EC = Text.getError())
+  return make_string_error(EC.message());
+if (std::error_code ec =
+parseConfiguration(Text.get()->getBuffer(), &Style)) {
+  if (ec == ParseError::Unsuitable) {
+if (!UnsuitableConfigFiles.empty())
+  UnsuitableConfigFiles.append(", ");
+UnsuitableConfigFiles.append(ConfigFile);
+continue;
+  }
+  return make_string_error("Error reading " + ConfigFile + ": " +
+   ec.message());
 }
-return make_string_error("Error reading " + ConfigFile + ": " +
- ec.message());
+LLVM_DEBUG(llvm::dbgs()
+   << "Using configuration file " << ConfigFile << "\n");
+return Style;
   }
-  LLVM_DEBUG(llvm::dbgs()
- << "Using configuration file " << ConfigFile << "\n");
-  return Style;
 }
   }
   if (!UnsuitableConfigFiles.empty())


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2600,6 +2600,10 @@
   if (std::error_code EC = FS->makeAbsolute(Path))
 return make_string_error(EC.message());
 
+  llvm::SmallVector FilesToLookFor;
+  FilesToLookFor.push_back(".clang-format");
+  FilesToLookFor.push_back("_clang-format");
+
   for (StringRef Directory = Path; !Directory.empty();
Directory = llvm::sys::path::parent_path(Directory)) {
 
@@ -2609,43 +2613,35 @@
   continue;
 }
 
-SmallString<128> ConfigFile(Directory);
-
-llvm::sys::path::append(ConfigFile, ".clang-format");
-LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
+for (const auto &F : FilesToLookFor) {
+  SmallString<128> ConfigFile(Directory);
 
-Status = FS->status(ConfigFile.str());
-bool FoundConfigFile =
-Status && (Status->getType() == llvm::sys::fs::file_type::regular_file);
-if (!FoundConfigFile) {
-  // Try _clang-format too, since dotfiles are not commonly used on Windows.
-  ConfigFile = Directory;
-  llvm::sys::path::append(ConfigFile, "_clang-format");
+  llvm::sys::path::append(ConfigFile, F);
   LLVM_DEBUG(llvm:

[clang] 86825db - [clang-format] Make '.clang-format' variants finding a loop (NFC)

2019-11-07 Thread Anders Waldenborg via cfe-commits


Author: Anders Waldenborg
Date: 2019-11-07T10:00:04+01:00
New Revision: 86825dbe3306d296094432feb4a7af7d385d6b1d

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

LOG: [clang-format] Make '.clang-format' variants finding a loop (NFC)

This simplifies logic making it trivial to add searching for other
files later.

Differential revision: https://reviews.llvm.org/D68568

Added: 


Modified: 
clang/lib/Format/Format.cpp

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index cd44c0be85f0..50a687730932 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2600,6 +2600,10 @@ llvm::Expected getStyle(StringRef 
StyleName, StringRef FileName,
   if (std::error_code EC = FS->makeAbsolute(Path))
 return make_string_error(EC.message());
 
+  llvm::SmallVector FilesToLookFor;
+  FilesToLookFor.push_back(".clang-format");
+  FilesToLookFor.push_back("_clang-format");
+
   for (StringRef Directory = Path; !Directory.empty();
Directory = llvm::sys::path::parent_path(Directory)) {
 
@@ -2609,43 +2613,35 @@ llvm::Expected getStyle(StringRef 
StyleName, StringRef FileName,
   continue;
 }
 
-SmallString<128> ConfigFile(Directory);
-
-llvm::sys::path::append(ConfigFile, ".clang-format");
-LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
+for (const auto &F : FilesToLookFor) {
+  SmallString<128> ConfigFile(Directory);
 
-Status = FS->status(ConfigFile.str());
-bool FoundConfigFile =
-Status && (Status->getType() == 
llvm::sys::fs::file_type::regular_file);
-if (!FoundConfigFile) {
-  // Try _clang-format too, since dotfiles are not commonly used on 
Windows.
-  ConfigFile = Directory;
-  llvm::sys::path::append(ConfigFile, "_clang-format");
+  llvm::sys::path::append(ConfigFile, F);
   LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
+
   Status = FS->status(ConfigFile.str());
-  FoundConfigFile = Status && (Status->getType() ==
-   llvm::sys::fs::file_type::regular_file);
-}
 
-if (FoundConfigFile) {
-  llvm::ErrorOr> Text =
-  FS->getBufferForFile(ConfigFile.str());
-  if (std::error_code EC = Text.getError())
-return make_string_error(EC.message());
-  if (std::error_code ec =
-  parseConfiguration(Text.get()->getBuffer(), &Style)) {
-if (ec == ParseError::Unsuitable) {
-  if (!UnsuitableConfigFiles.empty())
-UnsuitableConfigFiles.append(", ");
-  UnsuitableConfigFiles.append(ConfigFile);
-  continue;
+  if (Status &&
+  (Status->getType() == llvm::sys::fs::file_type::regular_file)) {
+llvm::ErrorOr> Text =
+FS->getBufferForFile(ConfigFile.str());
+if (std::error_code EC = Text.getError())
+  return make_string_error(EC.message());
+if (std::error_code ec =
+parseConfiguration(Text.get()->getBuffer(), &Style)) {
+  if (ec == ParseError::Unsuitable) {
+if (!UnsuitableConfigFiles.empty())
+  UnsuitableConfigFiles.append(", ");
+UnsuitableConfigFiles.append(ConfigFile);
+continue;
+  }
+  return make_string_error("Error reading " + ConfigFile + ": " +
+   ec.message());
 }
-return make_string_error("Error reading " + ConfigFile + ": " +
- ec.message());
+LLVM_DEBUG(llvm::dbgs()
+   << "Using configuration file " << ConfigFile << "\n");
+return Style;
   }
-  LLVM_DEBUG(llvm::dbgs()
- << "Using configuration file " << ConfigFile << "\n");
-  return Style;
 }
   }
   if (!UnsuitableConfigFiles.empty())



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

[clang-tools-extra] dec8d8d - [clangd] Add unit tests for comments in system headers

2019-11-07 Thread Ilya Biryukov via cfe-commits


Author: Ilya Biryukov
Date: 2019-11-07T10:24:27+01:00
New Revision: dec8d8d3f205268712a928d106ff2e6f799f7a9b

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

LOG: [clangd] Add unit tests for comments in system headers

Added: 


Modified: 
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 20f89895279c..02d7750607a4 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -872,6 +872,33 @@ TEST(CompletionTest, Documentation) {
   Contains(AllOf(Named("baz"), Doc("Multi-line\nblock comment";
 }
 
+TEST(CompletionTest, CommentsFromSystemHeaders) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+
+  auto Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+
+  FS.Files[testPath("foo.h")] = R"cpp(
+#pragma GCC system_header
+
+// This comment should be retained!
+int foo();
+  )cpp";
+
+  auto Results = completions(Server,
+ R"cpp(
+#include "foo.h"
+int x = foo^
+ )cpp");
+  EXPECT_THAT(
+  Results.Completions,
+  Contains(AllOf(Named("foo"), Doc("This comment should be retained!";
+}
+
 TEST(CompletionTest, GlobalCompletionFiltering) {
 
   Symbol Class = cls("XYZ");



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

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

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

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

With the new implemenation, we will have better coverage of various AST
nodes, and fix some known/potential bugs.

Aslo added the existing clang-renamae tests, it should not introduce any
regressions.

This fixes:

- https://github.com/clangd/clangd/issues/167
- https://github.com/clangd/clangd/issues/169
- https://github.com/clangd/clangd/issues/171


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69934

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

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

[clang] 5b9a072 - Revert a5c8ec4 "[CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood"

2019-11-07 Thread Hans Wennborg via cfe-commits


Author: Hans Wennborg
Date: 2019-11-07T10:30:07+01:00
New Revision: 5b9a072c39c0c34a290abd19e4aca8208a9afae6

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

LOG: Revert a5c8ec4 "[CGDebugInfo] Emit subprograms for decls when AT_tail_call 
is understood"

This caused Chromium builds to fail with "inlinable function call in a function
with debug info must have a !dbg location" errors. See
https://bugs.chromium.org/p/chromium/issues/detail?id=1022296#c1 for a
reproducer.

> Currently, clang emits subprograms for declared functions when the
> target debugger or DWARF standard is known to support entry values
> (DW_OP_entry_value & the GNU equivalent).
>
> Treat DW_AT_tail_call the same way to allow debuggers to follow cross-TU
> tail calls.
>
> Pre-patch debug session with a cross-TU tail call:
>
> ```
>   * frame #0: 0x00010fa4 main`target at b.c:4:3 [opt]
> frame #1: 0x00010f99 main`main at a.c:8:10 [opt]
> ```
>
> Post-patch (note that the tail-calling frame, "helper", is visible):
>
> ```
>   * frame #0: 0x00010fa4 main`target at b.c:4:3 [opt]
> frame #1: 0x00010f80 main`helper [opt] [artificial]
> frame #2: 0x00010f99 main`main at a.c:8:10 [opt]
> ```
>
> rdar://46577651
>
> Differential Revision: https://reviews.llvm.org/D69743

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index c0c8fd3c8f16..e0bb3fda7acf 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3748,7 +3748,9 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, 
SourceLocation Loc,
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
   QualType CalleeType,
   const FunctionDecl *CalleeDecl) {
-  if (!CallOrInvoke || getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
+  auto &CGOpts = CGM.getCodeGenOpts();
+  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
+  !CallOrInvoke)
 return;
 
   auto *Func = CallOrInvoke->getCalledFunction();
@@ -4822,10 +4824,10 @@ llvm::DINode::DIFlags 
CGDebugInfo::getCallSiteRelatedAttrs() const {
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
   (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
-   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
+   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
 
-  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5 &&
-  !CGM.getCodeGenOpts().EnableDebugEntryValues)
+  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
 return llvm::DINode::FlagZero;
 
   return llvm::DINode::FlagAllCallsDescribed;

diff  --git a/clang/test/CodeGen/debug-info-extern-call.c 
b/clang/test/CodeGen/debug-info-extern-call.c
index 7f58fe59a635..e35669b78f93 100644
--- a/clang/test/CodeGen/debug-info-extern-call.c
+++ b/clang/test/CodeGen/debug-info-extern-call.c
@@ -1,21 +1,8 @@
-// When entry values are emitted, expect a subprogram for extern decls so that
-// the dwarf generator can describe call site parameters at extern call sites.
-//
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=ENTRY-VAL
-// ENTRY-VAL: !DISubprogram(name: "fn1"
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CHECK-EXT
+// CHECK-EXT: !DISubprogram(name: "fn1"
 
-// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
-// decls so that the dwarf generator can describe information needed for tail
-// call frame reconstrution.
-//
-// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=GDB
-// GDB: !DISubprogram(name: "fn1"
-//
-// Do not emit a subprogram for extern decls when entry values are disabled and
-// the tuning is not set to gdb.
-//
-// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=SCE
-// SCE-NOT: !DISubprogram(name: "fn1"
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | 
FileCheck %s
+// CHECK-NOT: !DISubprogram(name: "fn1"
 
 extern int fn1(int a, int b);
 

diff  --git a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp 
b/clang/test/CodeGenCXX

[clang] 118f783 - [clang-rename] Respect the traversal scope when traversing the entire AST.

2019-11-07 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2019-11-07T10:43:54+01:00
New Revision: 118f7836a65e864ef3c7e015d58ca370fee65e89

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

LOG: [clang-rename] Respect the traversal scope when traversing the entire AST.

Summary:
This should be NFC to clang-rename, by default the traversal scope is
TUDecl. Traversing the TUDecl in clangd is a performance cliff, we should
avoid it.

Reviewers: ilya-biryukov

Subscribers: kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp 
b/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
index 966833137c26..d966a5ef23c2 100644
--- a/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ b/clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -67,7 +67,7 @@ class AdditionalUSRFinder : public 
RecursiveASTVisitor {
 
   std::vector Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
-TraverseDecl(Context.getTranslationUnitDecl());
+TraverseAST(Context);
 if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
   addUSRsOfOverridenFunctions(MethodDecl);
   for (const auto &OverriddenMethod : OverriddenMethods) {



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

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-07 Thread Csaba Dabis via Phabricator via cfe-commits

Charusso added a comment.

In D69813#1736611 , @aaron.ballman 
wrote:

> Would it make sense to use `cert.str.31.c` to remove the random dash? Would 
> this also help the user to do something like `cert.str.*.cpp`? if they want 
> just the CERT C++ STR rules checked? Or can they do that already even with 
> the `-`?

Well, we could introduce package `cert.str.c` and `cert.str.cpp` and then the 
rule-number follows: `cert.str.c.31` where the `31` is the name of the checker 
in this case, which sounds very strange. @Szelethus is the code owner of our 
frontend so I would wait how he imagine the layout. As I know to enable every C 
checker of the package `cert.str` we need to create a `c` package because we do 
not have such a logic to put `*` in the package name before the checker's name 
and the package `c` clarify what the user wants to do. Now I have checked your 
`cert.str.cpp` page [1] and I think the `cert.str.cpp` invocation could invoke 
the `cert.str.c` as a dependency, because the `c` rules apply to `cpp` as you 
have written.

On the other hand we are trying to avoid larger scope changes than the 
necessary because we do not know when `cert.str.c` would contain at least two 
checkers. That is why I was so minimal and only introduced the package `cert` 
because we already have two `FLP` checkers inside the `insecureAPI` 
base-checker.

[1] https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046330

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

https://reviews.llvm.org/D69813

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

[PATCH] D69892: [clang-rename] Respect the traversal scope when traversing the entire AST.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG118f7836a65e: [clang-rename] Respect the traversal scope 
when traversing the entire AST. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69892

Files:
  clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp


Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -67,7 +67,7 @@
 
   std::vector Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
-TraverseDecl(Context.getTranslationUnitDecl());
+TraverseAST(Context);
 if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
   addUSRsOfOverridenFunctions(MethodDecl);
   for (const auto &OverriddenMethod : OverriddenMethods) {


Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -67,7 +67,7 @@
 
   std::vector Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
-TraverseDecl(Context.getTranslationUnitDecl());
+TraverseAST(Context);
 if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
   addUSRsOfOverridenFunctions(MethodDecl);
   for (const auto &OverriddenMethod : OverriddenMethods) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] c5e4cf4 - [clangd] NFC, hide the internal-only utility function lex.

2019-11-07 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2019-11-07T10:58:09+01:00
New Revision: c5e4cf40ac459aae996180089a9831959ceb3d05

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

LOG: [clangd] NFC, hide the internal-only utility function lex.

To avoid any potential ODR violations.

Added: 


Modified: 
clang-tools-extra/clangd/SourceCode.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SourceCode.cpp 
b/clang-tools-extra/clangd/SourceCode.cpp
index e52588d6aa84..d505645a74f6 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -719,9 +719,10 @@ cleanupAndFormat(StringRef Code, const 
tooling::Replacements &Replaces,
   return formatReplacements(Code, std::move(*CleanReplaces), Style);
 }
 
-void lex(llvm::StringRef Code, const LangOptions &LangOpts,
- llvm::function_ref
- Action) {
+static void
+lex(llvm::StringRef Code, const LangOptions &LangOpts,
+llvm::function_ref
+Action) {
   // FIXME: InMemoryFileAdapter crashes unless the buffer is null terminated!
   std::string NullTerminatedCode = Code.str();
   SourceManagerForFile FileSM("dummy.cpp", NullTerminatedCode);



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

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D69813#1736667 , @Charusso wrote:

> In D69813#1736611 , @aaron.ballman 
> wrote:
>
> > Would it make sense to use `cert.str.31.c` to remove the random dash? Would 
> > this also help the user to do something like `cert.str.*.cpp`? if they want 
> > just the CERT C++ STR rules checked? Or can they do that already even with 
> > the `-`?
>
>
> Well, we could introduce package `cert.str.c` and `cert.str.cpp` and then the 
> rule-number follows: `cert.str.c.31` where the `31` is the name of the 
> checker in this case, which sounds very strange. @Szelethus is the code owner 
> of our frontend so I would wait how he imagine the layout.


I wouldn't want to go with that approach because it confuses the names from the 
coding standard it's meant to check. I think a good policy is to try to keep 
the check names similar to the coding standard names whenever possible 
(regardless of the coding standard).

> As I know to enable every C checker of the package `cert.str` we need to 
> create a `c` package because we do not have such a logic to put `*` in the 
> package name before the checker's name and the package `c` clarify what the 
> user wants to do. Now I have checked your `cert.str.cpp` page [1] and I think 
> the `cert.str.cpp` invocation could invoke the `cert.str.c` as a dependency, 
> because the `c` rules apply to `cpp` as you have written.

Yes, the C++ rules incorporate some of the C rules, but not all of them, which 
kind of complicates things. The STR section happens to take everything from the 
C STR section.

> On the other hand we are trying to avoid larger scope changes than the 
> necessary because we do not know when `cert.str.c` would contain at least two 
> checkers. That is why I was so minimal and only introduced the package `cert` 
> because we already have two `FLP` checkers inside the `insecureAPI` 
> base-checker.

Understandable.

> [1] https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046330




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

https://reviews.llvm.org/D69813



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

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

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits

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

I spot-checked the list and definitely saw some true positives in there. I did 
not exhaustively check the list, however. The timing numbers look reasonable 
enough as well. Unless someone finds an issue with the fp rate that I've 
missed, I think this is reasonable.


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

https://reviews.llvm.org/D68912



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

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

2019-11-07 Thread Tyker via Phabricator via cfe-commits

Tyker updated this revision to Diff 228197.
Tyker added a comment.

minor fixes
improved tests


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

https://reviews.llvm.org/D63960

Files:
  clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s -verify
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -Wno-unused-value %s -verify
 
 namespace basic_sema {
 
@@ -12,6 +12,7 @@
 }
 
 constexpr auto l_eval = [](int i) consteval {
+// expected-note@-1+ {{declared here}}
 
   return i;
 };
@@ -23,11 +24,12 @@
 
 struct A {
   consteval int f1(int i) const {
+// expected-note@-1 {{declared here}}
 return i;
   }
   consteval A(int i);
   consteval A() = default;
-  consteval ~A() = default;
+  consteval ~A() = default; // expected-error {{destructor cannot be declared consteval}}
 };
 
 consteval struct B {}; // expected-error {{struct cannot be marked consteval}}
@@ -51,14 +53,305 @@
 struct D {
   C c;
   consteval D() = default; // expected-error {{cannot be consteval}}
-  consteval ~D() = default; // expected-error {{cannot be consteval}}
+  consteval ~D() = default; // expected-error {{destructor cannot be declared consteval}}
 };
 
-struct E : C { // expected-note {{here}}
-  consteval ~E() {} // expected-error {{cannot be declared consteval because base class 'basic_sema::C' does not have a constexpr destructor}}
+struct E : C {
+  consteval ~E() {} // expected-error {{cannot be declared consteval}}
 };
 }
 
 consteval int main() { // expected-error {{'main' is not allowed to be declared consteval}}
   return 0;
 }
+
+consteval int f_eval(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+namespace taking_address {
+
+using func_type = int(int);
+
+func_type* p1 = (&f_eval);
+// expected-error@-1 {{take address}}
+func_type* p7 = __builtin_addressof(f_eval);
+// expected-error@-1 {{take address}}
+
+auto p = f_eval;
+// expected-error@-1 {{take address}}
+
+auto m1 = &basic_sema::A::f1;
+// expected-error@-1 {{take address}}
+auto l1 = &decltype(basic_sema::l_eval)::operator();
+// expected-error@-1 {{take address}}
+
+consteval int f(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+auto ptr = &f;
+// expected-error@-1 {{take address}}
+
+auto f1() {
+  return &f;
+// expected-error@-1 {{take address}}
+}
+
+}
+
+namespace invalid_function {
+using size_t = unsigned long;
+struct A {
+  consteval void *operator new(size_t count);
+  // expected-error@-1 {{'operator new' cannot be declared consteval}}
+  consteval void *operator new[](size_t count);
+  // expected-error@-1 {{'operator new[]' cannot be declared consteval}}
+  consteval void operator delete(void* ptr);
+  // expected-error@-1 {{'operator delete' cannot be declared consteval}}
+  consteval void operator delete[](void* ptr);
+  // expected-error@-1 {{'operator delete[]' cannot be declared consteval}}
+  consteval ~A() {}
+  // expected-error@-1 {{destructor cannot be declared consteval}}
+};
+
+}
+
+namespace nested {
+consteval int f() {
+  return 0;
+}
+
+consteval int f1(...) {
+  return 1;
+}
+
+enum E {};
+
+using T = int(&)();
+
+consteval auto operator+ (E, int(*a)()) {
+  return 0;
+}
+
+void d() {
+  auto i = f1(E() + &f);
+}
+
+auto l0 = [](auto) consteval {
+  return 0;
+};
+
+int i0 = l0(&f1);
+
+int i1 = f1(l0(4));
+
+int i2 = f1(&f1, &f1, &f1, &f1, &f1, &f1, &f1);
+
+int i3 = f1(f1(f1(&f1, &f1), f1(&f1, &f1), f1(f1(&f1, &f1), &f1)));
+
+}
+
+namespace user_defined_literal {
+
+consteval int operator"" _test(unsigned long long i) {
+// expected-note@-1+ {{declared here}}
+  return 0;
+}
+
+int i = 0_test;
+
+auto ptr = &operator"" _test;
+// expected-error@-1 {{take address}}
+
+consteval auto operator"" _test1(unsigned long long i) {
+  return &f_eval;
+}
+
+auto i1 = 0_test1; // expected-error {{could not be evaluated}}
+// expected-note@-1 {{is not a constant expression}}
+
+}
+
+namespace return_address {
+
+consteval int f() {
+// expected-note@-1 {{declared here}}
+  return 0;
+}
+
+consteval int(*ret1(int i))() {
+  return &f;
+}
+
+auto ptr = ret1(0);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{pointer to a consteva

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

2019-11-07 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

I think this change is reasonable, but I'd like @rsmith to agree before you 
commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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

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

2019-11-07 Thread Ehud Katz via Phabricator via cfe-commits

ekatz created this revision.
ekatz added reviewers: SouraVX, aprantl, dblaikie, JDevlieghere, uabelho, 
rsmith.
ekatz added a project: clang.
Herald added a subscriber: cfe-commits.

Partial revert of r372681 "Support for DWARF-5 C++ language tags".

The change introduced new external linkage languages ("C++11" and "C++14") 
which not supported in C++.

It also changed the definition of the existing enum to use the DWARF constants. 
The problem is that "LinkageSpecDeclBits.Language" (the field that reserves 
this enum) is actually defined as 3 bits length (bitfield), which cannot 
contain the new DWARF constants. Defining the enum as integer literals is more 
appropriate for maintaining valid values.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69935

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp

Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -31,8 +31,6 @@
 ExternCLoc = LSD->getBeginLoc();
   break;
 case LinkageSpecDecl::lang_cxx:
-case LinkageSpecDecl::lang_cxx_11:
-case LinkageSpecDecl::lang_cxx_14:
   break;
 }
 DC = LSD->getParent();
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -14205,10 +14205,6 @@
 Language = LinkageSpecDecl::lang_c;
   else if (Lang == "C++")
 Language = LinkageSpecDecl::lang_cxx;
-  else if (Lang == "C++11")
-Language = LinkageSpecDecl::lang_cxx_11;
-  else if (Lang == "C++14")
-Language = LinkageSpecDecl::lang_cxx_14;
   else {
 Diag(LangStr->getExprLoc(), diag::err_language_linkage_spec_unknown)
   << LangStr->getSourceRange();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5193,9 +5193,7 @@
 // EmitLinkageSpec - Emit all declarations in a linkage spec.
 void CodeGenModule::EmitLinkageSpec(const LinkageSpecDecl *LSD) {
   if (LSD->getLanguage() != LinkageSpecDecl::lang_c &&
-  LSD->getLanguage() != LinkageSpecDecl::lang_cxx &&
-  LSD->getLanguage() != LinkageSpecDecl::lang_cxx_11 &&
-  LSD->getLanguage() != LinkageSpecDecl::lang_cxx_14) {
+  LSD->getLanguage() != LinkageSpecDecl::lang_cxx) {
 ErrorUnsupported(LSD, "linkage spec");
 return;
   }
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1769,12 +1769,6 @@
   case LinkageSpecDecl::lang_cxx:
 OS << " C++";
 break;
-  case LinkageSpecDecl::lang_cxx_11:
-OS << " C++11";
-break;
-  case LinkageSpecDecl::lang_cxx_14:
-OS << " C++14";
-break;
   }
 }
 
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -874,12 +874,6 @@
   switch (LSD->getLanguage()) {
   case LinkageSpecDecl::lang_c: Lang = "C"; break;
   case LinkageSpecDecl::lang_cxx: Lang = "C++"; break;
-  case LinkageSpecDecl::lang_cxx_11:
-Lang = "C++11";
-break;
-  case LinkageSpecDecl::lang_cxx_14:
-Lang = "C++14";
-break;
   }
   JOS.attribute("language", Lang);
   attributeOnlyIfTrue("hasBraces", LSD->hasBraces());
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -1001,19 +1001,12 @@
 
 void DeclPrinter::VisitLinkageSpecDecl(LinkageSpecDecl *D) {
   const char *l;
-  switch (D->getLanguage()) {
-  case LinkageSpecDecl::lang_c:
+  if (D->getLanguage() == LinkageSpecDecl::lang_c)
 l = "C";
-break;
-  case LinkageSpecDecl::lang_cxx_14:
-l = "C++14";
-break;
-  case LinkageSpecDecl::lang_cxx_11:
-l = "C++11";
-break;
-  case LinkageSpecDecl::lang_cxx:
+  else {
+assert(D->getLanguage() == LinkageSpecDecl::lang_cxx &&
+   "unknown language in linkage specification");
 l = "C++";
-break;
   }
 
   Out << "extern \"" << l << "\" ";
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -42,7 +42,6 @@
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/iterator_range.h"
-#include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
@@ -2

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

2019-11-07 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

It's a bit unclear how we manage to still rename overrides. Is this handled by 
the USR-generating functions?
Could we factor out the part that collects `NamedDecl`s and use those instead 
of the USRs instead?




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

I would argue rename should not work in that case.
Could we check that the cursor is actually on the name token of the `NamedDecl` 
and not rename if it isn't?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:175
+  tooling::getCanonicalSymbolDeclaration(&RenameDecl), 
AST.getASTContext());
+  llvm::DenseSet TargetIDs;
+  for (auto &USR : RenameUSRs)

Why `USRs`? Could we just check whether the `ND.getCanonicalDecl()` is there 
instead?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:97
+};
+struct C : B {
+  void [[f^oo]]() override {}

Could we stop at `B`?
I don't think `C->D` cover any more code paths, they merely add some noise to 
the test, making it harder to read.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:109
+  A a;
+  a.[[f^oo]]();
+  B b;

NIT: alternatively, just do `A().foo()` to make the test smaller



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:131
+public:
+  [[Foo]](int value = 0) : x(value) {}
+

Could you also check that destructors are renamed properly?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:131
+public:
+  [[Foo]](int value = 0) : x(value) {}
+

ilya-biryukov wrote:
> Could you also check that destructors are renamed properly?
NIT: maybe remove the bodies of the functions that don't have references to 
`Foo`?
They seem to merely add noise, don't think they improve coverage.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:185
+};
+// FIXME: selection tree.
+Qux::Qux() : [[Foo]]() {}

The FIXME is a bit unclear. Could you explain in more detail?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:214
+  R"cpp(
+#define moo [[foo]]
+int [[fo^o]]() { return 42; }

I would argue we should fail in that case and not rename.
If we change the macro body, there's a chance other usages that we didn't want 
to update will also be updated.

However, if the current rename does that, also happy to keep as is. Up to you.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:380
+
+for (const auto &RenamePos : Code.points()) {
+  auto RenameResult =

NIT: also mention in the documentation comment above that rename is run on 
**all** points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

2019-11-07 Thread UTKARSH SAXENA via Phabricator via cfe-commits

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

We use the name from the IdentifierInfo of the Macro to compute its
SymbolID. It is better to just take the Name as a parameter to avoid
storing the IdentifierInfo whenever we need the SymbolID for the Macro.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69937

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp

Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -377,7 +377,7 @@
 Roles & static_cast(index::SymbolRole::Definition)))
 return true;
 
-  auto ID = getSymbolID(*Name, MI, SM);
+  auto ID = getSymbolID(Name->getName(), MI, SM);
   if (!ID)
 return true;
 
@@ -473,14 +473,14 @@
 // First, drop header guards. We can't identify these until EOF.
 for (const IdentifierInfo *II : IndexedMacros) {
   if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
   if (MI->isUsedForHeaderGuard())
 Symbols.erase(*ID);
 }
 // Now increment refcounts.
 for (const IdentifierInfo *II : ReferencedMacros) {
   if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
   IncRef(*ID);
 }
   }
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -492,7 +492,7 @@
 return clang::clangd::getSymbolID(R.Declaration);
   }
   case CodeCompletionResult::RK_Macro:
-return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM);
+return clang::clangd::getSymbolID(R.Macro->getName(), R.MacroDefInfo, SM);
   case CodeCompletionResult::RK_Keyword:
 return None;
   }
@@ -1765,8 +1765,7 @@
   Options.IncludeBriefComments = false;
   IncludeStructure PreambleInclusions; // Unused for signatureHelp
   semaCodeComplete(
-  std::make_unique(Options, Index, Result),
-  Options,
+  std::make_unique(Options, Index, Result), Options,
   {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)});
   return Result;
 }
Index: clang-tools-extra/clangd/AST.h
===
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -17,6 +17,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/MacroInfo.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 class SourceManager;
@@ -69,7 +70,7 @@
 /// macro (e.g. a change in definition offset can result in a different USR). We
 /// could change these semantics in the future by reimplementing this funcure
 /// (e.g. avoid USR for macros).
-llvm::Optional getSymbolID(const IdentifierInfo &II,
+llvm::Optional getSymbolID(const llvm::StringRef MacroName,
  const MacroInfo *MI,
  const SourceManager &SM);
 
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -203,13 +203,13 @@
   return SymbolID(USR);
 }
 
-llvm::Optional getSymbolID(const IdentifierInfo &II,
+llvm::Optional getSymbolID(const llvm::StringRef MacroName,
  const MacroInfo *MI,
  const SourceManager &SM) {
   if (MI == nullptr)
 return None;
   llvm::SmallString<128> USR;
-  if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, USR))
+  if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
 return None;
   return SymbolID(USR);
 }
@@ -225,7 +225,7 @@
 
   unsigned DifferentAt = 0;
   while (DifferentAt < MinLength &&
-  CurrentParts[DifferentAt] == OriginalParts[DifferentAt]) {
+ CurrentParts[DifferentAt] == OriginalParts[DifferentAt]) {
 DifferentAt++;
   }
 
@@ -235,15 +235,12 @@
   return join(Result, "::");
 }
 
-std::string printType(const QualType QT, const DeclContext & Context){
+std::string printType(const QualType QT, const DeclContext &Context) {
   PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());
   PP.SuppressUnwrittenScope = 1;
   PP.SuppressTagKeyword = 1;
-  return shor

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

2019-11-07 Thread Alexey Bader via Phabricator via cfe-commits

bader added a comment.

Hi @aaron.ballman,

Thanks a lot for the comments and sorry for the long delay. We've been working 
on complete implementation of the SYCL 1.2.1 specification.
Now I have more time to work on contributing the implementation to LLVM project.

I re-based the patch and started applying your suggestions. 
In addition to that I'd like to investigate slightly different approach to 
outlining suggested by @ABataev at LLVM Dev. Meeting conference and utilize the 
infrastructure OpenMP compiler uses in CodeGen library to emit "device part" of 
the single source.

Thanks,
Alexey


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

2019-11-07 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia created this revision.
Anastasia added a reviewer: rjmccall.
Herald added subscribers: ebevhan, yaxunl.

Since lambdas are represented by callable objects, we need to adjust addr space 
of implicit obj parameter.

This patch suggests to use `__generic` for OpenCL mode. Then any lambda 
variable declared in `__constant` addr space (which is not convertible to 
`__generic`)  would fail to compile with a diagnostic.

Towards generic C++ mode it might be better to just use add space from the 
lambda variable to qualify internal class member functions of lambda? However, 
I am not clear how to propagate the addr space since we are handling the 
initializers before the variable declaration. Alternatively it might make sense 
to just disallow qualifying lambdas with an addr space since they have internal 
compiler representation defined by the implementation.


https://reviews.llvm.org/D69938

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaOpenCLCXX/address-space-lambda.cl


Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -0,0 +1,14 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'int (int) const __generic'
+auto glambda = [](auto a) { return a; };
+
+__kernel void foo() {
+  int i;
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+  auto  llambda = [&]() {i++;};
+  llambda();
+  glambda(1);
+  __constant auto err = [&]() {}; //expected-note-re{{candidate function not 
viable: address space mismatch in 'this' argument ('__constant (lambda at 
{{.*}})'), parameter type must be 'const __generic (lambda at {{.*}})'}}
+  err(); //expected-error-re{{no matching function for call to object of type 
'__constant (lambda at {{.*}})'}}
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4916,7 +4916,9 @@
   .getScopeRep()
   ->getKind() == NestedNameSpecifier::TypeSpec) ||
  state.getDeclarator().getContext() ==
- DeclaratorContext::MemberContext;
+ DeclaratorContext::MemberContext ||
+ state.getDeclarator().getContext() ==
+ DeclaratorContext::LambdaExprContext;
 };
 
 if (state.getSema().getLangOpts().OpenCLCPlusPlus && IsClassMember()) {


Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -0,0 +1,14 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'int (int) const __generic'
+auto glambda = [](auto a) { return a; };
+
+__kernel void foo() {
+  int i;
+//CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
+  auto  llambda = [&]() {i++;};
+  llambda();
+  glambda(1);
+  __constant auto err = [&]() {}; //expected-note-re{{candidate function not viable: address space mismatch in 'this' argument ('__constant (lambda at {{.*}})'), parameter type must be 'const __generic (lambda at {{.*}})'}}
+  err(); //expected-error-re{{no matching function for call to object of type '__constant (lambda at {{.*}})'}}
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4916,7 +4916,9 @@
   .getScopeRep()
   ->getKind() == NestedNameSpecifier::TypeSpec) ||
  state.getDeclarator().getContext() ==
- DeclaratorContext::MemberContext;
+ DeclaratorContext::MemberContext ||
+ state.getDeclarator().getContext() ==
+ DeclaratorContext::LambdaExprContext;
 };
 
 if (state.getSema().getLangOpts().OpenCLCPlusPlus && IsClassMember()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69883: [OpenCL] Add math and common builtin functions

2019-11-07 Thread Anastasia Stulova via Phabricator via cfe-commits

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

LGTM! Thanks!

I guess we need to think about testing quite soon. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D69883



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

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

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

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

there are some unrelated changes in this patch (probably trigger by your editor 
setting?), though they are trivial, I would avoid these changes in a same patch.




Comment at: clang-tools-extra/clangd/AST.cpp:238
 
-std::string printType(const QualType QT, const DeclContext & Context){
+std::string printType(const QualType QT, const DeclContext &Context) {
   PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());

hmm, this is a unrelated change.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1768
   semaCodeComplete(
-  std::make_unique(Options, Index, Result),
-  Options,
+  std::make_unique(Options, Index, Result), 
Options,
   {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)});

same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69937



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

[PATCH] D69901: [OpenCL] Add integer functions to builtin functions

2019-11-07 Thread Anastasia Stulova via Phabricator via cfe-commits

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

LGTM! Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D69901



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

[PATCH] D69908: [OpenCL] Add geometric and relational builtin functions

2019-11-07 Thread Anastasia Stulova via Phabricator via cfe-commits

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

LGTM! Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D69908



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

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

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

hokein added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:764
+  /*WantFormat=*/true,
+  [this](PathRef File) { return DraftMgr.getDraft(File); },
+  [File, Params, Reply = std::move(Reply),

ilya-biryukov wrote:
> We should probably get a snapshot of all dirty buffers here instead.
> A racy  way (rename is run on a separate thread, new files updates might come 
> in in the meantime) to get contents of the file looks like a bad idea, this 
> will lead to hard-to-debug failures...
> 
> Note that `ClangdServer` also has access to all contents of all the files, 
> they are stored in `TUScheduler`, so we don't need to pass `GetDirtyBuffer` 
> callback up until the final run of `rename`
thanks, I like the idea. This could simplify the code, with this approach, the 
dirty buffer of the main file would be the same as the one used for building 
the AST.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

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

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

hokein updated this revision to Diff 228213.
hokein marked 5 inline comments as done.
hokein added a comment.
Herald added a subscriber: javed.absar.

- get dirty buffer in clangdServer layer;
- save a snpatshot for all dirty buffer;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

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

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,10 +96,9 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer &Server,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
   Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
   return std::move(*Result);
 }
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -9,8 +9,10 @@
 #include "Annotations.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,8 +20,43 @@
 namespace clangd {
 namespace {
 
-MATCHER_P2(RenameRange, Code, Range, "") {
-  return replacementToEdit(Code, arg).range == Range;
+using testing::Eq;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
+std::unique_ptr buildRefSlab(const Annotations &Code,
+  llvm::StringRef SymbolName,
+  llvm::StringRef Path) {
+  RefSlab::Builder Builder;
+  TestTU TU;
+  TU.HeaderCode = Code.code();
+  auto Symbols = TU.headerSymbols();
+  const auto &SymbolID = findSymbol(Symbols, SymbolName).ID;
+  for (const auto &Range : Code.ranges()) {
+Ref R;
+R.Kind = RefKind::Reference;
+R.Location.Start.setLine(Range.start.line);
+R.Location.Start.setColumn(Range.start.character);
+R.Location.End.setLine(Range.end.line);
+R.Location.End.setColumn(Range.end.character);
+auto U = URI::create(Path).toString();
+R.Location.FileURI = U.c_str();
+Builder.insert(SymbolID, R);
+  }
+
+  return std::make_unique(std::move(Builder).build());
+}
+
+std::vector<
+std::pair>
+applyRename(FileEdits FE) {
+  std::vector> Results;
+  for (auto &It : FE)
+Results.emplace_back(
+It.first().str(),
+llvm::cantFail(tooling::applyAllReplacements(
+It.getValue().InitialCode, It.getValue().Replacements)));
+  return Results;
 }
 
 // Generates an expected rename result by replacing all ranges in the given
@@ -73,11 +110,11 @@
 auto AST = TU.build();
 llvm::StringRef NewName = "abcde";
 auto RenameResult =
-renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName);
+rename({Code.point(), NewName, AST, testPath(TU.Filename)});
 ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
-auto ApplyResult = llvm::cantFail(
-tooling::applyAllReplacements(Code.code(), *RenameResult));
-EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
+EXPECT_THAT(applyRename(std::move(*RenameResult)),
+UnorderedElementsAre(
+Pair(testing::_, Eq(expectedR

[PATCH] D69878: Consoldiate internal denormal flushing controls

2019-11-07 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a comment.

> Stop emitting the denorms-are-zero attribute for the OpenCL flag. It
>  has no in-tree users. The meaning would also be target dependent, such
>  as the AMDGPU choice to treat this as only meaning allow flushing of
>  f32 and not f16 or f64. The naming is also potentially confusing,
>  since DAZ in other contexts refers to instructions implicitly treating
>  input denormals as zero, not necessarily flushing output denormals to
>  zero.

Would the targets supporting OpenCL need to define their own behavior in 
`getDefaultDenormalModeForType`?




Comment at: clang/include/clang/Driver/ToolChain.h:619
+  const llvm::fltSemantics *FPType = nullptr) const {
+// FIXME: This should be IEEE when default handling is fixed.
+return llvm::DenormalMode::Invalid;

Can you elaborate what has to be done in order to fix this?


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

https://reviews.llvm.org/D69878



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

[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

2019-11-07 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:4091
+V = Builder.CreatePointerBitCastOrAddrSpaceCast(
+V, IRFuncTy->getParamType(FirstIRArg));
+  else

svenvh wrote:
> @Anastasia pointed out that the AST might not be correct to begin with, which 
> could cause us to fail here.
> 
> ```
> | `-MemberExpr 0x558af2c0  ' type>' .getRef 0x558a4008
> |   `-ImplicitCastExpr 0x558af310  '__generic B2' lvalue 
> 
> | `-DeclRefExpr 0x558af2a0  'Derived' lvalue Var 
> 0x558ab290 'd' 'Derived'
> ```
> 
> The question is whether it's fine for the `UncheckedDerivedToBase 
> ImplicitCastExpr` to cast from `private` to `__generic`; or do we need 
> another `AddrSpaceConversion ImplicitCastExpr` node perhaps?
Yes, I am not sure AST is correct wrt addr spaces.

Also we should at least use `performAddrSpaceCast` that isn't easy here because 
the source addr spaces are not available at this point.

But overall I would like @rjmccall to have a look.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69810



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

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

2019-11-07 Thread Sanjay Patel via Phabricator via cfe-commits

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

LGTM - see inline for some leftover naming diffs.




Comment at: clang/lib/CodeGen/CGCall.cpp:1744-1745
   FuncAttrs.addAttribute("null-pointer-is-valid", "true");
-if (!CodeGenOpts.FPDenormalMode.empty())
-  FuncAttrs.addAttribute("denormal-fp-math", CodeGenOpts.FPDenormalMode);
+if (CodeGenOpts.FPSubnormalMode != llvm::SubnormalMode::Invalid)
+  FuncAttrs.addAttribute("denormal-fp-math",
+ 
llvm::subnormalModeName(CodeGenOpts.FPSubnormalMode));

If I'm seeing it correctly, this can't build as-is? FPSubnormalMode is called 
FPDenormalMode in the header change above here.



Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:20469
 ISD::NodeType SelOpcode = VT.isVector() ? ISD::VSELECT : ISD::SELECT;
-const Function &F = DAG.getMachineFunction().getFunction();
-Attribute Denorms = F.getFnAttribute("denormal-fp-math");
-if (Denorms.getValueAsString().equals("ieee")) {
+DenormalMode SubnormMode = DAG.getDenormalMode(VT);
+if (SubnormMode == DenormalMode::IEEE) {

SubnormMode -> DenormMode


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

https://reviews.llvm.org/D69598



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

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

2019-11-07 Thread UTKARSH SAXENA via Phabricator via cfe-commits

usaxena95 updated this revision to Diff 228220.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69937

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -377,7 +377,7 @@
 Roles & static_cast(index::SymbolRole::Definition)))
 return true;
 
-  auto ID = getSymbolID(*Name, MI, SM);
+  auto ID = getSymbolID(Name->getName(), MI, SM);
   if (!ID)
 return true;
 
@@ -473,14 +473,14 @@
 // First, drop header guards. We can't identify these until EOF.
 for (const IdentifierInfo *II : IndexedMacros) {
   if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
   if (MI->isUsedForHeaderGuard())
 Symbols.erase(*ID);
 }
 // Now increment refcounts.
 for (const IdentifierInfo *II : ReferencedMacros) {
   if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
   IncRef(*ID);
 }
   }
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -492,7 +492,7 @@
 return clang::clangd::getSymbolID(R.Declaration);
   }
   case CodeCompletionResult::RK_Macro:
-return clang::clangd::getSymbolID(*R.Macro, R.MacroDefInfo, SM);
+return clang::clangd::getSymbolID(R.Macro->getName(), R.MacroDefInfo, SM);
   case CodeCompletionResult::RK_Keyword:
 return None;
   }
Index: clang-tools-extra/clangd/AST.h
===
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -17,6 +17,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/MacroInfo.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 class SourceManager;
@@ -69,7 +70,7 @@
 /// macro (e.g. a change in definition offset can result in a different USR). 
We
 /// could change these semantics in the future by reimplementing this funcure
 /// (e.g. avoid USR for macros).
-llvm::Optional getSymbolID(const IdentifierInfo &II,
+llvm::Optional getSymbolID(const llvm::StringRef MacroName,
  const MacroInfo *MI,
  const SourceManager &SM);
 
Index: clang-tools-extra/clangd/AST.cpp
===
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -203,13 +203,13 @@
   return SymbolID(USR);
 }
 
-llvm::Optional getSymbolID(const IdentifierInfo &II,
+llvm::Optional getSymbolID(const llvm::StringRef MacroName,
  const MacroInfo *MI,
  const SourceManager &SM) {
   if (MI == nullptr)
 return None;
   llvm::SmallString<128> USR;
-  if (index::generateUSRForMacro(II.getName(), MI->getDefinitionLoc(), SM, 
USR))
+  if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
 return None;
   return SymbolID(USR);
 }


Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -377,7 +377,7 @@
 Roles & static_cast(index::SymbolRole::Definition)))
 return true;
 
-  auto ID = getSymbolID(*Name, MI, SM);
+  auto ID = getSymbolID(Name->getName(), MI, SM);
   if (!ID)
 return true;
 
@@ -473,14 +473,14 @@
 // First, drop header guards. We can't identify these until EOF.
 for (const IdentifierInfo *II : IndexedMacros) {
   if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager()))
   if (MI->isUsedForHeaderGuard())
 Symbols.erase(*ID);
 }
 // Now increment refcounts.
 for (const IdentifierInfo *II : ReferencedMacros) {
   if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
-if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceM

[clang] 6fc73f6 - [OpenCL] Add math and common builtin functions

2019-11-07 Thread Sven van Haastregt via cfe-commits


Author: Sven van Haastregt
Date: 2019-11-07T13:16:04Z
New Revision: 6fc73f63660b1fbe3a1af7b3f14d6fe441e0e938

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

LOG: [OpenCL] Add math and common builtin functions

Add the remaining math and common builtin functions from the OpenCL C
specification.

Patch by Pierre Gondois and Sven van Haastregt.

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

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 4f458652ff73..f31cb26e2c17 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -350,7 +350,160 @@ let MinVersion = CL20 in {
   }
 }
 
+
+//
+// OpenCL v1.1 s6.11.2, v1.2 s6.12.2, v2.0 s6.13.2 - Math functions
+// OpenCL Extension v2.0 s5.1.2 and s6.1.2 - Math Functions
+// --- Table 8 ---
+// --- 1 argument ---
+foreach name = ["acos", "acosh", "acospi",
+"asin", "asinh", "asinpi",
+"atan", "atanh", "atanpi",
+"cbrt", "ceil",
+"cos", "cosh", "cospi",
+"erfc", "erf",
+"exp", "exp2", "exp10", "expm1",
+"fabs", "floor",
+"log", "log2", "log10", "log1p", "logb",
+"rint", "round", "rsqrt",
+"sin", "sinh", "sinpi",
+"sqrt",
+"tan", "tanh", "tanpi",
+"tgamma", "trunc",
+"lgamma"] in {
+def : Builtin;
+}
+foreach name = ["nan"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 2 arguments ---
+foreach name = ["atan2", "atan2pi", "copysign", "fdim", "fmod", "hypot",
+"maxmag", "minmag", "nextafter", "pow", "powr",
+"remainder"] in {
+  def : Builtin;
+}
+foreach name = ["fmax", "fmin"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["ilogb"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["ldexp"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["pown", "rootn"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["fma", "mad"] in {
+  def : Builtin;
+}
+
+// --- Version dependent ---
+let MaxVersion = CL20 in {
+  foreach AS = [GlobalAS, LocalAS, PrivateAS] in {
+foreach name = ["fract", "modf", "sincos"] in {
+  def : Builtin]>;
+}
+foreach name = ["frexp", "lgamma_r"] in {
+  foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, 
GenTypeHalfVecAndScalar] in {
+def : Builtin]>;
+  }
+}
+foreach name = ["remquo"] in {
+  foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, 
GenTypeHalfVecAndScalar] in {
+def : Builtin]>;
+  }
+}
+  }
+}
+let MinVersion = CL20 in {
+  foreach name = ["fract", "modf", "sincos"] in {
+def : Builtin]>;
+  }
+  foreach name = ["frexp", "lgamma_r"] in {
+foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, 
GenTypeHalfVecAndScalar] in {
+  def : Builtin]>;
+}  }
+  foreach name = ["remquo"] in {
+foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, 
GenTypeHalfVecAndScalar] in {
+  def : Builtin]>;
+}
+  }
+}
+
+// --- Table 9 ---
+foreach name = ["half_cos",
+"half_exp", "half_exp2", "half_exp10",
+"half_log", "half_log2", "half_log10",
+"half_recip", "half_rsqrt",
+"half_sin", "half_sqrt", "half_tan",
+"native_cos",
+"native_exp", "native_exp2", "native_exp10",
+"native_log", "native_log2", "native_log10",
+"native_recip", "native_rsqrt",
+"native_sin", "native_sqrt", "native_tan"] in {
+  def : Builtin;
+}
+foreach name = ["half_divide", "half_powr",
+"native_divide", "native_powr"] in {
+  def : Builtin;
+}
+
 //
+// OpenCL v1.1 s6.11.4, v1.2 s6.12.4, v2.0 s6.13.4 - Common Functions
+// OpenCL Extension v2.0 s5.1.3 and s6.1.3 - Common Functions
+// --- Table 12 ---
+// --- 1 argument ---
+foreach name = ["degrees", "radians", "sign"] in {
+  def : Builtin;
+}
+
+// --- 2 arguments ---
+foreach name = ["max", "min"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["step"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["c

[PATCH] D69883: [OpenCL] Add math and common builtin functions

2019-11-07 Thread Sven van Haastregt via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6fc73f63660b: [OpenCL] Add math and common builtin functions 
(authored by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69883

Files:
  clang/lib/Sema/OpenCLBuiltins.td

Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -350,7 +350,160 @@
   }
 }
 
+
+//
+// OpenCL v1.1 s6.11.2, v1.2 s6.12.2, v2.0 s6.13.2 - Math functions
+// OpenCL Extension v2.0 s5.1.2 and s6.1.2 - Math Functions
+// --- Table 8 ---
+// --- 1 argument ---
+foreach name = ["acos", "acosh", "acospi",
+"asin", "asinh", "asinpi",
+"atan", "atanh", "atanpi",
+"cbrt", "ceil",
+"cos", "cosh", "cospi",
+"erfc", "erf",
+"exp", "exp2", "exp10", "expm1",
+"fabs", "floor",
+"log", "log2", "log10", "log1p", "logb",
+"rint", "round", "rsqrt",
+"sin", "sinh", "sinpi",
+"sqrt",
+"tan", "tanh", "tanpi",
+"tgamma", "trunc",
+"lgamma"] in {
+def : Builtin;
+}
+foreach name = ["nan"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 2 arguments ---
+foreach name = ["atan2", "atan2pi", "copysign", "fdim", "fmod", "hypot",
+"maxmag", "minmag", "nextafter", "pow", "powr",
+"remainder"] in {
+  def : Builtin;
+}
+foreach name = ["fmax", "fmin"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["ilogb"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["ldexp"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["pown", "rootn"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["fma", "mad"] in {
+  def : Builtin;
+}
+
+// --- Version dependent ---
+let MaxVersion = CL20 in {
+  foreach AS = [GlobalAS, LocalAS, PrivateAS] in {
+foreach name = ["fract", "modf", "sincos"] in {
+  def : Builtin]>;
+}
+foreach name = ["frexp", "lgamma_r"] in {
+  foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, GenTypeHalfVecAndScalar] in {
+def : Builtin]>;
+  }
+}
+foreach name = ["remquo"] in {
+  foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, GenTypeHalfVecAndScalar] in {
+def : Builtin]>;
+  }
+}
+  }
+}
+let MinVersion = CL20 in {
+  foreach name = ["fract", "modf", "sincos"] in {
+def : Builtin]>;
+  }
+  foreach name = ["frexp", "lgamma_r"] in {
+foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, GenTypeHalfVecAndScalar] in {
+  def : Builtin]>;
+}  }
+  foreach name = ["remquo"] in {
+foreach Type = [GenTypeFloatVecAndScalar, GenTypeDoubleVecAndScalar, GenTypeHalfVecAndScalar] in {
+  def : Builtin]>;
+}
+  }
+}
+
+// --- Table 9 ---
+foreach name = ["half_cos",
+"half_exp", "half_exp2", "half_exp10",
+"half_log", "half_log2", "half_log10",
+"half_recip", "half_rsqrt",
+"half_sin", "half_sqrt", "half_tan",
+"native_cos",
+"native_exp", "native_exp2", "native_exp10",
+"native_log", "native_log2", "native_log10",
+"native_recip", "native_rsqrt",
+"native_sin", "native_sqrt", "native_tan"] in {
+  def : Builtin;
+}
+foreach name = ["half_divide", "half_powr",
+"native_divide", "native_powr"] in {
+  def : Builtin;
+}
+
 //
+// OpenCL v1.1 s6.11.4, v1.2 s6.12.4, v2.0 s6.13.4 - Common Functions
+// OpenCL Extension v2.0 s5.1.3 and s6.1.3 - Common Functions
+// --- Table 12 ---
+// --- 1 argument ---
+foreach name = ["degrees", "radians", "sign"] in {
+  def : Builtin;
+}
+
+// --- 2 arguments ---
+foreach name = ["max", "min"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["step"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["clamp", "mix"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["smoothstep"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+
 // OpenCL v1.1 s6.11.7, v1.2 s6.12.7, v2.0 s6.13.7 - Vector Data Load and Store Functions
 // OpenCL Extension v1.1 s9.3.6 and s9.6.6, v1.2 s9.5.6, v2.0 s9.4.6, v2.0 s5.1.6 and 6.1.6 - V

[PATCH] D69625: [libTooling] Support implicit coercions in Stencil's `access` combinator.

2019-11-07 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel added a comment.

In D69625#1730324 , @gribozavr2 wrote:

> > Are you suggesting we remove text and selection entirely?
>
> Yes!


Sounds good. I'll do that in a separate patch. I think we can abandon this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69625



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

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-11-07 Thread Sam Elliott via Phabricator via cfe-commits

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

Thanks for the rebase. It looks good, let's get it committed!


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

https://reviews.llvm.org/D68391



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

[clang] 10e0d64 - CodeGen: set correct result for atomic compound expressions

2019-11-07 Thread Tim Northover via cfe-commits


Author: Tim Northover
Date: 2019-11-07T13:36:44Z
New Revision: 10e0d64337d64ebdb658bf9108bd9bb48fb5390c

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

LOG: CodeGen: set correct result for atomic compound expressions

Atomic compound expressions try to use atomicrmw if possible, but this
path doesn't set the Result variable, leaving it to crash in later code
if anything ever tries to use the result of the expression. This fixes
that issue by recalculating the new value based on the old one
atomically loaded.

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/atomic_ops.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index c1391d46f60c..822976640643 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2849,7 +2849,8 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
   CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) &&
 CGF.getLangOpts().getSignedOverflowBehavior() !=
 LangOptions::SOB_Trapping) {
-  llvm::AtomicRMWInst::BinOp aop = llvm::AtomicRMWInst::BAD_BINOP;
+  llvm::AtomicRMWInst::BinOp AtomicOp = llvm::AtomicRMWInst::BAD_BINOP;
+  llvm::Instruction::BinaryOps Op;
   switch (OpInfo.Opcode) {
 // We don't have atomicrmw operands for *, %, /, <<, >>
 case BO_MulAssign: case BO_DivAssign:
@@ -2858,30 +2859,40 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
 case BO_ShrAssign:
   break;
 case BO_AddAssign:
-  aop = llvm::AtomicRMWInst::Add;
+  AtomicOp = llvm::AtomicRMWInst::Add;
+  Op = llvm::Instruction::Add;
   break;
 case BO_SubAssign:
-  aop = llvm::AtomicRMWInst::Sub;
+  AtomicOp = llvm::AtomicRMWInst::Sub;
+  Op = llvm::Instruction::Sub;
   break;
 case BO_AndAssign:
-  aop = llvm::AtomicRMWInst::And;
+  AtomicOp = llvm::AtomicRMWInst::And;
+  Op = llvm::Instruction::And;
   break;
 case BO_XorAssign:
-  aop = llvm::AtomicRMWInst::Xor;
+  AtomicOp = llvm::AtomicRMWInst::Xor;
+  Op = llvm::Instruction::Xor;
   break;
 case BO_OrAssign:
-  aop = llvm::AtomicRMWInst::Or;
+  AtomicOp = llvm::AtomicRMWInst::Or;
+  Op = llvm::Instruction::Or;
   break;
 default:
   llvm_unreachable("Invalid compound assignment type");
   }
-  if (aop != llvm::AtomicRMWInst::BAD_BINOP) {
-llvm::Value *amt = CGF.EmitToMemory(
+  if (AtomicOp != llvm::AtomicRMWInst::BAD_BINOP) {
+llvm::Value *Amt = CGF.EmitToMemory(
 EmitScalarConversion(OpInfo.RHS, E->getRHS()->getType(), LHSTy,
  E->getExprLoc()),
 LHSTy);
-Builder.CreateAtomicRMW(aop, LHSLV.getPointer(), amt,
+Value *OldVal = Builder.CreateAtomicRMW(
+AtomicOp, LHSLV.getPointer(), Amt,
 llvm::AtomicOrdering::SequentiallyConsistent);
+
+// Since operation is atomic, the result type is guaranteed to be the
+// same as the input in LLVM terms.
+Result = Builder.CreateBinOp(Op, OldVal, Amt);
 return LHSLV;
   }
 }

diff  --git a/clang/test/CodeGen/atomic_ops.c b/clang/test/CodeGen/atomic_ops.c
index 0af1d387192d..a853ba9f739c 100644
--- a/clang/test/CodeGen/atomic_ops.c
+++ b/clang/test/CodeGen/atomic_ops.c
@@ -37,3 +37,56 @@ void baz(int y) {
 // CHECK: {{store atomic|call void @__atomic_store}}
   x += y;
 }
+
+_Atomic(int) compound_add(_Atomic(int) in) {
+// CHECK-LABEL: @compound_add
+// CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = add i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in += 5);
+}
+
+_Atomic(int) compound_sub(_Atomic(int) in) {
+// CHECK-LABEL: @compound_sub
+// CHECK: [[OLD:%.*]] = atomicrmw sub i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = sub i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in -= 5);
+}
+
+_Atomic(int) compound_xor(_Atomic(int) in) {
+// CHECK-LABEL: @compound_xor
+// CHECK: [[OLD:%.*]] = atomicrmw xor i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = xor i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in ^= 5);
+}
+
+_Atomic(int) compound_or(_Atomic(int) in) {
+// CHECK-LABEL: @compound_or
+// CHECK: [[OLD:%.*]] = atomicrmw or i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = or i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in |= 5);
+}
+
+_Atomic(int) compound_and(_Atomic(int) in) {
+// CHECK-LABEL: @compound_and
+// CHECK: [[OLD:%.*]] = atomicrmw and i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = and i32 [[OLD]], 5
+// CHECK: r

[PATCH] D67436: CodeGen: set correct result for atomic compound expressions

2019-11-07 Thread Tim Northover via Phabricator via cfe-commits

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks JF.

To github.com:llvm/llvm-project.git

  0ec6a4882ee..10e0d64337d  master -> master

> Separately, does this do floating-point add / sub properly? We added them too 
> C++20.

It looks like that already works because it doesn't/can't use LLVM's atomicrmw 
path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67436



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3489
+ !OMPRegionInfo->hasCancel())) {
+OMPBuilder->CreateBarrier({CGF.Builder.saveIP()}, Kind, ForceSimpleCall,
+  EmitChecks);

`createBarrier`



Comment at: clang/test/OpenMP/barrier_codegen.cpp:43
+// IRBUILDER:  ; Function Attrs: inaccessiblemem_or_argmemonly
+// IRBUILDER-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #2
+

Remove `#2`, it may be changed 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

2019-11-07 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Does that mean we identify each macro merely by its name?
It's not uncommon to have multiple `#define` for the same name, meaning 
completely different things.

If find references shows all of those, it's not very useful...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69937



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

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

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

hokein added a comment.

In D69937#1736980 , @ilya-biryukov 
wrote:

> Does that mean we identify each macro merely by its name?
>  It's not uncommon to have multiple `#define` for the same name, meaning 
> completely different things.
>
> If find references shows all of those, it's not very useful...


no, I would say this patch is NFC, we still construct macro symbol id with 
macro name and definition location.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69937



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

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

2019-11-07 Thread UTKARSH SAXENA via Phabricator via cfe-commits

usaxena95 added a comment.

We actually use both the name and the **source location** of the macro to 
calculate its ID.
I see that the subject of the patch might suggest otherwise. 
This is a trivial change which just changes the params of the function so that 
users don't have to carry the IdentifierInfo when we just want the name out of 
it.

We use `clang::index::generateUSRForMacro(StringRef MacroName, SourceLocation 
Loc, const SourceManager &SM, SmallVectorImpl &Buf)` to generate the 
SymbolID.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69937



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

[clang] 0e70c35 - [OpenCL] Add integer builtin functions

2019-11-07 Thread Sven van Haastregt via cfe-commits


Author: Sven van Haastregt
Date: 2019-11-07T14:59:33Z
New Revision: 0e70c350943f1a927f481529717c4f98a465777b

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

LOG: [OpenCL] Add integer builtin functions

This patch adds the integer builtin functions from the OpenCL C
specification.

Patch by Pierre Gondois and Sven van Haastregt.

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

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index f31cb26e2c17..40070cc4407f 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -279,6 +279,11 @@ def Vec1: IntList<"Vec1", [1]>;
 def TLAll   : TypeList<"TLAll", [Char, UChar, Short, UShort, Int, UInt, Long, 
ULong, Float, Double, Half]>;
 def TLFloat : TypeList<"TLFloat", [Float, Double, Half]>;
 
+// All unsigned integer types twice, to facilitate unsigned return types for 
e.g.
+// uchar abs(char) and
+// uchar abs(uchar).
+def TLAllUIntsTwice : TypeList<"TLAllUIntsTwice", [UChar, UChar, UShort, 
UShort, UInt, UInt, ULong, ULong]>;
+
 def TLAllInts : TypeList<"TLAllInts", [Char, UChar, Short, UShort, Int, UInt, 
Long, ULong]>;
 
 // GenType definitions for multiple base types (e.g. all floating point types,
@@ -290,6 +295,8 @@ def AGenTypeNNoScalar  : 
GenericType<"AGenTypeNNoScalar", TLAll, VecNoScalar
 def AIGenType1 : GenericType<"AIGenType1", TLAllInts, Vec1>;
 def AIGenTypeN : GenericType<"AIGenTypeN", TLAllInts, 
VecAndScalar>;
 def AIGenTypeNNoScalar : GenericType<"AIGenTypeNNoScalar", TLAllInts, 
VecNoScalar>;
+// All integer to unsigned
+def AI2UGenTypeN   : GenericType<"AI2UGenTypeN", TLAllUIntsTwice, 
VecAndScalar>;
 // Float
 def FGenTypeN  : GenericType<"FGenTypeN", TLFloat, VecAndScalar>;
 
@@ -466,6 +473,61 @@ foreach name = ["half_divide", "half_powr",
   def : Builtin;
 }
 
+//
+// OpenCL v1.1 s6.11.3, v1.2 s6.12.3, v2.0 s6.13.3 - Integer Functions
+// --- Table 10 ---
+// --- 1 argument ---
+foreach name = ["abs"] in {
+  def : Builtin;
+}
+foreach name = ["clz", "popcount"] in {
+  def : Builtin;
+}
+let MinVersion = CL20 in {
+  foreach name = ["ctz"] in {
+def : Builtin;
+  }
+}
+
+// --- 2 arguments ---
+foreach name = ["abs_
diff "] in {
+  def : Builtin;
+}
+foreach name = ["add_sat", "hadd", "rhadd", "mul_hi", "rotate", "sub_sat"] in {
+  def : Builtin;
+}
+foreach name = ["max", "min"] in {
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["upsample"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["clamp"] in {
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["mad_hi", "mad_sat"] in {
+  def : Builtin;
+}
+
+// --- Table 11 ---
+foreach name = ["mad24"] in {
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["mul24"] in {
+  def : Builtin;
+  def : Builtin;
+}
+
 //
 // OpenCL v1.1 s6.11.4, v1.2 s6.12.4, v2.0 s6.13.4 - Common Functions
 // OpenCL Extension v2.0 s5.1.3 and s6.1.3 - Common Functions
@@ -655,12 +717,6 @@ foreach Type = [Int, UInt] in {
   }
 }
 
-// OpenCL v1.1 s6.11.3, v1.2 s6.12.3, v2.0 s6.13.3 - Integer Functions
-foreach name = ["max", "min"] in {
-  def : Builtin;
-  def : Builtin;
-}
-
 //
 // OpenCL v1.1 s6.11.3, v1.2 s6.12.14, v2.0 s6.13.14: Image Read and Write 
Functions
 // OpenCL Extension v2.0 s5.1.8 and s6.1.8: Image Read and Write Functions

diff  --git a/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl 
b/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
index 84cbb7aeec9b..97a01a1fe931 100644
--- a/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ b/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -20,18 +20,19 @@
 
 // Provide typedefs when invoking clang without -finclude-default-header.
 #ifdef NO_HEADER
+typedef unsigned char uchar;
+typedef unsigned int uint;
+typedef unsigned long ulong;
+typedef unsigned short ushort;
+typedef __SIZE_TYPE__ size_t;
 typedef char char2 __attribute__((ext_vector_type(2)));
 typedef char char4 __attribute__((ext_vector_type(4)));
+typedef uchar uchar4 __attribute__((ext_vector_type(4)));
 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef half half4 __attribute__((ext_vector_type(4)));
 typedef int int2 __attribute__((ext_vector_type(2)));
 typedef int int4 __attribute__((ext_vector_type(4)));
 typedef long long2 __attribute__((ext_vector_typ

[PATCH] D69937: [clangd] Use name of Macro to compute its SymbolID.

2019-11-07 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Sorry for not reading through and assuming the wrong thing from the title. 
Thanks for the explanation! LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69937



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

[PATCH] D69901: [OpenCL] Add integer functions to builtin functions

2019-11-07 Thread Sven van Haastregt via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e70c350943f: [OpenCL] Add integer builtin functions 
(authored by svenvh).

Changed prior to commit:
  https://reviews.llvm.org/D69901?vs=228079&id=228238#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69901

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -20,18 +20,19 @@
 
 // Provide typedefs when invoking clang without -finclude-default-header.
 #ifdef NO_HEADER
+typedef unsigned char uchar;
+typedef unsigned int uint;
+typedef unsigned long ulong;
+typedef unsigned short ushort;
+typedef __SIZE_TYPE__ size_t;
 typedef char char2 __attribute__((ext_vector_type(2)));
 typedef char char4 __attribute__((ext_vector_type(4)));
+typedef uchar uchar4 __attribute__((ext_vector_type(4)));
 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef half half4 __attribute__((ext_vector_type(4)));
 typedef int int2 __attribute__((ext_vector_type(2)));
 typedef int int4 __attribute__((ext_vector_type(4)));
 typedef long long2 __attribute__((ext_vector_type(2)));
-typedef unsigned char uchar;
-typedef unsigned int uint;
-typedef unsigned long ulong;
-typedef unsigned short ushort;
-typedef __SIZE_TYPE__ size_t;
 #endif
 
 kernel void test_pointers(volatile global void *global_p, global const int4 *a) {
@@ -61,6 +62,8 @@
 char4 test_int(char c, char4 c4) {
   char m = max(c, c);
   char4 m4 = max(c4, c4);
+  uchar4 abs1 = abs(c4);
+  uchar4 abs2 = abs(abs1);
   return max(c4, c);
 }
 
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -279,6 +279,11 @@
 def TLAll   : TypeList<"TLAll", [Char, UChar, Short, UShort, Int, UInt, Long, ULong, Float, Double, Half]>;
 def TLFloat : TypeList<"TLFloat", [Float, Double, Half]>;
 
+// All unsigned integer types twice, to facilitate unsigned return types for e.g.
+// uchar abs(char) and
+// uchar abs(uchar).
+def TLAllUIntsTwice : TypeList<"TLAllUIntsTwice", [UChar, UChar, UShort, UShort, UInt, UInt, ULong, ULong]>;
+
 def TLAllInts : TypeList<"TLAllInts", [Char, UChar, Short, UShort, Int, UInt, Long, ULong]>;
 
 // GenType definitions for multiple base types (e.g. all floating point types,
@@ -290,6 +295,8 @@
 def AIGenType1 : GenericType<"AIGenType1", TLAllInts, Vec1>;
 def AIGenTypeN : GenericType<"AIGenTypeN", TLAllInts, VecAndScalar>;
 def AIGenTypeNNoScalar : GenericType<"AIGenTypeNNoScalar", TLAllInts, VecNoScalar>;
+// All integer to unsigned
+def AI2UGenTypeN   : GenericType<"AI2UGenTypeN", TLAllUIntsTwice, VecAndScalar>;
 // Float
 def FGenTypeN  : GenericType<"FGenTypeN", TLFloat, VecAndScalar>;
 
@@ -467,6 +474,61 @@
 }
 
 //
+// OpenCL v1.1 s6.11.3, v1.2 s6.12.3, v2.0 s6.13.3 - Integer Functions
+// --- Table 10 ---
+// --- 1 argument ---
+foreach name = ["abs"] in {
+  def : Builtin;
+}
+foreach name = ["clz", "popcount"] in {
+  def : Builtin;
+}
+let MinVersion = CL20 in {
+  foreach name = ["ctz"] in {
+def : Builtin;
+  }
+}
+
+// --- 2 arguments ---
+foreach name = ["abs_diff"] in {
+  def : Builtin;
+}
+foreach name = ["add_sat", "hadd", "rhadd", "mul_hi", "rotate", "sub_sat"] in {
+  def : Builtin;
+}
+foreach name = ["max", "min"] in {
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["upsample"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["clamp"] in {
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["mad_hi", "mad_sat"] in {
+  def : Builtin;
+}
+
+// --- Table 11 ---
+foreach name = ["mad24"] in {
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["mul24"] in {
+  def : Builtin;
+  def : Builtin;
+}
+
+//
 // OpenCL v1.1 s6.11.4, v1.2 s6.12.4, v2.0 s6.13.4 - Common Functions
 // OpenCL Extension v2.0 s5.1.3 and s6.1.3 - Common Functions
 // --- Table 12 ---
@@ -655,12 +717,6 @@
   }
 }
 
-// OpenCL v1.1 s6.11.3, v1.2 s6.12.3, v2.0 s6.13.3 - Integer Functions
-foreach name = ["max", "min"] in {
-  def : Builtin;
-  def : Builtin;
-}
-
 //
 // OpenCL v1.1 s6.11.3, v1.2 s6.12.14, v2.0 s6.13.14: Image Read and Write Functions
 // OpenCL Extension v2.0 s5.1.8 and s6.1.8: Image Read and Write Functions
___
cfe-commits mailing list
cfe-commi

[PATCH] D69908: [OpenCL] Add geometric and relational builtin functions

2019-11-07 Thread Sven van Haastregt via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3d30f2cff7a4: [OpenCL] Add geometric and relational builtin 
functions (authored by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69908

Files:
  clang/lib/Sema/OpenCLBuiltins.td

Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -274,10 +274,13 @@
 def VecAndScalar: IntList<"VecAndScalar", [1, 2, 3, 4, 8, 16]>;
 def VecNoScalar : IntList<"VecNoScalar", [2, 3, 4, 8, 16]>;
 def Vec1: IntList<"Vec1", [1]>;
+def Vec1234 : IntList<"Vec1234", [1, 2, 3, 4]>;
 
 // Type lists.
 def TLAll   : TypeList<"TLAll", [Char, UChar, Short, UShort, Int, UInt, Long, ULong, Float, Double, Half]>;
 def TLFloat : TypeList<"TLFloat", [Float, Double, Half]>;
+def TLSignedInts   : TypeList<"TLSignedInts", [Char, Short, Int, Long]>;
+def TLUnsignedInts : TypeList<"TLUnsignedInts", [UChar, UShort, UInt, ULong]>;
 
 // All unsigned integer types twice, to facilitate unsigned return types for e.g.
 // uchar abs(char) and
@@ -297,6 +300,10 @@
 def AIGenTypeNNoScalar : GenericType<"AIGenTypeNNoScalar", TLAllInts, VecNoScalar>;
 // All integer to unsigned
 def AI2UGenTypeN   : GenericType<"AI2UGenTypeN", TLAllUIntsTwice, VecAndScalar>;
+// Signed integer
+def SGenTypeN  : GenericType<"SGenTypeN", TLSignedInts, VecAndScalar>;
+// Unsigned integer
+def UGenTypeN  : GenericType<"UGenTypeN", TLUnsignedInts, VecAndScalar>;
 // Float
 def FGenTypeN  : GenericType<"FGenTypeN", TLFloat, VecAndScalar>;
 
@@ -313,6 +320,14 @@
   }
 }
 
+// GenType definitions for vec1234.
+foreach Type = [Float, Double, Half] in {
+  def "GenType" # Type # Vec1234 :
+  GenericType<"GenType" # Type # Vec1234,
+  TypeList<"GL" # Type.Name, [Type]>,
+  Vec1234>;
+}
+
 
 //===--===//
 // Definitions of OpenCL builtin functions
@@ -566,6 +581,91 @@
 }
 
 
+//
+// OpenCL v1.1 s6.11.5, v1.2 s6.12.5, v2.0 s6.13.5 - Geometric Functions
+// OpenCL Extension v2.0 s5.1.4 and s6.1.4 - Geometric Functions
+// --- Table 13 ---
+// --- 1 argument ---
+foreach name = ["length"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["normalize"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["fast_length"] in {
+  def : Builtin;
+}
+foreach name = ["fast_normalize"] in {
+  def : Builtin;
+}
+
+// --- 2 arguments ---
+foreach name = ["cross"] in {
+  foreach VSize = [3, 4] in {
+def : Builtin, VectorType, VectorType], Attr.Const>;
+def : Builtin, VectorType, VectorType], Attr.Const>;
+def : Builtin, VectorType, VectorType], Attr.Const>;
+  }
+}
+foreach name = ["dot", "distance"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["fast_distance"] in {
+  def : Builtin;
+}
+
+
+//
+// OpenCL v1.1 s6.11.6, v1.2 s6.12.6, v2.0 s6.13.6 - Relational Functions
+// OpenCL Extension v2.0 s5.1.5 and s6.1.5 - Relational Functions
+// --- Table 14 ---
+// --- 1 argument ---
+foreach name = ["isfinite", "isinf", "isnan", "isnormal", "signbit"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+foreach name = ["any", "all"] in {
+  def : Builtin;
+}
+
+// --- 2 arguments ---
+foreach name = ["isequal", "isnotequal", "isgreater", "isgreaterequal",
+"isless", "islessequal", "islessgreater", "isordered",
+"isunordered"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+// --- 3 arguments ---
+foreach name = ["bitselect"] in {
+  def : Builtin;
+}
+foreach name = ["select"] in {
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+  def : Builtin;
+}
+
+
 // OpenCL v1.1 s6.11.7, v1.2 s6.12.7, v2.0 s6.13.7 - Vector Data Load and Store Functions
 // OpenCL Extension v1.1 s9.3.6 and s9.6.6, v1.2 s9.5.6, v2.0 s9.4.6, v2.0 s5.1.6 and 6.1.6 - Vector Data Load and Store Functions
 // --- Table 15 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:620
+ SourceLocation Loc) {
+auto &DefaultmapInfo = getTopOfStack().DefaultmapMap[DMVC];
+DefaultmapInfo.ImplicitBehavior = DMIB;

`auto` -> to real type



Comment at: clang/lib/Sema/SemaOpenMP.cpp:626
+  bool hasSetDefaultmapCategory(OpenMPDefaultmapClauseKind VariableCategory) {
+int VC = static_cast(VariableCategory);
+return getTopOfStack().DefaultmapMap[VC].ImplicitBehavior != 
DMIB_unspecified;

No need for explicit typecast, enums are implcitly casted to ints.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2120-2121
+isDefaultmapDefaultBehaviorAtLevel(NewLevel, DMVC_pointer)) ||
+  (DSAStack->
+isDefaultmapDefaultBehaviorAtLevel(NewLevel, DMVC_aggregate)))
 OMPC = OMPC_firstprivate;

Why aggregates are firstprivate by default? I believe only scalars are 
firstprivates by default.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:3198
   }
-  ArrayRef getImplicitMap() const { return ImplicitMap; }
+  llvm::SmallVector
+getImplicitMap(DefaultMapVariableCategory DMVC) const {

Revert back to `ArrayRef`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:4435-4437
+if (DMIB == DMIB_alloc) Kind = OMPC_MAP_alloc;
+else if (DMIB == DMIB_to) Kind = OMPC_MAP_to;
+else if (DMIB == DMIB_from) Kind = OMPC_MAP_from;

Use `switch`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16349
 
+static DefaultMapImplicitBehavior DefaultmapModifierToImplicitBehavior(
+  OpenMPDefaultmapClauseModifier M) {

1. Function names must start from verb.
2. Functions must start from lower letter.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16372
+
+static DefaultMapVariableCategory DefaultmapKindToVariableCategory(
+  OpenMPDefaultmapClauseKind Kind) {

1. Function names must start from verb.
2. Functions must start from lower letter.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16411-16420
+bool isDefaultmapModifier = (M == OMPC_DEFAULTMAP_MODIFIER_alloc) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_to) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_from) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_tofrom) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_firstprivate) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_default) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_none);

Just `M != OMPC_DEFAULTMAP_MODIFIER_unknown` and `Kind != 
OMPC_DEFAULTMAP_unknown`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16426
+Value += "'scalar', 'aggregate', 'pointer'";
+Loc = KindLoc;
+Diag(Loc, diag::err_omp_unexpected_clause_value)

Why need to add a new `Loc` var here? Use the required `KindLoc`\`MLoc` 
directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[clang] de61aa3 - [RISCV] Improve sysroot computation if no GCC install detected

2019-11-07 Thread Edward Jones via cfe-commits


Author: Edward Jones
Date: 2019-11-07T15:17:40Z
New Revision: de61aa3118b9bac85c468ea7ec40604a086744f5

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

LOG: [RISCV] Improve sysroot computation if no GCC install detected

If a GCC installed is not detected, the driver would default to
the root of the filesystem. This is not ideal when this doesn't
match the install directory of the toolchain and can cause
undesireable behavior such as picking up system libraries or
the system linker when cross-compiling.

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

Added: 
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/bin/riscv32-unknown-elf-ld

clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crt0.o

clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o

clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld

clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crt0.o

clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtbegin.o

clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtend.o
clang/test/Driver/riscv32-toolchain-extra.c
clang/test/Driver/riscv64-toolchain-extra.c

Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 22dc5117f196..e14475551f57 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -33,6 +33,8 @@ RISCVToolChain::RISCVToolChain(const Driver &D, const 
llvm::Triple &Triple,
 getFilePaths().push_back(GCCInstallation.getInstallPath().str());
 getProgramPaths().push_back(
 (GCCInstallation.getParentLibPath() + "/../bin").str());
+  } else {
+getProgramPaths().push_back(D.Dir);
   }
 }
 
@@ -74,17 +76,22 @@ std::string RISCVToolChain::computeSysRoot() const {
   if (!getDriver().SysRoot.empty())
 return getDriver().SysRoot;
 
-  if (!GCCInstallation.isValid())
-return std::string();
-
-  StringRef LibDir = GCCInstallation.getParentLibPath();
-  StringRef TripleStr = GCCInstallation.getTriple().str();
-  std::string SysRootDir = LibDir.str() + "/../" + TripleStr.str();
+  SmallString<128> SysRootDir;
+  if (GCCInstallation.isValid()) {
+StringRef LibDir = GCCInstallation.getParentLibPath();
+StringRef TripleStr = GCCInstallation.getTriple().str();
+llvm::sys::path::append(SysRootDir, LibDir, "..", TripleStr);
+  } else {
+// Use the triple as provided to the driver. Unlike the parsed triple
+// this has not been normalized to always contain every field.
+llvm::sys::path::append(SysRootDir, getDriver().Dir, "..",
+getDriver().getTargetTriple());
+  }
 
   if (!llvm::sys::fs::exists(SysRootDir))
 return std::string();
 
-  return SysRootDir;
+  return SysRootDir.str();
 }
 
 void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,

diff  --git 
a/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/bin/riscv32-unknown-elf-ld 
b/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/bin/riscv32-unknown-elf-ld
new file mode 100755
index ..b23e55619b2f
--- /dev/null
+++ 
b/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/bin/riscv32-unknown-elf-ld
@@ -0,0 +1 @@
+#!/bin/true

diff  --git 
a/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crt0.o
 
b/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crt0.o
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o
 
b/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
 
b/clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld 
b/clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld
new file mode 100755
index ..b23e55619b2f
--- /dev/null
+++ 
b/clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld
@@ -0,0 +1 @@
+#!/bin/true

diff  --git 
a/clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crt0.o
 
b/clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crt0.o
new f

[clang] af57dbf - Add support for options -frounding-math, ftrapping-math, -ffp-model=, and -ffp-exception-behavior=

2019-11-07 Thread Melanie Blower via cfe-commits


Author: Melanie Blower
Date: 2019-11-07T07:22:45-08:00
New Revision: af57dbf12e54f3a8ff48534bf1078f4de104c1cd

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

LOG: Add support for options -frounding-math, ftrapping-math, -ffp-model=, and 
-ffp-exception-behavior=

Add options to control floating point behavior: trapping and
exception behavior, rounding, and control of optimizations that affect
floating point calculations. More details in UsersManual.rst.

Reviewers: rjmccall

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

Added: 
clang/test/CodeGen/fpconstrained.c
clang/test/Driver/fp-model.c

Modified: 
clang/docs/UsersManual.rst
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/clang_f_opts.c
clang/test/Driver/fast-math.c
llvm/include/llvm/Target/TargetOptions.h

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 714681d7f4ce..62e2575c6b26 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1231,10 +1231,10 @@ are listed below.
 
 **-f[no-]trapping-math**
 
-   ``-fno-trapping-math`` allows optimizations that assume that
-   floating point operations cannot generate traps such as divide-by-zero,
-   overflow and underflow. Defaults to ``-ftrapping-math``.
-   Currently this option has no effect.
+   Control floating point exception behavior. ``-fno-trapping-math`` allows 
optimizations that assume that floating point operations cannot generate traps 
such as divide-by-zero, overflow and underflow.
+
+- The option ``-ftrapping-math`` behaves identically to 
``-ffp-exception-behavior=strict``.
+- The option ``-fno-trapping-math`` behaves identically to 
``-ffp-exception-behavior=ignore``.   This is the default.
 
 .. option:: -ffp-contract=
 
@@ -1319,6 +1319,52 @@ are listed below.
 
Defaults to ``-fno-finite-math``.
 
+.. _opt_frounding-math:
+
+**-f[no-]rounding-math**
+
+Force floating-point operations to honor the dynamically-set rounding mode by 
default.
+
+The result of a floating-point operation often cannot be exactly represented 
in the result type and therefore must be rounded.  IEEE 754 describes 
diff erent rounding modes that control how to perform this rounding, not all of 
which are supported by all implementations.  C provides interfaces 
(``fesetround`` and ``fesetenv``) for dynamically controlling the rounding 
mode, and while it also recommends certain conventions for changing the 
rounding mode, these conventions are not typically enforced in the ABI.  Since 
the rounding mode changes the numerical result of operations, the compiler must 
understand something about it in order to optimize floating point operations.
+
+Note that floating-point operations performed as part of constant 
initialization are formally performed prior to the start of the program and are 
therefore not subject to the current rounding mode.  This includes the 
initialization of global variables and local ``static`` variables.  
Floating-point operations in these contexts will be rounded using 
``FE_TONEAREST``.
+
+- The option ``-fno-rounding-math`` allows the compiler to assume that the 
rounding mode is set to ``FE_TONEAREST``.  This is the default.
+- The option ``-frounding-math`` forces the compiler to honor the 
dynamically-set rounding mode.  This prevents optimizations which might affect 
results if the rounding mode changes or is 
diff erent from the default; for example, it prevents floating-point operations 
from being reordered across most calls and prevents constant-folding when the 
result is not exactly representable.
+
+.. option:: -ffp-model=
+
+   Specify floating point behavior. ``-ffp-model`` is an umbrella
+   option that encompasses functionality provided by other, single
+   purpose, floating point options.  Valid values are: ``precise``, ``strict``,
+   and ``fast``.
+   Details:
+
+   * ``precise`` Disables optimizations that are not value-safe on 
floating-point data, although FP contraction (FMA) is enabled 
(``-ffp-contract=fast``).  This is the default behavior.
+   * ``strict`` Enables ``-frounding-math`` and 
``-ffp-exception-behavior=strict``, and disables contractions (FMA).  All of 
the ``-ffast-math`` enablements are disabled.
+   * ``fast`` Behaves identically to specifying both ``-ffast-math`` and 
``ffp-contract=fast``
+
+   Note: If your command line specifies multiple instances
+   of the ``-ffp-model`` option, or if your command line option specifies
+   ``-ffp-mode

[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

2019-11-07 Thread Balázs Kéri via Phabricator via cfe-commits

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a project: clang.

Extend StreamChecker with a new evaluation function for API call 'freopen'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69948

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream.c

Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -20,6 +20,7 @@
 extern int feof(FILE *stream);
 extern int ferror(FILE *stream);
 extern int fileno(FILE *stream);
+extern FILE *freopen(const char *pathname, const char *mode, FILE *stream);
 
 void check_fread() {
   FILE *fp = tmpfile();
@@ -111,6 +112,13 @@
   fclose(p); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
 }
 
+void f_double_close_alias(void) {
+  FILE *p1 = fopen("foo", "r");
+  FILE *p2 = p1;
+  fclose(p1);
+  fclose(p2); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
+}
+
 void f_leak(int c) {
   FILE *p = fopen("foo.c", "r");
   if(c)
@@ -134,3 +142,36 @@
 void pr8081(FILE *stream, long offset, int whence) {
   fseek(stream, offset, whence);
 }
+
+void check_freopen_1() {
+  FILE *f1 = freopen("foo.c", "r", 0);
+  if (f1) {
+fclose(f1); // no-warning
+fclose(f1); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
+  } else {
+rewind(f1); // expected-warning {{Stream pointer might be NULL}}
+  }
+}
+
+void check_freopen_2() {
+  FILE *f1 = fopen("foo.c", "r");
+  if (f1) {
+FILE *f2 = freopen(0, "w", f1);
+if (f2) {
+  fclose(f1);
+  fclose(f2); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
+} else {
+  rewind(f1);
+  rewind(f2); // expected-warning {{Stream pointer might be NULL}}
+}
+  }
+}
+
+void check_freopen_3() {
+  FILE *f1 = fopen("foo.c", "r");
+  if (f1) {
+freopen(0, "w", f1);
+rewind(f1);
+fclose(f1);
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -66,6 +66,7 @@
   {{"fopen"}, &StreamChecker::evalFopen},
   {{"tmpfile"}, &StreamChecker::evalFopen},
   {{"fclose", 1}, &StreamChecker::evalFclose},
+  {{"freopen", 3}, &StreamChecker::evalFreopen},
   {{"fread", 4},
std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)},
   {{"fwrite", 4},
@@ -91,6 +92,7 @@
 
   void evalFopen(const CallEvent &Call, CheckerContext &C) const;
   void evalFclose(const CallEvent &Call, CheckerContext &C) const;
+  void evalFreopen(const CallEvent &Call, CheckerContext &C) const;
   void evalFseek(const CallEvent &Call, CheckerContext &C) const;
 
   void checkArgNullStream(const CallEvent &Call, CheckerContext &C,
@@ -166,6 +168,72 @@
 C.addTransition(State);
 }
 
+void StreamChecker::evalFreopen(const CallEvent &Call,
+CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SValBuilder &SValBuilder = C.getSValBuilder();
+  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  ConstraintManager &CM = C.getConstraintManager();
+  auto *CE = dyn_cast_or_null(Call.getOriginExpr());
+  if (!CE)
+return;
+
+  Optional StreamVal = Call.getArgSVal(2).getAs();
+  if (!StreamVal)
+return;
+
+  DefinedSVal RetVal =
+  SValBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+  .castAs();
+  SymbolRef RetSym = RetVal.getAsSymbol();
+  assert(RetSym && "RetVal must be a symbol here.");
+
+  ProgramStateRef StateStreamNotNull, StateStreamNull;
+  std::tie(StateStreamNotNull, StateStreamNull) =
+  CM.assumeDual(State, *StreamVal);
+
+  if (StateStreamNull) {
+StateStreamNull =
+StateStreamNull->BindExpr(CE, C.getLocationContext(), RetVal);
+
+ProgramStateRef StateRetNotNull, StateRetNull;
+std::tie(StateRetNotNull, StateRetNull) =
+CM.assumeDual(StateStreamNull, RetVal);
+if (StateRetNull) {
+  StateRetNull =
+  StateRetNull->set(RetSym, StreamState::getOpenFailed());
+  C.addTransition(StateRetNull);
+}
+if (StateRetNotNull) {
+  StateRetNotNull =
+  StateRetNotNull->set(RetSym, StreamState::getOpened());
+  C.addTransition(StateRetNotNull);
+}
+  }
+
+  if (StateStreamNotNull) {
+SymbolRef StreamSym = StreamVal->getAsSymbol();
+
+ProgramStateRef StateRetNull =
+StateStreamNotNull->BindExpr(CE, C.getLocationContext(), RetVal);
+StateRetNull = CM.assume(StateRetNull, RetVal, false);
+assert(StateRetNull && "This assumption should not fail.");
+StateRetNull =
+StateRetNull->set(

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGde61aa3118b9: [RISCV] Improve sysroot computation if no GCC 
install detected (authored by edward-jones).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68391

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/bin/riscv32-unknown-elf-ld
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
  clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtend.o
  clang/test/Driver/riscv32-toolchain-extra.c
  clang/test/Driver/riscv64-toolchain-extra.c

Index: clang/test/Driver/riscv64-toolchain-extra.c
===
--- /dev/null
+++ clang/test/Driver/riscv64-toolchain-extra.c
@@ -0,0 +1,33 @@
+// A basic clang -cc1 command-line, and simple environment check.
+
+// The tests here are similar to those in riscv64-toolchain.c, however
+// these tests need to create symlinks to test directory trees in order to
+// set up the environment and therefore shell support is required.
+// REQUIRES: shell
+// UNSUPPORTED: system-windows
+
+// If there is no GCC install detected then the driver searches for executables
+// and runtime starting from the directory tree above the driver itself.
+// The test below checks that the driver correctly finds the linker and
+// runtime if and only if they exist.
+//
+// RUN: mkdir -p %T/testroot-riscv64-baremetal-nogcc/bin
+// RUN: [ ! -s %T/testroot-riscv64-baremetal-nogcc/bin/clang ] || rm %T/testroot-riscv64-baremetal-nogcc/bin/clang
+// RUN: [ ! -s %T/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld ] || rm %T/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld
+// RUN: [ ! -s %T/testroot-riscv64-baremetal-nogcc/riscv64-unknown-elf ] || rm %T/testroot-riscv64-baremetal-nogcc/riscv64-unknown-elf
+// RUN: ln -s %clang %T/testroot-riscv64-baremetal-nogcc/bin/clang
+// RUN: ln -s %S/Inputs/basic_riscv64_nogcc_tree/bin/riscv64-unknown-elf-ld %T/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld
+// RUN: ln -s %S/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf %T/testroot-riscv64-baremetal-nogcc/riscv64-unknown-elf
+// RUN: %T/testroot-riscv64-baremetal-nogcc/bin/clang %s -### -no-canonical-prefixes \
+// RUN:-target riscv64-unknown-elf 2>&1 \
+// RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
+
+// C-RV64-BAREMETAL-LP64-NOGCC: InstalledDir: [[DRIVERDIR:.*]]
+// C-RV64-BAREMETAL-LP64-NOGCC: "-fuse-init-array"
+// C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "[[DRIVERDIR]]/../riscv64-unknown-elf/include"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crt0.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtbegin.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "-L[[DRIVERDIR]]/../riscv64-unknown-elf/lib"
+// C-RV64-BAREMETAL-LP64-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtend.o"
Index: clang/test/Driver/riscv32-toolchain-extra.c
===
--- /dev/null
+++ clang/test/Driver/riscv32-toolchain-extra.c
@@ -0,0 +1,33 @@
+// A basic clang -cc1 command-line, and simple environment check.
+
+// The tests here are similar to those in riscv32-toolchain.c, however
+// these tests need to create symlinks to test directory trees in order to
+// set up the environment and therefore shell support is required.
+// REQUIRES: shell
+// UNSUPPORTED: system-windows
+
+// If there is no GCC install detected then the driver searches for executables
+// and runtime starting from the directory tree above the driver itself.
+// The test below checks that the driver correctly finds the linker and
+// runtime if and only if they exist.
+//
+// RUN: mkdir -p %T/testroot-riscv32-baremetal-nogcc/bin
+// RUN: [ ! -s %T/testroot-riscv32-baremetal-nogcc/bin/clang ] || rm %T/testroot-riscv32-baremetal-nogcc/bin/clang
+// RUN: [ ! -s %T/testroot-riscv32-baremetal-nogcc/bin/riscv32-unknown-elf-ld ] || rm %T/testroot-riscv32-baremetal-nogcc/bin/riscv32-unknown-elf-ld
+// RUN: [ ! -s %T/testroot-riscv32-baremetal-nogcc/riscv32-unknown-elf ] || rm %T/testroot-riscv32-baremetal-nogcc/riscv32-unknown-elf
+// RUN: ln -s %clang %T/testroot-riscv32-baremetal-no

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

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



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3489
+ !OMPRegionInfo->hasCancel())) {
+OMPBuilder->CreateBarrier({CGF.Builder.saveIP()}, Kind, ForceSimpleCall,
+  EmitChecks);

ABataev wrote:
> `createBarrier`
I'd say we align ourselves with the IRBuilder (which has `CreateXXX` methods) 
or we are different enough to avoid confusion, e.g., `emit`. I don't care 
much which way but `create` for one builder and `Create` for another 
will be confusing, don't you think?



Comment at: clang/test/OpenMP/barrier_codegen.cpp:43
+// IRBUILDER:  ; Function Attrs: inaccessiblemem_or_argmemonly
+// IRBUILDER-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #2
+

ABataev wrote:
> Remove `#2`, it may be changed 
Agreed, I even added an option to the update_test script to do so but forgot to 
use it (D68851).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

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

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

hokein updated this revision to Diff 228243.
hokein marked 11 inline comments as done.
hokein added a comment.

simplify the testcases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934

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

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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3489
+ !OMPRegionInfo->hasCancel())) {
+OMPBuilder->CreateBarrier({CGF.Builder.saveIP()}, Kind, ForceSimpleCall,
+  EmitChecks);

jdoerfert wrote:
> ABataev wrote:
> > `createBarrier`
> I'd say we align ourselves with the IRBuilder (which has `CreateXXX` methods) 
> or we are different enough to avoid confusion, e.g., `emit`. I don't care 
> much which way but `create` for one builder and `Create` for another 
> will be confusing, don't you think?
Shall follow the LLVM coding document and format the new function name in 
accordance with it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

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

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

hokein added a comment.

In D69934#1736729 , @ilya-biryukov 
wrote:

> It's a bit unclear how we manage to still rename overrides. Is this handled 
> by the USR-generating functions?
>  Could we factor out the part that collects `NamedDecl`s and use those 
> instead of the USRs instead?


Yes, it is handled by the tooling USR-generating function. I would not touch 
that part, I plan to get rid of that API  in a followup.




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

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



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:175
+  tooling::getCanonicalSymbolDeclaration(&RenameDecl), 
AST.getASTContext());
+  llvm::DenseSet TargetIDs;
+  for (auto &USR : RenameUSRs)

ilya-biryukov wrote:
> Why `USRs`? Could we just check whether the `ND.getCanonicalDecl()` is there 
> instead?
checking `ND.getCanonicalDecl()` is not enough, thinking about virtual methods.

`tooling::getUSRsForDeclaration` does all the black-magic things here, it 
returns all rename-related decls.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:97
+};
+struct C : B {
+  void [[f^oo]]() override {}

ilya-biryukov wrote:
> Could we stop at `B`?
> I don't think `C->D` cover any more code paths, they merely add some noise to 
> the test, making it harder to read.
Stopped C.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:131
+public:
+  [[Foo]](int value = 0) : x(value) {}
+

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Could you also check that destructors are renamed properly?
> NIT: maybe remove the bodies of the functions that don't have references to 
> `Foo`?
> They seem to merely add noise, don't think they improve coverage.
Done. Add a new test for constructor/destructor case.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:185
+};
+// FIXME: selection tree.
+Qux::Qux() : [[Foo]]() {}

ilya-biryukov wrote:
> The FIXME is a bit unclear. Could you explain in more detail?
sorry, I forgot this.

This is a tricky case, if the cursor is on cxx initializer list, SelectionTree 
will return the CXXConsturctorExpr node of `Baz`, it actually rename the class 
`Baz` instead of renaming the `Foo` field.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:214
+  R"cpp(
+#define moo [[foo]]
+int [[fo^o]]() { return 42; }

ilya-biryukov wrote:
> I would argue we should fail in that case and not rename.
> If we change the macro body, there's a chance other usages that we didn't 
> want to update will also be updated.
> 
> However, if the current rename does that, also happy to keep as is. Up to you.
I was also surprised with this testcase, but it aligns with what the current 
rename does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69934



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

[clang] 7adab77 - [Sema] Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Edward Jones via cfe-commits


Author: Edward Jones
Date: 2019-11-07T15:45:44Z
New Revision: 7adab7719e55e1b29bfd521dcc73f202139e8f41

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

LOG: [Sema] Suppress -Wchar-subscripts if the index is a literal char

Assume that the user knows what they're doing if they provide a char
literal as an array index. This more closely matches the behavior of
GCC.

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/warn-char-subscripts.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e41cd5b6653a..2842e7199b1d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4741,7 +4741,8 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, 
SourceLocation LLoc,
 
   if ((IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_S) ||
IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_U))
- && !IndexExpr->isTypeDependent())
+ && !IndexExpr->isTypeDependent()
+ && !isa(IndexExpr))
 Diag(LLoc, diag::warn_subscript_is_char) << IndexExpr->getSourceRange();
 
   // C99 6.5.2.1p1: "shall have type "pointer to *object* type". Similarly,

diff  --git a/clang/test/SemaCXX/warn-char-subscripts.cpp 
b/clang/test/SemaCXX/warn-char-subscripts.cpp
index 84ea536b979e..7760ed0e5eba 100644
--- a/clang/test/SemaCXX/warn-char-subscripts.cpp
+++ b/clang/test/SemaCXX/warn-char-subscripts.cpp
@@ -14,6 +14,19 @@ void t2() {
   int val = subscript[array]; // expected-warning{{array subscript is of type 
'char'}}
 }
 
+void t3() {
+  int array[50] = { 0 };
+  int val = array[' ']; // no warning, subscript is a literal
+}
+void t4() {
+  int array[50] = { 0 };
+  int val = '('[array]; // no warning, subscript is a literal
+}
+void t5() {
+  int array[50] = { 0 };
+  int val = array['\x11']; // no warning, subscript is a literal
+}
+
 void test() {
   t1(); // expected-note {{in instantiation of function template 
specialization 't1' requested here}}
   t2(); // expected-note {{in instantiation of function template 
specialization 't2' requested here}}



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

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7adab7719e55: [Sema] Suppress -Wchar-subscripts if the index 
is a literal char (authored by edward-jones).
Herald added a subscriber: simoncook.

Changed prior to commit:
  https://reviews.llvm.org/D58896?vs=189127&id=228249#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-char-subscripts.cpp


Index: clang/test/SemaCXX/warn-char-subscripts.cpp
===
--- clang/test/SemaCXX/warn-char-subscripts.cpp
+++ clang/test/SemaCXX/warn-char-subscripts.cpp
@@ -14,6 +14,19 @@
   int val = subscript[array]; // expected-warning{{array subscript is of type 
'char'}}
 }
 
+void t3() {
+  int array[50] = { 0 };
+  int val = array[' ']; // no warning, subscript is a literal
+}
+void t4() {
+  int array[50] = { 0 };
+  int val = '('[array]; // no warning, subscript is a literal
+}
+void t5() {
+  int array[50] = { 0 };
+  int val = array['\x11']; // no warning, subscript is a literal
+}
+
 void test() {
   t1(); // expected-note {{in instantiation of function template 
specialization 't1' requested here}}
   t2(); // expected-note {{in instantiation of function template 
specialization 't2' requested here}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4741,7 +4741,8 @@
 
   if ((IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_S) ||
IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_U))
- && !IndexExpr->isTypeDependent())
+ && !IndexExpr->isTypeDependent()
+ && !isa(IndexExpr))
 Diag(LLoc, diag::warn_subscript_is_char) << IndexExpr->getSourceRange();
 
   // C99 6.5.2.1p1: "shall have type "pointer to *object* type". Similarly,


Index: clang/test/SemaCXX/warn-char-subscripts.cpp
===
--- clang/test/SemaCXX/warn-char-subscripts.cpp
+++ clang/test/SemaCXX/warn-char-subscripts.cpp
@@ -14,6 +14,19 @@
   int val = subscript[array]; // expected-warning{{array subscript is of type 'char'}}
 }
 
+void t3() {
+  int array[50] = { 0 };
+  int val = array[' ']; // no warning, subscript is a literal
+}
+void t4() {
+  int array[50] = { 0 };
+  int val = '('[array]; // no warning, subscript is a literal
+}
+void t5() {
+  int array[50] = { 0 };
+  int val = array['\x11']; // no warning, subscript is a literal
+}
+
 void test() {
   t1(); // expected-note {{in instantiation of function template specialization 't1' requested here}}
   t2(); // expected-note {{in instantiation of function template specialization 't2' requested here}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4741,7 +4741,8 @@
 
   if ((IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_S) ||
IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_U))
- && !IndexExpr->isTypeDependent())
+ && !IndexExpr->isTypeDependent()
+ && !isa(IndexExpr))
 Diag(LLoc, diag::warn_subscript_is_char) << IndexExpr->getSourceRange();
 
   // C99 6.5.2.1p1: "shall have type "pointer to *object* type". Similarly,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Stephan Bergmann via Phabricator via cfe-commits

sberg added a comment.

But how about literals like `'\x80'` where the promoted value depends on 
whether plain `char` is signed or unsigned?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896



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

[PATCH] D69923: [OPENMP][DOCS] Update OpenMP status (NFC)

2019-11-07 Thread Deepak Eachempati via Phabricator via cfe-commits

dreachem added a comment.

I don't believe I have commit permissions on this. @ABataev, can you commit 
this? Thanks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69923



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

2019-11-07 Thread Caroline Tice via Phabricator via cfe-commits

cmtice updated this revision to Diff 228250.
cmtice added a comment.

Fix small typo in comments.


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

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/debug-default-version.c

Index: clang/test/Driver/debug-default-version.c
===
--- /dev/null
+++ clang/test/Driver/debug-default-version.c
@@ -0,0 +1,45 @@
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -S -o - %s 2>&1 | FileCheck %s --check-prefixes=NODEBUGINFO,NODWARF4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, or you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -### -Werror -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -o - %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=DWARF4,CODEVIEW
+
+
+// Do Assembler testing most of the same test cases as those above.
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF2
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF3
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF4
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -gdwarf-5 -x assembler -c -fdebug-default-version=2 -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+// RUN: %clang -### -Werror -target x86_64-linux-gnu -fdebug-default-version=5 -g -x assembler -c -o - %s 2>&1 | FileCheck %s --check-prefix=DWARF5
+
+int main (void) {
+  return 0;
+}
+
+// NOCODEVIEW-NOT: -gcodeview
+// CODEVIEW: "-gcodeview"
+
+// NODEBUGINFO-NOT: -debug-info-kind=
+
+// DWARF2: "-dwarf-version=2"
+// DWARF3: "-dwarf-version=3"
+// DWARF4: "-dwarf-version=4"
+// DWARF5: "-dwarf-version=5"
+
+// NOCODEVIEW-NOT: -gcodeview
+// NODWARF4-NOT: -dwarf-version=4
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain &TC,
 const llvm::opt::ArgList &Args);
 
+unsigned ParseDebugDefaultVersion(const ToolChain &TC,
+  const llvm::opt::ArgList &Args);
+
 void AddAssemblerKPIC(const ToolChain &ToolChain,
   const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain &TC,
+ const ArgList &Args) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 2)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)
+<< A->getAsString(Args) << A->getVa

[clang] bcf754a - [OPENMP][DOCS] Update OpenMP status (NFC)

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


Author: Alexey Bataev
Date: 2019-11-07T11:07:56-05:00
New Revision: bcf754a3212c12ecc896bac66c599d571eca57d9

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

LOG: [OPENMP][DOCS] Update OpenMP status (NFC)

Summary: This is updating the OpenMP status table. Cray has volunteered for 
`defaultmap` and supporting `in_reduction` on the `target` construct, so the 
status on those entries from was changed from "unclaimed". Also, a new entry 
was added for supporting non-contiguous arrays sections on the `target update` 
directive.

Reviewers: ABataev, hfinkel, jdoerfert, kkwli0

Reviewed By: ABataev

Subscribers: guansong, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index 0ef4acf7631a..aafadcfa0058 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -173,13 +173,13 @@ implementation.
 
+--+--+--+---+
 | device extension | OMP_TARGET_OFFLOAD environment variable   
   | :good:`done` | D50522  
  |
 
+--+--+--+---+
-| device extension | support full 'defaultmap' functionality   
   | :part:`worked on`| 
  |
+| device extension | support full 'defaultmap' functionality   
   | :part:`worked on`| D69204  
  |
 
+--+--+--+---+
 | device extension | device specific functions 
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
 | device extension | clause: device_type   
   | :good:`done` | 
  |
 
+--+--+--+---+
-| device extension | clause: in_reduction  
   | :none:`unclaimed`| r308768 
  |
+| device extension | clause: in_reduction  
   | :part:`worked on`| r308768 
  |
 
+--+--+--+---+
 | device extension | omp_get_device_num()  
   | :part:`worked on`| D54342  
  |
 
+--+--+--+---+
@@ -211,6 +211,8 @@ implementation.
 
+--+--+--+---+
 | device extension | teams construct on the host device
   | :part:`worked on`| Clang part is done, r371553.
  |
 
+--+--+--+---+
+| device extension | support non-contiguous array sections for 
target update  | :part:`worked on`|

[PATCH] D69923: [OPENMP][DOCS] Update OpenMP status (NFC)

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbcf754a3212c: [OPENMP][DOCS] Update OpenMP status (NFC) 
(authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69923

Files:
  clang/docs/OpenMPSupport.rst


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -173,13 +173,13 @@
 
+--+--+--+---+
 | device extension | OMP_TARGET_OFFLOAD environment variable   
   | :good:`done` | D50522  
  |
 
+--+--+--+---+
-| device extension | support full 'defaultmap' functionality   
   | :part:`worked on`| 
  |
+| device extension | support full 'defaultmap' functionality   
   | :part:`worked on`| D69204  
  |
 
+--+--+--+---+
 | device extension | device specific functions 
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
 | device extension | clause: device_type   
   | :good:`done` | 
  |
 
+--+--+--+---+
-| device extension | clause: in_reduction  
   | :none:`unclaimed`| r308768 
  |
+| device extension | clause: in_reduction  
   | :part:`worked on`| r308768 
  |
 
+--+--+--+---+
 | device extension | omp_get_device_num()  
   | :part:`worked on`| D54342  
  |
 
+--+--+--+---+
@@ -211,6 +211,8 @@
 
+--+--+--+---+
 | device extension | teams construct on the host device
   | :part:`worked on`| Clang part is done, r371553.
  |
 
+--+--+--+---+
+| device extension | support non-contiguous array sections for 
target update  | :part:`worked on`| 
  |
++--+--+--+---+
 | atomic extension | hints for the atomic construct
   | :part:`worked on`| D51233  
  |
 
+--+--+--+---+
 | base language| C11 support

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

2019-11-07 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews created this revision.
eandrews added reviewers: rnk, gribozavr, gribozavr2, Szelethus, erichkeane, 
riccibruno.

This patch reapplies D61027 . When D61027 
 was previously committed (76945821b9cad3), 
buildbots failed due to clang-tidy test fails. The test fails are because some 
errors in templates are now diagnosed earlier (does not wait till 
instantiation).  I have modified the tests to add checks for these 
diagnostics/prevent these diagnostics.

Since I have not worked on clang-tidy in the past, I am hoping someone with 
more familiarity in this area can take a look and review my changes. There are 
no code changes in this second attempt (compared to D61027 
). I have only modified failing tests.

Summary of code changes (pasted from D61027 ) 
is below -

Clang currently crashes for switch statements inside a template when the 
condition is a non-integer field member because contextual implicit conversion 
is skipped when parsing the condition. This conversion is however later checked 
in an assert when the case statement is handled. The conversion is skipped when 
parsing the condition because the field member is set as type-dependent based 
on its containing class. This patch sets the type dependency based on the 
field's type instead. This patch fixes Bug 40982.


https://reviews.llvm.org/D69950

Files:
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaTemplate/dependent-names.cpp
  clang/test/SemaTemplate/enum-argument.cpp
  clang/test/SemaTemplate/member-access-expr.cpp
  clang/test/SemaTemplate/non-integral-switch-cond.cpp

Index: clang/test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
Index: clang/test/SemaTemplate/member-access-expr.cpp
===
--- clang/test/SemaTemplate/member-access-expr.cpp
+++ clang/test/SemaTemplate/member-access-expr.cpp
@@ -156,7 +156,7 @@
 void get(B **ptr) {
   // It's okay if at some point we figure out how to diagnose this
   // at instantiation time.
-  *ptr = field;
+  *ptr = field; // expected-error {{assigning to 'test6::B *' from incompatible type 'test6::A *}}
 }
   };
 }
Index: clang/test/SemaTemplate/enum-argument.cpp
===
--- clang/test/SemaTemplate/enum-argument.cpp
+++ clang/test/SemaTemplate/enum-argument.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 enum Enum { val = 1 };
 template  struct C {
@@ -31,7 +30,7 @@
 unsigned long long bitfield : e0;
 
 void f(int j) {
-  bitfield + j;
+  bitfield + j; // expected-warning {{expression result unused}}
 }
   };
 }
Index: clang/test/SemaTemplate/dependent-names.cpp
===
--- clang/test/SemaTemplate/dependent-names.cpp
+++ clang/test/SemaTemplate/dependent-names.cpp
@@ -273,9 +273,6 @@
   }
   int e[10];
 };
-void g() {
-  S().f(); // expected-note {{here}}
-}
   }
 
   namespace A2 {
Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -18,6 +18,7 @@
 [[nodiscard]] void *operator new(std::size_t, std::align_val_t, const std::nothrow_t&) noexcept;
 [[nodiscard]] void *operator new[](std::size_t, const std::nothrow_t&) noexcept;
 [[nodiscard]] void *operator new[](std::size_t, std::align_val_t, const std::nothrow_t&) noexcept;
+[[nodiscard]] void *operator new[](std::size_t, std::align_val_t);
 void operator delete(void*, const std::nothrow_t&) noexcept;
 void operator delete(void*, std::align_val_t, const std::nothrow_t&) noexcept;
 void operator delete[](void*, const std::nothrow_t&) noexcept;
@@ -1050,7 +1051,7 @@
 // Ensure that we don't try to evaluate these for overflow and crash. These
 // are all value-dependent expressions.
 p = new char[n];
-p = new (n) char[n];
+p = new ((std::align_val_t)n) char[n];
 p = new char(n);
   }
 }
Index: clang/lib/Sema/SemaChecking.cpp
==

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

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



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3489
+ !OMPRegionInfo->hasCancel())) {
+OMPBuilder->CreateBarrier({CGF.Builder.saveIP()}, Kind, ForceSimpleCall,
+  EmitChecks);

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > `createBarrier`
> > I'd say we align ourselves with the IRBuilder (which has `CreateXXX` 
> > methods) or we are different enough to avoid confusion, e.g., `emit`. I 
> > don't care much which way but `create` for one builder and `Create` 
> > for another will be confusing, don't you think?
> Shall follow the LLVM coding document and format the new function name in 
> accordance with it?
Changing the IRBuilder is something no one dared to do for years. I'm actually 
in favor but I would like to get this in rather sooner than later.

Two options I think are good:
  - Use `CreateXXX` now and, if someone ever comes around to rename the Builder 
methods we rename them in all Builders.
  - Use `emit` now to avoid confusion.

When someone in a review before asked for create instead of emit the reason was 
to be aligned with the IRBuilder.  I think that is a good justification so I 
went with `CreateXXX` even if it is against the coding standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

2019-11-07 Thread Balogh, Ádám via Phabricator via cfe-commits

baloghadamsoftware added a comment.

Thank you for working on this. Please add comments to the code and maybe also 
to the tests. I could not find anything in the standards about `freopen()` with 
null-pointer for the stream parameter.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:176
+  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  ConstraintManager &CM = C.getConstraintManager();
+  auto *CE = dyn_cast_or_null(Call.getOriginExpr());

The four lines above are typical cases for using auto, since the type is 
duplicated in the line. See: 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:220
+StateRetNull = CM.assume(StateRetNull, RetVal, false);
+assert(StateRetNull && "This assumption should not fail.");
+StateRetNull =

Please use a more elaborated error message here.



Comment at: clang/test/Analysis/stream.c:120
+  fclose(p2); // expected-warning {{Try to close a file Descriptor already 
closed. Cause undefined behaviour}}
+}
+

I wonder if this test belongs into this particular patch. It has nothing to do 
with `freopen()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948



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

[PATCH] D69951: [clang-format] NFC allow Format.h to be clang-formatted but still maintain the same doc layout in ClangFormatStyleOptions.rst

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

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: mitchell-stellar, klimek, sammccall, owenpan.
MyDeveloperDay added projects: clang, clang-format.

Format.h is used to generate ClangFormatStyleOptions.rst, the layout of the 
comments is critical to the rst file. Accidentally clang-formatting Format.h 
can lead to the .rst changing.

This revision simply add // clang-format off/on statement around the areas who 
formatting needs to be maintained, mainly around the options that are related 
to what happens when the line breaks due to `ColumnLimit` (which is what is 
happening to the comment)

This allows Format.h to be clang-formatted without causing a change in the 
documentation when dump_format_style.py is rerun, which is also part of the 
revision.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69951

Files:
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1161,6 +1161,7 @@
   /// \endcode
   bool CompactNamespaces;
 
+  // clang-format off
   /// If the constructor initializers don't fit on a line, put each
   /// initializer on its own line.
   /// \code
@@ -1178,6 +1179,7 @@
   ///   }
   /// \endcode
   bool ConstructorInitializerAllOnOneLineOrOnePerLine;
+  // clang-format on
 
   /// The number of characters to use for indentation of constructor
   /// initializer lists as well as inheritance lists.
@@ -1304,6 +1306,7 @@
 
   tooling::IncludeStyle IncludeStyle;
 
+  // clang-format off
   /// Indent case labels one level from the switch statement.
   ///
   /// When ``false``, use the same indentation level as for the switch 
statement.
@@ -1319,6 +1322,7 @@
   ///}  }
   /// \endcode
   bool IndentCaseLabels;
+  // clang-format on
 
   /// Indent goto labels.
   ///
@@ -1453,6 +1457,7 @@
   /// The JavaScriptQuoteStyle to use for JavaScript strings.
   JavaScriptQuoteStyle JavaScriptQuotes;
 
+  // clang-format off
   /// Whether to wrap JavaScript import/export statements.
   /// \code{.js}
   ///true:
@@ -1466,6 +1471,7 @@
   ///import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying, 
VeryLongImportsAreAnnoying,} from "some/module.js"
   /// \endcode
   bool JavaScriptWrapImports;
+  // clang-format on
 
   /// If true, the empty line at the start of blocks is kept.
   /// \code
@@ -1747,6 +1753,7 @@
   /// \endcode
   std::vector RawStringFormats;
 
+  // clang-format off
   /// If ``true``, clang-format will attempt to re-flow comments.
   /// \code
   ///false:
@@ -1760,6 +1767,7 @@
   /// * information */
   /// \endcode
   bool ReflowComments;
+  // clang-format on
 
   /// If ``true``, clang-format will sort ``#includes``.
   /// \code
@@ -2294,8 +2302,7 @@
 /// a non-recoverable syntax error.
 tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
ArrayRef Ranges,
-   StringRef FileName,
-   bool *IncompleteFormat);
+   StringRef FileName, bool *IncompleteFormat);
 
 /// Clean up any erroneous/redundant code in the given \p Ranges in \p
 /// Code.
@@ -2406,6 +2413,6 @@
 namespace std {
 template <>
 struct is_error_code_enum : std::true_type {};
-}
+} // namespace std
 
 #endif // LLVM_CLANG_FORMAT_FORMAT_H


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1161,6 +1161,7 @@
   /// \endcode
   bool CompactNamespaces;
 
+  // clang-format off
   /// If the constructor initializers don't fit on a line, put each
   /// initializer on its own line.
   /// \code
@@ -1178,6 +1179,7 @@
   ///   }
   /// \endcode
   bool ConstructorInitializerAllOnOneLineOrOnePerLine;
+  // clang-format on
 
   /// The number of characters to use for indentation of constructor
   /// initializer lists as well as inheritance lists.
@@ -1304,6 +1306,7 @@
 
   tooling::IncludeStyle IncludeStyle;
 
+  // clang-format off
   /// Indent case labels one level from the switch statement.
   ///
   /// When ``false``, use the same indentation level as for the switch statement.
@@ -1319,6 +1322,7 @@
   ///}  }
   /// \endcode
   bool IndentCaseLabels;
+  // clang-format on
 
   /// Indent goto labels.
   ///
@@ -1453,6 +1457,7 @@
   /// The JavaScriptQuoteStyle to use for JavaScript strings.
   JavaScriptQuoteStyle JavaScriptQuotes;
 
+  // clang-format off
   /// Whether to wrap JavaScript import/export statements.
   /// \code{.js}
   ///true:
@@ -1466,6 +1471,7 @@
   ///import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,} from "some/module.js"
   /// \endcode
   bool JavaScr

[PATCH] D68407: [WIP][RISCV] Use compiler-rt if no GCC installation detected

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits

edward-jones added a comment.

In D68407#1732685 , @lenary wrote:

> This patch is looking much better, thanks for updating it.
>
> Please may you clarify what RISC-V gcc does for `-lgcc`, `-lgcc_s`, 
> `-lgcc_eh`? Is it different to what gcc does on other targets? Being closer 
> to matching the linker arguments that gcc provides to ld seems like a good 
> idea, IMO.

The default behaviour on RISC-V GCC is just `-lgcc`, however for Clang if I 
call `tools::AddRunTimeLibs` then by default I get `-lgcc --as-needed -lgcc_s 
--no-as-needed` in the link step.

When libgcc is needed `tools::AddRunTimeLibs` unconditionally calls 
`AddUnwindLibrary`, which in turn calls `ToolChain::GetUnwindLibType` which 
always returns `ToolChain::UNW_Libgcc` for libgcc. The code in 
`AddUnwindLibrary` will then always add either `-lgcc_eh` (for a static 
libgcc), or `-lgcc_s` (for a shared libgcc) to the command line. I can override 
this by reimplementing `ToolChain::GetUnwindLibType` in the target and making 
it return `ToolChain::UNW_None`, but I'm not sure whether that's the desired 
behavior.

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

https://reviews.llvm.org/D68407

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

[PATCH] D69938: [OpenCL] Use __generic addr space when generating internal representation of lambda

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

rjmccall added a comment.

It does make logical sense to be able to qualify the call operator of a lambda. 
 The lambda always starts as a temporary, so presumably we want the default 
address space qualifier on lambdas to be compatible with the address space of 
temporaries.  However, that's probably also true of the default address 
qualifier on methods in general or you wouldn't be able to call them on 
temporaries and locals.  Thus, we should be able to use the same default in 
both cases, which seems to be what you've implemented.

It even makes sense to be able to qualify a lambda with an address space that's 
not compatible with the address space of temporaries; that just means that the 
lambda has to be copied elsewhere before it can be invoked.

The right place to write the qualifier is the same place where you have to 
write attributes and/or `mutable`, i.e. in the lambda declarator, something 
like `[]() __local { ... }`.

Arguably there ought to be a special-case rule allowing lambdas with no 
captures to be called in an arbitrary address space, but I don't think we need 
to go there in this patch.




Comment at: clang/test/SemaOpenCLCXX/address-space-lambda.cl:13
+  __constant auto err = [&]() {}; //expected-note-re{{candidate function not 
viable: address space mismatch in 'this' argument ('__constant (lambda at 
{{.*}})'), parameter type must be 'const __generic (lambda at {{.*}})'}}
+  err(); //expected-error-re{{no matching function for call to object of type 
'__constant (lambda at {{.*}})'}}
+}

Can we get a better diagnostic here when a candidate would have succeeded 
except for an address-space mismatch?  Something like "candidate function not 
viable: cannot convert 'this' from '__constant' address space to '__generic'"?  
You can do that in a separate patch, of course.


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

https://reviews.llvm.org/D69938



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

[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

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

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:4091
+V = Builder.CreatePointerBitCastOrAddrSpaceCast(
+V, IRFuncTy->getParamType(FirstIRArg));
+  else

Anastasia wrote:
> svenvh wrote:
> > @Anastasia pointed out that the AST might not be correct to begin with, 
> > which could cause us to fail here.
> > 
> > ```
> > | `-MemberExpr 0x558af2c0  ' > type>' .getRef 0x558a4008
> > |   `-ImplicitCastExpr 0x558af310  '__generic B2' lvalue 
> > 
> > | `-DeclRefExpr 0x558af2a0  'Derived' lvalue Var 
> > 0x558ab290 'd' 'Derived'
> > ```
> > 
> > The question is whether it's fine for the `UncheckedDerivedToBase 
> > ImplicitCastExpr` to cast from `private` to `__generic`; or do we need 
> > another `AddrSpaceConversion ImplicitCastExpr` node perhaps?
> Yes, I am not sure AST is correct wrt addr spaces.
> 
> Also we should at least use `performAddrSpaceCast` that isn't easy here 
> because the source addr spaces are not available at this point.
> 
> But overall I would like @rjmccall to have a look.
I agree that the AST should include an address-space conversion, not just a 
derived-to-base conversion.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69810



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

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

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

xbolva00 added a comment.

Well, i am not sure if one twitter report is good motivation to criple warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896



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

[PATCH] D53482: Add clang-format stability check with FormatTests

2019-11-07 Thread Wouter van Oortmerssen via Phabricator via cfe-commits

aardappel added a comment.

@MyDeveloperDay thanks for your thoughts.. while `-stable` would be helpful 
once you know you have the issue (or to permanently turn on in a git 
pre-submit), it does not help developers that don't even know this is a thing, 
and waste a bunch of time figuring out what is going on. In fact, it took me 
longer to figure out simply because I assumed it is unlikely that 
`clang-format` has such a bug :)

It might almost make sense in reverse: `-stable` by default, and have a 
`-un-stable` for those users where `clang-format` becomes a performance 
bottleneck :) But once you admit that, might as well fix the core problem 
instead.


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

https://reviews.llvm.org/D53482



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

[PATCH] D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types

2019-11-07 Thread Sam Elliott via Phabricator via cfe-commits

lenary added a comment.

In D69590#1728628 , @asb wrote:

> Thanks James - won't this still leave problems for structs that need 
> flattening?

The tests in clang/test/CodeGen/riscv32-ilp32d-abi.c already include lots of 
cases that include flattening.

I suppose the one place they lack tests is probably a complex type passed 
within a struct, which would be good to add @jrtc27. Though, we should probably 
greatly expand the testing of `_Complex` within ABIs.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69590

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

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

2019-11-07 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:342
+  // A snapshot of all file dirty buffers.
+  llvm::StringMap SnapShot = WorkScheduler.getAllFileContents();
   auto Action = [File = File.str(), NewName = NewName.str(), Pos, WantFormat,

NIT: s/SnapShot/Snapshot



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:371
+}
+CB(std::move(*Edits));
   };

NIT: `return CB(std::move(*Edits));`
to keep all calls to `CB` consistent.



Comment at: clang-tools-extra/clangd/TUScheduler.h:186
+
+  // std::vector
   /// Schedule an async task with no dependencies.

NIT: seems like a leftover from the previous version. Maybe remove?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:48
+  auto ID = getSymbolID(&D);
+  assert(ID);
+  Req.IDs.insert(*ID);

NIT: this assert is redundant, `Optional` asserts it has a value when 
`operator*` is called.
The code could be simplified to the equivalent: `Refs.insert(*getSymbolID(&D))`



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:135
+  // Note: within-file rename does support this through the AST.
+  if (const auto *S = llvm::dyn_cast(&RenameDecl))
+if (S->isVirtual())

nit: add braces to the outer if



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:244
+return EndOffset.takeError();
+  return std::make_pair<>(*StartOffset, *EndOffset);
+};

NIT: remove `<>`. should still work, right?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:297
+  FileEdits Results;
+  std::string OldName = RenameDecl->getNameAsString();
+  for (const auto &FileAndOccurrences : AffectedFiles) {

We don't seem to use it. Remove? Or am I missing the usage?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:345
+  SourceLocation SourceLocationBeg =
+  SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+  RInputs.Pos, SM, AST.getASTContext().getLangOpts()));

Why is this different from `prepareRename`, which does not call 
`getMacroArgExpandedLocation`?




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:379
+return FileEdits(
+{std::make_pair<>(RInputs.MainFilePath,
+  Edit{MainFileCode, 
std::move(*MainFileRenameEdit)})});

NIT: remove `<>`



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:80
 llvm::Optional ShowMessage;
 /// A mapping from file path(the one used for accessing the underlying VFS)
 /// to edits.

The comment is now redundant, since the typedef has the same comment.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:27
+
+std::unique_ptr buildRefSlab(const Annotations &Code,
+  llvm::StringRef SymbolName,

Could you document what this function is doing?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:52
+std::pair>
+applyRename(FileEdits FE) {
+  std::vector> Results;

NIT: I suggest to call it `applyEdits`, there's nothing rename-specific in this 
function.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:115
 ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
-auto ApplyResult = llvm::cantFail(
-tooling::applyAllReplacements(Code.code(), *RenameResult));
-EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
+EXPECT_THAT(applyRename(std::move(*RenameResult)),
+UnorderedElementsAre(

NIT: maybe simplify to `EXPECT_EQ(applyRename(...).second, expectedResult(Code, 
NewName))`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

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

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

jdoerfert updated this revision to Diff 228259.
jdoerfert added a comment.

Improve type generation & handling, provide examples for function types and
more attributes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785

Files:
  llvm/include/llvm/IR/OpenMPConstants.h
  llvm/include/llvm/IR/OpenMPIRBuilder.h
  llvm/include/llvm/IR/OpenMPKinds.def
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/OpenMPConstants.cpp
  llvm/lib/IR/OpenMPIRBuilder.cpp
  llvm/unittests/IR/CMakeLists.txt
  llvm/unittests/IR/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/IR/OpenMPIRBuilderTest.cpp
===
--- /dev/null
+++ llvm/unittests/IR/OpenMPIRBuilderTest.cpp
@@ -0,0 +1,77 @@
+//===- llvm/unittest/IR/OpenMPIRBuilderTest.cpp - OpenMPIRBuilder tests ---===//
+//
+// 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 "llvm/IR/OpenMPIRBuilder.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Verifier.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace omp;
+
+namespace {
+
+class OpenMPIRBuilderTest : public testing::Test {
+protected:
+  void SetUp() override {
+M.reset(new Module("MyModule", Ctx));
+FunctionType *FTy = FunctionType::get(Type::getVoidTy(Ctx),
+  /*isVarArg=*/false);
+F = Function::Create(FTy, Function::ExternalLinkage, "", M.get());
+BB = BasicBlock::Create(Ctx, "", F);
+  }
+
+  void TearDown() override {
+BB = nullptr;
+M.reset();
+  }
+
+  LLVMContext Ctx;
+  std::unique_ptr M;
+  Function *F;
+  BasicBlock *BB;
+};
+
+TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+
+  IRBuilder<> Builder(BB);
+
+  OMPBuilder.CreateBarrier({}, OMPD_for);
+  EXPECT_TRUE(M->global_empty());
+  EXPECT_EQ(M->size(), 1U);
+  EXPECT_EQ(F->size(), 1U);
+  EXPECT_EQ(BB->size(), 0U);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP()});
+  OMPBuilder.CreateBarrier(Loc, OMPD_for);
+  EXPECT_FALSE(M->global_empty());
+  EXPECT_EQ(M->size(), 3U);
+  EXPECT_EQ(F->size(), 1U);
+  EXPECT_EQ(BB->size(), 2U);
+
+  CallInst *GTID = dyn_cast(&BB->front());
+  EXPECT_NE(GTID, nullptr);
+  EXPECT_EQ(GTID->getNumArgOperands(), 1U);
+  EXPECT_EQ(GTID->getCalledFunction()->getName(), "__kmpc_global_thread_num");
+
+  CallInst *Barrier = dyn_cast(GTID->getNextNode());
+  EXPECT_NE(Barrier, nullptr);
+  EXPECT_EQ(Barrier->getNumArgOperands(), 2U);
+  EXPECT_EQ(Barrier->getCalledFunction()->getName(), "__kmpc_barrier");
+
+  EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
+
+  Builder.CreateUnreachable();
+  EXPECT_FALSE(verifyModule(*M));
+}
+} // namespace
Index: llvm/unittests/IR/CMakeLists.txt
===
--- llvm/unittests/IR/CMakeLists.txt
+++ llvm/unittests/IR/CMakeLists.txt
@@ -28,6 +28,7 @@
   ManglerTest.cpp
   MetadataTest.cpp
   ModuleTest.cpp
+  OpenMPIRBuilderTest.cpp
   PassManagerTest.cpp
   PatternMatch.cpp
   TimePassesTest.cpp
Index: llvm/lib/IR/OpenMPIRBuilder.cpp
===
--- /dev/null
+++ llvm/lib/IR/OpenMPIRBuilder.cpp
@@ -0,0 +1,222 @@
+//===- OpenMPIRBuilder.cpp - Builder for LLVM-IR for OpenMP directives ===//
+//
+// 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
+//
+//===--===//
+/// \file
+///
+/// This file implements the OpenMPIRBuilder class, which is used as a
+/// convenient way to create LLVM instructions for OpenMP directives.
+///
+//===--===//
+
+#include "llvm/IR/OpenMPIRBuilder.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/IR/IRBuilder.h"
+
+#define DEBUG_TYPE "openmp-ir-builder"
+
+using namespace llvm;
+using namespace omp;
+using namespace types;
+
+Function *OpenMPIRBuilder::getOrCreateRuntimeFunction(RuntimeFunction FnID,
+  bool Annotate) {
+  Function *Fn = nullptr;
+
+  // Try to find the declation in the module first.
+  switch (FnID) {
+#define OMP_RTL(Enum, Str, IsVarArg, ReturnType, ...)  \
+  case Enum:   \
+Fn = M.getFunction(Str);

[PATCH] D53482: Add clang-format stability check with FormatTests

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

MyDeveloperDay added a comment.

In D53482#1737203 , @aardappel wrote:

> @MyDeveloperDay thanks for your thoughts.. while `-stable` would be helpful 
> once you know you have the issue (or to permanently turn on in a git 
> pre-submit), it does not help developers that don't even know this is a 
> thing, and waste a bunch of time figuring out what is going on. In fact, it 
> took me longer to figure out simply because I assumed it is unlikely that 
> `clang-format` has such a bug :)
>
> It might almost make sense in reverse: `-stable` by default, and have a 
> `-un-stable` for those users where `clang-format` becomes a performance 
> bottleneck :) But once you admit that, might as well fix the core problem 
> instead.

Sorry I didn't make myself clear,I wasn't thinking of a final customer facing 
solution,  I was only really thinking of adding the -stable purely so we could 
pass a flag down and try different levels of repeating so we could do some 
performance tests. I feel it might be related to these 3 below, but was trying 
to think about how we could easily experiment.. I think ultimately we'd want a 
stable solution.

  alignConsecutiveDeclarations();
  alignConsecutiveAssignments();
  alignTrailingComments();

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

https://reviews.llvm.org/D53482

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

[PATCH] D69853: [OpenMP][NFCI] Introduce llvm/IR/OpenMPConstants.h

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

jdoerfert added a comment.

Are there  blocking issues on this one or can we go ahead and adjust minor 
details as we go?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853



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

[PATCH] D69952: [OPENMP50]Generalize handling of context matching/scoring.

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

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

Untie context matching/scoring from the attribute for declare variant
directive to simplify future uses in other context-dependent directives.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69952

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -388,25 +388,41 @@
   if (Expr *E = Attr.getVariantFuncRef())
 VariantFuncRef = Subst(E);
 
-  ExprResult Score;
-  if (Expr *E = Attr.getScore())
-Score = Subst(E);
-
   // Check function/variant ref.
   Optional> DeclVarData =
   S.checkOpenMPDeclareVariantFunction(
   S.ConvertDeclToDeclGroup(New), VariantFuncRef.get(), Attr.getRange());
   if (!DeclVarData)
 return;
-  // Instantiate the attribute.
-  Sema::OpenMPDeclareVariantCtsSelectorData Data(
-  Attr.getCtxSelectorSet(), Attr.getCtxSelector(),
-  llvm::makeMutableArrayRef(Attr.implVendors_begin(),
-Attr.implVendors_size()),
-  Score);
+  SmallVector Scores;
+  SmallVector Data;
+  for (unsigned I = 0, E = Attr.scores_size(); I < E; ++I) {
+ExprResult Score;
+if (Expr *E = *std::next(Attr.scores_begin(), I))
+  Score = Subst(E);
+Scores.push_back(Score);
+// Instantiate the attribute.
+auto CtxSet = static_cast(
+*std::next(Attr.ctxSelectorSets_begin(), I));
+auto Ctx = static_cast(
+*std::next(Attr.ctxSelectors_begin(), I));
+switch (CtxSet) {
+case OMPCtxSet_implementation:
+  switch (Ctx) {
+  case OMPCtx_vendor:
+Data.emplace_back(CtxSet, Ctx, Attr.implVendors());
+break;
+  case OMPCtxUnknown:
+llvm_unreachable("Unexpected context selector kind.");
+  }
+  break;
+case OMPCtxSetUnknown:
+  llvm_unreachable("Unexpected context selector set kind.");
+}
+  }
   S.ActOnOpenMPDeclareVariantDirective(DeclVarData.getValue().first,
DeclVarData.getValue().second,
-   Attr.getRange(), Data);
+   Attr.getRange(), Data, Scores);
 }
 
 static void instantiateDependentAMDGPUFlatWorkGroupSizeAttr(
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -5190,30 +5190,62 @@
   return std::make_pair(FD, cast(DRE));
 }
 
-void Sema::ActOnOpenMPDeclareVariantDirective(
-FunctionDecl *FD, Expr *VariantRef, SourceRange SR,
-const Sema::OpenMPDeclareVariantCtsSelectorData &Data) {
-  if (Data.CtxSet == OMPDeclareVariantAttr::CtxSetUnknown ||
-  Data.Ctx == OMPDeclareVariantAttr::CtxUnknown)
+void Sema::ActOnOpenMPDeclareVariantDirective(FunctionDecl *FD,
+  Expr *VariantRef, SourceRange SR,
+  ArrayRef Data,
+  ArrayRef Scores) {
+  if (Scores.empty())
 return;
-  Expr *Score = nullptr;
-  if (Data.CtxScore.isUsable()) {
-Score = Data.CtxScore.get();
-if (!Score->isTypeDependent() && !Score->isValueDependent() &&
-!Score->isInstantiationDependent() &&
-!Score->containsUnexpandedParameterPack()) {
-  llvm::APSInt Result;
-  ExprResult ICE = VerifyIntegerConstantExpression(Score, &Result);
-  if (ICE.isInvalid())
-return;
+  SmallVector CtxScores;
+  SmallVector CtxSets;
+  SmallVector Ctxs;
+  SmallVector ImplVendors;
+  bool IsError = false;
+  for (unsigned I = 0, E = Scores.size(); I < E; ++I) {
+OpenMPContextSelectorSetKind CtxSet = Data[I].CtxSet;
+OpenMPContextSelectorKind Ctx = Data[I].Ctx;
+if (CtxSet == OMPCtxSetUnknown || Ctx == OMPCtxUnknown)
+  return;
+Expr *Score = nullptr;
+if (Scores[I].isUsable()) {
+  Score = Scores[I].get();
+  if (!Score->isTypeDependent() && !Score->isValueDependent() &&
+  !Score->isInstantiationDependent() &&
+  !Score->containsUnexpandedParameterPack()) {
+Score =
+PerformOpenMPImplicitIntegerConversion(Score->getExprLoc(), Score)
+.get();
+if (Score)
+  Score = VerifyIntegerConstantExpression(Score).get();
+  }
+} else {
+  Score = ActOnIntegerConstant(SourceLocation(), 0).

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

jdoerfert updated this revision to Diff 228264.
jdoerfert added a comment.

Make the command line option shorter (mentioning openmp once should suffice)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/barrier_codegen.cpp

Index: clang/test/OpenMP/barrier_codegen.cpp
===
--- clang/test/OpenMP/barrier_codegen.cpp
+++ clang/test/OpenMP/barrier_codegen.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,CLANGCG
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CLANGCG
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -fopenmp-enable-irbuilder -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,IRBUILDER
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-enable-irbuilder -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fopenmp-enable-irbuilder -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,IRBUILDER
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
@@ -34,6 +38,10 @@
   return tmain(argc) + tmain(argv[0][0]) + a;
 }
 
+// CLANGCG-NOT: inaccessiblemem_or_argmemonly
+// IRBUILDER:  ; Function Attrs: inaccessiblemem_or_argmemonly
+// IRBUILDER-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*) #2
+
 // CHECK: define {{.+}} [[TMAIN_INT]](
 // CHECK: [[GTID:%.+]] = call i32 @__kmpc_global_thread_num([[IDENT_T]]* [[LOC]])
 // CHECK: call void @__kmpc_barrier([[IDENT_T]]* [[EXPLICIT_BARRIER_LOC]], i32 [[GTID]])
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2996,6 +2996,8 @@
   Opts.OpenMP && !Args.hasArg(options::OPT_fnoopenmp_use_tls);
   Opts.OpenMPIsDevice =
   Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_is_device);
+  Opts.OpenMPIRBuilder =
+  Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_enable_irbuilder);
   bool IsTargetSpecified =
   Opts.OpenMPIsDevice || Args.hasArg(options::OPT_fopenmp_targets_EQ);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4555,6 +4555,7 @@
 CmdArgs.push_back("-fnoopenmp-use-tls");
   Args.AddLastArg(CmdArgs, options::OPT_fopenmp_simd,
   options::OPT_fno_openmp_simd);
+  Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_enable_irbuilder);
   Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_version_EQ);
   Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_cuda_number_of_sm_EQ);
   Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_cuda_blocks_per_sm_EQ);
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -45,6 +45,7 @@
 class DataLayout;
 class FunctionType;
 class LLVMContext;
+class OpenMPIRBuilder;
 class IndexedInstrProfReader;
 }
 
@@ -319,6 +320,7 @@
   std::unique_ptr ObjCRuntime;
   std::unique_ptr OpenCLRuntime;
   std::unique_ptr OpenMPRuntime;
+  std::unique_ptr OMPBuilder;
   std::unique_ptr CUDARuntime;
   std::unique_ptr DebugInfo;
   std::unique_ptr ObjCData;
@@ -585,6 +587,9 @@
 return *OpenMPRuntime;
   }
 
+  /// Return a pointer to the configured OpenMPIRBuilder, if any.
+  llvm::OpenMPIRBuilder *getOpenMPIRBuilder() { return OMPBuilder.get(); }
+
   /// Return a reference to the configured CUDA runtime.
   CGCUDARuntime &getCUDARuntime() {
 assert(CUDARuntime != nullptr);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/C

[PATCH] D54214: [RISCV] Set triple based on -march flag

2019-11-07 Thread Simon Cook via Phabricator via cfe-commits

simoncook updated this revision to Diff 228261.
simoncook added a comment.

Rebase.

@lenary Following the discussion regarding D69383 
, I think it's best for now to keep the logic 
just keeping `-march` directly, rather than using `getRISCVArch`. I think in 
the case of `-target risc32-. -mabi=lp64` I think it would confuse users if 
the tools suddenly changed to doing an rv64 compile. If we disable that, all 
that function would provide me is the same StringRef I'm already evaluating. I 
think adding any extra flag to indicate whether a rv32<->rv64 switch is 
acceptable would just make the code unnecessarily more messy. I think in the 
future if `getRISCVArch` evaluates more flags, then it might make sense to 
reconsider this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54214

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/riscv-arch.c


Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -541,6 +541,17 @@
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 


Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -541,6 +541,17 @@
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68997: Allow searching for prebuilt implicit modules.

2019-11-07 Thread Alexandre Rames via Phabricator via cfe-commits

arames updated this revision to Diff 228267.
arames added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68997

Files:
  clang/docs/Modules.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/prebuilt-implicit-module/a.h
  clang/test/Modules/Inputs/prebuilt-implicit-module/module.modulemap
  clang/test/Modules/prebuilt-implicit-modules.m

Index: clang/test/Modules/prebuilt-implicit-modules.m
===
--- /dev/null
+++ clang/test/Modules/prebuilt-implicit-modules.m
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x objective-c -fmodules %S/Inputs/prebuilt-implicit-module/module.modulemap -emit-module -fmodule-name=module_a -fmodules-cache-path=%t
+// RUN: find %t -name "module_a*.pcm" | grep module_a
+//
+// Check we use a prebuilt module when available, and do not build an implicit module.
+// RUN: rm -rf %t1
+// RUN: mkdir -p %t1
+// RUN: %clang_cc1 -x objective-c %s -I%S/Inputs/prebuilt-implicit-module -fmodules -fmodule-map-file=%S/Inputs/prebuilt-implicit-module/module.modulemap -fprebuilt-implicit-modules -fprebuilt-module-path=%t -fmodules-cache-path=%t1
+// RUN: find %t1 -name "module_a*.pcm" | not grep module_e
+//
+// Check that we correctly fall back to implicit modules if the prebuilt implicit module is not found.
+// RUN: %clang_cc1 -x objective-c %s -I%S/Inputs/prebuilt-implicit-module -fmodules -fmodule-map-file=%S/Inputs/prebuilt-implicit-module/module.modulemap -fprebuilt-implicit-modules -fprebuilt-module-path=%t -fmodules-cache-path=%t1 -fno-signed-char
+// RUN: find %t1 -name "module_a*.pcm" | grep module_a
+
+// Check that non-implicit prebuilt modules are always preferred to prebuilt implicit modules.
+// RUN: rm -rf %t2
+// RUN: mkdir -p %t2
+// RUN: %clang_cc1 -x objective-c -fmodules %S/Inputs/prebuilt-implicit-module/module.modulemap -emit-module -fmodule-name=module_a -fmodules-cache-path=%t
+// RUN: %clang_cc1 -x objective-c -fmodules %S/Inputs/prebuilt-implicit-module/module.modulemap -emit-module -fmodule-name=module_a -o %t/module_a.pcm -fno-signed-char
+// RUN: not %clang_cc1 -x objective-c %s -I%S/Inputs/prebuilt-implicit-module -fmodules -fmodule-map-file=%S/Inputs/prebuilt-implicit-module/module.modulemap -fprebuilt-implicit-modules -fprebuilt-module-path=%t -fmodules-cache-path=%t2
+// RUN: find %t2 -name "module_a*.pcm" | not grep module_a
+
+// expected-no-diagnostics
+@import module_a;
+int test() {
+  return a;
+}
Index: clang/test/Modules/Inputs/prebuilt-implicit-module/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/prebuilt-implicit-module/module.modulemap
@@ -0,0 +1 @@
+module module_a { header "a.h" }
Index: clang/test/Modules/Inputs/prebuilt-implicit-module/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/prebuilt-implicit-module/a.h
@@ -0,0 +1 @@
+const int a = 1;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1677,6 +1677,7 @@
   Record.push_back(HSOpts.DisableModuleHash);
   Record.push_back(HSOpts.ImplicitModuleMaps);
   Record.push_back(HSOpts.ModuleMapFileHomeIsCwd);
+  Record.push_back(HSOpts.EnablePrebuiltImplicitModules);
   Record.push_back(HSOpts.UseBuiltinIncludes);
   Record.push_back(HSOpts.UseStandardSystemIncludes);
   Record.push_back(HSOpts.UseStandardCXXIncludes);
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5789,6 +5789,7 @@
   HSOpts.DisableModuleHash = Record[Idx++];
   HSOpts.ImplicitModuleMaps = Record[Idx++];
   HSOpts.ModuleMapFileHomeIsCwd = Record[Idx++];
+  HSOpts.EnablePrebuiltImplicitModules = Record[Idx++];
   HSOpts.UseBuiltinIncludes = Record[Idx++];
   HSOpts.UseStandardSystemIncludes = Record[Idx++];
   HSOpts.UseStandardCXXIncludes = Record[Idx++];
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -164,14 +164,41 @@
   return {};
 }
 
+std::string HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) {
+  const FileEntry *ModuleMap =
+  getModuleMap().getModuleMapFileForUniquing(Module);
+  StringRef ModuleName = Module->Name

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits

edward-jones added a comment.

In D58896#1737113 , @sberg wrote:

> But how about literals like `'\x80'` where the promoted value depends on 
> whether plain `char` is signed or unsigned?

If 'char' is signed and index into an array then this will typically trigger an 
`-Warray-bounds` warning because it references before the start of the array.

In D58896#1737200 , @xbolva00 wrote:

> Well, i am not sure if one twitter report is good motivation to criple 
> warning.

The motivation for suppressing the warning was that it is not uncommon to use a 
character literal in lookup tables. In addition in brings the warning inline 
with the behaviour of `-Wchar-subscripts` in GCC.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896

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

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

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

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16411-16420
+bool isDefaultmapModifier = (M == OMPC_DEFAULTMAP_MODIFIER_alloc) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_to) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_from) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_tofrom) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_firstprivate) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_default) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_none);

ABataev wrote:
> Just `M != OMPC_DEFAULTMAP_MODIFIER_unknown` and `Kind != 
> OMPC_DEFAULTMAP_unknown`
Thanks for your kindly suggestion, but I'm not able to use `M != 
OMPC_DEFAULTMAP_MODIFIER_unknown` to deal with the case that the defaultmap 
modifier  never being set. For this case:
```
#pragma omp target defaultmap(scalar:
```
The defaultmap modifier returned from the parsing phase is 0 while 
OMPC_DEFAULTMAP_MODIFIER_unknown is set to 3 (OMPC_DEFAULTMAP_unknown). I could 
make the condition correct by just comparing M with 0 but this is the use of 
magic number, so I'm wondering should I just comparing each case explicitly 
(alloc, to, from...) or I'll need to fix the parsing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69853: [OpenMP][NFCI] Introduce llvm/IR/OpenMPConstants.h

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

ABataev added a comment.

In D69853#1737218 , @jdoerfert wrote:

> Are there  blocking issues on this one or can we go ahead and adjust minor 
> details as we go?


I still think it would be good to separate patches into the LLVM part and into 
the clang part and commit them separately? E.g. flang people may try to look 
for the commits for constant in LLVM, but not for the clang changes. It will be 
much easier for them to skip the changes in clang.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:4011
   unsigned NamedModifiersNumber = 0;
-  SmallVector FoundNameModifiers(
-  OMPD_unknown + 1);
+  SmallVector
+  FoundNameModifiers(unsigned(OMPD_unknown) + 1);

jdoerfert wrote:
> Meinersbur wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > ABataev wrote:
> > > > > > > JonChesterfield wrote:
> > > > > > > > I wonder if it would be worth wrapping the accesses to 
> > > > > > > > FoundNameModifiers in functor that does the enum class to 
> > > > > > > > unsigned conversion. E.g. a class instance that contains the 
> > > > > > > > small vector and exposes operator[] that takes the enum class.
> > > > > > > > 
> > > > > > > > FoundNameModifiers[unsigned(val)] is quite a lot of line noise.
> > > > > > > Why `map`? It can be represented as a simple c-style array 
> > > > > > > without any problems.
> > > > > > No, these are not enums that auto-convert to unsigned.
> > > > > Convert them manually. Replacing the simple hash array with map does 
> > > > > not look like a good solution.
> > > > I feel this this is a micro optimization that will make the code less 
> > > > maintainable and, as Jon noted, "FoundNameModifiers[unsigned(val)] is 
> > > > quite a lot of line noise.". 
> > > > Take a look at the first version and let me know if you want to go back 
> > > > to that one for this part, if so, I can do that.
> > > You already introduced special wrapper class in the same file, maybe you 
> > > can reuse for implicit conversions?
> > We are introducing an `EnumeratedArray` class in 
> > https://reviews.llvm.org/D68827#change-XphX8PAWFr8V . It should reduce the 
> > need for casting and `OpenMPDirectiveKindExWrapper`.
> Arguably, the easiest and cleanest way is to use a `map` to *map directives 
> to if clauses*.
> 
> The wrapper is in a different file but I can write another one for here.
> 
> EnumeratedArray could probably be used here, it is not in yet though.
Then better to use `llvm::IndexedMap` or `llvm::DenseMap`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853



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

[PATCH] D68407: [WIP][RISCV] Use compiler-rt if no GCC installation detected

2019-11-07 Thread Edward Jones via Phabricator via cfe-commits

edward-jones updated this revision to Diff 228268.
edward-jones added a comment.

I've changed this to always return `ToolChain::UNW_None` from 
`RISCVToolChain::GetUnwindLibType` now. As a consequence we get the original 
behaviour of only `-lgcc` being added to the link command.


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

https://reviews.llvm.org/D68407

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv32_nogcc_tree/riscv32-unknown-elf/lib/crtend.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtbegin.o
  
clang/test/Driver/Inputs/basic_riscv64_nogcc_tree/riscv64-unknown-elf/lib/crtend.o
  clang/test/Driver/riscv32-toolchain-extra.c
  clang/test/Driver/riscv64-toolchain-extra.c

Index: clang/test/Driver/riscv64-toolchain-extra.c
===
--- clang/test/Driver/riscv64-toolchain-extra.c
+++ clang/test/Driver/riscv64-toolchain-extra.c
@@ -22,12 +22,12 @@
 // RUN:-target riscv64-unknown-elf 2>&1 \
 // RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
 
-// C-RV64-BAREMETAL-LP64-NOGCC: InstalledDir: [[DRIVERDIR:.*]]
 // C-RV64-BAREMETAL-LP64-NOGCC: "-fuse-init-array"
-// C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "[[DRIVERDIR]]/../riscv64-unknown-elf/include"
-// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/riscv64-unknown-elf-ld"
-// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crt0.o"
-// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtbegin.o"
-// C-RV64-BAREMETAL-LP64-NOGCC: "-L[[DRIVERDIR]]/../riscv64-unknown-elf/lib"
-// C-RV64-BAREMETAL-LP64-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
-// C-RV64-BAREMETAL-LP64-NOGCC: "[[DRIVERDIR]]/../riscv64-unknown-elf/lib/crtend.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/include"
+// C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/lib/crt0.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/lib/clang/{{[0-9.]*}}/lib/clang_rt.crtbegin-riscv64.o"
+// C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/lib"
+// C-RV64-BAREMETAL-LP64-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group"
+// C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/lib/clang/{{[0-9.]*}}/lib/libclang_rt.builtins-riscv64.a"
+// C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/lib/clang/{{[0-9.]*}}/lib/clang_rt.crtend-riscv64.o"
Index: clang/test/Driver/riscv32-toolchain-extra.c
===
--- clang/test/Driver/riscv32-toolchain-extra.c
+++ clang/test/Driver/riscv32-toolchain-extra.c
@@ -22,12 +22,12 @@
 // RUN:-target riscv32-unknown-elf 2>&1 \
 // RUN:| FileCheck -check-prefix=C-RV32-BAREMETAL-ILP32-NOGCC %s
 
-// C-RV32-BAREMETAL-ILP32-NOGCC: InstalledDir: [[DRIVERDIR:.*]]
 // C-RV32-BAREMETAL-ILP32-NOGCC: "-fuse-init-array"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "-internal-isystem" "[[DRIVERDIR]]/../riscv32-unknown-elf/include"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/riscv32-unknown-elf-ld"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/crt0.o"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/crtbegin.o"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "-L[[DRIVERDIR]]/../riscv32-unknown-elf/lib"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
-// C-RV32-BAREMETAL-ILP32-NOGCC: "[[DRIVERDIR]]/../riscv32-unknown-elf/lib/crtend.o"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "-internal-isystem" "{{.*}}Output/testroot-riscv32-baremetal-nogcc/bin/../riscv32-unknown-elf/include"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "{{.*}}Output/testroot-riscv32-baremetal-nogcc/bin/riscv32-unknown-elf-ld"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "{{.*}}Output/testroot-riscv32-baremetal-nogcc/bin/../riscv32-unknown-elf/lib/crt0.o"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "{{.*}}Output/testroot-riscv32-baremetal-nogcc/lib/clang/{{[0-9.]*}}/lib/clang_rt.crtbegin-riscv32.o"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "{{.*}}Output/testroot-riscv32-baremetal-nogcc/bin/../riscv32-unknown-elf/lib"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "--start-group" "-lc" "-lgloss" "--end-group"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "{{.*}}Output/testroot-riscv32-baremetal-nogcc/lib/clang/{{[0-9.]*}}/lib/libclang_rt.builtins-riscv32.a"
+// C-RV32-BAREMETAL-ILP32-NOGCC: "{{.*}}Output/testroot-riscv32-baremetal-nogcc/lib/clang/{{[0-9.]*}}/lib/clang_rt.crtend-riscv32.o"
Index: clang

[PATCH] D54214: [RISCV] Set triple based on -march flag

2019-11-07 Thread Sam Elliott via Phabricator via cfe-commits

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

In D54214#1737255 , @simoncook wrote:

> Rebase.
>
> @lenary Following the discussion regarding D69383 
> , I think it's best for now to keep the 
> logic just keeping `-march` directly, ...

Yep, I understand.

Ok, Logic and tests LGTM. I think this needs an entry in the Release Notes too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54214

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

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

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

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16411-16420
+bool isDefaultmapModifier = (M == OMPC_DEFAULTMAP_MODIFIER_alloc) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_to) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_from) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_tofrom) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_firstprivate) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_default) ||
+(M == OMPC_DEFAULTMAP_MODIFIER_none);

cchen wrote:
> ABataev wrote:
> > Just `M != OMPC_DEFAULTMAP_MODIFIER_unknown` and `Kind != 
> > OMPC_DEFAULTMAP_unknown`
> Thanks for your kindly suggestion, but I'm not able to use `M != 
> OMPC_DEFAULTMAP_MODIFIER_unknown` to deal with the case that the defaultmap 
> modifier  never being set. For this case:
> ```
> #pragma omp target defaultmap(scalar:
> ```
> The defaultmap modifier returned from the parsing phase is 0 while 
> OMPC_DEFAULTMAP_MODIFIER_unknown is set to 3 (OMPC_DEFAULTMAP_unknown). I 
> could make the condition correct by just comparing M with 0 but this is the 
> use of magic number, so I'm wondering should I just comparing each case 
> explicitly (alloc, to, from...) or I'll need to fix the parsing?
Could you set `OMPC_DEFAULTMAP_MODIFIER_unknown` as an initial value? Or set 
`OMPC_DEFAULTMAP_MODIFIER_unknown` as thr first enum with value `0` and instead 
add something like `OMPC_DEFAULTMAP_MODIFIER_last` and use it everywhere in the 
code where you need the number of values instead of 
`OMPC_DEFAULTMAP_MODIFIER_unknown`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1643-1644
   HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
+def fopenmp_enable_irbuilder : Flag<["-"], "fopenmp-enable-irbuilder">, 
Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
+  HelpText<"Use the experimental OpenMP-IR-Builder codegen path.">;
 def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group, 
Flags<[CC1Option, NoArgumentUnused]>;

Do we need to expose this option to driver or it is enough to have just a 
frontend option? If still need to have a driver option, add a test for driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

jdoerfert updated this revision to Diff 228270.
jdoerfert added a comment.

Update test attributes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/OpenMP/barrier_codegen.cpp

Index: clang/test/OpenMP/barrier_codegen.cpp
===
--- clang/test/OpenMP/barrier_codegen.cpp
+++ clang/test/OpenMP/barrier_codegen.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,CLANGCG
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CLANGCG
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -fopenmp-enable-irbuilder -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,IRBUILDER
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-enable-irbuilder -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fopenmp-enable-irbuilder -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,IRBUILDER
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
@@ -34,6 +38,10 @@
   return tmain(argc) + tmain(argv[0][0]) + a;
 }
 
+// CLANGCG-NOT: inaccessiblemem
+// IRBUILDER:  ; Function Attrs: inaccessiblemem_or_argmemonly nofree nosync nounwind readonly
+// IRBUILDER-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
+
 // CHECK: define {{.+}} [[TMAIN_INT]](
 // CHECK: [[GTID:%.+]] = call i32 @__kmpc_global_thread_num([[IDENT_T]]* [[LOC]])
 // CHECK: call void @__kmpc_barrier([[IDENT_T]]* [[EXPLICIT_BARRIER_LOC]], i32 [[GTID]])
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2996,6 +2996,8 @@
   Opts.OpenMP && !Args.hasArg(options::OPT_fnoopenmp_use_tls);
   Opts.OpenMPIsDevice =
   Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_is_device);
+  Opts.OpenMPIRBuilder =
+  Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_enable_irbuilder);
   bool IsTargetSpecified =
   Opts.OpenMPIsDevice || Args.hasArg(options::OPT_fopenmp_targets_EQ);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4555,6 +4555,7 @@
 CmdArgs.push_back("-fnoopenmp-use-tls");
   Args.AddLastArg(CmdArgs, options::OPT_fopenmp_simd,
   options::OPT_fno_openmp_simd);
+  Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_enable_irbuilder);
   Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_version_EQ);
   Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_cuda_number_of_sm_EQ);
   Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_cuda_blocks_per_sm_EQ);
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -45,6 +45,7 @@
 class DataLayout;
 class FunctionType;
 class LLVMContext;
+class OpenMPIRBuilder;
 class IndexedInstrProfReader;
 }
 
@@ -319,6 +320,7 @@
   std::unique_ptr ObjCRuntime;
   std::unique_ptr OpenCLRuntime;
   std::unique_ptr OpenMPRuntime;
+  std::unique_ptr OMPBuilder;
   std::unique_ptr CUDARuntime;
   std::unique_ptr DebugInfo;
   std::unique_ptr ObjCData;
@@ -585,6 +587,9 @@
 return *OpenMPRuntime;
   }
 
+  /// Return a pointer to the configured OpenMPIRBuilder, if any.
+  llvm::OpenMPIRBuilder *getOpenMPIRBuilder() { return OMPBuilder.get(); }
+
   /// Return a reference to the configured CUDA runtime.
   CGCUDARuntime &getCUDARuntime() {
 assert(CUDARuntime != nullptr);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/

[clang] f37b5c8 - [RISCV] Fix up tests on Windows after new usage of sys::path::append

2019-11-07 Thread Reid Kleckner via cfe-commits


Author: Reid Kleckner
Date: 2019-11-07T09:53:56-08:00
New Revision: f37b5c800e150ad915c4e0571edd2c92c0160d89

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

LOG: [RISCV] Fix up tests on Windows after new usage of sys::path::append

Added: 


Modified: 
clang/test/Driver/riscv32-toolchain.c
clang/test/Driver/riscv64-toolchain.c

Removed: 




diff  --git a/clang/test/Driver/riscv32-toolchain.c 
b/clang/test/Driver/riscv32-toolchain.c
index 198069edfad7..f8a2516779fb 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -10,7 +10,7 @@
 // RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-ILP32 %s
 
 // C-RV32-BAREMETAL-ILP32: "-fuse-init-array"
-// C-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
+// C-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv32-unknown-elf-ld"
 // C-RV32-BAREMETAL-ILP32: 
"--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // C-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
 // C-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
@@ -26,10 +26,10 @@
 // RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
 
 // C-RV32-BAREMETAL-NOSYSROOT-ILP32: "-fuse-init-array"
-// C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
-// C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib{{/|}}crt0.o"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv32-unknown-elf-ld"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf/lib{{/|}}crt0.o"
 // C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
-// C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf{{/|}}lib"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf{{/|}}lib"
 // C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // C-RV32-BAREMETAL-NOSYSROOT-ILP32: "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // C-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
@@ -42,7 +42,7 @@
 
 // CXX-RV32-BAREMETAL-ILP32: "-fuse-init-array"
 // CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" 
"{{.*}}Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++{{/|}}8.0.1"
-// CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
+// CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: 
"--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
@@ -58,11 +58,11 @@
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
 
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-fuse-init-array"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-internal-isystem" 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib{{/|}}crt0.o"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-internal-isystem" 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|\\

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-11-07 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

The test seems to fail on Windows, could you take a look and revert if it takes 
a while to fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68391



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

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



Comment at: clang/include/clang/Driver/Options.td:1643-1644
   HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
+def fopenmp_enable_irbuilder : Flag<["-"], "fopenmp-enable-irbuilder">, 
Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
+  HelpText<"Use the experimental OpenMP-IR-Builder codegen path.">;
 def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group, 
Flags<[CC1Option, NoArgumentUnused]>;

ABataev wrote:
> Do we need to expose this option to driver or it is enough to have just a 
> frontend option? If still need to have a driver option, add a test for driver.
How do I write a frontend option? Anything that we can query in the lib/CodeGen 
is fine with me. (I don't even need an option if we turn this on by default to 
get test coverage right away).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-11-07 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

nvm, rnk just got to it in 
https://github.com/llvm/llvm-project/commit/f37b5c800e150ad915c4e0571edd2c92c0160d89


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68391



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

[PATCH] D68391: [RISCV] Improve sysroot computation if no GCC install detected

2019-11-07 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Sorry, meant to also paste a link to a failing build: 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/11994/steps/stage%201%20check/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68391



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

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

ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1643-1644
   HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
+def fopenmp_enable_irbuilder : Flag<["-"], "fopenmp-enable-irbuilder">, 
Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>,
+  HelpText<"Use the experimental OpenMP-IR-Builder codegen path.">;
 def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group, 
Flags<[CC1Option, NoArgumentUnused]>;

jdoerfert wrote:
> ABataev wrote:
> > Do we need to expose this option to driver or it is enough to have just a 
> > frontend option? If still need to have a driver option, add a test for 
> > driver.
> How do I write a frontend option? Anything that we can query in the 
> lib/CodeGen is fine with me. (I don't even need an option if we turn this on 
> by default to get test coverage right away).
You nedd to move to CC1Options.td file. It means that you can't use it direcly 
when invoke the drive, instead you will need to use `-Xclang -fopenmp-...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[PATCH] D69853: [OpenMP][NFCI] Introduce llvm/IR/OpenMPConstants.h

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

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

In D69853#1737264 , @ABataev wrote:

> In D69853#1737218 , @jdoerfert wrote:
>
> > Are there  blocking issues on this one or can we go ahead and adjust minor 
> > details as we go?
>
>
> I still think it would be good to separate patches into the LLVM part and 
> into the clang part and commit them separately? E.g. flang people may try to 
> look for the commits for constant in LLVM, but not for the clang changes. It 
> will be much easier for them to skip the changes in clang.


But they might also want to see how to interact with the constants so having 
the clang parts in there is good. I do not see a clear benefit, e.g., Flang ppl 
might or might not prefer a separate patch, but I see a clear drawback (testing 
wise) so I still don't believe splitting the right thing.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:4011
   unsigned NamedModifiersNumber = 0;
-  SmallVector FoundNameModifiers(
-  OMPD_unknown + 1);
+  SmallVector
+  FoundNameModifiers(unsigned(OMPD_unknown) + 1);

ABataev wrote:
> jdoerfert wrote:
> > Meinersbur wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > ABataev wrote:
> > > > > > > > JonChesterfield wrote:
> > > > > > > > > I wonder if it would be worth wrapping the accesses to 
> > > > > > > > > FoundNameModifiers in functor that does the enum class to 
> > > > > > > > > unsigned conversion. E.g. a class instance that contains the 
> > > > > > > > > small vector and exposes operator[] that takes the enum class.
> > > > > > > > > 
> > > > > > > > > FoundNameModifiers[unsigned(val)] is quite a lot of line 
> > > > > > > > > noise.
> > > > > > > > Why `map`? It can be represented as a simple c-style array 
> > > > > > > > without any problems.
> > > > > > > No, these are not enums that auto-convert to unsigned.
> > > > > > Convert them manually. Replacing the simple hash array with map 
> > > > > > does not look like a good solution.
> > > > > I feel this this is a micro optimization that will make the code less 
> > > > > maintainable and, as Jon noted, "FoundNameModifiers[unsigned(val)] is 
> > > > > quite a lot of line noise.". 
> > > > > Take a look at the first version and let me know if you want to go 
> > > > > back to that one for this part, if so, I can do that.
> > > > You already introduced special wrapper class in the same file, maybe 
> > > > you can reuse for implicit conversions?
> > > We are introducing an `EnumeratedArray` class in 
> > > https://reviews.llvm.org/D68827#change-XphX8PAWFr8V . It should reduce 
> > > the need for casting and `OpenMPDirectiveKindExWrapper`.
> > Arguably, the easiest and cleanest way is to use a `map` to *map directives 
> > to if clauses*.
> > 
> > The wrapper is in a different file but I can write another one for here.
> > 
> > EnumeratedArray could probably be used here, it is not in yet though.
> Then better to use `llvm::IndexedMap` or `llvm::DenseMap`
It will not matter but sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853



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

1 2 3 >

1 - 100 of 232 matches

Mail list logo