[PATCH] D71272: [OpenCL] Pretty print __private addr space

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin added inline comments.



Comment at: clang/test/SemaOpenCL/access-qualifier.cl:28
 kernel void k1(img1d_wo img) {
-  myRead(img); // expected-error {{passing 'img1d_wo' (aka '__write_only 
image1d_t') to parameter of incompatible type '__read_only image1d_t'}}
+  myRead(img); // expected-error {{passing '__private img1d_wo' (aka 
'__private __write_only image1d_t') to parameter of incompatible type 
'__read_only image1d_t'}}
 }

Minor. An error message like this looks a bit confusing to me. User might 
wonder whether parameters are incompatible because of address space or because 
of access qualifiers. Should it print `passing '__private img1d_wo' (aka 
'__private __write_only image1d_t')  to parameter of incompatible type 
'__private __read_only image1d_t'`? In this case it is clear that there is a 
mismatch in access qualifiers.



Comment at: clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl:373
 #if !__OPENCL_CPP_VERSION__
-// expected-error@-3{{passing '__constant int *' to parameter of type 
'__generic int *' changes address space of pointer}}
+// expected-error@-3{{passing '__constant int *__private' to parameter of type 
'__generic int *' changes address space of pointer}}
 #else

Again, I think it would be more clear if it was: `passing '__constant int 
*__private' to parameter of type '__generic int *__private' changes address 
space of pointer`


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

https://reviews.llvm.org/D71272



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D71345#1782647 , @nridge wrote:

> Here, both navigations target the overloaded operator, but if you comment out 
> the overloaded operator, they target the respective local variable 
> declarations. I think this is pretty good do-what-I-mean behaviour, arguably 
> better than a categorical preference for the right side over the left side.


That's reasonable in eclipse (though personally I'd find it frustrating to have 
no way to target `b` in `a+b+c`).
It seems broken for editors where the cursor is on a character rather than 
between characters, though.

> - For the purpose of determining "encloses", the cursor being at either 
> boundary of a node counts as a match (so A and B are both considered to 
> enclose the first cursor, and B and C are both considered to enclose the 
> second cursor).
> - The algorithm for go-to-definition is: first look for an enclosing 
> `ImplicitName` node; if one is found, go to its target. Otherwise, look for 
> an enclosing `Name` node and go to its target.
> 
>   Other features use the same API, but perhaps with a different node type; 
> for example, the "define out of line" refactoring looks for an enclosing 
> `FunctionDeclarator` node.
> 
>   Perhaps we could have a simpler `SelectionTree` API along these lines, 
> where the type of desired node informs the selection at each call site?

Thanks for the details on CDT. This diverges quite a lot from what we currently 
have:

- we have abstractions at the preprocessor level (tokens) and at the semantic 
level (clang AST), but nothing modeling the concept of e.g. "name" that we 
would need here.
- our refactorings (tweak) relies on computing the selection by traversing the 
AST first and then querying it for each operation, which doesn't work if the 
query affects the traversal and the traversal is expensive

Syntax trees would address this by providing a tree with the relevant syntactic 
constructs that's cheap to traverse, and would certainly end up replacing 
SelectionTree altogether.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71422: [clangd] Introduce bulletlists

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FormattedString.h:65
 
+/// Prints a list of documents while prepending "- " marker. Also indents the
+/// documents.

This is a class, not a function - comment should focus on what it *is*.



Comment at: clang-tools-extra/clangd/FormattedString.h:95
 
+  /// Causes every line of the document to be indented by 2 spaces. Except the
+  /// first line, it is containers responsibility to adjust padding for first

Why is it the responsibility of this class to keep track of its indentation 
within the parent?
This seems like it should be a parameter to the render functions, rather than 
state.

BTW, it appears to be legal to write lists as:
```
-
  this is the
  first item
-
  second item
```
Which is much easier/more regular to generate, because it doesn't require 
`Document` to special-case the first line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71422



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 233746.
serge-sans-paille marked 6 inline comments as done.
serge-sans-paille added a comment.

Handle comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc

Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -0,0 +1,19 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
+#ifdef WITH_SELF_REFERENCE_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  if (n != 0)
+memcpy(dest, from, n);
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_SELF_REFERENCE_DECL | FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
+//
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+// CHECK-SELF-REF-DECL: @llvm.memcpy
+//
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+
+  static char buffer[1];
+  memcpy(buffer, from, 2); // expected-warning {{'memcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isInlineBuiltinDeclaration()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4596,8 +4596,14 @@
 }
 
 static CGCallee EmitDirectCallee(CodeGenFunction &CGF, const FunctionDecl *FD) {
+
   if (auto builtinID = FD->getBuiltinID()) {
-return CGCallee::forBuiltin(builtinID, FD);
+// Replaceable builtin provide their own implementation of a builtin. Unless
+// we are in the builtin implementation itself, don't call the actual
+// builtin. If we are in the builtin implementation, avoid trivial infinite
+// recursion.
+if (!FD->isInlineBuiltinDeclaration() || CGF.CurFn->getName() == FD->getName())
+  return CGCallee::forBuiltin(builtinID, FD);
   }
 
   llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,24 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isInlineBuiltinDeclaration() const {
+  if (!getBuiltinID())
+return false;
+
+  const FunctionDecl *Definition;
+  if (hasBody(Definition)) {
+
+if (!Definition->isInlineSpecified())
+  return false;
+
+const SourceManager &SM = getASTContext().getSourceManager();
+SourceLocation SL = Definition->getLocation();
+if (SL.isValid())
+  return SM.isInSystemHeader(SL);
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides an inline implementation of a builtin.
+  bool isInlineBuiltinDeclaration() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 

[clang-tools-extra] 7c13fe8 - [clangd] Introduce codeblocks

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-13T09:58:55+01:00
New Revision: 7c13fe8a6a643497d49036e6ea368e1adb06f57e

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

LOG: [clangd] Introduce codeblocks

Summary: Follow-up to the patch D71248

Reviewers: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/FormattedString.cpp
clang-tools-extra/clangd/FormattedString.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FormattedString.cpp 
b/clang-tools-extra/clangd/FormattedString.cpp
index 8bb17f38012b..69d33a45ada1 100644
--- a/clang-tools-extra/clangd/FormattedString.cpp
+++ b/clang-tools-extra/clangd/FormattedString.cpp
@@ -72,10 +72,10 @@ std::string renderInlineBlock(llvm::StringRef Input) {
   return "`" + std::move(R) + "`";
 }
 
-/// Render \p Input as markdown code block with a specified \p Language. The
-/// result is surrounded by >= 3 backticks. Although markdown also allows to 
use
-/// '~' for code blocks, they are never used.
-std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
+/// Get marker required for \p Input to represent a markdown codeblock. It
+/// consists of at least 3 backticks(`). Although markdown also allows to use
+/// tilde(~) for code blocks, they are never used.
+std::string getMarkerForCodeBlock(llvm::StringRef Input) {
   // Count the maximum number of consecutive backticks in \p Input. We need to
   // start and end the code block with more.
   unsigned MaxBackticks = 0;
@@ -90,8 +90,7 @@ std::string renderCodeBlock(llvm::StringRef Input, 
llvm::StringRef Language) {
   }
   MaxBackticks = std::max(Backticks, MaxBackticks);
   // Use the corresponding number of backticks to start and end a code block.
-  std::string BlockMarker(/*Repeat=*/std::max(3u, MaxBackticks + 1), '`');
-  return BlockMarker + Language.str() + "\n" + Input.str() + "\n" + 
BlockMarker;
+  return std::string(/*Repeat=*/std::max(3u, MaxBackticks + 1), '`');
 }
 
 // Trims the input and concatanates whitespace blocks into a single ` `.
@@ -131,6 +130,26 @@ class Spacer : public Block {
   void renderPlainText(llvm::raw_ostream &OS) const override { OS << '\n'; }
 };
 
+class CodeBlock : public Block {
+public:
+  void renderMarkdown(llvm::raw_ostream &OS) const override {
+std::string Marker = getMarkerForCodeBlock(Contents);
+// No need to pad from previous blocks, as they should end with a new line.
+OS << Marker << Language << '\n' << Contents << '\n' << Marker << '\n';
+  }
+
+  void renderPlainText(llvm::raw_ostream &OS) const override {
+// In plaintext we want one empty line before and after codeblocks.
+OS << '\n' << Contents << "\n\n";
+  }
+
+  CodeBlock(std::string Contents, std::string Language)
+  : Contents(std::move(Contents)), Language(std::move(Language)) {}
+
+private:
+  std::string Contents;
+  std::string Language;
+};
 } // namespace
 
 std::string Block::asMarkdown() const {
@@ -204,6 +223,11 @@ Paragraph &Document::addParagraph() {
 
 void Document::addSpacer() { Children.push_back(std::make_unique()); }
 
+void Document::addCodeBlock(std::string Code, std::string Language) {
+  Children.emplace_back(
+  std::make_unique(std::move(Code), std::move(Language)));
+}
+
 std::string Document::asMarkdown() const {
   return renderBlocks(Children, &Block::renderMarkdown);
 }

diff  --git a/clang-tools-extra/clangd/FormattedString.h 
b/clang-tools-extra/clangd/FormattedString.h
index 02753950f0cf..4ded2fdee315 100644
--- a/clang-tools-extra/clangd/FormattedString.h
+++ b/clang-tools-extra/clangd/FormattedString.h
@@ -70,6 +70,9 @@ class Document {
   Paragraph &addParagraph();
   /// Inserts a vertical space into the document.
   void addSpacer();
+  /// Adds a block of code. This translates to a ``` block in markdown. In 
plain
+  /// text representation, the code block will be surrounded by newlines.
+  void addCodeBlock(std::string Code, std::string Language = "cpp");
 
   std::string asMarkdown() const;
   std::string asPlainText() const;

diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index d6b270c826d9..f55a9c3d1f67 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -95,7 +95,7 @@ std::string printDefinition(const Decl *D) {
 }
 
 void printParams(llvm::raw_ostream &OS,
-const std::vector &Params) {
+ const std::vector &Params) {
   for (size_t I = 0, E = Params.size(); I != E; ++I) {
 if (I)
   OS << ", ";
@@ -456,12

[clang-tools-extra] 597c6b6 - [clangd] Introduce paragraph, the first part of new rendering structs

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-13T09:58:55+01:00
New Revision: 597c6b65552a777a40f2afed07c543f6789318b1

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

LOG: [clangd] Introduce paragraph, the first part of new rendering structs

Summary:
Initial patch for new rendering structs in clangd.

Splitting implementation into smaller chunks, for a full view of the API see 
D71063.

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

Tags: #clang

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

Reviewers: sammccall

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/FormattedString.cpp
clang-tools-extra/clangd/FormattedString.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index cb1f02ff68b9..69b4308a1c9e 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1095,10 +1095,10 @@ void ClangdLSPServer::onHover(const 
TextDocumentPositionParams &Params,
   R.range = (*H)->SymRange;
   switch (HoverContentFormat) {
   case MarkupKind::PlainText:
-R.contents.value = (*H)->present().renderAsPlainText();
+R.contents.value = (*H)->present().asPlainText();
 return Reply(std::move(R));
   case MarkupKind::Markdown:
-R.contents.value = (*H)->present().renderAsMarkdown();
+R.contents.value = (*H)->present().asMarkdown();
 return Reply(std::move(R));
   };
   llvm_unreachable("unhandled MarkupKind");

diff  --git a/clang-tools-extra/clangd/FormattedString.cpp 
b/clang-tools-extra/clangd/FormattedString.cpp
index 27fd37b9ae62..8bb17f38012b 100644
--- a/clang-tools-extra/clangd/FormattedString.cpp
+++ b/clang-tools-extra/clangd/FormattedString.cpp
@@ -7,19 +7,27 @@
 
//===--===//
 #include "FormattedString.h"
 #include "clang/Basic/CharInfo.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
+namespace markup {
 
 namespace {
 /// Escape a markdown text block. Ensures the punctuation will not introduce
 /// any of the markdown constructs.
-static std::string renderText(llvm::StringRef Input) {
+std::string renderText(llvm::StringRef Input) {
   // Escaping ASCII punctiation ensures we can't start a markdown construct.
   constexpr llvm::StringLiteral Punctuation =
   R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
@@ -40,7 +48,7 @@ static std::string renderText(llvm::StringRef Input) {
 
 /// Renders \p Input as an inline block of code in markdown. The returned value
 /// is surrounded by backticks and the inner contents are properly escaped.
-static std::string renderInlineBlock(llvm::StringRef Input) {
+std::string renderInlineBlock(llvm::StringRef Input) {
   std::string R;
   // Double all backticks to make sure we don't close the inline block early.
   for (size_t From = 0; From < Input.size();) {
@@ -63,11 +71,11 @@ static std::string renderInlineBlock(llvm::StringRef Input) 
{
 return "` " + std::move(R) + " `";
   return "`" + std::move(R) + "`";
 }
+
 /// Render \p Input as markdown code block with a specified \p Language. The
 /// result is surrounded by >= 3 backticks. Although markdown also allows to 
use
 /// '~' for code blocks, they are never used.
-static std::string renderCodeBlock(llvm::StringRef Input,
-   llvm::StringRef Language) {
+std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
   // Count the maximum number of consecutive backticks in \p Input. We need to
   // start and end the code block with more.
   unsigned MaxBackticks = 0;
@@ -86,114 +94,123 @@ static std::string renderCodeBlock(llvm::StringRef Input,
   return BlockMarker + Language.str() + "\n" + Input.str() + "\n" + 
BlockMarker;
 }
 
-} // namespace
-
-void FormattedString::appendText(std::string Text) {
-  Chunk C;
-  C.Kind = ChunkKind::PlainText;
-  C.Contents = Text;
-  Chunks.push_back(C);
+// Trims the input and concatanates whitespace blo

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG597c6b65552a: [clangd] Introduce paragraph, the first part 
of new rendering structs (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71248

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -8,192 +8,109 @@
 #include "FormattedString.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
-
+#include "llvm/Support/raw_ostream.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
+namespace markup {
 namespace {
 
-TEST(FormattedString, Basic) {
-  FormattedString S;
-  EXPECT_EQ(S.renderAsPlainText(), "");
-  EXPECT_EQ(S.renderAsMarkdown(), "");
-
-  S.appendText("foobar  ");
-  S.appendText("baz");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foobar  baz");
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foobar");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "foobar\n"
-  "```\n");
-}
-
-TEST(FormattedString, CodeBlocks) {
-  FormattedString S;
-  S.appendCodeBlock("foobar");
-  S.appendCodeBlock("bazqux", "javascript");
-  S.appendText("after");
-
-  std::string ExpectedText = R"(foobar
-
-bazqux
-
-after)";
-  EXPECT_EQ(S.renderAsPlainText(), ExpectedText);
-  std::string ExpectedMarkdown = R"md(```cpp
-foobar
-```
-```javascript
-bazqux
-```
-after)md";
-  EXPECT_EQ(S.renderAsMarkdown(), ExpectedMarkdown);
-
-  S = FormattedString();
-  S.appendInlineCode("foobar");
-  S.appendInlineCode("bazqux");
-  EXPECT_EQ(S.renderAsPlainText(), "foobar bazqux");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foobar` `bazqux`");
-
-  S = FormattedString();
-  S.appendText("foo");
-  S.appendInlineCode("bar");
-  S.appendText("baz");
-
-  EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
-}
-
-TEST(FormattedString, Escaping) {
+TEST(Render, Escaping) {
   // Check some ASCII punctuation
-  FormattedString S;
-  S.appendText("*!`");
-  EXPECT_EQ(S.renderAsMarkdown(), "\\*\\!\\`");
+  Paragraph P;
+  P.appendText("*!`");
+  EXPECT_EQ(P.asMarkdown(), "\\*\\!\\`");
 
   // Check all ASCII punctuation.
-  S = FormattedString();
+  P = Paragraph();
   std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
   // Same text, with each character escaped.
   std::string EscapedPunctuation;
   EscapedPunctuation.reserve(2 * Punctuation.size());
   for (char C : Punctuation)
 EscapedPunctuation += std::string("\\") + C;
-  S.appendText(Punctuation);
-  EXPECT_EQ(S.renderAsMarkdown(), EscapedPunctuation);
+  P.appendText(Punctuation);
+  EXPECT_EQ(P.asMarkdown(), EscapedPunctuation);
 
   // In code blocks we don't need to escape ASCII punctuation.
-  S = FormattedString();
-  S.appendInlineCode("* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`* foo !+ bar * baz`");
-  S = FormattedString();
-  S.appendCodeBlock("#define FOO\n* foo !+ bar * baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "#define FOO\n* foo !+ bar * baz\n"
-  "```\n");
+  P = Paragraph();
+  P.appendCode("* foo !+ bar * baz");
+  EXPECT_EQ(P.asMarkdown(), "`* foo !+ bar * baz`");
 
   // But we have to escape the backticks.
-  S = FormattedString();
-  S.appendInlineCode("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "`foo``bar``baz`");
-
-  S = FormattedString();
-  S.appendCodeBlock("foo`bar`baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "```cpp\n"
-  "foo`bar`baz\n"
-  "```\n");
+  P = Paragraph();
+  P.appendCode("foo`bar`baz");
+  EXPECT_EQ(P.asMarkdown(), "`foo``bar``baz`");
 
   // Inline code blocks starting or ending with backticks should add spaces.
-  S = FormattedString();
-  S.appendInlineCode("`foo");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo `");
-  S = FormattedString();
-  S.appendInlineCode("foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` foo`` `");
-  S = FormattedString();
-  S.appendInlineCode("`foo`");
-  EXPECT_EQ(S.renderAsMarkdown(), "` ``foo`` `");
-
-  // Should also add extra spaces if

[PATCH] D71414: [clangd] Introduce codeblocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c13fe8a6a64: [clangd] Introduce codeblocks (authored by 
kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71414

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -54,6 +54,28 @@
   P = Paragraph();
   P.appendCode("`foo`");
   EXPECT_EQ(P.asMarkdown(), "` ``foo`` `");
+
+  // Code blocks might need more than 3 backticks.
+  Document D;
+  D.addCodeBlock("foobarbaz `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "```cpp\n"
+"foobarbaz `\nqux\n"
+"```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ``\nqux");
+  EXPECT_THAT(D.asMarkdown(), "```cpp\n"
+  "foobarbaz ``\nqux\n"
+  "```");
+  D = Document();
+  D.addCodeBlock("foobarbaz ```\nqux");
+  EXPECT_EQ(D.asMarkdown(), "cpp\n"
+"foobarbaz ```\nqux\n"
+"");
+  D = Document();
+  D.addCodeBlock("foobarbaz ` `` ```  `\nqux");
+  EXPECT_EQ(D.asMarkdown(), "`cpp\n"
+"foobarbaz ` `` ```  `\nqux\n"
+"`");
 }
 
 TEST(Paragraph, SeparationOfChunks) {
@@ -96,9 +118,18 @@
 TEST(Document, Separators) {
   Document D;
   D.addParagraph().appendText("foo");
+  D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), "foo\nbar");
-  EXPECT_EQ(D.asPlainText(), "foo\nbar");
+  EXPECT_EQ(D.asMarkdown(), R"md(foo
+```cpp
+test
+```
+bar)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+
+test
+
+bar)pt");
 }
 
 TEST(Document, Spacer) {
@@ -110,6 +141,38 @@
   EXPECT_EQ(D.asPlainText(), "foo\n\nbar");
 }
 
+TEST(CodeBlock, Render) {
+  Document D;
+  // Code blocks preserves any extra spaces.
+  D.addCodeBlock("foo\n  bar\n  baz");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```)md");
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz)pt");
+  D.addCodeBlock("foo");
+  EXPECT_EQ(D.asMarkdown(), R"md(```cpp
+foo
+  bar
+  baz
+```
+```cpp
+foo
+```)md");
+  // FIXME: we shouldn't have 2 empty lines in between. A solution might be
+  // having a `verticalMargin` method for blocks, and let container insert new
+  // lines according to that before/after blocks.
+  EXPECT_EQ(D.asPlainText(), R"pt(foo
+  bar
+  baz
+
+
+foo)pt");
+}
+
 } // namespace
 } // namespace markup
 } // namespace clangd
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -95,7 +95,7 @@
 }
 
 void printParams(llvm::raw_ostream &OS,
-const std::vector &Params) {
+ const std::vector &Params) {
   for (size_t I = 0, E = Params.size(); I != E; ++I) {
 if (I)
   OS << ", ";
@@ -456,12 +456,11 @@
   P.appendCode(llvm::StringRef(*NamespaceScope).drop_back(2));
   }
 
-  Output.addSpacer();
   if (!Definition.empty()) {
-Output.addParagraph().appendCode(Definition);
+Output.addCodeBlock(Definition);
   } else {
 // Builtin types
-Output.addParagraph().appendCode(Name);
+Output.addCodeBlock(Name);
   }
 
   if (!Documentation.empty())
Index: clang-tools-extra/clangd/FormattedString.h
===
--- clang-tools-extra/clangd/FormattedString.h
+++ clang-tools-extra/clangd/FormattedString.h
@@ -70,6 +70,9 @@
   Paragraph &addParagraph();
   /// Inserts a vertical space into the document.
   void addSpacer();
+  /// Adds a block of code. This translates to a ``` block in markdown. In plain
+  /// text representation, the code block will be surrounded by newlines.
+  void addCodeBlock(std::string Code, std::string Language = "cpp");
 
   std::string asMarkdown() const;
   std::string asPlainText() const;
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -72,10 +72,10 @@
   return "`" + std::move(R) + "`";
 }
 
-/// Render \p Input as markdown code block with a specified \p Language. The
-/// result is surrounded by >= 3 backticks. Although markdown also allows to use
-/// '~' for code blocks, they are never used.
-std::string renderCodeBlock(llvm::StringRef Input, llvm::StringRef Language) {
+/// Get marker req

[clang-tools-extra] 087528a - [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-13T10:07:18+01:00
New Revision: 087528a331786228221d7a56a51ab97a3fcac8f1

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

LOG: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

Reviewers: ilya-biryukov, hokein

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
index 3bc5df0edbfd..57690ee3d684 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -32,6 +32,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Driver/Types.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
@@ -360,6 +361,25 @@ const SourceLocation getBeginLoc(const FunctionDecl *FD) {
   return FD->getBeginLoc();
 }
 
+llvm::Optional
+addInlineIfInHeader(const FunctionDecl *FD) {
+  // This includes inline functions and constexpr functions.
+  if (FD->isInlined() || llvm::isa(FD))
+return llvm::None;
+  // Primary template doesn't need inline.
+  if (FD->isTemplated() && !FD->isFunctionTemplateSpecialization())
+return llvm::None;
+
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  llvm::StringRef FileName = SM.getFilename(FD->getLocation());
+
+  // If it is not a header we don't need to mark function as "inline".
+  if (!isHeaderFile(FileName, FD->getASTContext().getLangOpts()))
+return llvm::None;
+
+  return tooling::Replacement(SM, FD->getInnerLocStart(), 0, "inline ");
+}
+
 /// Moves definition of a function/method to its declaration location.
 /// Before:
 /// a.h:
@@ -436,6 +456,7 @@ class DefineInline : public Tweak {
   "Couldn't find semicolon for target declaration.");
 }
 
+auto AddInlineIfNecessary = addInlineIfInHeader(Target);
 auto ParamReplacements = renameParameters(Target, Source);
 if (!ParamReplacements)
   return ParamReplacements.takeError();
@@ -446,6 +467,13 @@ class DefineInline : public Tweak {
 
 const tooling::Replacement SemicolonToFuncBody(SM, *Semicolon, 1,
*QualifiedBody);
+tooling::Replacements TargetFileReplacements(SemicolonToFuncBody);
+TargetFileReplacements = TargetFileReplacements.merge(*ParamReplacements);
+if (AddInlineIfNecessary) {
+  if (auto Err = TargetFileReplacements.add(*AddInlineIfNecessary))
+return std::move(Err);
+}
+
 auto DefRange = toHalfOpenFileRange(
 SM, AST.getLangOpts(),
 SM.getExpansionRange(CharSourceRange::getCharRange(getBeginLoc(Source),
@@ -462,9 +490,8 @@ class DefineInline : public Tweak {
 
 llvm::SmallVector, 2> Edits;
 // Edit for Target.
-auto FE = Effect::fileEdit(
-SM, SM.getFileID(*Semicolon),
-tooling::Replacements(SemicolonToFuncBody).merge(*ParamReplacements));
+auto FE = Effect::fileEdit(SM, SM.getFileID(*Semicolon),
+   std::move(TargetFileReplacements));
 if (!FE)
   return FE.takeError();
 Edits.push_back(std::move(*FE));

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index d50d27afb1c8..ebeea82864cb 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1864,6 +1864,66 @@ TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
   EXPECT_EQ(apply(Test), Expected) << Test;
 }
 
+TEST_F(DefineInlineTest, AddInline) {
+  llvm::StringMap EditedFiles;
+  ExtraFiles["a.h"] = "void foo();";
+  apply(R"cpp(#include "a.h"
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Check we put inline before cv-qualifiers.
+  ExtraFiles["a.h"] = "const int foo();";
+  apply(R"cpp(#include "a.h"
+  const int fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline const int foo(){}")));
+
+  // No double inline.
+  ExtraFiles["a.h"] = "inline void foo();";
+  apply(R"cpp(#include "a.h"
+  inline void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedF

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1913
+
+  // Specializations needs to be marked "inline".
+  ExtraFiles["a.h"] = R"cpp(

hokein wrote:
> could you add a test case for  partial template specializations? I think we 
> don't need the inline for that.
there's no such thing as partial specialization for functions, and we don't 
allow inlining inside templated classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68261



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

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG087528a33178: [clangd] Add "inline" keyword to 
prevent ODR-violations in DefineInline (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68261

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1864,6 +1864,66 @@
   EXPECT_EQ(apply(Test), Expected) << Test;
 }
 
+TEST_F(DefineInlineTest, AddInline) {
+  llvm::StringMap EditedFiles;
+  ExtraFiles["a.h"] = "void foo();";
+  apply(R"cpp(#include "a.h"
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Check we put inline before cv-qualifiers.
+  ExtraFiles["a.h"] = "const int foo();";
+  apply(R"cpp(#include "a.h"
+  const int fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline const int foo(){}")));
+
+  // No double inline.
+  ExtraFiles["a.h"] = "inline void foo();";
+  apply(R"cpp(#include "a.h"
+  inline void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Constexprs don't need "inline".
+  ExtraFiles["a.h"] = "constexpr void foo();";
+  apply(R"cpp(#include "a.h"
+  constexpr void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "constexpr void foo(){}")));
+
+  // Class members don't need "inline".
+  ExtraFiles["a.h"] = "struct Foo { void foo(); }";
+  apply(R"cpp(#include "a.h"
+  void Foo::fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("a.h"), "struct Foo { void foo(){} }")));
+
+  // Function template doesn't need to be "inline"d.
+  ExtraFiles["a.h"] = "template  void foo();";
+  apply(R"cpp(#include "a.h"
+  template 
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("a.h"), "template  void foo(){}")));
+
+  // Specializations needs to be marked "inline".
+  ExtraFiles["a.h"] = R"cpp(
+template  void foo();
+template <> void foo();)cpp";
+  apply(R"cpp(#include "a.h"
+  template <>
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(
+template  void foo();
+template <> inline void foo(){})cpp")));
+}
+
 TWEAK_TEST(DefineOutline);
 TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
   FileName = "Test.cpp";
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -32,6 +32,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Driver/Types.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
@@ -360,6 +361,25 @@
   return FD->getBeginLoc();
 }
 
+llvm::Optional
+addInlineIfInHeader(const FunctionDecl *FD) {
+  // This includes inline functions and constexpr functions.
+  if (FD->isInlined() || llvm::isa(FD))
+return llvm::None;
+  // Primary template doesn't need inline.
+  if (FD->isTemplated() && !FD->isFunctionTemplateSpecialization())
+return llvm::None;
+
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  llvm::StringRef FileName = SM.getFilename(FD->getLocation());
+
+  // If it is not a header we don't need to mark function as "inline".
+  if (!isHeaderFile(FileName, FD->getASTContext().getLangOpts()))
+return llvm::None;
+
+  return tooling::Replacement(SM, FD->getInnerLocStart(), 0, "inline ");
+}
+
 /// Moves definition of a function/method to its declaration location.
 /// Before:
 /// a.h:
@@ -436,6 +456,7 @@
   "Couldn't find semicolon for target declaration.");
 }
 
+auto AddInlineIfNecessary = addInlineIfInHeader(Target);
  

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 233751.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68261

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1864,6 +1864,66 @@
   EXPECT_EQ(apply(Test), Expected) << Test;
 }
 
+TEST_F(DefineInlineTest, AddInline) {
+  llvm::StringMap EditedFiles;
+  ExtraFiles["a.h"] = "void foo();";
+  apply(R"cpp(#include "a.h"
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Check we put inline before cv-qualifiers.
+  ExtraFiles["a.h"] = "const int foo();";
+  apply(R"cpp(#include "a.h"
+  const int fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline const int foo(){}")));
+
+  // No double inline.
+  ExtraFiles["a.h"] = "inline void foo();";
+  apply(R"cpp(#include "a.h"
+  inline void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "inline void foo(){}")));
+
+  // Constexprs don't need "inline".
+  ExtraFiles["a.h"] = "constexpr void foo();";
+  apply(R"cpp(#include "a.h"
+  constexpr void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+   testPath("a.h"), "constexpr void foo(){}")));
+
+  // Class members don't need "inline".
+  ExtraFiles["a.h"] = "struct Foo { void foo(); }";
+  apply(R"cpp(#include "a.h"
+  void Foo::fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("a.h"), "struct Foo { void foo(){} }")));
+
+  // Function template doesn't need to be "inline"d.
+  ExtraFiles["a.h"] = "template  void foo();";
+  apply(R"cpp(#include "a.h"
+  template 
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(
+  testPath("a.h"), "template  void foo(){}")));
+
+  // Specializations needs to be marked "inline".
+  ExtraFiles["a.h"] = R"cpp(
+template  void foo();
+template <> void foo();)cpp";
+  apply(R"cpp(#include "a.h"
+  template <>
+  void fo^o() {})cpp", &EditedFiles);
+  EXPECT_THAT(EditedFiles,
+  testing::ElementsAre(FileWithContents(testPath("a.h"),
+R"cpp(
+template  void foo();
+template <> inline void foo(){})cpp")));
+}
+
 TWEAK_TEST(DefineOutline);
 TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
   FileName = "Test.cpp";
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -32,6 +32,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
+#include "clang/Driver/Types.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
@@ -360,6 +361,25 @@
   return FD->getBeginLoc();
 }
 
+llvm::Optional
+addInlineIfInHeader(const FunctionDecl *FD) {
+  // This includes inline functions and constexpr functions.
+  if (FD->isInlined() || llvm::isa(FD))
+return llvm::None;
+  // Primary template doesn't need inline.
+  if (FD->isTemplated() && !FD->isFunctionTemplateSpecialization())
+return llvm::None;
+
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  llvm::StringRef FileName = SM.getFilename(FD->getLocation());
+
+  // If it is not a header we don't need to mark function as "inline".
+  if (!isHeaderFile(FileName, FD->getASTContext().getLangOpts()))
+return llvm::None;
+
+  return tooling::Replacement(SM, FD->getInnerLocStart(), 0, "inline ");
+}
+
 /// Moves definition of a function/method to its declaration location.
 /// Before:
 /// a.h:
@@ -436,6 +456,7 @@
   "Couldn't find semicolon for target declaration.");
 }
 
+auto AddInlineIfNecessary = addInlineIfInHeader(Target);
 auto ParamReplacements = renameParameters(Target, Sou

[PATCH] D68261: [clangd] Add "inline" keyword to prevent ODR-violations in DefineInline

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60834 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 clang-format.patch 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68261



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

[PATCH] D71455: [NFC] Fix typos in Clangd and Clang

[Kirill Bobyrev via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D71455

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/Transport.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang/include/clang/Index/IndexDataConsumer.h
  clang/lib/Index/IndexingContext.cpp

Index: clang/lib/Index/IndexingContext.cpp
===
--- clang/lib/Index/IndexingContext.cpp
+++ clang/lib/Index/IndexingContext.cpp
@@ -99,7 +99,7 @@
 return;
   reportModuleReferences(Mod->Parent, IdLocs.drop_back(), ImportD,
  DataConsumer);
-  DataConsumer.handleModuleOccurence(ImportD, Mod,
+  DataConsumer.handleModuleOccurrence(ImportD, Mod,
  (SymbolRoleSet)SymbolRole::Reference,
  IdLocs.back());
 }
@@ -145,7 +145,7 @@
   if (ImportD->isImplicit())
 Roles |= (unsigned)SymbolRole::Implicit;
 
-  return DataConsumer.handleModuleOccurence(ImportD, Mod, Roles, Loc);
+  return DataConsumer.handleModuleOccurrence(ImportD, Mod, Roles, Loc);
 }
 
 bool IndexingContext::isTemplateImplicitInstantiation(const Decl *D) {
@@ -443,26 +443,26 @@
   }
 
   IndexDataConsumer::ASTNodeInfo Node{OrigE, OrigD, Parent, ContainerDC};
-  return DataConsumer.handleDeclOccurence(D, Roles, FinalRelations, Loc, Node);
+  return DataConsumer.handleDeclOccurrence(D, Roles, FinalRelations, Loc, Node);
 }
 
 void IndexingContext::handleMacroDefined(const IdentifierInfo &Name,
  SourceLocation Loc,
  const MacroInfo &MI) {
   SymbolRoleSet Roles = (unsigned)SymbolRole::Definition;
-  DataConsumer.handleMacroOccurence(&Name, &MI, Roles, Loc);
+  DataConsumer.handleMacroOccurrence(&Name, &MI, Roles, Loc);
 }
 
 void IndexingContext::handleMacroUndefined(const IdentifierInfo &Name,
SourceLocation Loc,
const MacroInfo &MI) {
   SymbolRoleSet Roles = (unsigned)SymbolRole::Undefinition;
-  DataConsumer.handleMacroOccurence(&Name, &MI, Roles, Loc);
+  DataConsumer.handleMacroOccurrence(&Name, &MI, Roles, Loc);
 }
 
 void IndexingContext::handleMacroReference(const IdentifierInfo &Name,
SourceLocation Loc,
const MacroInfo &MI) {
   SymbolRoleSet Roles = (unsigned)SymbolRole::Reference;
-  DataConsumer.handleMacroOccurence(&Name, &MI, Roles, Loc);
+  DataConsumer.handleMacroOccurrence(&Name, &MI, Roles, Loc);
 }
Index: clang/include/clang/Index/IndexDataConsumer.h
===
--- clang/include/clang/Index/IndexDataConsumer.h
+++ clang/include/clang/Index/IndexDataConsumer.h
@@ -39,14 +39,14 @@
   virtual void setPreprocessor(std::shared_ptr PP) {}
 
   /// \returns true to continue indexing, or false to abort.
-  virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
+  virtual bool handleDeclOccurrence(const Decl *D, SymbolRoleSet Roles,
ArrayRef Relations,
SourceLocation Loc, ASTNodeInfo ASTNode) {
 return true;
   }
 
   /// \returns true to continue indexing, or false to abort.
-  virtual bool handleMacroOccurence(const IdentifierInfo *Name,
+  virtual bool handleMacroOccurrence(const IdentifierInfo *Name,
 const MacroInfo *MI, SymbolRoleSet Roles,
 SourceLocation Loc) {
 return true;
@@ -57,7 +57,7 @@
   /// This will be called for each module reference in an import decl.
   /// For "@import MyMod.SubMod", there will be a call for 'MyMod' with the
   /// 'reference' role, and a call for 'SubMod' with the 'declaration' role.
-  virtual bool handleModuleOccurence(const ImportDecl *ImportD,
+  virtual bool handleModuleOccurrence(const ImportDecl *ImportD,
  const Module *Mod, SymbolRoleSet Roles,
  SourceLocation Loc) {
 return true;
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Ren

[clang-tools-extra] 6b8ff5e - [clangd] Fix windows builds

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-13T10:30:44+01:00
New Revision: 6b8ff5e43b405d255259196b6a53a3b5671aa5c7

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

LOG: [clangd] Fix windows builds

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index ebeea82864cb..8627439d4555 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1865,38 +1865,44 @@ TEST_F(DefineInlineTest, QualifyWithUsingDirectives) {
 }
 
 TEST_F(DefineInlineTest, AddInline) {
+  ExtraArgs.push_back("-fno-delayed-template-parsing");
   llvm::StringMap EditedFiles;
   ExtraFiles["a.h"] = "void foo();";
   apply(R"cpp(#include "a.h"
-  void fo^o() {})cpp", &EditedFiles);
+  void fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
testPath("a.h"), "inline void foo(){}")));
 
   // Check we put inline before cv-qualifiers.
   ExtraFiles["a.h"] = "const int foo();";
   apply(R"cpp(#include "a.h"
-  const int fo^o() {})cpp", &EditedFiles);
+  const int fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
testPath("a.h"), "inline const int foo(){}")));
 
   // No double inline.
   ExtraFiles["a.h"] = "inline void foo();";
   apply(R"cpp(#include "a.h"
-  inline void fo^o() {})cpp", &EditedFiles);
+  inline void fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
testPath("a.h"), "inline void foo(){}")));
 
   // Constexprs don't need "inline".
   ExtraFiles["a.h"] = "constexpr void foo();";
   apply(R"cpp(#include "a.h"
-  constexpr void fo^o() {})cpp", &EditedFiles);
+  constexpr void fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
testPath("a.h"), "constexpr void foo(){}")));
 
   // Class members don't need "inline".
   ExtraFiles["a.h"] = "struct Foo { void foo(); }";
   apply(R"cpp(#include "a.h"
-  void Foo::fo^o() {})cpp", &EditedFiles);
+  void Foo::fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles,
   testing::ElementsAre(FileWithContents(
   testPath("a.h"), "struct Foo { void foo(){} }")));
@@ -1905,7 +1911,8 @@ TEST_F(DefineInlineTest, AddInline) {
   ExtraFiles["a.h"] = "template  void foo();";
   apply(R"cpp(#include "a.h"
   template 
-  void fo^o() {})cpp", &EditedFiles);
+  void fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles,
   testing::ElementsAre(FileWithContents(
   testPath("a.h"), "template  void foo(){}")));
@@ -1916,7 +1923,8 @@ TEST_F(DefineInlineTest, AddInline) {
 template <> void foo();)cpp";
   apply(R"cpp(#include "a.h"
   template <>
-  void fo^o() {})cpp", &EditedFiles);
+  void fo^o() {})cpp",
+&EditedFiles);
   EXPECT_THAT(EditedFiles,
   testing::ElementsAre(FileWithContents(testPath("a.h"),
 R"cpp(



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 233753.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc

Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -0,0 +1,19 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
+#ifdef WITH_SELF_REFERENCE_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  if (n != 0)
+memcpy(dest, from, n);
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_SELF_REFERENCE_DECL | FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
+//
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+// CHECK-SELF-REF-DECL: @llvm.memcpy
+//
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+
+  static char buffer[1];
+  memcpy(buffer, from, 2); // expected-warning {{'memcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isInlineBuiltinDeclaration()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4596,8 +4596,15 @@
 }
 
 static CGCallee EmitDirectCallee(CodeGenFunction &CGF, const FunctionDecl *FD) {
+
   if (auto builtinID = FD->getBuiltinID()) {
-return CGCallee::forBuiltin(builtinID, FD);
+// Replaceable builtin provide their own implementation of a builtin. Unless
+// we are in the builtin implementation itself, don't call the actual
+// builtin. If we are in the builtin implementation, avoid trivial infinite
+// recursion.
+if (!FD->isInlineBuiltinDeclaration() ||
+CGF.CurFn->getName() == FD->getName())
+  return CGCallee::forBuiltin(builtinID, FD);
   }
 
   llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,24 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isInlineBuiltinDeclaration() const {
+  if (!getBuiltinID())
+return false;
+
+  const FunctionDecl *Definition;
+  if (hasBody(Definition)) {
+
+if (!Definition->isInlineSpecified())
+  return false;
+
+const SourceManager &SM = getASTContext().getSourceManager();
+SourceLocation SL = Definition->getLocation();
+if (SL.isValid())
+  return SM.isInSystemHeader(SL);
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides an inline implementation of a builtin.
+  bool isInlineBuiltinDeclaration() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/ma

[PATCH] D70048: [LLD] Add NetBSD support as a new flavor of LLD (nb.lld)

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

We attempted to patch internally LLD rebasing our code for the buildbot 
purposes but LLD without being used rots quickly, see: 
https://reviews.llvm.org/D58892

We use LD as `ld` and there are no plans neither intentions to break it. It was 
already decided internally in the project.

If you insist, I can rewrite nb.lld from C++ to shell and upstream to the LLD 
repository. We require `-fuse-ld=lld` to be rerouted through this shell script 
and lld invocation call it so we can revert back to the concept from 
https://reviews.llvm.org/D69755. Personally I see no gain in this as I prefer 
to rely on LLVM libs at least for Target Triplet detection.

As I can understand that Linux/GNU behavior is promoted (and enforced to other 
ELF OSs) it does not work for us. I propose a patch that keeps intact the 
GNU/Linux code.

If 200 LOC for NetBSD is a show stopper, please see e.g. compiler-rt where we 
maintain 2 orders of magnitude more NetBSD-only LOC.

I try to avoid forking LLD for downstream and this shall be appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70048



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

[PATCH] D71201: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as __attribute__((objc_direct))

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 233756.
teemperor added a comment.

- Removed Objective-C metadata from LLVM test.
- Removed quoted attributes.
- Update DebugInfoFlags.def to be compatible with D68117 
  assuming probinson are applied there.


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

https://reviews.llvm.org/D71201

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenObjC/debug-info-direct-method.m
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/test/DebugInfo/X86/objc_direct.ll

Index: llvm/test/DebugInfo/X86/objc_direct.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/objc_direct.ll
@@ -0,0 +1,114 @@
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s
+
+; Source code to regenerate:
+; __attribute__((objc_root_class))
+; @interface Root
+; - (int)direct_method __attribute__((objc_direct));
+; @end
+;
+; @implementation Root
+; - (int)direct_method __attribute__((objc_direct)) {
+;   return 42;
+; }
+; @end
+;
+; clang -O0 -g -gdwarf-5 direct.m -c
+
+; CHECK: DW_TAG_subprogram [3]
+; CHECK: DW_AT_APPLE_objc_direct
+; CHECK-SAME: DW_FORM_flag_present
+; CHECK: DW_TAG_formal_parameter [4]
+
+; ModuleID = 'direct.bc'
+source_filename = "direct.m"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+%0 = type opaque
+%struct._objc_cache = type opaque
+%struct._class_t = type { %struct._class_t*, %struct._class_t*, %struct._objc_cache*, i8* (i8*, i8*)**, %struct._class_ro_t* }
+%struct._class_ro_t = type { i32, i32, i32, i8*, i8*, %struct.__method_list_t*, %struct._objc_protocol_list*, %struct._ivar_list_t*, i8*, %struct._prop_list_t* }
+%struct.__method_list_t = type { i32, i32, [0 x %struct._objc_method] }
+%struct._objc_method = type { i8*, i8*, i8* }
+%struct._objc_protocol_list = type { i64, [0 x %struct._protocol_t*] }
+%struct._protocol_t = type { i8*, i8*, %struct._objc_protocol_list*, %struct.__method_list_t*, %struct.__method_list_t*, %struct.__method_list_t*, %struct.__method_list_t*, %struct._prop_list_t*, i32, i32, i8**, i8*, %struct._prop_list_t* }
+%struct._ivar_list_t = type { i32, i32, [0 x %struct._ivar_t] }
+%struct._ivar_t = type { i64*, i8*, i8*, i32, i32 }
+%struct._prop_list_t = type { i32, i32, [0 x %struct._prop_t] }
+%struct._prop_t = type { i8*, i8* }
+
+; Function Attrs: noinline optnone ssp uwtable
+define hidden i32 @"\01-[Root direct_method]"(%0* %self, i8* %_cmd) !dbg !24 {
+entry:
+  %retval = alloca i32, align 4
+  %self.addr = alloca %0*, align 8
+  %_cmd.addr = alloca i8*, align 8
+  store %0* %self, %0** %self.addr, align 8
+  call void @llvm.dbg.declare(metadata %0** %self.addr, metadata !25, metadata !DIExpression()), !dbg !27
+  store i8* %_cmd, i8** %_cmd.addr, align 8
+  call void @llvm.dbg.declare(metadata i8** %_cmd.addr, metadata !28, metadata !DIExpression()), !dbg !27
+  %0 = load %0*, %0** %self.addr, align 8, !dbg !30
+  %1 = icmp eq %0* %0, null, !dbg !30
+  br i1 %1, label %objc_direct_method.self_is_nil, label %objc_direct_method.cont, !dbg !30, !prof !31
+
+objc_direct_method.self_is_nil:   ; preds = %entry
+  %2 = bitcast i32* %retval to i8*, !dbg !30
+  call void @llvm.memset.p0i8.i64(i8* align 4 %2, i8 0, i64 4, i1 false), !dbg !30
+  br label %return, !dbg !30
+
+objc_direct_method.cont:  ; preds = %entry
+  store i32 42, i32* %retval, align 4, !dbg !32
+  br label %return, !dbg !32
+
+return:   ; preds = %objc_direct_method.cont, %objc_direct_method.self_is_nil
+  %3 = load i32, i32* %retval, align 4, !dbg !33
+  ret i32 %3, !dbg !33
+}
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #2
+
+attributes #0 = { noinline optnone ssp uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+attributes #2 = { argmemonly nounwind willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!14, !15, !16, !17, !18, !19, !20, !21, !22}
+!llvm.ident = !{!23}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_ObjC, file: !1, producer: "clang version 10.0.0 (https://github.com/llvm/llvm-project d6b2f33e2b6338d24cf756ba220939aecc81210d)", isOptimized: false, runtimeVersion: 2, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+!1 = !DIFile(filename: "direct.m", directory: "/", checksumkind: CSK_MD5, checksum: "6b49fad130344b0011fc0eef65949390")
+!2 = !{}
+!3 = !{!4}
+!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "Root", scope: !1, file: !1, line: 2, 

[PATCH] D71374: Improve support of GNU mempcpy

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D71374#1783032 , @Jim wrote:

> I am curious what is difference of code generation after applying your 
> changes?


Before, when compiling

  #define _GNU_SOURCE
  #include 
  
  void* foo(void* to, void* from, unsigned n) {
return mempcpy(mempcpy(to, from, n), from, n);
  }

We get (clang -O3)

  define i8* @foo(i8*, i8*, i32) #0 {
%4 = alloca i8*, align 8
%5 = alloca i8*, align 8
%6 = alloca i32, align 4
store i8* %0, i8** %4, align 8
store i8* %1, i8** %5, align 8
store i32 %2, i32* %6, align 4
%7 = load i8*, i8** %4, align 8
%8 = load i8*, i8** %5, align 8
%9 = load i32, i32* %6, align 4
%10 = zext i32 %9 to i64
%11 = call i8* @mempcpy(i8* %7, i8* %8, i64 %10) #2
%12 = load i8*, i8** %5, align 8
%13 = load i32, i32* %6, align 4
%14 = zext i32 %13 to i64
%15 = call i8* @mempcpy(i8* %11, i8* %12, i64 %14) #2
ret i8* %15
  }

And we now get

  define dso_local i8* @foo(i8* %to, i8* nocapture readonly %from, i32 %n) 
local_unnamed_addr #0 {
  entry:
%conv = zext i32 %n to i64
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %to, i8* align 1 
%from, i64 %conv, i1 false)
%0 = getelementptr i8, i8* %to, i64 %conv
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %0, i8* align 1 
%from, i64 %conv, i1 false)
%1 = getelementptr i8, i8* %0, i64 %conv
ret i8* %1
  }

Which looks much better to me, esp. as it unlocks memcpy-specific optimisations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71374: Improve support of GNU mempcpy

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

xbolva00 added a comment.

LLVM already converts mempcpy to memcpy..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71458: [ARM][MVE] Add intrinsics for more immediate shifts.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: dmgreen, MarkMurrayARM, miyuki, ostannard.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls.
Herald added projects: clang, LLVM.

This fills in the remaining shift operations that take a single vector
input and an immediate shift count: the `vqshl`, `vqshlu`, `vrshr` and
`vshll[bt]` families.

`vshll[bt]` (which shifts each input lane left into a double-width
output lane) is the most interesting one. There are separate MC
instruction ids for shifting by exactly the input lane width and
shifting by less than that, because the instruction encoding is so
completely different for the lane-width special case. So I had to
write two sets of patterns to match based on the immediate shift
count, which involved adding a ComplexPattern matcher to avoid the
general-case pattern accidentally matching the special case too. For
that family I've made sure to add an llc codegen test for both
versions of each instruction.

I'm experimenting with a new strategy for parametrising the isel
patterns for all these instructions: adding extra fields to the
relevant `Instruction` subclass itself, which are ignored by the
Tablegen backends that generate the MC data, but can be retrieved from
each instance of that instruction subclass when it's passed as a
template parameter to the multiclass that generates its isel patterns.
A nice effect of that is that I can fill in those informational fields
using `let` blocks, rather than having to type them out once per
instruction at `defm` time.

(As a result, quite a lot of existing instruction `def`s are
reindented by this patch, so it's clearer to read with whitespace
changes ignored.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71458

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll
===
--- llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll
@@ -385,6 +385,1058 @@
   ret <4 x i32> %2
 }
 
+define arm_aapcs_vfpcc <16 x i8> @test_vqshlq_n_s8(<16 x i8> %a) {
+; CHECK-LABEL: test_vqshlq_n_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.s8 q0, q0, #3
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vqshl.imm.v16i8(<16 x i8> %a, i32 3, i32 0)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vqshlq_n_s16(<8 x i16> %a) {
+; CHECK-LABEL: test_vqshlq_n_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.s16 q0, q0, #4
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vqshl.imm.v8i16(<8 x i16> %a, i32 4, i32 0)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vqshlq_n_s32(<4 x i32> %a) {
+; CHECK-LABEL: test_vqshlq_n_s32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.s32 q0, q0, #4
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vqshl.imm.v4i32(<4 x i32> %a, i32 4, i32 0)
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vqshlq_n_u8(<16 x i8> %a) {
+; CHECK-LABEL: test_vqshlq_n_u8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.u8 q0, q0, #0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vqshl.imm.v16i8(<16 x i8> %a, i32 0, i32 1)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vqshlq_n_u16(<8 x i16> %a) {
+; CHECK-LABEL: test_vqshlq_n_u16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.u16 q0, q0, #13
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vqshl.imm.v8i16(<8 x i16> %a, i32 13, i32 1)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vqshlq_n_u32(<4 x i32> %a) {
+; CHECK-LABEL: test_vqshlq_n_u32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.u32 q0, q0, #6
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vqshl.imm.v4i32(<4 x i32> %a, i32 6, i32 1)
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vqshluq_n_s8(<16 x i8> %a) {
+; CHECK-LABEL: test_vqshluq_n_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshlu.s8 q0, q0, #5
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vqshlu.imm.v16i8(<16 x i8> %a, i32 5)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vqshluq_n_s16(<8 x i16> %a) {
+; CHECK-LABEL: test_vqshluq_n_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshlu.s16 q0, q0, #5
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vqshlu.imm.v8i16(<8 x i16> %a, i32 5)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs

[PATCH] D71421: [ARM][MVE][Intrinsics] Add *_x() variants of my *_m() intrinsics.

[Simon Tatham via Phabricator via cfe-commits]

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

LGTM. I must admit that I probably didn't manage to take in every single detail 
of the revised test collection, but the shape of it looks generally nice, and 
everything I spot-checked looked spot-on. And the diffs in actual source files 
are definitely an improvement :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71421



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

[PATCH] D71167: [Driver] Default to -momit-leaf-frame-pointer for AArch64

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

The patch looks structurally fine, but I'm missing the argumentation for 
changing the default and related, the history of why the default is to //not// 
omit the frame pointer on leaf functions.
Can you provide some insight here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71167



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

[PATCH] D71458: [ARM][MVE] Add intrinsics for more immediate shifts.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:2399
+  foreach pred_int = [int_arm_mve_vshll_imm_predicated] in
+  foreach imm = [inst_imm.immediateType] in {
+

simon_tatham wrote:
> MarkMurrayARM wrote:
> > *OUCH* 6 deep :-)
> Yes, but each of those loops is over a length-1 list, so it's not actually 
> taking any time!
> 
> This is just the kind of thing I'd have preferred to do using an actual 
> 'define local variable' command. But Tablegen doesn't have one. If D74107 is 
> approved then I'll be able to convert all of these into `defvar`, along with 
> many other uses of this singleton-foreach idiom.
Ahem. D71407, I mean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71458



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

[PATCH] D71458: [ARM][MVE] Add intrinsics for more immediate shifts.

[Mark Murray via Phabricator via cfe-commits]

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

LGTM




Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:2399
+  foreach pred_int = [int_arm_mve_vshll_imm_predicated] in
+  foreach imm = [inst_imm.immediateType] in {
+

*OUCH* 6 deep :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71458



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

[PATCH] D71458: [ARM][MVE] Add intrinsics for more immediate shifts.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:2399
+  foreach pred_int = [int_arm_mve_vshll_imm_predicated] in
+  foreach imm = [inst_imm.immediateType] in {
+

MarkMurrayARM wrote:
> *OUCH* 6 deep :-)
Yes, but each of those loops is over a length-1 list, so it's not actually 
taking any time!

This is just the kind of thing I'd have preferred to do using an actual 'define 
local variable' command. But Tablegen doesn't have one. If D74107 is approved 
then I'll be able to convert all of these into `defvar`, along with many other 
uses of this singleton-foreach idiom.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71458



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

[PATCH] D71062: [ARM][MVE] Add vector reduction intrinsics with two vector operands

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM accepted this revision.
MarkMurrayARM added a comment.

Tiny observation of types - fix only if you feel like it.




Comment at: clang/include/clang/Basic/arm_mve.td:845
+
+let params = [s16, s32] in {
+defm vmlaldav : MVEBinaryVectorHoriz64;

Types again?



Comment at: clang/include/clang/Basic/arm_mve.td:854
+
+let params = [s32] in {
+defm vrmlaldavh : MVEBinaryVectorHoriz64R;

Types again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71062



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

[PATCH] D71460: [OpenCL] Fix support for cl_khr_mipmap_image_writes

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin created this revision.
AlexeySotkin added reviewers: Anastasia, svenvh, yaxunl, asavonic.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Patch by Ilya Mashkov


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71460

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Headers/opencl-c.h
  clang/test/SemaOpenCL/extension-version.cl


Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/extension-version.cl
+++ clang/test/SemaOpenCL/extension-version.cl
@@ -243,6 +243,18 @@
 #pragma OPENCL EXTENSION cl_khr_mipmap_image : enable
 
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+#ifndef cl_khr_mipmap_image_writes
+#error "Missing cl_khr_mipmap_image_writes define"
+#endif
+#else
+#ifdef cl_khr_mipmap_image_writes
+#error "Incorrect cl_khr_mipmap_image_writes define"
+#endif
+// expected-warning@+2{{unsupported OpenCL extension 
'cl_khr_mipmap_image_writes' - ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : enable
+
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #ifndef cl_khr_srgb_image_writes
 #error "Missing cl_khr_srgb_image_writes define"
 #endif
Index: clang/lib/Headers/opencl-c.h
===
--- clang/lib/Headers/opencl-c.h
+++ clang/lib/Headers/opencl-c.h
@@ -14682,7 +14682,8 @@
 
 // OpenCL Extension v2.0 s9.18 - Mipmaps
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#if defined(cl_khr_mipmap_image_writes)
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : begin
 void __ovld write_imagef(write_only image1d_t image, int coord, int lod, 
float4 color);
 void __ovld write_imagei(write_only image1d_t image, int coord, int lod, int4 
color);
 void __ovld write_imageui(write_only image1d_t image, int coord, int lod, 
uint4 color);
@@ -14699,15 +14700,17 @@
 void __ovld write_imagei(write_only image2d_array_t image_array, int4 coord, 
int lod, int4 color);
 void __ovld write_imageui(write_only image2d_array_t image_array, int4 coord, 
int lod, uint4 color);
 
-void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int 
lod, float color);
-void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, 
int lod, float color);
+void __ovld write_imagef(write_only image2d_depth_t image, int2 coord, int 
lod, float depth);
+void __ovld write_imagef(write_only image2d_array_depth_t image, int4 coord, 
int lod, float depth);
 
 #ifdef cl_khr_3d_image_writes
 void __ovld write_imagef(write_only image3d_t image, int4 coord, int lod, 
float4 color);
 void __ovld write_imagei(write_only image3d_t image, int4 coord, int lod, int4 
color);
 void __ovld write_imageui(write_only image3d_t image, int4 coord, int lod, 
uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : end
+#endif //defined(cl_khr_mipmap_image_writes)
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // Image write functions for half4 type
@@ -14756,7 +14759,8 @@
 #endif //cl_khr_depth_images
 
 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
-#ifdef cl_khr_mipmap_image
+#ifdef cl_khr_mipmap_image_writes
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : begin
 void __ovld write_imagef(read_write image1d_t image, int coord, int lod, 
float4 color);
 void __ovld write_imagei(read_write image1d_t image, int coord, int lod, int4 
color);
 void __ovld write_imageui(read_write image1d_t image, int coord, int lod, 
uint4 color);
@@ -14780,8 +14784,10 @@
 void __ovld write_imagef(read_write image3d_t image, int4 coord, int lod, 
float4 color);
 void __ovld write_imagei(read_write image3d_t image, int4 coord, int lod, int4 
color);
 void __ovld write_imageui(read_write image3d_t image, int4 coord, int lod, 
uint4 color);
-#endif
-#endif //cl_khr_mipmap_image
+#endif //cl_khr_3d_image_writes
+
+#pragma OPENCL EXTENSION cl_khr_mipmap_image_writes : end
+#endif //cl_khr_mipmap_image_writes
 #endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // Image write functions for half4 type
Index: clang/include/clang/Basic/OpenCLExtensions.def
===
--- clang/include/clang/Basic/OpenCLExtensions.def
+++ clang/include/clang/Basic/OpenCLExtensions.def
@@ -70,6 +70,7 @@
 OPENCLEXT_INTERNAL(cl_khr_egl_event, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_egl_image, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
+OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)


Index: clang/

[PATCH] D71374: Improve support of GNU mempcpy

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D71374#1783245 , @xbolva00 wrote:

> LLVM already converts mempcpy to memcpy..


Indeed, the clang version I was using as base reference was clang-9, and the 
mempcpy optimisation at IR level got introduced after that.
Nevertheless, this patch does much more than lowering mempcpy, it also triggers 
new warnings, so It's still useful that clang understands it.

@xbolva00 should I remove the lowering part and leave it to llvm?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71374: Improve support of GNU mempcpy

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

[[edited the example to reflect actual state]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

> Hmm, maybe this feature/suggestion is broken or at least not exactly awesome 
> when it comes to auto-returning functions that are eventually void-returning 
> functions? Now the function definition has no DW_AT_type to override the 
> unspecified_type in the declaration... :/ that's unfortunate (@probinson - 
> thoughts?)

Normally, the DW_AT_specification on the definition would mean, look at the 
declaration for additional attributes, such as DW_AT_type.  However, the 
declaration's unspecified_type means, look at the definition.  The definition 
omits DW_AT_type, therefore the return type is "void".
It's a wee bit circular, but I think it's not unreasonable to expect the 
consumer to figure this out.


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

https://reviews.llvm.org/D70524



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

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782963 , @jdoerfert wrote:

> In D71241#1782668 , @ABataev wrote:
>
> > In D71241#1782650 , @jdoerfert 
> > wrote:
> >
> > > While we talk a lot about what you think is bad about this solution it 
> > > seems we ignore the problems in the current one. Let me summarize a few:
> > >
> > > - Take https://godbolt.org/z/XCjQUA where the wrong function is called in 
> > > the target region (because the "hack" to inject code in the wrong 
> > > definition is not applicable).
> >
> >
> > No time for it, just short answers. No definition for the variant - no 
> > definition for the base.
>
>
> This is perfectly valid code and with the current scheme impossible to 
> support.
>
> >> - Take https://godbolt.org/z/Yi9Lht where the wrong function is called on 
> >> the host (no there is *no* alias hidden)
> > 
> > GlobalAlias can be emitted only for definitions. No definition for variant 
> > - no aliasing.
>
> Exactly, as above, this is a problem.
>
> >> - Take https://godbolt.org/z/2evvtN which shows that the alias solution is 
> >> incompatible with linking.
> > 
> > Undefined behavior according to the standard.
>
> I don't think so. If you do, please reference the rules this would violate.


Page 59, 25-27.

> 
> 
>>> - Take the `construct` context selector and the `begin/end declare variant` 
>>> construct which both cannot be implemented with aliases.
> 
> This can also not be implemented in the alias scheme.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[PATCH] D71111: [Sema] Improve diagnostic about addr spaces for overload candidates

[Anastasia Stulova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGed8dadb37c7e: [Sema] Improve diagnostic about addr spaces 
for overload candidates (authored by Anastasia).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D7?vs=233579&id=233777#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Diagnostic.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/address-space-references.cpp
  clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  clang/test/SemaOpenCLCXX/address-space-lambda.cl
  clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl

Index: clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
===
--- clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
+++ clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
@@ -7,8 +7,8 @@
 };
 
 void bar(__local C*);
-// expected-note@-1{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka '__global C *')), parameter type must be '__local C *'}}
-// expected-note@-2{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka 'C *')), parameter type must be '__local C *'}}
+// expected-note@-1{{candidate function not viable: cannot pass pointer to address space '__global' as a pointer to address space '__local' in 1st argument}}
+// expected-note@-2{{candidate function not viable: cannot pass pointer to default address space as a pointer to address space '__local' in 1st argument}}
 
 __global C Glob;
 void foo(){
Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- clang/test/SemaOpenCLCXX/address-space-lambda.cl
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -12,7 +12,7 @@
   // Test lambda with default parameters
 //CHECK: CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
   [&] {i++;} ();
-  __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 {{.*}})'}}
+  __constant auto err = [&]() {}; //expected-note{{candidate function not viable: 'this' object is in address space '__constant', but method expects object in address space '__generic'}}
   err();  //expected-error-re{{no matching function for call to object of type '__constant (lambda at {{.*}})'}}
   // FIXME: There is very limited addr space functionality
   // we can test when taking lambda type from the object.
@@ -31,19 +31,19 @@
 //CHECK: |-CXXMethodDecl {{.*}} constexpr operator() 'void () const __generic'
   auto priv2 = []() __generic {};
   priv2();
-  auto priv3 = []() __global {}; //expected-note-re{{candidate function not viable: address space mismatch in 'this' argument ('(lambda at {{.*}})'), parameter type must be 'const __global (lambda at {{.*}})'}} //expected-note{{conversion candidate of type 'void (*)()'}}
+  auto priv3 = []() __global {}; //expected-note{{candidate function not viable: 'this' object is in default address space, but method expects object in address space '__global'}} //expected-note{{conversion candidate of type 'void (*)()'}}
   priv3(); //expected-error{{no matching function for call to object of type}}
 
-  __constant auto const1 = []() __private{}; //expected-note-re{{candidate function not viable: address space mismatch in 'this' argument ('__constant (lambda at {{.*}})'), parameter type must be 'const (lambda at {{.*}}'}} //expected-note{{conversion candidate of type 'void (*)()'}}
+  __constant auto const1 = []() __private{}; //expected-note{{candidate function not viable: 'this' object is in address space '__constant', but method expects object in default address space}} //expected-note{{conversion candidate of type 'void (*)()'}}
   const1(); //expected-error{{no matching function for call to object of type '__constant (lambda at}}
-  __constant auto const2 = []() __generic{}; //expected-note-re{{candidate function not viable: address space mismatch in 'this' argument ('__constant (lambda at {{.*}})'), parameter type must be 'const __generic (lambda at {{.*}}'}} //expected-note{{conversion candidate of type 'void (*)()'}}
+  __constant auto const2 = []() __generic{}; //expected-note{{candidate function not viable: 'this' object is in address space '__constant', but method expects object in address space '__generic'}} //expected-note{{conversion candidate of type 'void (*)()'}}
   const2(); //expected-error{{no matching function fo

[clang] ed8dadb - [Sema] Improve diagnostic about addr spaces for overload candidates

[Anastasia Stulova via cfe-commits]

Author: Anastasia Stulova
Date: 2019-12-13T12:35:18Z
New Revision: ed8dadb37c7e1a7f4889d868ac9b19bfe7762237

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

LOG: [Sema] Improve diagnostic about addr spaces for overload candidates

Allow sending address spaces into diagnostics to simplify and improve
error reporting. Improved wording of diagnostics for address spaces
in overloading.

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/AST/Type.h
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ASTDiagnostic.cpp
clang/lib/AST/TypePrinter.cpp
clang/lib/Basic/Diagnostic.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/address-space-references.cpp
clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
clang/test/SemaOpenCLCXX/address-space-lambda.cl
clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl

Removed: 




diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index caf2a3dd79a3..6d1d69bd87a2 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -553,6 +553,8 @@ class Qualifiers {
   std::string getAsString() const;
   std::string getAsString(const PrintingPolicy &Policy) const;
 
+  static std::string getAddrSpaceAsString(LangAS AS);
+
   bool isEmptyWhenPrinted(const PrintingPolicy &Policy) const;
   void print(raw_ostream &OS, const PrintingPolicy &Policy,
  bool appendSpaceIfNonEmpty = false) const;
@@ -6838,6 +6840,23 @@ inline const Type *Type::getPointeeOrArrayElementType() 
const {
 return type->getBaseElementTypeUnsafe();
   return type;
 }
+/// Insertion operator for diagnostics. This allows sending address spaces into
+/// a diagnostic with <<.
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+   LangAS AS) {
+  DB.AddTaggedVal(static_cast>(AS),
+  DiagnosticsEngine::ArgumentKind::ak_addrspace);
+  return DB;
+}
+
+/// Insertion operator for partial diagnostics. This allows sending adress
+/// spaces into a diagnostic with <<.
+inline const PartialDiagnostic &operator<<(const PartialDiagnostic &PD,
+   LangAS AS) {
+  PD.AddTaggedVal(static_cast>(AS),
+  DiagnosticsEngine::ArgumentKind::ak_addrspace);
+  return PD;
+}
 
 /// Insertion operator for diagnostics. This allows sending Qualifiers into a
 /// diagnostic with <<.

diff  --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index 94ae011a0b20..ce996b615bba 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -177,6 +177,9 @@ class DiagnosticsEngine : public 
RefCountedBase {
 /// IdentifierInfo
 ak_identifierinfo,
 
+/// address space
+ak_addrspace,
+
 /// Qualifiers
 ak_qual,
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c91ef356f944..b91fd262c9d3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3987,8 +3987,12 @@ def note_ovl_candidate_bad_lvalue : Note<
 "%select{%ordinal4 argument|object argument}3">;
 def note_ovl_candidate_bad_addrspace : Note<
 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-"address space mismatch in %select{%ordinal6|'this'}5 argument (%3), "
-"parameter type must be %4">;
+"cannot %select{pass pointer to|bind reference in}5 %3 "
+"%select{as a pointer to|to object in}5 %4 in %ordinal6 "
+"argument">;
+def note_ovl_candidate_bad_addrspace_this : Note<
+"candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
+"'this' object is in %3, but method expects object in %4">;
 def note_ovl_candidate_bad_gc : Note<
 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
 "%select{%ordinal7|'this'}6 argument (%3) has %select{no|__weak|__strong}4 
"

diff  --git a/clang/lib/AST/ASTDiagnostic.cpp b/clang/lib/AST/ASTDiagnostic.cpp
index 30985441031d..8ff3cebb1d83 100644
--- a/clang/lib/AST/ASTDiagnostic.cpp
+++ b/clang/lib/AST/ASTDiagnostic.cpp
@@ -338,6 +338,21 @@ void clang::FormatASTNodeDiagnosticArgument(
 
   switch (Kind) {
 default: llvm_unreachable("unknown ArgumentKind");
+case DiagnosticsEngine::ak_addrspace: {
+  assert(Modifier.empty() && Argument.empty() &&
+ "Invalid modifier for Qualfiers argument");
+
+  auto S = Qualifiers::getAddrSpaceAsString(static_cast(Val));
+  if (S.empty()) {
+OS << (Context.getLangOpts().OpenCL ? "default" : "generic");
+

[PATCH] D71433: [analyzer] CERT: POS34-C

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a reviewer: aaron.ballman.
Charusso added a subscriber: aaron.ballman.
Charusso added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:1881
+
+  #include 
+

I would remove that line.



Comment at: clang/docs/analyzer/checkers.rst:1893
+
+This check corresponds to the CERT C Coding Standard rule.
+

I think it is clear it is a CERT-checker.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:765
+  HelpText<"Finds calls to the `putenv` function which pass a pointer to "
+   "an automatic variable as the argument. (CERT POS 34C)">,
+  Documentation;

I would write ##`putenv`## -> `'putenv'` and the CERT rule-number should be 
clear from that point so you could emit it.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:52
+   isa(MSR) ||
+   isa(MSR) || isa(MSR)))
+return;

I think you could shrink that into:
```lang=c
!IsPossiblyAutoVar && !isa(MSR)
```

What you have specified is a negation of 
```lang=c
!isa(MSR) && !isa(MSR) && 
!isa(MSR)
```
~ according to: 
https://clang.llvm.org/doxygen/classclang_1_1ento_1_1MemSpaceRegion.html

where the `CodeSpaceRegion` and `GlobalInternalSpaceRegion` is not that 
interesting for the checker.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:58
+this, "'putenv' function should not be called with auto variables",
+categories::SecurityError));
+  ExplodedNode *N = C.generateErrorNode();

@NoQ, it is from your documentation. Would we prefer that or the 
`registerChecker(CheckerManager &)` is the new way to construct the `BugType`? 
I believe the latter is more appropriate.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:60
+  ExplodedNode *N = C.generateErrorNode();
+  auto Report = llvm::make_unique(*BT, BT->getName(), N);
+  C.emitReport(std::move(Report));

We would like to point out the allocation's site, where it comes from.
We have two facilities to do so: `MallocBugVisitor` to track dynamic allocation 
and `trackExpressionValue()` to track any kind of an expression.

You could find examples from my CERT-checker: D70411. The rough idea is that:
```lang=c
// Track the argument.
if (isa(MSR)) {
  bugreporter::trackExpressionValue(C.getPredecessor(), ArgExpr, *Report);
} else if (const SymbolRef Sym = ArgV.getAsSymbol()) { // It is a 
`HeapSpaceRegion`
  Report->addVisitor(allocation_state::getMallocBRVisitor(Sym));
}
```
~ here you need to copy-paste the `getMallocBRVisitor()` from my review. Sorry!



Comment at: clang/test/Analysis/cert/pos34-c.cpp:4
+// RUN:  -verify %s
+
+#include "../Inputs/system-header-simulator.h"

Could you inject the rule's page please?



Comment at: clang/test/Analysis/cert/pos34-c.cpp:13
+
+// example from cert
+int volatile_memory1(const char *var) {

I would name the rule properly, as a sentence like: `Example from the CERT 
rule's page.`

My idea was that to have a `/cert/pos34-c.cpp` which only consists of the 
rule's page stuff mentioning the rule's page on the top. And having a separate 
file which only consists of arbitrary tests called like 
`/cert/pos34-c-fp-suppression.cpp` as it shows some false-positive suppression 
what you have found on tests or something. Like @aaron.ballman wrote two tests 
in D70823 and I would copy-paste them from his comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71433



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

[PATCH] D70537: [clang] CGDebugInfo asserts `!DT.isNull()` when compiling with debug symbols

[kamlesh kumar via Phabricator via cfe-commits]

kamleshbhalui added a comment.

In D70537#1769458 , @dblaikie wrote:

> I feel like there's something missing about this bug/issue - is there a good 
> explanation/understanding for why does the bug only occur with the two levels 
> of static member inline variable templates & not one? Perhaps there's some 
> existing code that works for simple cases but is insufficiently general and 
> should be modified to be so, rather than adding a new case?


Here is the situation which is occuring in the particuler case(referring added 
testcase pr42710.cpp),

while instantiating the template variable `value` it also needs the type to 
declare it 
(see at 
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaTemplateInstantiateDecl.cpp#L4590)
 and it sees that it is undeduced type , so it starts instantiating the
initialization expression and  in order to doing so it comes to at this point
https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CGDebugInfo.cpp#L1424
which iterates over all the  VarDecl in record and try to generate debug info.
again here template variable `value` comes , which was undeduced and fails to 
unwrap the type and finally assert fails.


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

https://reviews.llvm.org/D70537



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

[PATCH] D71374: Improve support of GNU mempcpy

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

xbolva00 added a comment.

> @xbolva00 should I remove the lowering part and leave it to llvm?

Probably okay to leave it + inbounds, but please add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71460: [OpenCL] Fix support for cl_khr_mipmap_image_writes

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added a comment.

What about `get_image_num_mip_levels` functions defined in the extension 
specification?




Comment at: clang/lib/Headers/opencl-c.h:14780
 
 void __ovld write_imagef(read_write image2d_depth_t image, int2 coord, int 
lod, float color);
 void __ovld write_imagef(read_write image2d_array_depth_t image, int4 coord, 
int lod, float color);

Shouldn't "color" be renamed to "depth" here as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71460



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

Re: [PATCH] D71439: Allow redeclaration of __declspec(uuid)

[Zachary Henkel via cfe-commits]

Same as my other pending patch. I'll need someone else to submit on my behalf 
since I don't have commit rights. @reid can that be you?

Sent from my Palm III


From: Reid Kleckner via Phabricator 
Sent: Thursday, December 12, 2019 6:29:57 PM
To: Zachary Henkel ; h...@chromium.org 
; ztur...@roblox.com 
Cc: cfe-commits@lists.llvm.org ; 
mlek...@skidmore.edu ; blitzrak...@gmail.com 
; shen...@google.com 
Subject: [PATCH] D71439: Allow redeclaration of __declspec(uuid)

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

Seems reasonable, looks good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD71439%2Fnew%2F&data=02%7C01%7Czachary.henkel%40microsoft.com%7C6dcc6227975b4a827b8808d77f6395e5%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637117938012071507&sdata=i2RCVhsNZy658VmJ8zTYtLFTO9KWBc1ankV%2BFBCzPmM%3D&reserved=0

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD71439&data=02%7C01%7Czachary.henkel%40microsoft.com%7C6dcc6227975b4a827b8808d77f6395e5%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637117938012071507&sdata=XLpn61ZLgIesvdDuAWHBRslFCEBqVim09STh2Tqcjw4%3D&reserved=0



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

[clang] 25305a9 - [ARM][MVE] Add intrinsics for more immediate shifts.

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2019-12-13T13:07:39Z
New Revision: 25305a9311d45bc602014b7ee7584e80675aaf59

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

LOG: [ARM][MVE] Add intrinsics for more immediate shifts.

Summary:
This fills in the remaining shift operations that take a single vector
input and an immediate shift count: the `vqshl`, `vqshlu`, `vrshr` and
`vshll[bt]` families.

`vshll[bt]` (which shifts each input lane left into a double-width
output lane) is the most interesting one. There are separate MC
instruction ids for shifting by exactly the input lane width and
shifting by less than that, because the instruction encoding is so
completely different for the lane-width special case. So I had to
write two sets of patterns to match based on the immediate shift
count, which involved adding a ComplexPattern matcher to avoid the
general-case pattern accidentally matching the special case too. For
that family I've made sure to add an llc codegen test for both
versions of each instruction.

I'm experimenting with a new strategy for parametrising the isel
patterns for all these instructions: adding extra fields to the
relevant `Instruction` subclass itself, which are ignored by the
Tablegen backends that generate the MC data, but can be retrieved from
each instance of that instruction subclass when it's passed as a
template parameter to the multiclass that generates its isel patterns.
A nice effect of that is that I can fill in those informational fields
using `let` blocks, rather than having to type them out once per
instruction at `defm` time.

(As a result, quite a lot of existing instruction `def`s are
reindented by this patch, so it's clearer to read with whitespace
changes ignored.)

Reviewers: dmgreen, MarkMurrayARM, miyuki, ostannard

Reviewed By: MarkMurrayARM

Subscribers: kristof.beyls, hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/include/clang/Basic/arm_mve.td
clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
llvm/include/llvm/IR/IntrinsicsARM.td
llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
llvm/lib/Target/ARM/ARMInstrInfo.td
llvm/lib/Target/ARM/ARMInstrMVE.td
llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll

Removed: 




diff  --git a/clang/include/clang/Basic/arm_mve.td 
b/clang/include/clang/Basic/arm_mve.td
index 8bb567c573a3..9d9c067ade1c 100644
--- a/clang/include/clang/Basic/arm_mve.td
+++ b/clang/include/clang/Basic/arm_mve.td
@@ -606,6 +606,47 @@ let params = T.Int in {
   }
 }
 
+let params = T.Int in {
+  def vqshlq_n: Intrinsic $v, $sh, (unsignedflag Scalar))>;
+  def vqshlq_m_n: Intrinsic
+$v, $sh, (unsignedflag Scalar), $pred, $inactive)>;
+
+  let pnt = PNT_NType in {
+def vrshrq_n: Intrinsic $v, $sh, (unsignedflag Scalar))>;
+defm vrshrq: IntrinsicMX
+  $v, $sh, (unsignedflag Scalar), $pred, $inactive), "_n">;
+  }
+}
+
+let params = T.Signed, pnt = PNT_NType in {
+  def vqshluq_n: Intrinsic $v, $sh)>;
+  def vqshluq_m_n: Intrinsic
+$v, $sh, $pred, $inactive)>;
+}
+
+multiclass vshll_imm {
+  let params = !listconcat(T.Int8, T.Int16), pnt = PNT_NType in {
+def _n: Intrinsic
+$v, $sh, (unsignedflag Scalar), top)>;
+defm "": IntrinsicMX
+$v, $sh, (unsignedflag Scalar), top, $pred, $inactive), "_n">;
+  }
+}
+defm vshllbq : vshll_imm<0>;
+defm vshlltq : vshll_imm<1>;
+
 // Base class for the scalar shift intrinsics.
 class ScalarShift:
   Intrinsic 
{

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
index 200273c03654..2128d0801c6a 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
@@ -720,3 +720,918 @@ uint32x4_t test_vshrq_x_n_u32(uint32x4_t a, mve_pred16_t 
p)
 return vshrq_x_n_u32(a, 6, p);
 #endif /* POLYMORPHIC */
 }
+
+// CHECK-LABEL: @test_vqshlq_n_s8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call <16 x i8> 
@llvm.arm.mve.vqshl.imm.v16i8(<16 x i8> [[A:%.*]], i32 3, i32 0)
+// CHECK-NEXT:ret <16 x i8> [[TMP0]]
+//
+int8x16_t test_vqshlq_n_s8(int8x16_t a)
+{
+#ifdef POLYMORPHIC
+return vqshlq_n(a, 3);
+#else /* POLYMORPHIC */
+return vqshlq_n_s8(a, 3);
+#endif /* POLYMORPHIC */
+}
+
+// CHECK-LABEL: @test_vqshlq_n_s16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call <8 x i16> 
@llvm.arm.mve.vqshl.imm.v8i16(<8 x i16> [[A:%.*]], i32 4, i32 0)
+// CHECK-NEXT:ret <8 x i16> [[TMP0]]
+//
+int16x8_t test_vqshlq_n_s16(int16x8_t a)
+{
+#ifdef POLYMORPHIC
+return vqshlq_n(a, 4);
+#else /* POLYMORPHIC */
+return vqshlq_n_s16(a,

[PATCH] D71039: Add support for the MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you also add documentation to the attribute in AttrDocs.td and hook it up 
to the attribute in Attr.td now that we're actually processing these attributes 
rather than ignoring them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71039



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

[PATCH] D71458: [ARM][MVE] Add intrinsics for more immediate shifts.

[Simon Tatham via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG25305a9311d4: [ARM][MVE] Add intrinsics for more immediate 
shifts. (authored by simon_tatham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71458

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll
===
--- llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll
@@ -385,6 +385,1058 @@
   ret <4 x i32> %2
 }
 
+define arm_aapcs_vfpcc <16 x i8> @test_vqshlq_n_s8(<16 x i8> %a) {
+; CHECK-LABEL: test_vqshlq_n_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.s8 q0, q0, #3
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vqshl.imm.v16i8(<16 x i8> %a, i32 3, i32 0)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vqshlq_n_s16(<8 x i16> %a) {
+; CHECK-LABEL: test_vqshlq_n_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.s16 q0, q0, #4
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vqshl.imm.v8i16(<8 x i16> %a, i32 4, i32 0)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vqshlq_n_s32(<4 x i32> %a) {
+; CHECK-LABEL: test_vqshlq_n_s32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.s32 q0, q0, #4
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vqshl.imm.v4i32(<4 x i32> %a, i32 4, i32 0)
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vqshlq_n_u8(<16 x i8> %a) {
+; CHECK-LABEL: test_vqshlq_n_u8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.u8 q0, q0, #0
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vqshl.imm.v16i8(<16 x i8> %a, i32 0, i32 1)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vqshlq_n_u16(<8 x i16> %a) {
+; CHECK-LABEL: test_vqshlq_n_u16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.u16 q0, q0, #13
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vqshl.imm.v8i16(<8 x i16> %a, i32 13, i32 1)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vqshlq_n_u32(<4 x i32> %a) {
+; CHECK-LABEL: test_vqshlq_n_u32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshl.u32 q0, q0, #6
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vqshl.imm.v4i32(<4 x i32> %a, i32 6, i32 1)
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vqshluq_n_s8(<16 x i8> %a) {
+; CHECK-LABEL: test_vqshluq_n_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshlu.s8 q0, q0, #5
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vqshlu.imm.v16i8(<16 x i8> %a, i32 5)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vqshluq_n_s16(<8 x i16> %a) {
+; CHECK-LABEL: test_vqshluq_n_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshlu.s16 q0, q0, #5
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vqshlu.imm.v8i16(<8 x i16> %a, i32 5)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vqshluq_n_s32(<4 x i32> %a) {
+; CHECK-LABEL: test_vqshluq_n_s32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vqshlu.s32 q0, q0, #4
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vqshlu.imm.v4i32(<4 x i32> %a, i32 4)
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vrshrq_n_s8(<16 x i8> %a) {
+; CHECK-LABEL: test_vrshrq_n_s8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vrshr.s8 q0, q0, #4
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <16 x i8> @llvm.arm.mve.vrshr.imm.v16i8(<16 x i8> %a, i32 4, i32 0)
+  ret <16 x i8> %0
+}
+
+define arm_aapcs_vfpcc <8 x i16> @test_vrshrq_n_s16(<8 x i16> %a) {
+; CHECK-LABEL: test_vrshrq_n_s16:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vrshr.s16 q0, q0, #12
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <8 x i16> @llvm.arm.mve.vrshr.imm.v8i16(<8 x i16> %a, i32 12, i32 0)
+  ret <8 x i16> %0
+}
+
+define arm_aapcs_vfpcc <4 x i32> @test_vrshrq_n_s32(<4 x i32> %a) {
+; CHECK-LABEL: test_vrshrq_n_s32:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vrshr.s32 q0, q0, #30
+; CHECK-NEXT:bx lr
+entry:
+  %0 = tail call <4 x i32> @llvm.arm.mve.vrshr.imm.v4i32(<4 x i32> %a, i32 30, i32 0)
+  ret <4 x i32> %0
+}
+
+define arm_aapcs_vfpcc <16 x i8> @test_vrshrq_n_u8(<16 x i8> %a) {
+; CHECK-LABEL: test_vrshrq_n_u8:
+; CHECK:   @ %bb.0: @ %entry
+; CHECK-NEXT:vrshr.u8 q0, q0, #1
+; CHECK-NEXT:bx lr
+entry:
+  %

[PATCH] D71062: [ARM][MVE] Add vector reduction intrinsics with two vector operands

[Mikhail Maltsev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/arm_mve.td:845
+
+let params = [s16, s32] in {
+defm vmlaldav : MVEBinaryVectorHoriz64;

MarkMurrayARM wrote:
> Types again?
I don't it's worth adding a new list to `T` if it is only used in a single 
place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71062



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

[PATCH] D71272: [OpenCL] Pretty print __private addr space

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaOpenCL/access-qualifier.cl:28
 kernel void k1(img1d_wo img) {
-  myRead(img); // expected-error {{passing 'img1d_wo' (aka '__write_only 
image1d_t') to parameter of incompatible type '__read_only image1d_t'}}
+  myRead(img); // expected-error {{passing '__private img1d_wo' (aka 
'__private __write_only image1d_t') to parameter of incompatible type 
'__read_only image1d_t'}}
 }

AlexeySotkin wrote:
> Minor. An error message like this looks a bit confusing to me. User might 
> wonder whether parameters are incompatible because of address space or 
> because of access qualifiers. Should it print `passing '__private img1d_wo' 
> (aka '__private __write_only image1d_t')  to parameter of incompatible type 
> '__private __read_only image1d_t'`? In this case it is clear that there is a 
> mismatch in access qualifiers.
Yes, I agree. However, we are printing the full `QualType` in both places in 
this diagnostics, so the problem is that the addr space is either not deduced 
or being dropped from `QualType` somewhere. I don't think we have got addr 
spaces working yet for all cases correctly and printing `__private` have 
revealed a number of such issues. I suggest however to fix them in isolation 
case by case as we discover them. This commit is already pretty big and I don't 
want to expand it even more.  I have opened a bug to track this issues: 
https://bugs.llvm.org/show_bug.cgi?id=44294
Hopefully we can fix it asap. Does it make sense?


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

https://reviews.llvm.org/D71272



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

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

[Eric Astor via Phabricator via cfe-commits]

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

- Fix tests expecting use of 'r' constraints for offset operands


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,15 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default: llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,8 +279,8 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
+  bool isOffsetOfLocal() const override {
+return isImm() && Imm.LocalRef;
   }
 
   bool needAddressOf() const override {
@@ -613,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 @@
 IOK_LENGTH,
 IOK_SIZE,
 IOK_TYPE,
-IOK_OFFSET
   };
 
   class InfixCalculator {
@@ -326,6 +325,7 @@
 IES_RSHIFT,
 IES_PLUS,
 IES_MINUS,
+IES_OFFSET,
 IES_NOT,
 IES_MULTIPLY,
 IES_DIVIDE,
@@ -350,16 +350,30 @@
 InlineAsmIdentifierInfo Info;
 short BracCount;
 bool MemExpr;
+bool OffsetOperator;
+SMLoc OffsetOperatorLoc;
+
+bool setSymRef(const MCExpr *Val, StringRef ID, StringRef &ErrMsg) {
+  if (Sym) {
+ErrMsg = "cannot use more than one symbol in memory operand";
+return true;
+  }
+  Sym = Val;
+  SymName = ID;
+  return false;
+}
 
   public:
   

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1782586 , @hfinkel wrote:

> In D71241#1782460 , @JonChesterfield 
> wrote:
>
> > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> > > 
> > >   Faithfulness¶
> > >   The AST intends to provide a representation of the program that is 
> > > faithful to the original source. 
> >
> > That's pretty convincing.
>
>
> No, you're misinterpreting the intent of the statement. Here's the entire 
> section...
>
> > Faithfulness
> >  The AST intends to provide a representation of the program that is 
> > faithful to the original source. We intend for it to be possible to write 
> > refactoring tools using only information stored in, or easily 
> > reconstructible from, the Clang AST. This means that the AST representation 
> > should either not desugar source-level constructs to simpler forms, or – 
> > where made necessary by language semantics or a clear engineering tradeoff 
> > – should desugar minimally and wrap the result in a construct representing 
> > the original source form.
> > 
> > For example, CXXForRangeStmt directly represents the syntactic form of a 
> > range-based for statement, but also holds a semantic representation of the 
> > range declaration and iterator declarations. It does not contain a 
> > fully-desugared ForStmt, however.
> > 
> > Some AST nodes (for example, ParenExpr) represent only syntax, and others 
> > (for example, ImplicitCastExpr) represent only semantics, but most nodes 
> > will represent a combination of syntax and associated semantics. 
> > Inheritance is typically used when representing different (but related) 
> > syntaxes for nodes with the same or similar semantics.
>
> First, being "faithful" to the original source means both syntax and 
> semantics. I realize that AST is a somewhat-ambiguous term - we have semantic 
> elements in our AST - but Clang's AST is not just some kind of minimized 
> parse tree. The AST even has semantics-only nodes (e.g., for implicit casts). 
> As you can see, the design considerations here are not just "record the local 
> syntactic elements", but require semantic interpretation of these elements.
>
> Again, Clang's AST is used for various kinds of static analysis tools, and 
> these depend on having overload resolution correctly performed prior to that 
> analysis. This includes overload resolution that depends on context (whether 
> that's qualifications on `this` or host/device in CUDA or anything else).
>
> None of this is to say that we should not record the original spelling of the 
> function call, we should do that *also*, and that should be done when 
> constructing the AST in Sema in addition to performing the variant selection.


You are not corret. Check the implementation of decltype, for example 
https://godbolt.org/z/R76Nn. We keep the original decltype in AST, though we 
could easily lower it to the real type. This is the design of AST - keep it as 
much as possible close to the original code and modify it only if it is 
absolutely impossible (again, check 
https://clang.llvm.org/docs/InternalsManual.html#the-ast-library).

Constexprs are not lowered in AST. They are lowered in place where it is 
required. constexpr is just evaluated. It can be evaluated in Sema, if 
required, or in codegen, in the analysis tool. Check 
https://godbolt.org/z/wr9RFk  as an example. You will see, the constexprs are 
not evaluated in AST, instead AST tries to do everything to keep them in their 
original form.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

tl;dr; LGTM, from my side as long as you are also happy with the extra 
complexity introduced by this to all call sites.

As told in the offline discussions, only problem I have with this approach is 
its intrusiveness. But arguably all of the call sites that cares about what to 
do in such situations already has some sort of handling themselves. So I am OK 
with landing this generic situation and adding some more mental overhead to all 
of the callers, hopefully we could come up with some helpers that would enable 
callers to choose one behavior or the other all the time and get rid of extra 
complexity(not sure if they are likely to stick though).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71272: [OpenCL] Pretty print __private addr space

[Alexey Sotkin via Phabricator via cfe-commits]

AlexeySotkin added inline comments.



Comment at: clang/test/SemaOpenCL/access-qualifier.cl:28
 kernel void k1(img1d_wo img) {
-  myRead(img); // expected-error {{passing 'img1d_wo' (aka '__write_only 
image1d_t') to parameter of incompatible type '__read_only image1d_t'}}
+  myRead(img); // expected-error {{passing '__private img1d_wo' (aka 
'__private __write_only image1d_t') to parameter of incompatible type 
'__read_only image1d_t'}}
 }

Anastasia wrote:
> AlexeySotkin wrote:
> > Minor. An error message like this looks a bit confusing to me. User might 
> > wonder whether parameters are incompatible because of address space or 
> > because of access qualifiers. Should it print `passing '__private img1d_wo' 
> > (aka '__private __write_only image1d_t')  to parameter of incompatible type 
> > '__private __read_only image1d_t'`? In this case it is clear that there is 
> > a mismatch in access qualifiers.
> Yes, I agree. However, we are printing the full `QualType` in both places in 
> this diagnostics, so the problem is that the addr space is either not deduced 
> or being dropped from `QualType` somewhere. I don't think we have got addr 
> spaces working yet for all cases correctly and printing `__private` have 
> revealed a number of such issues. I suggest however to fix them in isolation 
> case by case as we discover them. This commit is already pretty big and I 
> don't want to expand it even more.  I have opened a bug to track this issues: 
> https://bugs.llvm.org/show_bug.cgi?id=44294
> Hopefully we can fix it asap. Does it make sense?
Yes, fair enough.


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

https://reviews.llvm.org/D71272



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

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D71213#1781322 , @gchatelet wrote:

> LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
> warning but it only works when building without `-Wall`.


Did you mean to write _with_ -Wall? I fail to see anything that stops the macro 
from working with -Wall?

Generally, I'd say using that macro a bit more for this kind of thing would be 
a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



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

[PATCH] D67661: [RISCV] Headers: Add Bitmanip extension Clang header files and rvintrin.h

[Scott Egerton via Phabricator via cfe-commits]

s.egerton planned changes to this revision.
s.egerton added a comment.

In D67661#1767141 , @lewis-revill 
wrote:

> So I have a quick comment about this patch, perhaps it might help to get 
> things moving again.
>
> I'd like to see the actual frontend changes, IE separate from the header 
> implementations, to be split into a separate patch. So we can have things 
> like the __riscv_bitmanip macro and the target attribute parsing potentially 
> landed sooner?


Sounds good to me. I'll work on separating these out.


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

https://reviews.llvm.org/D67661



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

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

In D71213#1783467 , @nhaehnle wrote:

> In D71213#1781322 , @gchatelet wrote:
>
> > LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
> > warning but it only works when building without `-Wall`.
>
>
> Did you mean to write _with_ -Wall? I fail to see anything that stops the 
> macro from working with -Wall?
>
> Generally, I'd say using that macro a bit more for this kind of thing would 
> be a good idea.


Sorry I meant `-Werror` which will turn warnings into errors, then deprecation 
messages will break compilation.
I thought it was ON by default but it's not the case 
.
I'll go this route then. Thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



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

[clang] 9757277 - Reland [DataLayout] Fix occurrences that size and range of pointers are assumed to be the same.

[Nicola Zaghen via cfe-commits]

Author: Nicola Zaghen
Date: 2019-12-13T14:30:21Z
New Revision: 97572775d2fe088d8059b3a9423f6d8539fafe33

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

LOG: Reland [DataLayout] Fix occurrences that size and range of pointers are 
assumed to be the same.

GEP index size can be specified in the DataLayout, introduced in D42123. 
However, there were still places
in which getIndexSizeInBits was used interchangeably with getPointerSizeInBits. 
This notably caused issues
with Instcombine's visitPtrToInt; but the unit tests was incorrect, so this 
remained undiscovered.

This fixes the buildbot failures.

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

Patch by Joseph Faulls!

Added: 
llvm/test/Transforms/InstCombine/builtin-object-size-custom-dl.ll
llvm/test/Transforms/InstCombine/stdio-custom-dl.ll

Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
llvm/include/llvm/Analysis/PtrUseVisitor.h
llvm/include/llvm/Analysis/Utils/Local.h
llvm/lib/Analysis/ConstantFolding.cpp
llvm/lib/Analysis/InlineCost.cpp
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Analysis/Loads.cpp
llvm/lib/Analysis/MemoryBuiltins.cpp
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/lib/Analysis/ScalarEvolutionExpander.cpp
llvm/lib/Analysis/ValueTracking.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
llvm/lib/IR/DataLayout.cpp
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/test/Transforms/InstCombine/gep-custom-dl.ll
llvm/test/Transforms/InstCombine/icmp-custom-dl.ll
llvm/test/Transforms/PhaseOrdering/scev-custom-dl.ll
llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index b1dcbc51f58e..b8846162508e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3264,10 +3264,10 @@ static Value *emitPointerArithmetic(CodeGenFunction 
&CGF,
expr->getRHS()))
 return CGF.Builder.CreateIntToPtr(index, pointer->getType());
 
-  if (width != DL.getTypeSizeInBits(PtrTy)) {
+  if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
 // Zero-extend or sign-extend the pointer value according to
 // whether the index is signed or not.
-index = CGF.Builder.CreateIntCast(index, DL.getIntPtrType(PtrTy), isSigned,
+index = CGF.Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
   "idx.ext");
   }
 

diff  --git a/llvm/include/llvm/Analysis/PtrUseVisitor.h 
b/llvm/include/llvm/Analysis/PtrUseVisitor.h
index fbf04c841d30..05bca2226742 100644
--- a/llvm/include/llvm/Analysis/PtrUseVisitor.h
+++ b/llvm/include/llvm/Analysis/PtrUseVisitor.h
@@ -222,9 +222,9 @@ class PtrUseVisitor : protected InstVisitor,
 // offsets on this pointer.
 // FIXME: Support a vector of pointers.
 assert(I.getType()->isPointerTy());
-IntegerType *IntPtrTy = cast(DL.getIntPtrType(I.getType()));
+IntegerType *IntIdxTy = cast(DL.getIndexType(I.getType()));
 IsOffsetKnown = true;
-Offset = APInt(IntPtrTy->getBitWidth(), 0);
+Offset = APInt(IntIdxTy->getBitWidth(), 0);
 PI.reset();
 
 // Enqueue the uses of this pointer.

diff  --git a/llvm/include/llvm/Analysis/Utils/Local.h 
b/llvm/include/llvm/Analysis/Utils/Local.h
index 98b931f93451..ca505960cbeb 100644
--- a/llvm/include/llvm/Analysis/Utils/Local.h
+++ b/llvm/include/llvm/Analysis/Utils/Local.h
@@ -29,15 +29,15 @@ template 
 Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout &DL, User *GEP,
  bool NoAssumptions = false) {
   GEPOperator *GEPOp = cast(GEP);
-  Type *IntPtrTy = DL.getIntPtrType(GEP->getType());
-  Value *Result = Constant::getNullValue(IntPtrTy);
+  Type *IntIdxTy = DL.getIndexType(GEP->getType());
+  Value *Result = Constant::getNullValue(IntIdxTy);
 
   // If the GEP is inbounds, we know that none of the addressing operations 
will
   // overflow in a signed sense.
   bool isInBounds = GEPOp->isInBounds() && !NoAssumptions;
 
   // Build a mask for high order bits.
-  unsigned IntPtrWidth = IntPtrTy->getScalarType()->getIntegerBitWidth();
+  unsigned IntPtrWidth = IntIdxTy->getScalarType()->getIntegerBitWidth();
   uint64_t PtrSizeMask =
   std::numeric_limits::max() >> (64 - IntPtrWidth);
 
@@ -56,17 +56,17 @@ Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout 
&DL, User *GEP,
 Size = DL.getStructLayout(STy)->getElementOffset(OpValue);
 
 if (Size)
-  Result = Builder->CreateAdd(Result, Const

[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaSYCL.cpp:417
+  Util::DeclContextDesc{clang::Decl::Kind::ClassTemplateSpecialization,
+"accessor"}};
+  return matchQualifiedTypeName(Ty, Scopes);

keryell wrote:
> After more thought, perhaps the solution proposed by Victor Lomüller during 
> the last SYCL upstreaming meeting about marking accessor classes with some 
> attribute instead of detecting concrete type names is a better generic 
> approach.
> I am more convinced now by his argument allowing more experimenting with 
> alternative runtimes, since it looks like it is the only place we detect type 
> names. For example the kernels are marked with an attribute in the runtime 
> instead of concretely detecting the `parallel_for()` functions and so on.
@Naghasan , what do you think about upstreaming the solution which you proposed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71016



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Sorry in advance for the reviewer choices... Blame got me you all for all 
touching these things in the early 2010s.  Since then, Richard and I seem to be 
the only one to have touched these types.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: eli.friedman, aaron.ballman, echristo, rjmccall.
Herald added a subscriber: Anastasia.
Herald added a project: clang.
erichkeane added a comment.

Sorry in advance for the reviewer choices... Blame got me you all for all 
touching these things in the early 2010s.  Since then, Richard and I seem to be 
the only one to have touched these types.


GCC supports the conditional operator on VectorTypes that acts as a
'select' in C++ mode. This patch implements the support. Types are
converted as closely to GCC's behavior as possible, though in a few
places consistency with our existing vector type support was preferred.

Note that this implementation is different from the OpenCL version in a
number of ways, so it unfortunately required a different implementation.

First, the SEMA rules and promotion rules are significantly different.

Secondly, GCC implements COND[i] != 0 ? LHS[i] : RHS[i] (where i is in
the range 0- VectorSize, for each element).  In OpenCL, the condition is
COND[i] < 0 ? LHS[i]: RHS[i].

In the process of implementing this, it was also required to make the
expression COND ? LHS : RHS type dependent if COND is type dependent,
since the type is now dependent on the condition.  For example:

  T ? 1 : 2;

Is not typically type dependent, since the result can be deduced from
the operands.  HOWEVER, if T is a VectorType now, it could change this
to a 'select' (basically a swizzle with a non-constant mask) with the 1
and 2 being promoted to vectors themselves.

While this is a change, it is NOT a standards incompatible change. Based
on my (and D. Gregor's, at the time of writing the code) reading of the
standard, the expression is supposed to be type dependent if ANY
sub-expression is type dependent.


Repository:
  rC Clang

https://reviews.llvm.org/D71463

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/SemaCXX/vector-conditional.cpp

Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using FourShorts = short __attribute__((__vector_size__(8)));
+using TwoInts = int __attribute__((__vector_size__(8)));
+using TwoUInts = unsigned __attribute__((__vector_size__(8)));
+using FourInts = int __attribute__((__vector_size__(16)));
+using FourUInts = unsigned __attribute__((__vector_size__(16)));
+using TwoLongLong = long long __attribute__((__vector_size__(16)));
+using FourLongLong = long long __attribute__((__vector_size__(32)));
+using TwoFloats = float __attribute__((__vector_size__(8)));
+using FourFloats = float __attribute__((__vector_size__(16)));
+using TwoDoubles = double __attribute__((__vector_size__(16)));
+using FourDoubles = double __attribute__((__vector_size__(32)));
+
+FourShorts four_shorts;
+TwoInts two_ints;
+TwoUInts two_uints;
+FourInts four_ints;
+FourUInts four_uints;
+TwoLongLong two_ll;
+FourLongLong four_ll;
+TwoFloats two_floats;
+FourFloats four_floats;
+TwoDoubles two_doubles;
+FourDoubles four_doubles;
+
+enum E {};
+enum class SE {};
+E e;
+SE se;
+
+// Check the rules of the condition of the conditional operator.
+void Condition() {
+  // Only int types are allowed here, the rest should fail to convert to bool.
+  (void)(four_floats ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+  (void)(two_doubles ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+}
+
+// Check the rules of the LHS/RHS of the conditional operator.
+void Operands() {
+  (void)(four_ints ? four_ints : throw 1); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? throw 1 : four_ints); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ?: throw 1);// expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? (void)1 : four_ints); // expected-error {{GNU vector conditional operand cannot be void}}
+  (void)(four_ints ?: (void)1);// expected-error {{GNU vector conditional operand cannot be void}}
+
+  // Vector types must be the same element size as the condition.
+  (void)(four_ints ? two_ll : two_ll); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoLongLong' (vector of 2 'long long' values) do not

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71241#1783444 , @ABataev wrote:

> In D71241#1782586 , @hfinkel wrote:
>
> > In D71241#1782460 , 
> > @JonChesterfield wrote:
> >
> > > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> > > > 
> > > >   Faithfulness¶
> > > >   The AST intends to provide a representation of the program that is 
> > > > faithful to the original source. 
> > >
> > > That's pretty convincing.
> >
> >
> > No, you're misinterpreting the intent of the statement. Here's the entire 
> > section...
> >
> > > Faithfulness
> > >  The AST intends to provide a representation of the program that is 
> > > faithful to the original source. We intend for it to be possible to write 
> > > refactoring tools using only information stored in, or easily 
> > > reconstructible from, the Clang AST. This means that the AST 
> > > representation should either not desugar source-level constructs to 
> > > simpler forms, or – where made necessary by language semantics or a clear 
> > > engineering tradeoff – should desugar minimally and wrap the result in a 
> > > construct representing the original source form.
> > > 
> > > For example, CXXForRangeStmt directly represents the syntactic form of a 
> > > range-based for statement, but also holds a semantic representation of 
> > > the range declaration and iterator declarations. It does not contain a 
> > > fully-desugared ForStmt, however.
> > > 
> > > Some AST nodes (for example, ParenExpr) represent only syntax, and others 
> > > (for example, ImplicitCastExpr) represent only semantics, but most nodes 
> > > will represent a combination of syntax and associated semantics. 
> > > Inheritance is typically used when representing different (but related) 
> > > syntaxes for nodes with the same or similar semantics.
> >
> > First, being "faithful" to the original source means both syntax and 
> > semantics. I realize that AST is a somewhat-ambiguous term - we have 
> > semantic elements in our AST - but Clang's AST is not just some kind of 
> > minimized parse tree. The AST even has semantics-only nodes (e.g., for 
> > implicit casts). As you can see, the design considerations here are not 
> > just "record the local syntactic elements", but require semantic 
> > interpretation of these elements.
> >
> > Again, Clang's AST is used for various kinds of static analysis tools, and 
> > these depend on having overload resolution correctly performed prior to 
> > that analysis. This includes overload resolution that depends on context 
> > (whether that's qualifications on `this` or host/device in CUDA or anything 
> > else).
> >
> > None of this is to say that we should not record the original spelling of 
> > the function call, we should do that *also*, and that should be done when 
> > constructing the AST in Sema in addition to performing the variant 
> > selection.
>
>
> You are not corret. Check the implementation of decltype, for example 
> https://godbolt.org/z/R76Nn. We keep the original decltype in AST, though we 
> could easily lower it to the real type. This is the design of AST - keep it 
> as much as possible close to the original code and modify it only if it is 
> absolutely impossible (again, check 
> https://clang.llvm.org/docs/InternalsManual.html#the-ast-library).


Yes, but our decltype representation is a semantically-accurate representation 
of the source constructs. Your CodeGen approach to variant selection leads to a 
semantically-inaccurate representation: it produces a CallExpr that has a 
DeclRefExpr to the wrong function declaration.

In any case, our decltype representation is fairly analogous to what I proposed 
above. DecltypeType derives from Type, and stores both the original expression 
and the underlying type. If we have an OpenMPVariantCallExpr, it would also 
store a deference to the base function definition, but like desugar() returns 
the "resolved" regular type, OpenMPVariantCallExpr's getCallee() would return 
the resolved function that will be called.

> Constexprs are not lowered in AST. They are lowered in place where it is 
> required. constexpr is just evaluated. It can be evaluated in Sema, if 
> required, or in codegen, in the analysis tool. Check 
> https://godbolt.org/z/wr9RFk  as an example. You will see, the constexprs are 
> not evaluated in AST, instead AST tries to do everything to keep them in 
> their original form.

I'm aware of how this works. If we see a need for a lazy evaluation strategy 
here we can certainly discuss that. And I agree that we should not drop all 
information about the base function. I'm simply saying that's not what 
getCallee() should return. However, what you're doing here is not analogous to 
the evaluation of constexprs. In short, keeping close to the original source 
does justify misrepresenting the semantics.


Repository:
  rG LLVM Github Monorepo

CHANG

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71241#1783499 , @hfinkel wrote:

> In D71241#1783444 , @ABataev wrote:
>
> > In D71241#1782586 , @hfinkel wrote:
> >
> > > In D71241#1782460 , 
> > > @JonChesterfield wrote:
> > >
> > > > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library
> > > > > 
> > > > >   Faithfulness¶
> > > > >   The AST intends to provide a representation of the program that is 
> > > > > faithful to the original source. 
> > > >
> > > > That's pretty convincing.
> > >
> > >
> > > No, you're misinterpreting the intent of the statement. Here's the entire 
> > > section...
> > >
> > > > Faithfulness
> > > >  The AST intends to provide a representation of the program that is 
> > > > faithful to the original source. We intend for it to be possible to 
> > > > write refactoring tools using only information stored in, or easily 
> > > > reconstructible from, the Clang AST. This means that the AST 
> > > > representation should either not desugar source-level constructs to 
> > > > simpler forms, or – where made necessary by language semantics or a 
> > > > clear engineering tradeoff – should desugar minimally and wrap the 
> > > > result in a construct representing the original source form.
> > > > 
> > > > For example, CXXForRangeStmt directly represents the syntactic form of 
> > > > a range-based for statement, but also holds a semantic representation 
> > > > of the range declaration and iterator declarations. It does not contain 
> > > > a fully-desugared ForStmt, however.
> > > > 
> > > > Some AST nodes (for example, ParenExpr) represent only syntax, and 
> > > > others (for example, ImplicitCastExpr) represent only semantics, but 
> > > > most nodes will represent a combination of syntax and associated 
> > > > semantics. Inheritance is typically used when representing different 
> > > > (but related) syntaxes for nodes with the same or similar semantics.
> > >
> > > First, being "faithful" to the original source means both syntax and 
> > > semantics. I realize that AST is a somewhat-ambiguous term - we have 
> > > semantic elements in our AST - but Clang's AST is not just some kind of 
> > > minimized parse tree. The AST even has semantics-only nodes (e.g., for 
> > > implicit casts). As you can see, the design considerations here are not 
> > > just "record the local syntactic elements", but require semantic 
> > > interpretation of these elements.
> > >
> > > Again, Clang's AST is used for various kinds of static analysis tools, 
> > > and these depend on having overload resolution correctly performed prior 
> > > to that analysis. This includes overload resolution that depends on 
> > > context (whether that's qualifications on `this` or host/device in CUDA 
> > > or anything else).
> > >
> > > None of this is to say that we should not record the original spelling of 
> > > the function call, we should do that *also*, and that should be done when 
> > > constructing the AST in Sema in addition to performing the variant 
> > > selection.
> >
> >
> > You are not corret. Check the implementation of decltype, for example 
> > https://godbolt.org/z/R76Nn. We keep the original decltype in AST, though 
> > we could easily lower it to the real type. This is the design of AST - keep 
> > it as much as possible close to the original code and modify it only if it 
> > is absolutely impossible (again, check 
> > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library).
>
>
> Yes, but our decltype representation is a semantically-accurate 
> representation of the source constructs. Your CodeGen approach to variant 
> selection leads to a semantically-inaccurate representation: it produces a 
> CallExpr that has a DeclRefExpr to the wrong function declaration.
>
> In any case, our decltype representation is fairly analogous to what I 
> proposed above. DecltypeType derives from Type, and stores both the original 
> expression and the underlying type. If we have an OpenMPVariantCallExpr, it 
> would also store a deference to the base function definition, but like 
> desugar() returns the "resolved" regular type, OpenMPVariantCallExpr's 
> getCallee() would return the resolved function that will be called.


But we don't need it. We have all the required information in the AST tree. 
Each function has associated list of attributes with the context traits and 
variant function. When you process CallExpr, just check the list of these 
attributes and return the address of the variant function instead of the 
original.

> 
> 
>> Constexprs are not lowered in AST. They are lowered in place where it is 
>> required. constexpr is just evaluated. It can be evaluated in Sema, if 
>> required, or in codegen, in the analysis tool. Check 
>> https://godbolt.org/z/wr9RFk  as an example. You will see, the constexprs 
>> are not evaluated i

[PATCH] D71374: Improve support of GNU mempcpy

[serge via Phabricator via cfe-commits]

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

Added test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/bstring.c
  clang/test/CodeGen/mempcpy-libcall.c

Index: clang/test/CodeGen/mempcpy-libcall.c
===
--- /dev/null
+++ clang/test/CodeGen/mempcpy-libcall.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-llvm < %s| FileCheck %s
+
+typedef __SIZE_TYPE__ size_t;
+
+void *mempcpy(void *, void const *, size_t);
+
+char *test(char *d, char *s, size_t n) {
+  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}} %[[REG1:[^ ]+]], i8* {{.*}} %1, i64 %[[REG2:[^ ]+]], i1 false)
+  // CHECK-NEXT: %[[REGr:[^ ]+]] = getelementptr inbounds i8, i8* %[[REG1]], i64 %[[REG2]]
+  // CHECK-NEXT: ret i8* %[[REGr]]
+  return mempcpy(d, s, n);
+}
Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -222,6 +222,9 @@
   char dst[1];
 
   mempcpy(dst, src, 4); // expected-warning{{Memory copy function overflows destination buffer}}
+#ifndef VARIANT
+// expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 4}}
+#endif
 }
 
 void mempcpy3 () {
@@ -243,6 +246,9 @@
   char dst[3];
 
   mempcpy(dst+2, src+2, 2); // expected-warning{{Memory copy function overflows destination buffer}}
+#ifndef VARIANT
+// expected-warning@-2{{'mempcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
+#endif
 }
 
 void mempcpy6() {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -340,7 +340,8 @@
   case Builtin::BI__builtin___strncat_chk:
   case Builtin::BI__builtin___strncpy_chk:
   case Builtin::BI__builtin___stpncpy_chk:
-  case Builtin::BI__builtin___memccpy_chk: {
+  case Builtin::BI__builtin___memccpy_chk:
+  case Builtin::BI__builtin___mempcpy_chk: {
 DiagID = diag::warn_builtin_chk_overflow;
 IsChkVariant = true;
 SizeIndex = TheCall->getNumArgs() - 2;
@@ -379,7 +380,9 @@
   case Builtin::BImemmove:
   case Builtin::BI__builtin_memmove:
   case Builtin::BImemset:
-  case Builtin::BI__builtin_memset: {
+  case Builtin::BI__builtin_memset:
+  case Builtin::BImempcpy:
+  case Builtin::BI__builtin_mempcpy: {
 DiagID = diag::warn_fortify_source_overflow;
 SizeIndex = TheCall->getNumArgs() - 1;
 ObjectIndex = 0;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2506,7 +2506,9 @@
 return RValue::get(nullptr);
   }
   case Builtin::BImemcpy:
-  case Builtin::BI__builtin_memcpy: {
+  case Builtin::BI__builtin_memcpy:
+  case Builtin::BImempcpy:
+  case Builtin::BI__builtin_mempcpy: {
 Address Dest = EmitPointerWithAlignment(E->getArg(0));
 Address Src = EmitPointerWithAlignment(E->getArg(1));
 Value *SizeVal = EmitScalarExpr(E->getArg(2));
@@ -2515,7 +2517,11 @@
 EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
 E->getArg(1)->getExprLoc(), FD, 1);
 Builder.CreateMemCpy(Dest, Src, SizeVal, false);
-return RValue::get(Dest.getPointer());
+if (BuiltinID == Builtin::BImempcpy ||
+BuiltinID == Builtin::BI__builtin_mempcpy)
+  return RValue::get(Builder.CreateInBoundsGEP(Dest.getPointer(), SizeVal));
+else
+  return RValue::get(Dest.getPointer());
   }
 
   case Builtin::BI__builtin_char_memchr:
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3866,6 +3866,11 @@
   case Builtin::BImemcpy:
 return Builtin::BImemcpy;
 
+  case Builtin::BI__builtin_mempcpy:
+  case Builtin::BI__builtin___mempcpy_chk:
+  case Builtin::BImempcpy:
+return Builtin::BImempcpy;
+
   case Builtin::BI__builtin_memmove:
   case Builtin::BI__builtin___memmove_chk:
   case Builtin::BImemmove:
@@ -3923,6 +3928,8 @@
 return Builtin::BImemset;
   else if (FnInfo->isStr("memcpy"))
 return Builtin::BImemcpy;
+  else if (FnInfo->isStr("mempcpy"))
+return Builtin::BImempcpy;
   else if (FnInfo->isStr("memmove"))
 return Builtin::BImemmove;
   else if (FnInfo->isStr("memcmp"))
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -984,6 +9

[PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Mark Murray via Phabricator via cfe-commits]

MarkMurrayARM created this revision.
MarkMurrayARM added a reviewer: simon_tatham.
Herald added subscribers: cfe-commits, dmgreen, kristof.beyls.
Herald added a project: clang.

I overstepped my reach and generated too many intrinsics; these never
made it into the tests.

Remove these extras. Some needed to be signed-olny, and there were some
possible but unrequired _x variants that needed an extra argument to
IntrinsicMX to allow [de-]selection at compile-time.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71466

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td

Index: clang/include/clang/Basic/arm_mve_defs.td
===
--- clang/include/clang/Basic/arm_mve_defs.td
+++ clang/include/clang/Basic/arm_mve_defs.td
@@ -435,6 +435,7 @@
 // A wrapper to define both _m and _x versions of a predicated
 // intrinsic.
 multiclass IntrinsicMX {
   // The _m variant takes an initial parameter called $inactive, which
@@ -444,15 +445,17 @@
   def "_m" # nameSuffix:
  Intrinsic;
 
-  // The _x variant leaves off that parameter, and simply uses an
-  // undef value of the same type.
-  def "_x" # nameSuffix:
- Intrinsic {
-// Allow overriding of the polymorphic name type, because
-// sometimes the _m and _x variants polymorph differently
-// (typically because the type of the inactive parameter can be
-// used as a disambiguator if it's present).
-let pnt = pnt_x;
+  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+// The _x variant leaves off that parameter, and simply uses an
+// undef value of the same type.
+def "_x" # nameSuffix:
+   Intrinsic {
+  // Allow overriding of the polymorphic name type, because
+  // sometimes the _m and _x variants polymorph differently
+  // (typically because the type of the inactive parameter can be
+  // used as a disambiguator if it's present).
+  let pnt = pnt_x;
+}
   }
 }
 
Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -79,10 +79,6 @@
   (IRInt<"vmulh", [Vector]> $a, $b)>;
 def vrmulhq: Intrinsic $a, $b)>;
-def vqdmulhq: Intrinsic $a, $b)>;
-def vqrdmulhq: Intrinsic $a, $b)>;
 def vmullbq_int: Intrinsic
$a, $b, 0)>;
@@ -90,6 +86,12 @@
   (IRInt<"vmull", [DblVector, Vector]>
$a, $b, 1)>;
 }
+let params = T.Signed in {
+def vqdmulhq: Intrinsic $a, $b)>;
+def vqrdmulhq: Intrinsic $a, $b)>;
+}
 
 let params = T.Poly, overrideKindLetter = "p" in {
 def vmullbq_poly: Intrinsic $a, $b)>;
 }
 
-multiclass VectorVectorArithmetic {
+multiclass VectorVectorArithmetic {
   defm "" : IntrinsicMX $a, $b,
- $pred, $inactive)>;
+ $pred, $inactive),
+wantXVariant>;
 }
 
 multiclass VectorVectorArithmeticBitcast {
@@ -179,16 +182,18 @@
   defm vmaxq : VectorVectorArithmetic<"max_predicated">;
   defm vmulhq : VectorVectorArithmetic<"mulh_predicated">;
   defm vrmulhq : VectorVectorArithmetic<"rmulh_predicated">;
-  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated">;
-  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated">;
-  defm vqaddq : VectorVectorArithmetic<"qadd_predicated">;
+  defm vqaddq : VectorVectorArithmetic<"qadd_predicated", 0>;
   defm vhaddq : VectorVectorArithmetic<"hadd_predicated">;
   defm vrhaddq : VectorVectorArithmetic<"rhadd_predicated">;
-  defm vqsubq : VectorVectorArithmetic<"qsub_predicated">;
+  defm vqsubq : VectorVectorArithmetic<"qsub_predicated", 0>;
   defm vhsubq : VectorVectorArithmetic<"hsub_predicated">;
   defm vmullbq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 0)>;
   defm vmulltq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 1)>;
 }
+let params = T.Signed in {
+  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated", 0>;
+  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated", 0>;
+}
 
 let params = T.Poly, overrideKindLetter = "p" in {
   defm vmullbq_poly : DblVectorVectorArithmetic<"mull_poly_predicated", (u32 0)>;
@@ -594,7 +599,7 @@
   defm vshlq: IntrinsicMX
-   $v, $sh, $pred, $inactive), "_n">;
+   $v, $sh, $pred, $inactive), 1, "_n">;
 
   let pnt = PNT_NType in {
 def vshrq_n: Intrinsic
- $v, $sh, (unsignedflag Scalar), $pred, $inactive), "_n">;
+ $v, $sh, (unsignedflag Scalar), $pred, $inactive), 1, "_n">;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand created this revision.
uweigand added reviewers: kpn, andrew.w.kaylor, craig.topper, cameron.mcinally, 
RKSimon, spatel, rjmccall, rsmith.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Update the IRBuilder to generate constrained FP comparisons in CreateFCmp when 
IsFPConstrained is true, similar to the other places in the IRBuilder.

Also, add a new CreateFCmpS to emit **signaling** FP comparisons, and use it in 
clang where comparisons are supposed to be signaling (currently, only when 
emitting code for the <, <=, >, >= operators).  Most other places are supposed 
to emit quiet comparisons, including the equality comparisons, the various 
builtins like isless, and uses of floating-point values in boolean contexts.  A 
few places that I haven't touched may need some extra thought (e.g. are 
comparisons implicitly generated to implement sanitizer checks supposed to be 
signaling?).

I've noticed two potential problems while implementing this:

- There is currently no way to add fast-math flags to a constrained FP 
comparison, since this is implemented as an intrinsic call that returns a 
boolean type, and FMF are only allowed for calls returning a floating-point 
type.  However, given the discussion around 
https://bugs.llvm.org/show_bug.cgi?id=42179, it seems that FCmp itself really 
shouldn't have any FMF either, so this is probably OK.

- Using builtins like __builtin_isless on a "float" type will implicitly 
convert the float argument to a double; apparently this is because the builtin 
is declared as having a variable argument list?   In any case, that means that 
even though a quiet comparison is generated, the semantics still isn't correct 
for quiet NaNs, as that implicit conversion will already signal an exception.  
This probably needs to be fixed, but I guess that can be done as a separate 
patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71467

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/fpconstrained-cmp.c
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -1161,6 +1161,18 @@
 return MetadataAsValue::get(Context, ExceptMDS);
   }
 
+  Value *getConstrainedFPPredicate(CmpInst::Predicate Predicate) {
+assert(CmpInst::isFPPredicate(Predicate) &&
+   Predicate != CmpInst::FCMP_FALSE &&
+   Predicate != CmpInst::FCMP_TRUE &&
+   "Invalid constrained FP comparison predicate!");
+
+StringRef PredicateStr = CmpInst::getPredicateName(Predicate);
+auto *PredicateMDS = MDString::get(Context, PredicateStr);
+
+return MetadataAsValue::get(Context, PredicateMDS);
+  }
+
 public:
   Value *CreateAdd(Value *LHS, Value *RHS, const Twine &Name = "",
bool HasNUW = false, bool HasNSW = false) {
@@ -2308,12 +2320,41 @@
 
   Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
 const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmp,
+P, LHS, RHS, Name);
+
+if (auto *LC = dyn_cast(LHS))
+  if (auto *RC = dyn_cast(RHS))
+return Insert(Folder.CreateFCmp(P, LC, RC), Name);
+return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
+  }
+
+  Value *CreateFCmpS(CmpInst::Predicate P, Value *LHS, Value *RHS,
+ const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmps,
+P, LHS, RHS, Name);
+
 if (auto *LC = dyn_cast(LHS))
   if (auto *RC = dyn_cast(RHS))
 return Insert(Folder.CreateFCmp(P, LC, RC), Name);
 return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
   }
 
+  CallInst *CreateConstrainedFPCmp(
+  Intrinsic::ID ID, CmpInst::Predicate P, Value *L, Value *R,
+  const Twine &Name = "",
+  Optional Except = None) {
+Value *PredicateV = getConstrainedFPPredicate(P);
+Value *ExceptV = getConstrainedFPExcept(Except);
+
+CallInst *C = CreateIntrinsic(ID, {L->getType()},
+  {L, R, PredicateV, ExceptV}, nullptr, Name);
+setConstrainedFPCallAttr(C);
+return C;
+  }
+
   //======//
   // Instruction creation methods: Other Instructions
   //======//
Index: clang/test/CodeGen/fpconstrained-cmp.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained-cmp.c
@@ -0,0 +1,151 @@
+// RUN: %clang_cc1 -ffp-exception-behavior=ignore -emit-llvm -o - %s

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Has anyone actually asked Richard to look at this? He isn't subscribed to the 
diff and may not be watching openmp-dev.

I don't think it's reasonable to stall progress on optimising openmp 
indefinitely. Richard may find it difficult to find time to resolve this. Would 
you accept a time out of a week, after which the majority vote carries it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D71345#1783092 , @sammccall wrote:

> (though personally I'd find it frustrating to have no way to target `b` in 
> `a+b+c`).


(For completeness, there is a way to target `b` in `a+b+c`: by selecting `b` 
(such that your selection range has length 1 rather than 0). Then, neither `+` 
node will enclose the selection range, but the `b` token will.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D71345#1783579 , @nridge wrote:

> (For completeness, there is a way to target `b` in `a+b+c`: by selecting `b` 
> (such that your selection range has length 1 rather than 0). Then, neither 
> `+` node will enclose the selection range, but the `b` node will.)


yes but this doesn't work with most of the LSP features, e.g. go-to-def, hover, 
etc.




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:443
+break;
+  } else {
+Effect = T.takeError();

nit: no need for else


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D71345#1783579 , @nridge wrote:

> In D71345#1783092 , @sammccall wrote:
>
> > (though personally I'd find it frustrating to have no way to target `b` in 
> > `a+b+c`).
>
>
> (For completeness, there is a way to target `b` in `a+b+c`: by selecting `b` 
> (such that your selection range has length 1 rather than 0). Then, neither 
> `+` node will enclose the selection range, but the `b` node will.)


Indeed, sorry - I meant that if we incorporated it into an LSP server, there'd 
be no way to target it in methods that take a position rather than a selection 
(go to defn, hover etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Sam McCall via Phabricator via cfe-commits]

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

OK, I'm going to land this with some reservations. It's a mess, but the number 
of callsites isn't overwhelming.
I'd been hoping to get away with bias-right, but we keep getting complaints 
about this.
It's hard to find an approach that solves this, doesn't obviously break other 
things, and is close enough to what we do now.
Happy to revisit in future, would be nice to get rid of this wart.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[clang] 0eb0992 - [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Mark Murray via cfe-commits]

Author: Mark Murray
Date: 2019-12-13T15:51:31Z
New Revision: 0eb0992739189dba0d86af33722bc27260a9b555

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

LOG: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

Summary:
I overstepped my reach and generated too many intrinsics; these never
made it into the tests.

Remove these extras. Some needed to be signed-olny, and there were some
possible but unrequired _x variants that needed an extra argument to
IntrinsicMX to allow [de-]selection at compile-time.

Reviewers: simon_tatham

Subscribers: kristof.beyls, dmgreen, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td

Removed: 




diff  --git a/clang/include/clang/Basic/arm_mve.td 
b/clang/include/clang/Basic/arm_mve.td
index 6a27bdb807a9..15ed4ce83f01 100644
--- a/clang/include/clang/Basic/arm_mve.td
+++ b/clang/include/clang/Basic/arm_mve.td
@@ -79,10 +79,6 @@ def vmulhq: Intrinsic $a, $b)>;
 def vrmulhq: Intrinsic $a, $b)>;
-def vqdmulhq: Intrinsic $a, $b)>;
-def vqrdmulhq: Intrinsic $a, $b)>;
 def vmullbq_int: Intrinsic
$a, $b, 0)>;
@@ -90,6 +86,12 @@ def vmulltq_int: Intrinsic
$a, $b, 1)>;
 }
+let params = T.Signed in {
+def vqdmulhq: Intrinsic $a, $b)>;
+def vqrdmulhq: Intrinsic $a, $b)>;
+}
 
 let params = T.Poly, overrideKindLetter = "p" in {
 def vmullbq_poly: Intrinsic $a, $b)>;
 }
 
-multiclass VectorVectorArithmetic {
+multiclass VectorVectorArithmetic {
   defm "" : IntrinsicMX $a, $b,
- $pred, $inactive)>;
+ $pred, $inactive),
+wantXVariant>;
 }
 
 multiclass VectorVectorArithmeticBitcast {
@@ -179,16 +182,18 @@ let params = T.Int in {
   defm vmaxq : VectorVectorArithmetic<"max_predicated">;
   defm vmulhq : VectorVectorArithmetic<"mulh_predicated">;
   defm vrmulhq : VectorVectorArithmetic<"rmulh_predicated">;
-  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated">;
-  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated">;
-  defm vqaddq : VectorVectorArithmetic<"qadd_predicated">;
+  defm vqaddq : VectorVectorArithmetic<"qadd_predicated", 0>;
   defm vhaddq : VectorVectorArithmetic<"hadd_predicated">;
   defm vrhaddq : VectorVectorArithmetic<"rhadd_predicated">;
-  defm vqsubq : VectorVectorArithmetic<"qsub_predicated">;
+  defm vqsubq : VectorVectorArithmetic<"qsub_predicated", 0>;
   defm vhsubq : VectorVectorArithmetic<"hsub_predicated">;
   defm vmullbq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 0)>;
   defm vmulltq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 1)>;
 }
+let params = T.Signed in {
+  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated", 0>;
+  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated", 0>;
+}
 
 let params = T.Poly, overrideKindLetter = "p" in {
   defm vmullbq_poly : DblVectorVectorArithmetic<"mull_poly_predicated", (u32 
0)>;
@@ -594,7 +599,7 @@ let params = T.Int in {
   defm vshlq: IntrinsicMX
-   $v, $sh, $pred, $inactive), "_n">;
+   $v, $sh, $pred, $inactive), 1, "_n">;
 
   let pnt = PNT_NType in {
 def vshrq_n: Intrinsic
- $v, $sh, (unsignedflag Scalar), $pred, $inactive), "_n">;
+ $v, $sh, (unsignedflag Scalar), $pred, $inactive), 1, "_n">;
   }
 }
 

diff  --git a/clang/include/clang/Basic/arm_mve_defs.td 
b/clang/include/clang/Basic/arm_mve_defs.td
index 03472fb47b6c..939d5eb0cd6b 100644
--- a/clang/include/clang/Basic/arm_mve_defs.td
+++ b/clang/include/clang/Basic/arm_mve_defs.td
@@ -440,6 +440,7 @@ class NameOverride {
 // A wrapper to define both _m and _x versions of a predicated
 // intrinsic.
 multiclass IntrinsicMX {
   // The _m variant takes an initial parameter called $inactive, which
@@ -449,15 +450,17 @@ multiclass IntrinsicMX;
 
-  // The _x variant leaves off that parameter, and simply uses an
-  // undef value of the same type.
-  def "_x" # nameSuffix:
- Intrinsic {
-// Allow overriding of the polymorphic name type, because
-// sometimes the _m and _x variants polymorph 
diff erently
-// (typically because the type of the inactive parameter can be
-// used as a disambiguator if it's present).
-let pnt = pnt_x;
+  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+// The _x variant leaves off that parameter, and simply uses an
+// undef value of the same type.
+def "_x" # nameSuffix:
+   Intrinsic {
+  // Allow overriding of the polymorphic name type, because
+  // sometimes the _m and _x variants polymorph 
diff erently
+  // (typical

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Sam McCall via Phabricator via cfe-commits]

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

Bias selection-tree right for Hover.
In VSCode (and presumably other GUI-based editors) the location sent is always
the one on the left of the character under the cursor.

Clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang/lib/Tooling/Syntax/Tokens.cpp

Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -254,7 +254,6 @@
   assert(Loc.isFileID());
   llvm::ArrayRef All =
   Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
-  // Comparing SourceLocations is well-defined within a FileID.
   auto *Right = llvm::partition_point(
   All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
   bool AcceptRight = Right != All.end() && Right->location() <= Loc;
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1971,7 +1971,7 @@
   // Basic check for function body and signature.
   EXPECT_AVAILABLE(R"cpp(
 class Bar {
-  [[void [[f^o^o]]() [[{ return; }
+  [[void [[f^o^o^]]() [[{ return; }
 };
 
 void foo();
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,12 +63,33 @@
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
+// Prepare and apply the specified tweak based on the selection in Input.
+// Returns None if and only if prepare() failed.
+llvm::Optional>
+applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
+   const SymbolIndex *Index) {
+  auto Range = rangeOrPoint(Input);
+  llvm::Optional> Result;
+  SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
+Range.second, [&](SelectionTree ST) {
+  Tweak::Selection S(Index, AST, Range.first,
+ Range.second, std::move(ST));
+  if (auto T = prepareTweak(TweakID, S)) {
+Result = (*T)->apply(S);
+return true;
+  } else {
+llvm::consumeError(T.takeError());
+return false;
+  }
+});
+  return Result;
+}
+
 MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
FileName,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
-  auto Selection = rangeOrPoint(Input);
   TestTU TU;
   TU.Filename = FileName;
   TU.HeaderCode = Header;
@@ -76,12 +97,11 @@
   TU.ExtraArgs = ExtraArgs;
   TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
-  Tweak::Selection S(Index, AST, Selection.first, Selection.second);
-  auto PrepareResult = prepareTweak(TweakID, S);
-  if (PrepareResult)
-return true;
-  llvm::consumeError(PrepareResult.takeError());
-  return false;
+  auto Result = applyTweak(AST, Input, TweakID, Index);
+  // We only care if prepare() succeeded, but must handle Errors.
+  if (Result && !*Result)
+consumeError(Result->takeError());
+  return Result.hasValue();
 }
 
 } // namespace
@@ -90,8 +110,6 @@
  llvm::StringMap *EditedFiles) const {
   std::string WrappedCode = wrap(Context, MarkedCode);
   Annotations Input(WrappedCode);
-  auto Selection = rangeOrPoint(Input);
-
   TestTU TU;
   TU.Filename = FileName;
   TU.HeaderCode = Header;
@@ -99,23 +117,20 @@
   TU.Code = Input.code();
   TU.ExtraArgs = ExtraArgs;
   ParsedAST AST = TU.build();
-  Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second);
 
-  auto T = prepareTweak(Tweak

[PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Mark Murray via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0eb099273918: [ARM][MVE][Intrinsics] remove extraneous 
intrinsics. (authored by MarkMurrayARM).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71466

Files:
  clang/include/clang/Basic/arm_mve.td
  clang/include/clang/Basic/arm_mve_defs.td

Index: clang/include/clang/Basic/arm_mve_defs.td
===
--- clang/include/clang/Basic/arm_mve_defs.td
+++ clang/include/clang/Basic/arm_mve_defs.td
@@ -440,6 +440,7 @@
 // A wrapper to define both _m and _x versions of a predicated
 // intrinsic.
 multiclass IntrinsicMX {
   // The _m variant takes an initial parameter called $inactive, which
@@ -449,15 +450,17 @@
   def "_m" # nameSuffix:
  Intrinsic;
 
-  // The _x variant leaves off that parameter, and simply uses an
-  // undef value of the same type.
-  def "_x" # nameSuffix:
- Intrinsic {
-// Allow overriding of the polymorphic name type, because
-// sometimes the _m and _x variants polymorph differently
-// (typically because the type of the inactive parameter can be
-// used as a disambiguator if it's present).
-let pnt = pnt_x;
+  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
+// The _x variant leaves off that parameter, and simply uses an
+// undef value of the same type.
+def "_x" # nameSuffix:
+   Intrinsic {
+  // Allow overriding of the polymorphic name type, because
+  // sometimes the _m and _x variants polymorph differently
+  // (typically because the type of the inactive parameter can be
+  // used as a disambiguator if it's present).
+  let pnt = pnt_x;
+}
   }
 }
 
Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -79,10 +79,6 @@
   (IRInt<"vmulh", [Vector]> $a, $b)>;
 def vrmulhq: Intrinsic $a, $b)>;
-def vqdmulhq: Intrinsic $a, $b)>;
-def vqrdmulhq: Intrinsic $a, $b)>;
 def vmullbq_int: Intrinsic
$a, $b, 0)>;
@@ -90,6 +86,12 @@
   (IRInt<"vmull", [DblVector, Vector]>
$a, $b, 1)>;
 }
+let params = T.Signed in {
+def vqdmulhq: Intrinsic $a, $b)>;
+def vqrdmulhq: Intrinsic $a, $b)>;
+}
 
 let params = T.Poly, overrideKindLetter = "p" in {
 def vmullbq_poly: Intrinsic $a, $b)>;
 }
 
-multiclass VectorVectorArithmetic {
+multiclass VectorVectorArithmetic {
   defm "" : IntrinsicMX $a, $b,
- $pred, $inactive)>;
+ $pred, $inactive),
+wantXVariant>;
 }
 
 multiclass VectorVectorArithmeticBitcast {
@@ -179,16 +182,18 @@
   defm vmaxq : VectorVectorArithmetic<"max_predicated">;
   defm vmulhq : VectorVectorArithmetic<"mulh_predicated">;
   defm vrmulhq : VectorVectorArithmetic<"rmulh_predicated">;
-  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated">;
-  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated">;
-  defm vqaddq : VectorVectorArithmetic<"qadd_predicated">;
+  defm vqaddq : VectorVectorArithmetic<"qadd_predicated", 0>;
   defm vhaddq : VectorVectorArithmetic<"hadd_predicated">;
   defm vrhaddq : VectorVectorArithmetic<"rhadd_predicated">;
-  defm vqsubq : VectorVectorArithmetic<"qsub_predicated">;
+  defm vqsubq : VectorVectorArithmetic<"qsub_predicated", 0>;
   defm vhsubq : VectorVectorArithmetic<"hsub_predicated">;
   defm vmullbq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 0)>;
   defm vmulltq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 1)>;
 }
+let params = T.Signed in {
+  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated", 0>;
+  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated", 0>;
+}
 
 let params = T.Poly, overrideKindLetter = "p" in {
   defm vmullbq_poly : DblVectorVectorArithmetic<"mull_poly_predicated", (u32 0)>;
@@ -594,7 +599,7 @@
   defm vshlq: IntrinsicMX
-   $v, $sh, $pred, $inactive), "_n">;
+   $v, $sh, $pred, $inactive), 1, "_n">;
 
   let pnt = PNT_NType in {
 def vshrq_n: Intrinsic
- $v, $sh, (unsignedflag Scalar), $pred, $inactive), "_n">;
+ $v, $sh, (unsignedflag Scalar), $pred, $inactive), 1, "_n">;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] b60896f - [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-12-13T16:57:03+01:00
New Revision: b60896fad926754f715acc5d771555577e0f

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

LOG: [clangd] Fall back to selecting token-before-cursor if token-after-cursor 
fails.

Summary:
The problem:

LSP specifies that Positions are between characters. Therefore when a position
(or an empty range) is used to target elements of the source code, there is an
ambiguity - should we look left or right of the cursor?

Until now, SelectionTree resolved this to the right except in trivial cases
(where there's whitespace, semicolon, or eof on the right).
This meant that it's unable to e.g. out-line `int foo^()` today.

Complicating this, LSP notwithstanding the cursor is *on* a character in many
editors (mostly terminal-based). In these cases there's no ambiguity - we must
"look right" - but there's also no way to tell in LSP.

(Several features currently resolve this by using getBeginningOfIdentifier,
which tries to rewind and supports end-of-identifier. But this relies on
raw lexing and is limited and buggy).

Precedent: well - most other languages aren't so full of densely packed symbols
that we might want to target. Bias-towards-identifier works well enough.
MS C++ for vscode seems to mostly use bias-toward-identifier too.
The problem with this solution is it doesn't provide any way to target some
things such as the constructor call in Foo^(bar());

Presented solution:

When an ambiguous selection is found, we generate *both* possible selection
trees. We try to run the feature on the rightward tree first, and then on the
leftward tree if it fails.

This is basically do-what-I-mean, the main downside is the need to do this on
a feature-by-feature basis (because each feature knows what "fail" means).
The most complicated instance of this is Tweaks, where the preferred selection
may vary tweak-by-tweak.

Wrinkles:

While production behavior is pretty consistent, this introduces some
inconsistency in testing, depending whether the interface we're testing is
inside or outside the "retry" wrapper.

In particular, for many features like Hover, the unit tests will show production
behavior, while for Tweaks the harness would have to run the loop itself if
we want this.

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/Selection.h
clang-tools-extra/clangd/SemanticSelection.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index a858680d4067..7934408ff141 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -374,7 +374,8 @@ void ClangdServer::rename(PathRef File, Position Pos, 
llvm::StringRef NewName,
   WorkScheduler.runWithAST("Rename", File, std::move(Action));
 }
 
-static llvm::Expected
+// May generate several candidate selections, due to SelectionTree ambiguity.
+static llvm::Expected>
 tweakSelection(const Range &Sel, const InputsAndAST &AST) {
   auto Begin = positionToOffset(AST.Inputs.Contents, Sel.start);
   if (!Begin)
@@ -382,7 +383,15 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
 return End.takeError();
-  return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
+  std::vector Result;
+  SelectionTree::createEach(AST.AST.getASTContext(), AST.AST.getTokens(),
+*Begin, *End, [&](SelectionTree T) {
+  Result.emplace_back(AST.Inputs.Index, AST.AST,
+  *Begin, *End, std::move(T));
+  return false;
+});
+  assert(!Result.empty() && "Expected at least one SelectionTree");
+  return Result;
 }
 
 void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
@@ -391,12 +400,21 @@ void ClangdServer::enumerateTweaks(PathRef File, Range 
Sel,
  this](Expected InpAST) mutable {
 if (!InpA

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb60896fad926: [clangd] Fall back to selecting 
token-before-cursor if token-after-cursor fails. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D71345?vs=233804&id=233805#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1971,7 +1971,7 @@
   // Basic check for function body and signature.
   EXPECT_AVAILABLE(R"cpp(
 class Bar {
-  [[void [[f^o^o]]() [[{ return; }
+  [[void [[f^o^o^]]() [[{ return; }
 };
 
 void foo();
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,12 +63,33 @@
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
+// Prepare and apply the specified tweak based on the selection in Input.
+// Returns None if and only if prepare() failed.
+llvm::Optional>
+applyTweak(ParsedAST &AST, const Annotations &Input, StringRef TweakID,
+   const SymbolIndex *Index) {
+  auto Range = rangeOrPoint(Input);
+  llvm::Optional> Result;
+  SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Range.first,
+Range.second, [&](SelectionTree ST) {
+  Tweak::Selection S(Index, AST, Range.first,
+ Range.second, std::move(ST));
+  if (auto T = prepareTweak(TweakID, S)) {
+Result = (*T)->apply(S);
+return true;
+  } else {
+llvm::consumeError(T.takeError());
+return false;
+  }
+});
+  return Result;
+}
+
 MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
FileName,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
-  auto Selection = rangeOrPoint(Input);
   TestTU TU;
   TU.Filename = FileName;
   TU.HeaderCode = Header;
@@ -76,12 +97,11 @@
   TU.ExtraArgs = ExtraArgs;
   TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
-  Tweak::Selection S(Index, AST, Selection.first, Selection.second);
-  auto PrepareResult = prepareTweak(TweakID, S);
-  if (PrepareResult)
-return true;
-  llvm::consumeError(PrepareResult.takeError());
-  return false;
+  auto Result = applyTweak(AST, Input, TweakID, Index);
+  // We only care if prepare() succeeded, but must handle Errors.
+  if (Result && !*Result)
+consumeError(Result->takeError());
+  return Result.hasValue();
 }
 
 } // namespace
@@ -90,8 +110,6 @@
  llvm::StringMap *EditedFiles) const {
   std::string WrappedCode = wrap(Context, MarkedCode);
   Annotations Input(WrappedCode);
-  auto Selection = rangeOrPoint(Input);
-
   TestTU TU;
   TU.Filename = FileName;
   TU.HeaderCode = Header;
@@ -99,23 +117,20 @@
   TU.Code = Input.code();
   TU.ExtraArgs = ExtraArgs;
   ParsedAST AST = TU.build();
-  Tweak::Selection S(Index.get(), AST, Selection.first, Selection.second);
 
-  auto T = prepareTweak(TweakID, S);
-  if (!T) {
-llvm::consumeError(T.takeError());
-return "unavailable";
-  }
-  llvm::Expected Result = (*T)->apply(S);
+  auto Result = applyTweak(AST, Input, TweakID, Index.get());
   if (!Result)
-return "fail: " + llvm::toString(Result.takeError());
-  if (Result->ShowMessage)
-return "message:\n" + *Result->ShowMessage;
-  if (Result->ApplyEdits.empty())
+return "unavailable";
+  if (!*Result)
+return "fail: " + llvm::toString(Result->takeError());
+  const auto &Effect = **Result;
+  if ((*Result)->ShowMessage)
+return "message:\n" + *Ef

[clang] 22f8125 - [Tooling/Syntax] Helpers to find spelled tokens touching a location.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-12-13T16:57:03+01:00
New Revision: 22f81250889b2e366187ee1465ba0ec71a6e457d

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

LOG: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

Summary: Useful when positions are used to target nodes, with before/after 
ambiguity.

Reviewers: ilya-biryukov, kbobyrev

Subscribers: cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/Tokens.cpp 
b/clang/lib/Tooling/Syntax/Tokens.cpp
index 7c7a442dff03..96595826ddb7 100644
--- a/clang/lib/Tooling/Syntax/Tokens.cpp
+++ b/clang/lib/Tooling/Syntax/Tokens.cpp
@@ -254,7 +254,6 @@ syntax::spelledTokensTouching(SourceLocation Loc,
   assert(Loc.isFileID());
   llvm::ArrayRef All =
   Tokens.spelledTokens(Tokens.sourceManager().getFileID(Loc));
-  // Comparing SourceLocations is well-defined within a FileID.
   auto *Right = llvm::partition_point(
   All, [&](const syntax::Token &Tok) { return Tok.location() < Loc; });
   bool AcceptRight = Right != All.end() && Right->location() <= Loc;



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

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

[Eric Astor via Phabricator via cfe-commits]

epastor updated this revision to Diff 233806.
epastor added a comment.

- Fix Intel named operator parsing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,15 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default: llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,8 +279,8 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
+  bool isOffsetOfLocal() const override {
+return isImm() && Imm.LocalRef;
   }
 
   bool needAddressOf() const override {
@@ -613,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 @@
 IOK_LENGTH,
 IOK_SIZE,
 IOK_TYPE,
-IOK_OFFSET
   };
 
   class InfixCalculator {
@@ -326,6 +325,7 @@
 IES_RSHIFT,
 IES_PLUS,
 IES_MINUS,
+IES_OFFSET,
 IES_NOT,
 IES_MULTIPLY,
 IES_DIVIDE,
@@ -350,16 +350,30 @@
 InlineAsmIdentifierInfo Info;
 short BracCount;
 bool MemExpr;
+bool OffsetOperator;
+SMLoc OffsetOperatorLoc;
+
+bool setSymRef(const MCExpr *Val, StringRef ID, StringRef &ErrMsg) {
+  if (Sym) {
+ErrMsg = "cannot use more than one symbol in memory operand";
+return true;
+  }
+  Sym = Val;
+  SymName = ID;
+  return false;
+}
 
   public:
 IntelExprStateMachine()
 : State(IES_INIT), PrevState(IES_ERROR), BaseReg(0), IndexReg(0),

[PATCH] D71469: [AArch64] Add sq(r)dmulh_lane(q) LLVM IR intrinsics

[Sanne Wouda via Phabricator via cfe-commits]

sanwou01 created this revision.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, hiraditya, 
kristof.beyls.
Herald added projects: clang, LLVM.
sanwou01 added reviewers: SjoerdMeijer, dmgreen, t.p.northover.

Currently, sqdmulh_lane and friends from the ACLE (implemented in arm_neon.h),
are represented in LLVM IR as a by vector sqdmulh and a vector of (repeated)
indices, like so:

  %shuffle = shufflevector <4 x i16> %v, <4 x i16> undef, <4 x i32> 
  %vqdmulh2.i = tail call <4 x i16> @llvm.aarch64.neon.sqdmulh.v4i16(<4 x i16> 
%a, <4 x i16> %shuffle)

When %v's values are known, the shufflevector is optimized away and we are no
longer able to select the lane variant of sqdmulh in the backend.

This makes it impossible to do a neat trick when using NEON intrinsics: one can
load a number of constants using a single vector load, which are then repeatedly
used to multiply whole vectors by one of the constants. This trick is used for a
nice performance upside (2.5% to 4% on one microbenchmark) in libjpeg-turbo.

This patch adds four LLVM IR intrinsics to the AArch64 backend:

- sqdmulh_lane
- sqdmulh_laneq
- sqrdmulh_lane
- sqrdmulh_laneq.

This prevents the constant propagation when it is not wanted.

In order to represent the type of these intrinsics, the patch adds
LLVMNarrowType and LLVMWideType to the IntrinsicEmitter.  The second parameter
of the 'lane' variants is a vector with the same element type, restricted to a
total of 64 bits.  Similarly, the 'laneq' variants' second parameter is widened
to a total of 128 bits.

The 'lane' variants also need an additional register class.  The second argument
must be in the lower half of the 64-bit NEON register file, but only when
operating on i16 elements.

Note that the existing patterns for shufflevector and sqdmulh into sqdmulh_lane
(etc.) remain, so code that does not rely on NEON intrinsics to generate these
instructions is not affected.

This patch also changes clang to emit the new LLVM IR intrinsics for the
corresponding NEON intrinsics (AArch64 only).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71469

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-neon-2velem.c
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/IR/Function.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64RegisterBankInfo.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.td
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/test/CodeGen/AArch64/arm64-neon-2velem.ll
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -224,7 +224,9 @@
   IIT_SCALABLE_VEC = 43,
   IIT_SUBDIVIDE2_ARG = 44,
   IIT_SUBDIVIDE4_ARG = 45,
-  IIT_VEC_OF_BITCASTS_TO_INT = 46
+  IIT_VEC_OF_BITCASTS_TO_INT = 46,
+  IIT_NARROW_VEC = 47,
+  IIT_WIDE_VEC = 48
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -302,6 +304,10 @@
   Sig.push_back(IIT_SUBDIVIDE4_ARG);
 else if (R->isSubClassOf("LLVMVectorOfBitcastsToInt"))
   Sig.push_back(IIT_VEC_OF_BITCASTS_TO_INT);
+else if (R->isSubClassOf("LLVMNarrowType"))
+  Sig.push_back(IIT_NARROW_VEC);
+else if (R->isSubClassOf("LLVMWideType"))
+  Sig.push_back(IIT_WIDE_VEC);
 else
   Sig.push_back(IIT_ARG);
 return Sig.push_back((Number << 3) | 7 /*IITDescriptor::AK_MatchType*/);
Index: llvm/test/CodeGen/AArch64/arm64-neon-2velem.ll
===
--- llvm/test/CodeGen/AArch64/arm64-neon-2velem.ll
+++ llvm/test/CodeGen/AArch64/arm64-neon-2velem.ll
@@ -9,20 +9,36 @@
 declare <2 x float> @llvm.aarch64.neon.fmulx.v2f32(<2 x float>, <2 x float>)
 
 declare <4 x i32> @llvm.aarch64.neon.sqrdmulh.v4i32(<4 x i32>, <4 x i32>)
+declare <4 x i32> @llvm.aarch64.neon.sqrdmulh.lane.v4i32(<4 x i32>, <2 x i32>, i32)
+declare <4 x i32> @llvm.aarch64.neon.sqrdmulh.laneq.v4i32(<4 x i32>, <4 x i32>, i32)
 
 declare <2 x i32> @llvm.aarch64.neon.sqrdmulh.v2i32(<2 x i32>, <2 x i32>)
+declare <2 x i32> @llvm.aarch64.neon.sqrdmulh.lane.v2i32(<2 x i32>, <2 x i32>, i32)
+declare <2 x i32> @llvm.aarch64.neon.sqrdmulh.laneq.v2i32(<2 x i32>, <4 x i32>, i32)
 
 declare <8 x i16> @llvm.aarch64.neon.sqrdmulh.v8i16(<8 x i16>, <8 x i16>)
+declare <8 x i16> @llvm.aarch64.neon.sqrdmulh.lane.v8i16(<8 x i16>, <4 x i16>, i32)
+declare <8 x i16> @llvm.aarch64.neon.sqrdmulh.laneq.v8i16(<8 x i16>, <8 x i16>, i32)
 
 declare <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16>, <4 x i16>)
+declare <4 x i16> @llvm.aarch64.neon.sqrdmulh.lane.v4i16(<4 x 

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

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

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60824 tests passed, 4 failed 
and 726 were skipped.

  failed: Clang.CodeGen/ms-inline-asm-64.c
  failed: Clang.CodeGen/ms-inline-asm.c
  failed: Clang.SemaTemplate/instantiation-depth-default.cpp
  failed: LLVM.CodeGen/X86/ms-inline-asm.ll

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436



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

[clang] 34536db - Revert "[ARM][MVE][Intrinsics] remove extraneous intrinsics."

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2019-12-13T17:16:13+01:00
New Revision: 34536db7bbe0b8c5f8ffa70df307312b451aca2e

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

LOG: Revert "[ARM][MVE][Intrinsics] remove extraneous intrinsics."

This reverts commit 0eb0992739189dba0d86af33722bc27260a9b555.

The code does not compile:
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/20462

Added: 


Modified: 
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td

Removed: 




diff  --git a/clang/include/clang/Basic/arm_mve.td 
b/clang/include/clang/Basic/arm_mve.td
index 15ed4ce83f01..6a27bdb807a9 100644
--- a/clang/include/clang/Basic/arm_mve.td
+++ b/clang/include/clang/Basic/arm_mve.td
@@ -79,6 +79,10 @@ def vmulhq: Intrinsic $a, $b)>;
 def vrmulhq: Intrinsic $a, $b)>;
+def vqdmulhq: Intrinsic $a, $b)>;
+def vqrdmulhq: Intrinsic $a, $b)>;
 def vmullbq_int: Intrinsic
$a, $b, 0)>;
@@ -86,12 +90,6 @@ def vmulltq_int: Intrinsic
$a, $b, 1)>;
 }
-let params = T.Signed in {
-def vqdmulhq: Intrinsic $a, $b)>;
-def vqrdmulhq: Intrinsic $a, $b)>;
-}
 
 let params = T.Poly, overrideKindLetter = "p" in {
 def vmullbq_poly: Intrinsic $a, $b)>;
 }
 
-multiclass VectorVectorArithmetic {
+multiclass VectorVectorArithmetic {
   defm "" : IntrinsicMX $a, $b,
- $pred, $inactive),
-wantXVariant>;
+ $pred, $inactive)>;
 }
 
 multiclass VectorVectorArithmeticBitcast {
@@ -182,18 +179,16 @@ let params = T.Int in {
   defm vmaxq : VectorVectorArithmetic<"max_predicated">;
   defm vmulhq : VectorVectorArithmetic<"mulh_predicated">;
   defm vrmulhq : VectorVectorArithmetic<"rmulh_predicated">;
-  defm vqaddq : VectorVectorArithmetic<"qadd_predicated", 0>;
+  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated">;
+  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated">;
+  defm vqaddq : VectorVectorArithmetic<"qadd_predicated">;
   defm vhaddq : VectorVectorArithmetic<"hadd_predicated">;
   defm vrhaddq : VectorVectorArithmetic<"rhadd_predicated">;
-  defm vqsubq : VectorVectorArithmetic<"qsub_predicated", 0>;
+  defm vqsubq : VectorVectorArithmetic<"qsub_predicated">;
   defm vhsubq : VectorVectorArithmetic<"hsub_predicated">;
   defm vmullbq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 0)>;
   defm vmulltq_int : DblVectorVectorArithmetic<"mull_int_predicated", (u32 1)>;
 }
-let params = T.Signed in {
-  defm vqdmulhq : VectorVectorArithmetic<"qdmulh_predicated", 0>;
-  defm vqrdmulhq : VectorVectorArithmetic<"qrdmulh_predicated", 0>;
-}
 
 let params = T.Poly, overrideKindLetter = "p" in {
   defm vmullbq_poly : DblVectorVectorArithmetic<"mull_poly_predicated", (u32 
0)>;
@@ -599,7 +594,7 @@ let params = T.Int in {
   defm vshlq: IntrinsicMX
-   $v, $sh, $pred, $inactive), 1, "_n">;
+   $v, $sh, $pred, $inactive), "_n">;
 
   let pnt = PNT_NType in {
 def vshrq_n: Intrinsic
- $v, $sh, (unsignedflag Scalar), $pred, $inactive), 1, "_n">;
+ $v, $sh, (unsignedflag Scalar), $pred, $inactive), "_n">;
   }
 }
 

diff  --git a/clang/include/clang/Basic/arm_mve_defs.td 
b/clang/include/clang/Basic/arm_mve_defs.td
index 939d5eb0cd6b..03472fb47b6c 100644
--- a/clang/include/clang/Basic/arm_mve_defs.td
+++ b/clang/include/clang/Basic/arm_mve_defs.td
@@ -440,7 +440,6 @@ class NameOverride {
 // A wrapper to define both _m and _x versions of a predicated
 // intrinsic.
 multiclass IntrinsicMX {
   // The _m variant takes an initial parameter called $inactive, which
@@ -450,17 +449,15 @@ multiclass IntrinsicMX;
 
-  foreach unusedVar = !if(!eq(wantXVariant, 1), [1], []) in {
-// The _x variant leaves off that parameter, and simply uses an
-// undef value of the same type.
-def "_x" # nameSuffix:
-   Intrinsic {
-  // Allow overriding of the polymorphic name type, because
-  // sometimes the _m and _x variants polymorph 
diff erently
-  // (typically because the type of the inactive parameter can be
-  // used as a disambiguator if it's present).
-  let pnt = pnt_x;
-}
+  // The _x variant leaves off that parameter, and simply uses an
+  // undef value of the same type.
+  def "_x" # nameSuffix:
+ Intrinsic {
+// Allow overriding of the polymorphic name type, because
+// sometimes the _m and _x variants polymorph 
diff erently
+// (typically because the type of the inactive parameter can be
+// used as a disambiguator if it's present).
+let pnt = pnt_x;
   }
 }
 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60850 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D71345#1783587 , @sammccall wrote:

> Indeed, sorry - I meant that if we incorporated it into an LSP server, 
> there'd be no way to target it in methods that take a position rather than a 
> selection (go to defn, hover etc).


Ah, right, I forgot that LSP tends to send positions rather than ranges as 
inputs. (I hope that changes in the future, e.g. an issue 
 is on file 
for hover.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I reverted this change in 34536db7bbe0b8c5f8ffa70df307312b451aca2e 
. This 
change didn't compile: 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/20462. Please 
always run `ninja check-all` before pushing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71466



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

[PATCH] D68362: [libunwind][RISCV] Add 64-bit RISC-V support

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.

LGTM! Thanks for correcting the preprocessor issues!


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

https://reviews.llvm.org/D68362



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

Re: [PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Mark Murray via cfe-commits]

Hi

Sorry about the breakage; a commit crossed my build/check/push run, and I was 
testing a fix when your revert came.

M

On 13/12/2019, 16:24, "Dmitri Gribenko via Phabricator" 
 wrote:

gribozavr2 added a comment.

I reverted this change in 34536db7bbe0b8c5f8ffa70df307312b451aca2e 
. This 
change didn't compile: 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/20462. Please 
always run `ninja check-all` before pushing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71466





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

[PATCH] D64573: [Syntax] Allow to mutate syntax trees

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Generally looks good, just nitpicks.




Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:24
  const clang::TranslationUnitDecl &TU);
+/// Allows to create syntax trees from subtrees not backed by the source code.
+class Factory {

"Creates syntax trees..."



Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:25
+/// Allows to create syntax trees from subtrees not backed by the source code.
+class Factory {
+public:

Why a class to contain static functions? Is it like a namespace, or is there 
some future design intent?..

Or is it for the friendship?



Comment at: clang/include/clang/Tooling/Syntax/Mutations.h:26
+
+/// Either removes a statement or replaces it with an empty statement.
+/// EXPECTS: S->canModify() == true

Removes a statement, or replaces it with an empty statement where one is 
required syntactically.

(provide an explanation why it may sometimes use an empty statement -- I think 
it is not obvious)



Comment at: clang/include/clang/Tooling/Syntax/Mutations.h:33
+
+#endif

Please add the newline.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:87
+  bool isDetached() const;
+  /// Whether the node was created from the AST backed by the source code
+  /// rather than added later through mutation APIs or created with factory

Please clarify what happens to mixed content (some subtrees are from the source 
code, some are synthesized).



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:90
+  /// functions.
+  bool isOriginal() const { return Original; }
+  /// If this function return false, the tree cannot be modified because there

Please also document what `isOriginal` means for syntax nodes that were moved 
within the tree (re-parented). For example, think of a refactoring that swaps 
"then" and "else" branches of an "if" statement.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:123
+  unsigned Original : 1;
+  unsigned CanModify : 1;
 };

Is it worth eagerly computing the "can modify" bit? Mapping expanded tokens to 
spelled is log(n) after all, and doing it for every syntax node can add up to 
nontrivial costs...



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:163
+  /// Only used by MutationsImpl to implement higher-level mutation operations.
+  void replaceChildRangeLowLevel(Node *Before, Node *End, Node *New);
+  friend class MutationsImpl;

s/Before/BeforeBegin/? This way begin/end pairing is more obvious.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:483
+
+syntax::Leaf *syntax::Factory::createPunctuation(syntax::Arena &A,
+ tok::TokenKind K) {

I can imagine that both building the syntax tree from the AST, and synthesizing 
syntax trees from thin air will eventually grow pretty long. So I'd like to 
suggest to split synthesis of syntax trees into a separate file, even though it 
will be pretty short today.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:498
+  return S;
+}

Please add the newline below.



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:20
+struct EnumerateTokenSpans {
+  EnumerateTokenSpans(const syntax::Tree *Root, ProcessTokensFn Callback)
+  : BeginSpan(nullptr), EndSpan(nullptr), SpanIsOriginal(false),

I'd probably change this struct into a function with a local lambda... I feel 
like it is weird to call a constructor just for the side-effects.



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:53
+  const syntax::Token *BeginSpan;
+  const syntax::Token *EndSpan;
+  bool SpanIsOriginal;

BeginOfSpan/EndOfSpan or SpanBegin/SpanEnd sound more natural to me.



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:79
+syntax::computeReplacements(const syntax::Arena &A,
+const syntax::TranslationUnit *TU) {
+  auto &Buffer = A.tokenBuffer();

Why not a reference for the TU?



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:86
+  std::string Replacement;
+  auto emitReplacement = [&](llvm::ArrayRef RemovedRange) {
+if (RemovedRange.empty() && Replacement.empty())

s/RemovedRange/ReplacedRange/ ?



Comment at: clang/lib/Tooling/Syntax/ComputeReplacements.cpp:90
+
+// Removed range points into expanded tokens.
+assert(Buffer.expandedTokens().begin() <= RemovedRange.begin());

Unclear whether this comment is explaining the assumption, or e

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

[Eric Astor via Phabricator via cfe-commits]

epastor updated this revision to Diff 233813.
epastor added a comment.

- Respect correct usage of offset.
- Don't assume implicit sizing on offset.
- Check that offset works in compound expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,15 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default: llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,8 +279,8 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
+  bool isOffsetOfLocal() const override {
+return isImm() && Imm.LocalRef;
   }
 
   bool needAddressOf() const override {
@@ -613,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 @@
 IOK_LENGTH,
 IOK_SIZE,
 IOK_TYPE,
-IOK_OFFSET
   };
 
   class InfixCalculator 

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

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

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 clang-format.patch 
,
 test-results.xml 
,
 diff.json 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71436



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

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71241#1783362 , @ABataev wrote:

> >>> - Take https://godbolt.org/z/2evvtN which shows that the alias solution 
> >>> is incompatible with linking.
> >> 
> >> Undefined behavior according to the standard.
> > 
> > I don't think so. If you do, please reference the rules this would violate.
>
> Page 59, 25-27.


OpenMP 5.0, Page 59, 25-27:

> • If the function has any declarations, then the declare variant directives 
> for any declarations that have one must be equivalent. If the function 
> definition has a declare variant, it must also be equivalent. Otherwise, the 
> result is unspecified.

In the example (https://godbolt.org/z/2evvtN), all `declare variant` directives 
on declarations *that have one* are equivalent, since only on declaration has a 
`declare variant`. Since the function definition does not have a `declare 
variant`, the second restriction is also fulfilled.

To summarize, this example is broken in the current scheme and valid according 
to the spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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

[clang] a2cd460 - [ARM][MVE][Intrinsics] All vqdmulhq/vqrdmulhq tests should be for signed numbers.

[Mark Murray via cfe-commits]

Author: Mark Murray
Date: 2019-12-13T17:29:59Z
New Revision: a2cd4600ec6710f3218f071128e2a81edd23a2b2

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

LOG: [ARM][MVE][Intrinsics] All vqdmulhq/vqrdmulhq tests should be for signed 
numbers.

Fix broken tests. I can't yet explain how they worked locally pre-commit.

Added: 


Modified: 
clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
llvm/test/CodeGen/Thumb2/mve-intrinsics/vqdmulhq.ll
llvm/test/CodeGen/Thumb2/mve-intrinsics/vqrdmulhq.ll

Removed: 




diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
index 05e890e40078..eb7e0a0afdf3 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
@@ -4,17 +4,17 @@
 
 #include 
 
-// CHECK-LABEL: @test_vqdmulhq_u8(
+// CHECK-LABEL: @test_vqdmulhq_s8(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = call <16 x i8> 
@llvm.arm.mve.vqdmulh.v16i8(<16 x i8> [[A:%.*]], <16 x i8> [[B:%.*]])
 // CHECK-NEXT:ret <16 x i8> [[TMP0]]
 //
-uint8x16_t test_vqdmulhq_u8(uint8x16_t a, uint8x16_t b)
+int8x16_t test_vqdmulhq_s8(int8x16_t a, int8x16_t b)
 {
 #ifdef POLYMORPHIC
 return vqdmulhq(a, b);
 #else /* POLYMORPHIC */
-return vqdmulhq_u8(a, b);
+return vqdmulhq_s8(a, b);
 #endif /* POLYMORPHIC */
 }
 
@@ -32,17 +32,17 @@ int16x8_t test_vqdmulhq_s16(int16x8_t a, int16x8_t b)
 #endif /* POLYMORPHIC */
 }
 
-// CHECK-LABEL: @test_vqdmulhq_u32(
+// CHECK-LABEL: @test_vqdmulhq_s32(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = call <4 x i32> @llvm.arm.mve.vqdmulh.v4i32(<4 
x i32> [[A:%.*]], <4 x i32> [[B:%.*]])
 // CHECK-NEXT:ret <4 x i32> [[TMP0]]
 //
-uint32x4_t test_vqdmulhq_u32(uint32x4_t a, uint32x4_t b)
+int32x4_t test_vqdmulhq_s32(int32x4_t a, int32x4_t b)
 {
 #ifdef POLYMORPHIC
 return vqdmulhq(a, b);
 #else /* POLYMORPHIC */
-return vqdmulhq_u32(a, b);
+return vqdmulhq_s32(a, b);
 #endif /* POLYMORPHIC */
 }
 
@@ -62,19 +62,19 @@ int8x16_t test_vqdmulhq_m_s8(int8x16_t inactive, int8x16_t 
a, int8x16_t b, mve_p
 #endif /* POLYMORPHIC */
 }
 
-// CHECK-LABEL: @test_vqdmulhq_m_u16(
+// CHECK-LABEL: @test_vqdmulhq_m_s16(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:[[TMP1:%.*]] = call <8 x i1> @llvm.arm.mve.pred.i2v.v8i1(i32 
[[TMP0]])
 // CHECK-NEXT:[[TMP2:%.*]] = call <8 x i16> 
@llvm.arm.mve.qdmulh.predicated.v8i16.v8i1(<8 x i16> [[A:%.*]], <8 x i16> 
[[B:%.*]], <8 x i1> [[TMP1]], <8 x i16> [[INACTIVE:%.*]])
 // CHECK-NEXT:ret <8 x i16> [[TMP2]]
 //
-uint16x8_t test_vqdmulhq_m_u16(uint16x8_t inactive, uint16x8_t a, uint16x8_t 
b, mve_pred16_t p)
+int16x8_t test_vqdmulhq_m_s16(int16x8_t inactive, int16x8_t a, int16x8_t b, 
mve_pred16_t p)
 {
 #ifdef POLYMORPHIC
 return vqdmulhq_m(inactive, a, b, p);
 #else /* POLYMORPHIC */
-return vqdmulhq_m_u16(inactive, a, b, p);
+return vqdmulhq_m_s16(inactive, a, b, p);
 #endif /* POLYMORPHIC */
 }
 

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
index 4916faead32c..27b7efd31014 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
@@ -4,17 +4,17 @@
 
 #include 
 
-// CHECK-LABEL: @test_vqrdmulhq_u8(
+// CHECK-LABEL: @test_vqrdmulhq_s8(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = call <16 x i8> 
@llvm.arm.mve.vqrdmulh.v16i8(<16 x i8> [[A:%.*]], <16 x i8> [[B:%.*]])
 // CHECK-NEXT:ret <16 x i8> [[TMP0]]
 //
-uint8x16_t test_vqrdmulhq_u8(uint8x16_t a, uint8x16_t b)
+int8x16_t test_vqrdmulhq_s8(int8x16_t a, int8x16_t b)
 {
 #ifdef POLYMORPHIC
 return vqrdmulhq(a, b);
 #else /* POLYMORPHIC */
-return vqrdmulhq_u8(a, b);
+return vqrdmulhq_s8(a, b);
 #endif /* POLYMORPHIC */
 }
 
@@ -32,17 +32,17 @@ int16x8_t test_vqrdmulhq_s16(int16x8_t a, int16x8_t b)
 #endif /* POLYMORPHIC */
 }
 
-// CHECK-LABEL: @test_vqrdmulhq_u32(
+// CHECK-LABEL: @test_vqrdmulhq_s32(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = call <4 x i32> 
@llvm.arm.mve.vqrdmulh.v4i32(<4 x i32> [[A:%.*]], <4 x i32> [[B:%.*]])
 // CHECK-NEXT:ret <4 x i32> [[TMP0]]
 //
-uint32x4_t test_vqrdmulhq_u32(uint32x4_t a, uint32x4_t b)
+int32x4_t test_vqrdmulhq_s32(int32x4_t a, int32x4_t b)
 {
 #ifdef POLYMORPHIC
 return vqrdmulhq(a, b);
 #else /* POLYMORPHIC */
-return vqrdmulhq_u32(a, b);
+return vqrdmulhq_s32(a, b);
 #endif /* POLYMORPHIC */
 }
 
@@ -62,19 +62,19 @@ int8x16_t test_vqrdmulhq_m_s8(int8x16_t inactive, int8x16_t 
a, int8x16_t b, mve_
 #endif /* POLYMORPHIC */
 }
 
-// CHECK-LABEL: @test_vqrdmulhq_m

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Chi Chun Chen via Phabricator via cfe-commits]

cchen created this revision.
cchen added a reviewer: ABataev.
Herald added subscribers: cfe-commits, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

For this example:

  int a[100];
  
  int f2 (int i, int k)
  {
  for (int j = 16; j < 64; j++)
  {
  a[i] = j;
  i += 4;
  }
  return i;
  }

Clang will crash since it does not capture k in OpenMP outlined
function (failed assertion: "DeclRefExpr for Decl not entered in 
LocalDeclMap?").
By evaluating k inside the for loop, the code can run without issue.
Therefore, my change in CGStmtOpenMP.cpp is just inserting a alloca for
k to make sure the issue is due to not capturing the variable correctly.

I think the correct way might be adding a checker in SemaOpenMP to find if
there is any step expression contain any non-constant var and add them to
the parameter of OpenMP outlined function. However, I haven't figured
out how to add the var as parameter of OpenMP outlined function 
(ActOnOpenMPRegionStart
is for directive not for clause).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71475

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include 
 using namespace clang;
 using namespace CodeGen;
 using namespace llvm::omp;
@@ -1501,6 +1502,39 @@
 if (const auto *CS = cast_or_null(C->getCalcStep()))
   if (const auto *SaveRef = cast(CS->getLHS())) {
 EmitVarDecl(*cast(SaveRef->getDecl()));
+// Run DFS to find all the non-constant node under linear-step 
expression
+const Expr* Step = CS->getRHS();
+std::queue StmtQueue;
+StmtQueue.push(Step);
+while (!StmtQueue.empty()) {
+  const Stmt* CurStep = StmtQueue.front();
+  StmtQueue.pop();
+  if (const auto *BO = dyn_cast(CurStep)) {
+StmtQueue.push(BO->getLHS());
+StmtQueue.push(BO->getRHS());
+  } else if (isa(CurStep)) {
+const auto *IC = dyn_cast(CurStep);
+for (const Stmt *Child: IC->children()) {
+  StmtQueue.push(Child);
+}
+  } else if (isa(CurStep)) {
+// Generate a `alloca` for the variable in linear-step that
+// is not inside LocalDeclMap since compiler does not capture
+// this variable inside openmp-construct AST (which also
+// don't consider this variable as LValue either).
+// TODO The runtime behavior is incorrect for now since I only add
+// an `alloca`, and haven't store a meaningful value yet.
+if (const auto *DRE = dyn_cast(CurStep)) {
+  const ValueDecl* VD = DRE->getDecl();
+  const VarDecl* VarD = cast(VD);
+  if (LocalDeclMap.find(VarD) == LocalDeclMap.end()) {
+AutoVarEmission Emission = EmitAutoVarAlloca(*VarD);
+LocalDeclMap.insert({VarD, Emission.getAllocatedAddress()});
+EmitAutoVarCleanups(Emission);
+  }
+}
+  }
+}
 // Emit calculation of the linear step.
 EmitIgnoredExpr(CS);
   }


Index: clang/test/OpenMP/parallel_for_linear_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_linear_codegen.cpp
+++ clang/test/OpenMP/parallel_for_linear_codegen.cpp
@@ -28,6 +28,19 @@
 float f;
 char cnt;
 
+int a[100];
+
+int foo (int i, int k)
+{
+#pragma omp parallel for linear (i: k + 1)
+  for (int j = 16; j < 64; j++)
+  {
+a[i] = j;
+i += 4;
+  }
+  return i;
+}
+
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[F:@.+]] = global float 0.0
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include 
 using namesp

[PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests: http://45.33.8.238/mac/4132/step_7.txt

Please 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/D71466/new/

https://reviews.llvm.org/D71466



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

Re: [PATCH] D71466: [ARM][MVE][Intrinsics] remove extraneous intrinsics.

[Mark Murray via cfe-commits]

Hi

I've committed a fix.

M

On 13/12/2019, 17:38, "Nico Weber via Phabricator"  
wrote:

thakis added a comment.

This breaks tests: http://45.33.8.238/mac/4132/step_7.txt

Please 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/D71466/new/

https://reviews.llvm.org/D71466





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

[PATCH] D71476: [OpenCL] Add builtin function extension handling

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added reviewers: Anastasia, Nicola.
Herald added subscribers: jfb, yaxunl.
Herald added a reviewer: rengolin.

Provide a mechanism to attach OpenCL extension information to builtin
functions, so that their use can be restricted according to the
extension(s) the builtin is part of.

Patch by Pierre Gondois and Sven van Haastregt.


https://reviews.llvm.org/D71476

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -26,6 +26,11 @@
 //
 //  * Structs and enums to represent types and function signatures.
 //
+//  * const char *FunctionExtensionTable[]
+//List of space-separated OpenCL extensions.  A builtin references an
+//entry in this table when the builtin requires a particular (set of)
+//extension(s) to be enabled.
+//
 //  * OpenCLTypeStruct TypeTable[]
 //Type information for return types and arguments.
 //
@@ -133,6 +138,9 @@
   // function names.
   void GroupBySignature();
 
+  // Emit the FunctionExtensionTable that lists all function extensions.
+  void EmitExtensionTable();
+
   // Emit the TypeTable containing all types used by OpenCL builtins.
   void EmitTypeTable();
 
@@ -150,12 +158,13 @@
   // each function, and is a struct OpenCLBuiltinDecl.
   // E.g.:
   // // 891 convert_float2_rtn
-  //   { 58, 2, 100, 0 },
+  //   { 58, 2, 3, 100, 0 },
   // This means that the signature of this convert_float2_rtn overload has
   // 1 argument (+1 for the return type), stored at index 58 in
-  // the SignatureTable.  The last two values represent the minimum (1.0) and
-  // maximum (0, meaning no max version) OpenCL version in which this overload
-  // is supported.
+  // the SignatureTable.  This prototype requires extension "3" in the
+  // FunctionExtensionTable.  The last two values represent the minimum (1.0)
+  // and maximum (0, meaning no max version) OpenCL version in which this
+  // overload is supported.
   void EmitBuiltinTable();
 
   // Emit a StringMatcher function to check whether a function name is an
@@ -191,6 +200,10 @@
   // Contains the map of OpenCL types to their index in the TypeTable.
   MapVector TypeMap;
 
+  // List of OpenCL function extensions mapping extension strings to
+  // an index into the FunctionExtensionTable.
+  StringMap FunctionExtensionIndex;
+
   // List of OpenCL type names in the same order as in enum OpenCLTypeID.
   // This list does not contain generic types.
   std::vector TypeList;
@@ -227,16 +240,18 @@
   // Emit enums and structs.
   EmitDeclarations();
 
+  // Parse the Records to populate the internal lists.
   GetOverloads();
   GroupBySignature();
 
   // Emit tables.
+  EmitExtensionTable();
   EmitTypeTable();
   EmitSignatureTable();
   EmitBuiltinTable();
 
+  // Emit functions.
   EmitStringMatcher();
-
   EmitQualTypeFinder();
 }
 
@@ -323,6 +338,8 @@
   const bool IsConst;
   // Function attribute __attribute__((convergent))
   const bool IsConv;
+  // OpenCL extension(s) required for this overload.
+  const unsigned short Extension;
   // First OpenCL version in which this overload was introduced (e.g. CL20).
   const unsigned short MinVersion;
   // First OpenCL version in which this overload was removed (e.g. CL20).
@@ -413,6 +430,23 @@
   }
 }
 
+void BuiltinNameEmitter::EmitExtensionTable() {
+  OS << "static const char *FunctionExtensionTable[] = {\n";
+  unsigned Index = 0;
+  std::vector FuncExtensions =
+  Records.getAllDerivedDefinitions("FunctionExtension");
+
+  for (const auto &FE : FuncExtensions) {
+// Emit OpenCL extension table entry.
+OS << "  // " << Index << ": " << FE->getName() << "\n"
+   << "  \"" << FE->getValueAsString("ExtName") << "\",\n";
+
+// Record index of this extension.
+FunctionExtensionIndex[FE->getName()] = Index++;
+  }
+  OS << "};\n\n";
+}
+
 void BuiltinNameEmitter::EmitTypeTable() {
   OS << "static const OpenCLTypeStruct TypeTable[] = {\n";
   for (const auto &T : TypeMap) {
@@ -463,11 +497,13 @@
 OS << "\n";
 
 for (const auto &Overload : SLM.second.Signatures) {
+  StringRef ExtName = Overload.first->getValueAsDef("Extension")->getName();
   OS << "  { " << Overload.second << ", "
  << Overload.first->getValueAsListOfDefs("Signature").size() << ", "
  << (Overload.first->getValueAsBit("IsPure")) << ", "
  << (Overload.first->getValueAsBit("IsConst")) << ", "
  << (Overload.first->getValueAsBit("IsConv")) << ", "
+ << FunctionExtensionIndex[ExtName] << ", "
  << Overload.first->getValueAsDef("MinVersion")->getValueAsInt("ID")
  << ", "
  << Overload.first->getValueAsDef("MaxVers

[PATCH] D70289: [OpenMP][NFCI] Use the libFrontend ProcBindKind in Clang

[Jon Chesterfield via Phabricator via cfe-commits]

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

Nice cleanup, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70289



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

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Your commit message example lacks the #pragma.

What if you add k to the list of explicit firstprivate? (I mean, you can try it 
in C first).

And how do I reproduce the crash? I tried: https://godbolt.org/z/FDPSnA


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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

[PATCH] D71167: [Driver] Default to -momit-leaf-frame-pointer for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added subscribers: dyung, ychen.
MaskRay added a comment.

In D71167#1783298 , @sdesmalen wrote:

> The patch looks structurally fine, but I'm missing the argumentation for 
> changing the default and related, the history of why the default is to 
> //not// omit the frame pointer on leaf functions.
>  Can you provide some insight here?


Honestly we probably don't need this change if the consistency with GCC 
regarding -momit-leaf-frame-pointer is not useful. I touched this function in 
D64294  and I hoped isPS4CPU did not have the 
special rule (@dyung @ychen).

I can abondon this patch if you think it is unnecessary. We may only need 
D71168 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71167



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

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D71475#1783834 , @jdoerfert wrote:

> Your commit message example lacks the #pragma.
>
> What if you add k to the list of explicit firstprivate? (I mean, you can try 
> it in C first).
>
> And how do I reproduce the crash? I tried: https://godbolt.org/z/FDPSnA


Add firstprivate make the code work. Also, the code crash due to assertion 
failure and I guess the compiler explorer is using release version instead of 
debug version?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Thomas Lively via Phabricator via cfe-commits]

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

This issue is still not resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

More info: https://docs.oracle.com/cd/E23824_01/html/819-0690/ggdlu.html


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

https://reviews.llvm.org/D68101



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Can you update 
https://clang.llvm.org/docs/LanguageExtensions.html#langext-vectors with a 
description of the interaction between the different language features here?

It's unfortunate that the gcc extension is explicitly incompatible with OpenCL, 
but I guess we're stuck with it.




Comment at: clang/include/clang/AST/Expr.h:3738
+// implementation of this that the standard implies that this
+// could be true in the C++ standard as well.
+(cond->isTypeDependent() || lhs->isTypeDependent() ||

You can just refer to [temp.dep.expr] here; the type does in fact "depend" on 
the type of the condition.  I'm a little concerned that the C++ standard might 
not preserve this guarantee in the future.



Comment at: clang/lib/Sema/SemaExpr.cpp:8981
+else
+  return true;
   } else if (VectorEltTy->isRealFloatingType()) {

Can you separate this out into an independent patch?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5826
+else
+  ResultElementTy = UsualArithmeticConversions(LHS, RHS);
+

I'm not completely sure this works the way you want it to... in particular, if 
the types don't match, and both operands can be promoted to int, you end up 
with an int vector.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71463



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

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71475#1783928 , @cchen wrote:

> In D71475#1783834 , @jdoerfert wrote:
>
> > Your commit message example lacks the #pragma.
> >
> > What if you add k to the list of explicit firstprivate? (I mean, you can 
> > try it in C first).
> >
> > And how do I reproduce the crash? I tried: https://godbolt.org/z/FDPSnA
>
>
> Add firstprivate make the code work. Also, the code crash due to assertion 
> failure and I guess the compiler explorer is using release version instead of 
> debug version?


I see. That makes sense.

What do you think about making all variables used in the linear-step 
(implicitly) firstprivate? Doing this might allow us to (easily) verify they 
are not shared/private/lastprivate etc. already. If I run this with k 
lastprivate there is no assertion but the code is not updating k, potentially 
because it is not legal but then we want to error out. k in a reduction is 
similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/Decl.cpp:3019
+if (SL.isValid())
+  return SM.isInSystemHeader(SL);
+  }

I'm a little concerned about this; we're subtly changing our generated code 
based on the "system-headerness" of the definition.  We generally avoid 
depending on this for anything other than warnings. It could lead to weird 
results with preprocessed source, or if someone specifies their include paths 
incorrectly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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

[PATCH] D71485: [profile] Fix a crash when -fprofile-remapping-file= triggers an error

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: rsmith, wenlei, wmi.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71485

Files:
  clang/test/CodeGenCXX/Inputs/profile-remap-error.map
  clang/test/CodeGenCXX/profile-remap-error.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp


Index: llvm/lib/Transforms/IPO/SampleProfile.cpp
===
--- llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1973,7 +1973,8 @@
: ProfileRemappingFileName,
   IsThinLTOPreLink, GetAssumptionCache, GetTTI);
 
-  SampleLoader.doInitialization(M);
+  if (!SampleLoader.doInitialization(M))
+return PreservedAnalyses::all();
 
   ProfileSummaryInfo *PSI = &AM.getResult(M);
   CallGraph &CG = AM.getResult(M);
Index: clang/test/CodeGenCXX/profile-remap-error.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/profile-remap-error.cpp
@@ -0,0 +1,9 @@
+// REQUIRES: x86-registered-target
+
+// RUN: not %clang_cc1 -triple x86_64-linux-gnu 
-fprofile-sample-use=%S/Inputs/profile-remap.samples 
-fprofile-remapping-file=%S/Inputs/profile-remap-error.map 
-fexperimental-new-pass-manager -O2 %s -emit-llvm -o - 2>&1 | FileCheck %s
+
+// CHECK:  error: {{.*}}/profile-remap-error.map:1: Could not demangle 
'unmangled' as a ; invalid mangling?
+// CHECK-NEXT: error: {{.*}}/profile-remap-error.map: Could not create 
remapper: Malformed sample profile data
+// CHECK-NEXT: error: {{.*}}/profile-remap.samples: Could not open profile: 
Malformed sample profile data
+
+void foo();
Index: clang/test/CodeGenCXX/Inputs/profile-remap-error.map
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/profile-remap-error.map
@@ -0,0 +1 @@
+name unmangled ignored


Index: llvm/lib/Transforms/IPO/SampleProfile.cpp
===
--- llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1973,7 +1973,8 @@
: ProfileRemappingFileName,
   IsThinLTOPreLink, GetAssumptionCache, GetTTI);
 
-  SampleLoader.doInitialization(M);
+  if (!SampleLoader.doInitialization(M))
+return PreservedAnalyses::all();
 
   ProfileSummaryInfo *PSI = &AM.getResult(M);
   CallGraph &CG = AM.getResult(M);
Index: clang/test/CodeGenCXX/profile-remap-error.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/profile-remap-error.cpp
@@ -0,0 +1,9 @@
+// REQUIRES: x86-registered-target
+
+// RUN: not %clang_cc1 -triple x86_64-linux-gnu -fprofile-sample-use=%S/Inputs/profile-remap.samples -fprofile-remapping-file=%S/Inputs/profile-remap-error.map -fexperimental-new-pass-manager -O2 %s -emit-llvm -o - 2>&1 | FileCheck %s
+
+// CHECK:  error: {{.*}}/profile-remap-error.map:1: Could not demangle 'unmangled' as a ; invalid mangling?
+// CHECK-NEXT: error: {{.*}}/profile-remap-error.map: Could not create remapper: Malformed sample profile data
+// CHECK-NEXT: error: {{.*}}/profile-remap.samples: Could not open profile: Malformed sample profile data
+
+void foo();
Index: clang/test/CodeGenCXX/Inputs/profile-remap-error.map
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/profile-remap-error.map
@@ -0,0 +1 @@
+name unmangled ignored
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70073: [ConstExprPreter] Implemented function calls and if statements

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70073



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