[PATCH] D77527: [AST] Adjust existing tests for recovery-exprs

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

When enabling recovery-expr (in https://reviews.llvm.org/D76696, which has been
revertedj), we need to update a few tests to adjust new diagnostic changes.

This patch updates all relevant tests, it will make the developing life
easier (if recovery-expr is enabled by default, we can revert this
patch).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77527

Files:
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp
  clang/test/Parser/objcxx0x-lambda-expressions.mm
  clang/test/Parser/objcxx11-invalid-lambda.cpp
  clang/test/SemaCXX/cast-conversion.cpp
  clang/test/SemaCXX/constructor-initializer.cpp
  clang/test/SemaCXX/cxx1z-copy-omission.cpp
  clang/test/SemaCXX/decltype-crash.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/test/SemaOpenCLCXX/address-space-references.cl
  clang/test/SemaTemplate/instantiate-init.cpp

Index: clang/test/SemaTemplate/instantiate-init.cpp
===
--- clang/test/SemaTemplate/instantiate-init.cpp
+++ clang/test/SemaTemplate/instantiate-init.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -frecovery-ast -verify -std=c++11 %s
 
 struct X0 { // expected-note 8{{candidate}}
   X0(int*, float*); // expected-note 4{{candidate}}
@@ -100,7 +100,7 @@
 integral_c<1> ic1 = array_lengthof(Description::data);
 (void)sizeof(array_lengthof(Description::data));
 
-sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
+(void)sizeof(array_lengthof( // expected-error{{no matching function for call to 'array_lengthof'}}
   Description::data // expected-note{{in instantiation of static data member 'PR7985::Description::data' requested here}}
   ));
 
Index: clang/test/SemaOpenCLCXX/address-space-references.cl
===
--- clang/test/SemaOpenCLCXX/address-space-references.cl
+++ clang/test/SemaOpenCLCXX/address-space-references.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -pedantic -verify -fsyntax-only
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -frecovery-ast -cl-std=clc++ -pedantic -verify -fsyntax-only
 
 __global const int& f(__global float &ref) {
   return ref; // expected-error{{reference of type 'const __global int &' cannot bind to a temporary object because of address space mismatch}}
@@ -11,7 +11,7 @@
 int bar(const unsigned int &i);
 
 void foo() {
-  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+  bar(1); // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
 }
 
 // Test addr space conversion with nested pointers
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++03 -verify %s
-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++03 -frecovery-ast -verify %s
+// RUN: %clang_cc1 -std=c++11 -frecovery-ast -verify %s
 
 __builtin_va_list ap;
 
@@ -22,7 +22,8 @@
 // default ctor.
 void record_context(int a, ...) {
   struct Foo {
-// expected-error@+1 {{'va_start' cannot be used outside a function}}
+// expected-error@+2 {{'va_start' cannot be used outside a function}}
+// expected-error@+1 {{default argument references parameter 'a'}}
 void meth(int a, int b = (__builtin_va_start(ap, a), 0)) {}
   };
 }
Index: clang/test/SemaCXX/decltype-crash.cpp
===
--- clang/test/SemaCXX/decltype-crash.cpp
+++ clang/test/SemaCXX/decltype-crash.cpp
@@ -1,7 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wc++11-compat -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -frecovery-ast -verify -Wc++11-compat -std=c++98 %s
 
 int& a();
 
 void f() {
-  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} expected-error {{use of undeclared identifier 'decltype'}}
+  decltype(a()) c; // expected-warning {{'decltype' is a keyword in C++11}} \
+   // expected-error {{use of undeclared identifier 'decltype'}} \
+   // expected-error {{expected ';' after expression}} \
+   // expected-error {{use of undeclared identifier 'c'}}
 }
Index: clang/test/SemaCXX/cxx1z-copy-omission.cpp
===
--- clang/test/SemaCXX/cxx1z-copy-omission.cpp
+++ clang/test/SemaCXX/cxx1z-copy-omission.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z -verify %s
+// RUN: %clang_c

[PATCH] D77468: [clang] fix undefined behaviour in RawComment::getFormattedText()

[Raphael Isemann via Phabricator via cfe-commits]

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

We don't usually use alternative operator spelling in Clang, but otherwise this 
LGTM. I'll land this for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77468



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

[PATCH] D77461: [clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I expect different style guides to have a different opinion on this, depending 
on the justification for the rule.

> The purpose of the rule is to avoid code which causes hidden dependencies.

That's one justification, which would prohibit such static variables.

Another justification is to avoid "spooky action-at-a-distance" (one piece of 
code communicating with another piece of code through a global variable), which 
would allow such static variables, because they are encapsulated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77461



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

[PATCH] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Cheers, LGTM


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

https://reviews.llvm.org/D76617



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Ping.
It is now testable, a sub-checker was added for testing (that can be useful at 
later improvements too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const ElementRegion *TheElementRegion =
+MRegion->getAs()) {

The sequence of `FieldRegion`s and `ElementRegion`s on top of a base region may 
be arbitrary: `var.a[0].b[1][2].c.d[3]` etc.

I'd rather unwrap those regions one-by-one in a loop and look at the alignment 
of each layer.


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

https://reviews.llvm.org/D76996



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

[PATCH] D77410: [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl

[Artem Dergachev via Phabricator via cfe-commits]

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

Excellent! No objections. It should have absolutely been like that from the 
start: summaries are of functions, not of call sites.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77410



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

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

I reported a small issue here: https://bugs.llvm.org/show_bug.cgi?id=45441


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68554



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

[PATCH] D75431: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Great, thanks!!


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

https://reviews.llvm.org/D75431



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

[PATCH] D77461: [clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global
 is in "Interfaces" section, it only covers inter-procedural stuff.
Diagnosing function-local static non-const variable is a plain false-positive 
diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77461



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

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis updated this revision to Diff 255250.
abelkocsis added a comment.

Small fixes


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D75229

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-in-multithreaded-program.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-con37-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std::thread.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std::thread.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) { return 0; };
+enum {
+  thrd_success = 0,
+};
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+int func(void *data) {
+  while (!flag) {
+  }
+  return 0;
+}
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+  thrd_t tid;
+
+  if (thrd_success != thrd_create(&tid, func, NULL)) {
+  }
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std::thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std::thread.cpp
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+void threadFunction() {}
+
+namespace std {
+class thread {
+public:
+  thread() noexcept;
+  template 
+  explicit thread(Function &&f, Args &&... args);
+  thread(const thread &) = delete;
+  thread(thread &&) noexcept;
+};
+} // namespace std
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+
+  std::thread threadObj(threadFunction);
+
+  return 0;
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: bugprone-signal-in-multithreaded-program.ThreadList, value: "thrd_create"}]}'
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) { return 0; };
+enum {
+  thrd_success = 0,
+};
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+int func(void *data) {
+  while (!flag) {
+  }
+  return 0;
+}
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+  thrd_t tid;
+
+  if (thrd_success != thrd_create(&tid, func, NULL)) {
+  }
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std::t

[PATCH] D77410: [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:553
+QualType ActualT = FD->getParamDecl(I)->getType().getCanonicalType();
 assert(ActualT.isCanonical());
 if (ActualT != FormalT)

This assert can be removed, after the previous `getCanonicalType` call it looks 
redundant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77410



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

[clang] ad7211d - [clang] fix undefined behaviour in RawComment::getFormattedText()

[Raphael Isemann via cfe-commits]

Author: Oliver Bruns
Date: 2020-04-06T10:48:25+02:00
New Revision: ad7211df6f257e39da2e5a11b2456b4488f32a1e

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

LOG: [clang] fix undefined behaviour in RawComment::getFormattedText()

Summary:
Calling `back()` and `pop_back()` on the empty string is undefined
behavior [1,2].

The issue manifested itself as an uncaught `std::out_of_range` exception
when running `clangd` compiled on RHEL7 using devtoolset-9.

[1] https://en.cppreference.com/w/cpp/string/basic_string/back
[2] https://en.cppreference.com/w/cpp/string/basic_string/pop_back

Fixes: 1ff7c32fc91c607b690d4bb9cf42f406be8dde68

Reviewers: teemperor, ioeric, cfe-commits

Reviewed By: teemperor

Subscribers: ilya-biryukov, kadircet, usaxena95

Tags: #clang

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

Added: 


Modified: 
clang/lib/AST/RawCommentList.cpp

Removed: 




diff  --git a/clang/lib/AST/RawCommentList.cpp 
b/clang/lib/AST/RawCommentList.cpp
index 8552b4fcd2b3..a8d15036cab9 100644
--- a/clang/lib/AST/RawCommentList.cpp
+++ b/clang/lib/AST/RawCommentList.cpp
@@ -431,7 +431,7 @@ std::string RawComment::getFormattedText(const 
SourceManager &SourceMgr,
   };
 
   auto DropTrailingNewLines = [](std::string &Str) {
-while (Str.back() == '\n')
+while (!Str.empty() && Str.back() == '\n')
   Str.pop_back();
   };
 



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

[PATCH] D77532: Fix a typo in clang/lib/Frontend/FrontendAction.cpp

[BoYao Zhang via Phabricator via cfe-commits]

rZhBoYao created this revision.
rZhBoYao added reviewers: rsmith, harlanhaskins, clang.
rZhBoYao added a project: clang.
rZhBoYao updated this revision to Diff 255254.
rZhBoYao added a comment.

revert the change to trailing newlines


There is a typo in clang/lib/Frontend/FrontendAction.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77532

Files:
  clang/lib/Frontend/FrontendAction.cpp


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -787,7 +787,7 @@
   auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : SrcMgr::C_User;
   auto &SourceMgr = CI.getSourceManager();
   auto BufferID = SourceMgr.createFileID(std::move(Buffer), Kind);
-  assert(BufferID.isValid() && "couldn't creaate module buffer ID");
+  assert(BufferID.isValid() && "couldn't create module buffer ID");
   SourceMgr.setMainFileID(BufferID);
 }
   }


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -787,7 +787,7 @@
   auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : SrcMgr::C_User;
   auto &SourceMgr = CI.getSourceManager();
   auto BufferID = SourceMgr.createFileID(std::move(Buffer), Kind);
-  assert(BufferID.isValid() && "couldn't creaate module buffer ID");
+  assert(BufferID.isValid() && "couldn't create module buffer ID");
   SourceMgr.setMainFileID(BufferID);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Thibault North via Phabricator via cfe-commits]

tnorth added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72326



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

[PATCH] D77468: [clang] fix undefined behaviour in RawComment::getFormattedText()

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Landed as ad7211df6f257e39da2e5a11b2456b4488f32a1e 


Btw, usually patches also need a test (especially if they are less trivial than 
this one).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77468



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

[PATCH] D77532: Fix a typo in clang/lib/Frontend/FrontendAction.cpp

[BoYao Zhang via Phabricator via cfe-commits]

rZhBoYao updated this revision to Diff 255254.
rZhBoYao added a comment.

revert the change to trailing newlines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77532

Files:
  clang/lib/Frontend/FrontendAction.cpp


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -787,7 +787,7 @@
   auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : SrcMgr::C_User;
   auto &SourceMgr = CI.getSourceManager();
   auto BufferID = SourceMgr.createFileID(std::move(Buffer), Kind);
-  assert(BufferID.isValid() && "couldn't creaate module buffer ID");
+  assert(BufferID.isValid() && "couldn't create module buffer ID");
   SourceMgr.setMainFileID(BufferID);
 }
   }


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -787,7 +787,7 @@
   auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : SrcMgr::C_User;
   auto &SourceMgr = CI.getSourceManager();
   auto BufferID = SourceMgr.createFileID(std::move(Buffer), Kind);
-  assert(BufferID.isValid() && "couldn't creaate module buffer ID");
+  assert(BufferID.isValid() && "couldn't create module buffer ID");
   SourceMgr.setMainFileID(BufferID);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77468: [clang] fix undefined behaviour in RawComment::getFormattedText()

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGad7211df6f25 (authored by obruns, committed by teemperor).

Changed prior to commit:
  https://reviews.llvm.org/D77468?vs=255059&id=255256#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77468

Files:
  clang/lib/AST/RawCommentList.cpp


Index: clang/lib/AST/RawCommentList.cpp
===
--- clang/lib/AST/RawCommentList.cpp
+++ clang/lib/AST/RawCommentList.cpp
@@ -431,7 +431,7 @@
   };
 
   auto DropTrailingNewLines = [](std::string &Str) {
-while (Str.back() == '\n')
+while (!Str.empty() && Str.back() == '\n')
   Str.pop_back();
   };
 


Index: clang/lib/AST/RawCommentList.cpp
===
--- clang/lib/AST/RawCommentList.cpp
+++ clang/lib/AST/RawCommentList.cpp
@@ -431,7 +431,7 @@
   };
 
   auto DropTrailingNewLines = [](std::string &Str) {
-while (Str.back() == '\n')
+while (!Str.empty() && Str.back() == '\n')
   Str.pop_back();
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

[Kristóf Umann via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:59
 enum CallEventKind {
+  CE_CXXDeallocator,
   CE_Function,

I need to move this below `CE_Function`, as its not in the range of 
`CE_BEG_FUNCTION_CALLS` and `CE_END_FUNCTION_CALLS`.


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

https://reviews.llvm.org/D75430



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

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

[George Rimar via Phabricator via cfe-commits]

grimar added a comment.

In D77184#1960437 , @thakis wrote:

> grimar, andrewng: You both have checkout and build on different drives too, 
> yes?


Nope. I think my configuration is just normal, except I do not use "C:".
My checkout is "D:\Work3\LLVM\llvm-project" and the build folder is inside: 
"D:\Work3\LLVM\llvm-project\build".

(Just in case, to clarify: I have no issues with building the todays LLVM).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77184



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

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 255258.
njames93 added a comment.

- Remove format artefacts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77499

Files:
  clang/include/clang/ASTMatchers/Dynamic/Diagnostics.h
  clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
  clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h

Index: clang/lib/ASTMatchers/Dynamic/Marshallers.h
===
--- clang/lib/ASTMatchers/Dynamic/Marshallers.h
+++ clang/lib/ASTMatchers/Dynamic/Marshallers.h
@@ -29,6 +29,7 @@
 #include "clang/Basic/OpenMPKinds.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -64,6 +65,10 @@
   static ArgKind getKind() {
 return ArgKind(ArgKind::AK_String);
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &) {
+return llvm::None;
+  }
 };
 
 template <>
@@ -82,6 +87,10 @@
   static ArgKind getKind() {
 return ArgKind(ASTNodeKind::getFromNodeKind());
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &) {
+return llvm::None;
+  }
 };
 
 template <> struct ArgTypeTraits {
@@ -94,6 +103,10 @@
   static ArgKind getKind() {
 return ArgKind(ArgKind::AK_Boolean);
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &) {
+return llvm::None;
+  }
 };
 
 template <> struct ArgTypeTraits {
@@ -106,6 +119,10 @@
   static ArgKind getKind() {
 return ArgKind(ArgKind::AK_Double);
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &) {
+return llvm::None;
+  }
 };
 
 template <> struct ArgTypeTraits {
@@ -118,6 +135,10 @@
   static ArgKind getKind() {
 return ArgKind(ArgKind::AK_Unsigned);
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &) {
+return llvm::None;
+  }
 };
 
 template <> struct ArgTypeTraits {
@@ -141,6 +162,8 @@
   static ArgKind getKind() {
 return ArgKind(ArgKind::AK_String);
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &Value);
 };
 
 template <> struct ArgTypeTraits {
@@ -164,6 +187,8 @@
   static ArgKind getKind() {
 return ArgKind(ArgKind::AK_String);
   }
+
+  static llvm::Optional getBestGuess(const VariantValue &Value);
 };
 
 template <> struct ArgTypeTraits {
@@ -186,6 +211,8 @@
   }
 
   static ArgKind getKind() { return ArgKind(ArgKind::AK_String); }
+
+  static llvm::Optional getBestGuess(const VariantValue &Value);
 };
 
 /// Matcher descriptor interface.
@@ -319,7 +346,7 @@
 /// polymorphic matcher. For the former, we just construct the VariantMatcher.
 /// For the latter, we instantiate all the possible Matcher of the poly
 /// matcher.
-static VariantMatcher outvalueToVariantMatcher(const DynTypedMatcher &Matcher) {
+inline VariantMatcher outvalueToVariantMatcher(const DynTypedMatcher &Matcher) {
   return VariantMatcher::SingleMatcher(Matcher);
 }
 
@@ -496,9 +523,16 @@
 
 #define CHECK_ARG_TYPE(index, type)\
   if (!ArgTypeTraits::is(Args[index].Value)) {   \
-Error->addError(Args[index].Range, Error->ET_RegistryWrongArgType) \
-<< (index + 1) << ArgTypeTraits::getKind().asString()\
-<< Args[index].Value.getTypeAsString();\
+if (llvm::Optional BestGuess =\
+ArgTypeTraits::getBestGuess(Args[index].Value)) {\
+  Error->addError(Args[index].Range,   \
+  Error->ET_RegistryUnknownEnumWithReplace)\
+  << index + 1 << Args[index].Value.getString() << *BestGuess; \
+} else {   \
+  Error->addError(Args[index].Range, Error->ET_RegistryWrongArgType)   \
+  << (index + 1) << ArgTypeTraits::getKind().asString()  \
+  << Args[index].Value.getTypeAsString();  \
+}  \
 return VariantMatcher();   \
   }
 
Index: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
===
--- /dev/null
+++ clang/lib/ASTMatchers/Dynamic/Marshallers.cpp
@@ -0,0 +1,91 @@
+#include "Marshallers.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+static llvm::Optional
+getBestGuess(llvm::StringRef Search, llvm::ArrayRef Allowed,
+ llvm::StringRef DropPrefix = "", unsigned MaxEditDistance = 3) {
+  if (MaxEditDistance != -1u) {
+++MaxEditDistance;
+  }
+  

[PATCH] D77410: [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus accepted this revision.
Szelethus added a comment.

LGTM! Maybe its worth glancing over the rest of the code (comments in 
particular) whether anything would be outdated by this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77410



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

[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

[Kerry McLaughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG944e322f8897: [AArch64][SVE] Add SVE intrinsics for 
saturating add & subtract (authored by kmclaughlin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77054

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-int-arith-imm.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-int-arith.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-int-arith.ll
===
--- llvm/test/CodeGen/AArch64/sve-intrinsics-int-arith.ll
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-int-arith.ll
@@ -134,6 +134,82 @@
   ret  %out
 }
 
+; SQADD
+
+define  @sqadd_i8( %a,  %b) {
+; CHECK-LABEL: sqadd_i8:
+; CHECK: sqadd z0.b, z0.b, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqadd.x.nxv16i8( %a,
+%b)
+  ret  %out
+}
+
+define  @sqadd_i16( %a,  %b) {
+; CHECK-LABEL: sqadd_i16:
+; CHECK: sqadd z0.h, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqadd.x.nxv8i16( %a,
+%b)
+  ret  %out
+}
+
+define  @sqadd_i32( %a,  %b) {
+; CHECK-LABEL: sqadd_i32:
+; CHECK: sqadd z0.s, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqadd.x.nxv4i32( %a,
+%b)
+  ret  %out
+}
+
+define  @sqadd_i64( %a,  %b) {
+; CHECK-LABEL: sqadd_i64:
+; CHECK: sqadd z0.d, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqadd.x.nxv2i64( %a,
+%b)
+  ret  %out
+}
+
+; SQSUB
+
+define  @sqsub_i8( %a,  %b) {
+; CHECK-LABEL: sqsub_i8:
+; CHECK: sqsub z0.b, z0.b, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqsub.x.nxv16i8( %a,
+%b)
+  ret  %out
+}
+
+define  @sqsub_i16( %a,  %b) {
+; CHECK-LABEL: sqsub_i16:
+; CHECK: sqsub z0.h, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqsub.x.nxv8i16( %a,
+%b)
+  ret  %out
+}
+
+define  @sqsub_i32( %a,  %b) {
+; CHECK-LABEL: sqsub_i32:
+; CHECK: sqsub z0.s, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqsub.x.nxv4i32( %a,
+%b)
+  ret  %out
+}
+
+define  @sqsub_i64( %a,  %b) {
+; CHECK-LABEL: sqsub_i64:
+; CHECK: sqsub z0.d, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.sqsub.x.nxv2i64( %a,
+%b)
+  ret  %out
+}
+
 ; UDOT
 
 define  @udot_i32( %a,  %b,  %c) {
@@ -169,6 +245,82 @@
   ret  %out
 }
 
+; UQADD
+
+define  @uqadd_i8( %a,  %b) {
+; CHECK-LABEL: uqadd_i8:
+; CHECK: uqadd z0.b, z0.b, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqadd.x.nxv16i8( %a,
+%b)
+  ret  %out
+}
+
+define  @uqadd_i16( %a,  %b) {
+; CHECK-LABEL: uqadd_i16:
+; CHECK: uqadd z0.h, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqadd.x.nxv8i16( %a,
+%b)
+  ret  %out
+}
+
+define  @uqadd_i32( %a,  %b) {
+; CHECK-LABEL: uqadd_i32:
+; CHECK: uqadd z0.s, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqadd.x.nxv4i32( %a,
+%b)
+  ret  %out
+}
+
+define  @uqadd_i64( %a,  %b) {
+; CHECK-LABEL: uqadd_i64:
+; CHECK: uqadd z0.d, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqadd.x.nxv2i64( %a,
+%b)
+  ret  %out
+}
+
+; UQSUB
+
+define  @uqsub_i8( %a,  %b) {
+; CHECK-LABEL: uqsub_i8:
+; CHECK: uqsub z0.b, z0.b, z1.b
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqsub.x.nxv16i8( %a,
+%b)
+  ret  %out
+}
+
+define  @uqsub_i16( %a,  %b) {
+; CHECK-LABEL: uqsub_i16:
+; CHECK: uqsub z0.h, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqsub.x.nxv8i16( %a,
+%b)
+  ret  %out
+}
+
+define  @uqsub_i32( %a,  %b) {
+; CHECK-LABEL: uqsub_i32:
+; CHECK: uqsub z0.s, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.uqsub.x.nxv4i32( %a,
+%b)
+  ret  %out
+}
+
+define  @uqsub_i64( %a,  %b) {
+; CHECK-LABEL: uqsub_i64:
+; CHECK: uqsub z0.d, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch6

[PATCH] D77484: [Vector] Pass VectLib to LTO backend so TLI build correct vector function list

[George Rimar via Phabricator via cfe-commits]

grimar added inline comments.



Comment at: lld/ELF/Options.td:491
   HelpText<"Sample profile file path">;
+def lto_vector_library: J<"lto-vector-library=">,
+  HelpText<"Vector functions library">;

You need to update the documentation either (`lld/docs/ld.lld.1`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77484



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

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: sammccall, kadircet, hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.
njames93 added a project: clang-tools-extra.

Removes the `static` token when defining a function out of line if the function 
is a `CXXMethodDecl`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77534

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


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2142,6 +2142,17 @@
 };)cpp",
   "void B::foo()   {}\n",
   },
+  {
+  R"cpp(
+struct A {
+  static void fo^o() {}
+};)cpp",
+  R"cpp(
+struct A {
+  static void foo() ;
+};)cpp",
+  " void A::foo() {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
@@ -2236,6 +2247,24 @@
 STUPID_MACRO(sizeof sizeof int) void foo() ;
   };)cpp",
" void A::foo() {}\n"},
+  {R"cpp(#define STAT static
+  struct A {
+STAT void f^oo() {}
+  };)cpp",
+   R"cpp(#define STAT static
+  struct A {
+STAT void foo() ;
+  };)cpp",
+   " void A::foo() {}\n"},
+  {R"cpp(#define STUPID_MACRO(X) static
+  struct A {
+STUPID_MACRO(sizeof sizeof int) void f^oo() {}
+  };)cpp",
+   R"cpp(#define STUPID_MACRO(X) static
+  struct A {
+STUPID_MACRO(sizeof sizeof int) void foo() ;
+  };)cpp",
+   " void A::foo() {}\n"},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -270,6 +270,41 @@
 }
   }
 
+  if (const auto *MD = dyn_cast(FD)) {
+if (MD->isStatic()) {
+  SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()};
+  bool FoundTok = false;
+  for (const auto &Tok : TokBuf.expandedTokens(SpecRange)) {
+if (Tok.kind() != tok::kw_static)
+  continue;
+FoundTok = true;
+auto Spelling = TokBuf.spelledForExpanded(llvm::makeArrayRef(Tok));
+if (!Spelling) {
+  Errors =
+  llvm::joinErrors(std::move(Errors),
+   llvm::createStringError(
+   llvm::inconvertibleErrorCode(),
+   "define outline: Can't move out of line as "
+   "function has a macro `static` 
specifier."));
+  break;
+}
+CharSourceRange DelRange =
+syntax::Token::range(SM, Spelling->front(), Spelling->back())
+.toCharRange(SM);
+if (auto Err =
+DeclarationCleanups.add(tooling::Replacement(SM, DelRange, 
"")))
+  Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+break;
+  }
+  if (!FoundTok)
+Errors = llvm::joinErrors(
+std::move(Errors),
+llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"define outline: Couldn't remove methods `static` keyword"));
+}
+  }
+
   if (Errors)
 return std::move(Errors);
   return getFunctionSourceAfterReplacements(FD, DeclarationCleanups);


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2142,6 +2142,17 @@
 };)cpp",
   "void B::foo()   {}\n",
   },
+  {
+  R"cpp(
+struct A {
+  static void fo^o() {}
+};)cpp",
+  R"cpp(
+struct A {
+  static void foo() ;
+};)cpp",
+  " void A::foo() {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
@@ -2236,6 +2247,24 @@
 STUPID_MACRO(sizeof sizeof int) void foo() ;
   };)cpp",
" void A::foo() {}\n"},
+  {R"cpp(#define STAT static
+  struct A {
+STAT void f^oo() {}
+  };)cpp",
+   R"cpp(#define STAT static
+  struct A {
+STAT void foo() ;
+  };)cpp",
+   " void A::foo() {}\n"},
+  {R"cpp(#define STUPID_MACRO(X) static
+  struct A {
+STUPID_MACRO(sizeof sizeof int) void f^oo() {}
+  };)

[PATCH] D77379: [FPEnv] Use single enum to represent rounding mode

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 255267.
sepavloff added a comment.

Updated patch

- enhancements of LangOptions.def are moved into separate patch,
- fixed some clang-format errors,
- added more comments to RoundingMode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77379

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/LangOptions.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReader.cpp
  llvm/include/llvm/ADT/APFloat.h
  llvm/include/llvm/ADT/FloatingPointMode.h
  llvm/include/llvm/IR/FPEnv.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/IR/FPEnv.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/Support/APFloat.cpp
  llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
  llvm/unittests/IR/IRBuilderTest.cpp

Index: llvm/unittests/IR/IRBuilderTest.cpp
===
--- llvm/unittests/IR/IRBuilderTest.cpp
+++ llvm/unittests/IR/IRBuilderTest.cpp
@@ -257,51 +257,51 @@
   ASSERT_TRUE(isa(V));
   auto *CII = cast(V);
   EXPECT_EQ(fp::ebStrict, CII->getExceptionBehavior());
-  EXPECT_EQ(fp::rmDynamic, CII->getRoundingMode());
+  EXPECT_EQ(RoundingMode::Dynamic, CII->getRoundingMode());
 
   Builder.setDefaultConstrainedExcept(fp::ebIgnore);
-  Builder.setDefaultConstrainedRounding(fp::rmUpward);
+  Builder.setDefaultConstrainedRounding(RoundingMode::TowardPositive);
   V = Builder.CreateFAdd(V, V);
   CII = cast(V);
   EXPECT_EQ(fp::ebIgnore, CII->getExceptionBehavior());
-  EXPECT_EQ(CII->getRoundingMode(), fp::rmUpward);
+  EXPECT_EQ(CII->getRoundingMode(), RoundingMode::TowardPositive);
 
   Builder.setDefaultConstrainedExcept(fp::ebIgnore);
-  Builder.setDefaultConstrainedRounding(fp::rmToNearest);
+  Builder.setDefaultConstrainedRounding(RoundingMode::NearestTiesToEven);
   V = Builder.CreateFAdd(V, V);
   CII = cast(V);
   EXPECT_EQ(fp::ebIgnore, CII->getExceptionBehavior());
-  EXPECT_EQ(fp::rmToNearest, CII->getRoundingMode());
+  EXPECT_EQ(RoundingMode::NearestTiesToEven, CII->getRoundingMode());
 
   Builder.setDefaultConstrainedExcept(fp::ebMayTrap);
-  Builder.setDefaultConstrainedRounding(fp::rmDownward);
+  Builder.setDefaultConstrainedRounding(RoundingMode::TowardNegative);
   V = Builder.CreateFAdd(V, V);
   CII = cast(V);
   EXPECT_EQ(fp::ebMayTrap, CII->getExceptionBehavior());
-  EXPECT_EQ(fp::rmDownward, CII->getRoundingMode());
+  EXPECT_EQ(RoundingMode::TowardNegative, CII->getRoundingMode());
 
   Builder.setDefaultConstrainedExcept(fp::ebStrict);
-  Builder.setDefaultConstrainedRounding(fp::rmTowardZero);
+  Builder.setDefaultConstrainedRounding(RoundingMode::TowardZero);
   V = Builder.CreateFAdd(V, V);
   CII = cast(V);
   EXPECT_EQ(fp::ebStrict, CII->getExceptionBehavior());
-  EXPECT_EQ(fp::rmTowardZero, CII->getRoundingMode());
+  EXPECT_EQ(RoundingMode::TowardZero, CII->getRoundingMode());
 
   Builder.setDefaultConstrainedExcept(fp::ebIgnore);
-  Builder.setDefaultConstrainedRounding(fp::rmDynamic);
+  Builder.setDefaultConstrainedRounding(RoundingMode::Dynamic);
   V = Builder.CreateFAdd(V, V);
   CII = cast(V);
   EXPECT_EQ(fp::ebIgnore, CII->getExceptionBehavior());
-  EXPECT_EQ(fp::rmDynamic, CII->getRoundingMode());
+  EXPECT_EQ(RoundingMode::Dynamic, CII->getRoundingMode());
 
   // Now override the defaults.
   Call = Builder.CreateConstrainedFPBinOp(
 Intrinsic::experimental_constrained_fadd, V, V, nullptr, "", nullptr,
-fp::rmDownward, fp::ebMayTrap);
+RoundingMode::TowardNegative, fp::ebMayTrap);
   CII = cast(Call);
   EXPECT_EQ(CII->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
   EXPECT_EQ(fp::ebMayTrap, CII->getExceptionBehavior());
-  EXPECT_EQ(fp::rmDownward, CII->getRoundingMode());
+  EXPECT_EQ(RoundingMode::TowardNegative, CII->getRoundingMode());
 
   Builder.CreateRetVoid();
   EXPECT_FALSE(verifyModule(*M));
Index: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -270,7 +270,7 @@
 }
 
 void FAddendCoef::operator+=(const FAddendCoef &That) {
-  enum APFloat::roundingMode RndMode = APFloat::rmNearestTiesToEven;
+  RoundingMode RndMode = RoundingMode::NearestTiesToEven;
   if (isInt() == That.isInt()) {
 if (isInt())
   IntVal += That.IntVal;
Index: llvm/lib/Support/APFloat.cpp
===
--- llvm/lib/Support/APFloat.cpp
+++ llvm/lib/Support/APFloat.cpp
@@ -171,6 +171,12 @@
 return semPPCDoubleDo

[PATCH] D77379: [FPEnv] Use single enum to represent rounding mode

[Serge Pavlov via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/ADT/FloatingPointMode.h:26
+/// assigned to the rounding modes must agree with the values used by 
FLT_ROUNDS
+/// (C11, 5.2.4.2.2p8).
+enum class RoundingMode : int8_t {

rjmccall wrote:
> I agree that we should use one enum across LLVM and Clang.  I'm not sure that 
> using the `FLT_ROUNDS` values is worthwhile, especially since (1) 
> `FLT_ROUNDS` doesn't specify a value for some of these (like 
> `NearestTiesToAway`) and (2) some of the values it does use (e.g. for 
> "indeterminable") make this actively more awkward to store.  And the most 
> useful thing we could do — matching the values of `FE_TONEAREST` and so on — 
> isn't possible because those values are unportable.  I'd rather we just pick 
> arbitrary, non-ABI-stable values, like we normally would, and then make the 
> places that rely on matching some external schema translate.
>  (1) FLT_ROUNDS doesn't specify a value for some of these (like 
> NearestTiesToAway)

In the recent C standard draft 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2454.pdf), there is support 
of all 5 rounding modes including values returned by `FLT_ROUNDS` 
(5.2.4.2.2p11), values used by `fegetround` and `fesetround` 
(FE_TONEARESTFROMZERO in 7.6p13)

>  (2) some of the values it does use (e.g. for "indeterminable") make this 
> actively more awkward to store.

This is not a rounding mode value, it is just error indicator returned by 
intrinsic functions, it does not need to be stored. Added comment about that.

> And the most useful thing we could do — matching the values of FE_TONEAREST 
> and so on — isn't possible because those values are unportable.

I am working on patch that implements `fesetround` as intrinsic function. It 
introduces two intrinsic functions, one is `llvm.set_rounding` (D74729 [FPEnv] 
Intrinsic for setting rounding mode) and the other is `llvm.convert_rounding` 
(unpublished yet). The latter translates platform-dependent values like 
FE_DOWNWARD into platform independent representation, which is the same as used 
by FLT_ROUNDS.

Actually the motivation for this patch was just the need to have 
platform-independent representation of rounding mode that could be used in IR, 
which is more or less platform-independent. The representation used by 
`FLT_ROUNDS` fits these purposes because:
* it is platform-neutral,
* it is defined by standard, and
* it encodes all IEEE rounding modes. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77379



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

[PATCH] D77305: [Analyzer][VLASize] Support multi-dimensional arrays.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

It would be great to see a test about properly setting the arrays extent. 
Otherwise LGTM.




Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:61
+  // warned about that already.
+  // FIXME: report bug?
+  if (SizeV.isUnknown())

Hmm, why would we? The size is most probably unknown to us.



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:162-172
+  const VariableArrayType *VLALast = nullptr;
+  while (VLA) {
+const Expr *SizeE = VLA->getSizeExpr();
+State = checkVLASize(C, State, SizeE);
+if (!State)
+  return;
+SizeExpr.push_back(SizeE);

Ahaa, so if we have a `vla[x][y][a][b]` variable, `VLALast` would be `vla[x]`, 
which allows you to get the actual element type. Could you explain that (maybe 
even with a small example like this) in the code?



Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:194-196
+  // Binary operation not created, return with the current state set.
+  C.addTransition(State);
   return;

How about 
```
We were not able to determine extent of the array, return with the original 
state.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77305



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

[PATCH] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:240
+  // Returns the SVETypeFlags for the given memory element type.
+  uint64_t encodeMemoryElementType(unsigned MT) {
+return encodeFlag(MT, "MemEltTypeMask");

Shouldn't `MT` be `uint64_t` instead of `unsigned`?


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

https://reviews.llvm.org/D76617



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

[PATCH] D75903: [AArch64][CodeGen] Fixing stack alignment of HFA arguments on AArch64 PCS

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.

From the AAPCS64's Parameter Passing Rules section 
(https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#642parameter-passing-rules),
 I believe the proposed handling is correct. The HFA related rules described in 
this section are:

  Stage B – Pre-padding and extension of arguments
  [...]
  B.2   If the argument type is an HFA or an HVA, then the argument is used 
unmodified.
  [...]

  Stage C – Assignment of arguments to registers and stack
  [...]
  C.2   If the argument is an HFA or an HVA and there are sufficient 
unallocated SIMD and Floating-point registers (NSRN + number of members <= 8), 
then the argument is allocated to SIMD and Floating-point Registers (with one 
register per member of the HFA or HVA). The NSRN is incremented by the number 
of registers used. The argument has now been allocated.
  C.3   If the argument is an HFA or an HVA then the NSRN is set to 8 and the 
size of the argument is rounded up to the nearest multiple of 8 bytes.
  C.4   If the argument is an HFA, an HVA, a Quad-precision Floating-point or 
Short Vector Type then the NSAA is rounded up to the larger of 8 or the Natural 
Alignment of the argument’s type.
  [...]

As per rule `C.4`, the argument should be allocated on the stack address 
rounded to the larger of 8 and its Natural Alignment, which is 32 according to 
what is specified by the Composite Types rules in sectoin 5.6 of that same 
document 
(https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#composite-types):

  5.6   Composite Types
  [...]
  - The natural alignment of a composite type is the maximum of each of the 
member alignments of the 'top-level' members of the composite type i.e. before 
any alignment adjustment of the entire composite is applied

In regards to the compatibility with other compilers, I'm not sure that 
following what seems to be an uncompliant behavior would be the best way to 
proceed. @rnk and @ostannard, what would be your take on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75903



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Kristóf Umann via Phabricator via cfe-commits]

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

LGTM, let's go! Thank you so much for sticking this out!




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:42-51
   /// The error state of a stream.
+  /// Valid only if the stream is opened.
   enum ErrorKindTy {
-NoError,/// No error flag is set or stream is not open.
-EofError,   /// EOF condition (`feof` is true).
-OtherError, /// Other (non-EOF) error (`ferror` is true).
-AnyError/// EofError or OtherError, used if exact error is unknown.
+/// No error flag is set (or stream is not open).
+NoError,
+/// EOF condition (`feof` is true).
+FEof,

Can a stream be both `feof` and `ferror`? Let's not delay this patch one moment 
longer, but a `TODO` to give this some thought might be nice.



Comment at: clang/test/Analysis/stream-error.c:1
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream 
-analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.StreamTester 
-analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
 

`core` isn't enabled! :o


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam marked 5 inline comments as done.
saiislam added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13630
+
+  // Map C11/C++11 memory ordering to LLVM memory ordering
+  switch (static_cast(ord)) {

sameerds wrote:
> There should no mention of any high-level language here. The correct enum to 
> validate against is llvm::AtomicOrdering from llvm/Support/AtomicOrdering.h, 
> and not the C ABI or any other language ABI.
Even though this builtin is supposed to be language-independent, here intention 
was to provide interface in terms of well known standard C11/C++11 enums for 
memory order (__ATOMIC_ACQUIRE, etc.), so that user of the builtin don't have 
to remember and modify their code. The builtin internally maps it as per the 
expectation of fence instruction.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13651
+  llvm::getConstantStringInfo(Scope, scp);
+  SSID = getLLVMContext().getOrInsertSyncScopeID(scp);
+

sameerds wrote:
> This seems to be creating a new ID for any arbitrary string passed as sync 
> scope. This should be validated against LLVMContext::getSyncScopeNames(). 
As the FE is not aware about specific target and implementation of sync scope 
for that target, getSyncScopeNames() here returns llvm'sdefault sync scopes, 
which only supports singlethreaded and system as valid scopes. Validity 
checking of memory scope string is being intentionally left for the later 
stages which deal with the generated IR.



Comment at: clang/test/CodeGenHIP/builtin_memory_fence.cpp:9
+  // CHECK: fence syncscope("workgroup") seq_cst
+  __builtin_memory_fence(__ATOMIC_SEQ_CST,  "workgroup");
+  

sameerds wrote:
> Orderings like `__ATOMIC_SEQ_CST` are defined for C/C++ memory models. They 
> should not be used with the new builtin because this new builtin does not 
> follow any specific language model. For user convenience, the right thing to 
> do is to introduce new tokens in the Clang preprocessor, similar to the 
> `__ATOMIC_*` tokens. The convenient shortcut is to just tell the user to 
> supply numerical values by looking at the LLVM source code.
> 
> From llvm/Support/AtomicOrdering.h, note how the numerical value for 
> `__ATOMIC_SEQ_CST` is 5, but the numerical value for the LLVM 
> SequentiallyConsistent ordering is 7. The numerical value 5 refers to the 
> LLVM ordering "release". So, if the implementation were correct, this line 
> should result in the following unexpected LLVM IR:
>   fence syncscope("workgroup") release
As you pointed out, the range of acquire to sequentiallly consistent memory 
orders for llvm::AtomicOrdering is [4, 7], while for llvm::AtomicOrderingCABI 
is [2, 5]. Enums of C ABI was taken to ensure easy of use for the users who are 
familiar with C/C++ standard memory model. It allows them to use macros like 
__ATOMIC_ACQUIRE etc.
Clang CodeGen of the builtin internally maps C ABI ordering to llvm atomic 
ordering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75917



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

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

[Andrew Ng via Phabricator via cfe-commits]

andrewng added a comment.

In D77184#1960437 , @thakis wrote:

> grimar, andrewng: You both have checkout and build on different drives too, 
> yes?


Sorry for not replying earlier. Yes, for most of my setups I have the source 
checkout on one drive and build output on another drive if available.

Thanks for committing the Windows fix.

Cheers,
Andrew


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77184



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

[PATCH] D75364: [clang-format] Handle macros in function params and return value

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

I think situations where we infer M(x) to expand to a "type" without 
const/volatile close-by are too ambiguous an we will have the opposite problem: 
formatting certain binary expressions as type pointer/references. Some sort of 
a list of macros that clang-format assumes expand to  a "type" could eliminate 
ambiguities, but has its own problems.
I think the case where there's a const/volatile/typename/typedef etc. around 
M(x) is very interesting; not sure if there are many practical ambiguities 
there.
There was an idea of giving a bunch of #define-s to clang-format (maybe with a 
small subset of what's possible) and use these rules to decide how to format 
matching forms, but that's hard (requires some sort of "virtual expanded token 
sequences" that the formatter should undestrand how to handle) to do and I 
don't know what's the status of that.




Comment at: clang/unittests/Format/FormatTest.cpp:7360
+  verifyFormat("int f(M(x) *p1 = nullptr, M(x) *p2, volatile M(x) *p3);");
+  verifyFormat("M(x) *foo();");
+  verifyFormat("const M(x) *foo(M(x) *a = nullptr);");

This is ambiguous: the `*` could be a binary operator: 
https://godbolt.org/z/n7Jr-h


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

https://reviews.llvm.org/D75364



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

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

[Sam McCall via Phabricator via cfe-commits]

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

Thanks for your patience!
This is very nice, only +100 lines on TUScheduler.cpp in the end!




Comment at: clang-tools-extra/clangd/TUScheduler.cpp:224
+  std::vector CIDiags, WantDiagnostics WantDiags) {
 // Make possibly expensive copy while not holding the lock.
+Request Req = {std::move(CI), std::move(PI), std::move(CIDiags), WantDiags,

comment is now obsolete, remove it and consider inlining `Req`



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:325
   mutable std::condition_variable ReqCV;   /* GUARDED_BY(Mutex) */
   std::shared_ptr LatestBuild; /* GUARDED_BY(Mutex) */
 

this is no longer locked or concurrently accessed right?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:378
+  /// Used to inform ASTWorker about a new preamble build by PreambleThread.
+  /// Diagnostics are only published through this callback, which ensures they
+  /// are always for newer versions of the file. As this callback gets called 
in

nit: last sentence is a fragment, I think you want to swap the dot and comma:

...through this callback. This ensures... of the file, as the callback...



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:408
+
+  /// Returns the latest built preamble, might be null if no preamble has been
+  /// built or latest attempt resulted in a failure.

Unused, I think?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:793
+  // Used to check whether we can update AST cache.
+  bool InputsAreTheSame =
+  std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==

I'd suggest InputsAreLatest or something that hints at *why* they might differ



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:810
+  });
+  llvm::Optional> AST = IdleASTs.take(this);
+  if (!AST) {

This is reusing the AST built for a read if the file (and preamble) hasn't 
changed, right?

I think it's helpful to explain where teh cache data might be coming from.

IIRC this is the case we thought wasn't so important. It competes with a 
different optimization: skipping clang-tidy, warning analysis, etc when 
building ASTs because we know their diagnostics will never be used.

This might be a good place to leave a comment like "FIXME: maybe we're better 
off never reusing this AST, so queued AST builds aren't required to produce 
diagnostics" or so



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:862
+  // Stash the AST in the cache for further use if it was build for current
+  // inputs.
+  if (InputsAreTheSame) {

This explains the what but not the why.
(We may be building an older version of the source, as the queue raced ahead 
while we were waiting on the preamble. In that case the queue can't reuse the 
AST we built.)
Comment could go here or on InputsAreTheSame


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76725



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

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

[Raul Tambre via Phabricator via cfe-commits]

tambre added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:4337
+EmitCXXThrowExpr(ThrowExpr);
+return EmitLValue(dead);
+  }

rsmith wrote:
> The IR we emit for the dead operand is unreachable; it's a bit wasteful to 
> generate it here and (hopefully!) leave it to the optimizer to remove it 
> again. Perhaps we could produce an `undef` lvalue instead? See 
> `CodeGenFunction::EmitUnsupportedLValue` for an example of how to build such 
> a value.
Done.



Comment at: clang/test/CodeGenCXX/throw-expressions.cpp:84-87
+  void conditional_throw() {
+int a, b;
+(true ? throw 0 : a) = 0;
+  }

rsmith wrote:
> Please add some `CHECK`s in here to make sure we actually emit a call to 
> `__cxa_throw`.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77502



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

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 255275.
tambre marked 4 inline comments as done.
tambre added a comment.

Emit undef lvalue, add throw CHECK to test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77502

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCXX/throw-expressions.cpp


Index: clang/test/CodeGenCXX/throw-expressions.cpp
===
--- clang/test/CodeGenCXX/throw-expressions.cpp
+++ clang/test/CodeGenCXX/throw-expressions.cpp
@@ -79,6 +79,12 @@
   // CHECK-NOT: call {{.*}}@_ZN6DR15601AD1Ev
   // CHECK: call {{.*}} @__cxa_atexit({{.*}} @_ZN6DR15601AD1Ev {{.*}} 
@_ZGRN6DR15601rE
   // CHECK-NOT: call {{.*}}@_ZN6DR15601AD1Ev
+
+  // PR28184
+  void conditional_throw() {
+int a;
+(true ? throw 0 : a) = 0; // CHECK: call void @__cxa_throw({{.*}})
+  }
 }
 
 // CHECK-LABEL: define void @_Z5test7b(
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4331,6 +4331,15 @@
   // If the true case is live, we need to track its region.
   if (CondExprBool)
 incrementProfileCounter(expr);
+  // If a throw expression we need to return dead as lvalue.
+  if (auto *ThrowExpr = dyn_cast(live->IgnoreParens())) {
+EmitCXXThrowExpr(ThrowExpr);
+llvm::Type *Ty =
+llvm::PointerType::getUnqual(ConvertType(dead->getType()));
+return MakeAddrLValue(
+Address(llvm::UndefValue::get(Ty), CharUnits::One()),
+dead->getType());
+  }
   return EmitLValue(live);
 }
   }


Index: clang/test/CodeGenCXX/throw-expressions.cpp
===
--- clang/test/CodeGenCXX/throw-expressions.cpp
+++ clang/test/CodeGenCXX/throw-expressions.cpp
@@ -79,6 +79,12 @@
   // CHECK-NOT: call {{.*}}@_ZN6DR15601AD1Ev
   // CHECK: call {{.*}} @__cxa_atexit({{.*}} @_ZN6DR15601AD1Ev {{.*}} @_ZGRN6DR15601rE
   // CHECK-NOT: call {{.*}}@_ZN6DR15601AD1Ev
+
+  // PR28184
+  void conditional_throw() {
+int a;
+(true ? throw 0 : a) = 0; // CHECK: call void @__cxa_throw({{.*}})
+  }
 }
 
 // CHECK-LABEL: define void @_Z5test7b(
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4331,6 +4331,15 @@
   // If the true case is live, we need to track its region.
   if (CondExprBool)
 incrementProfileCounter(expr);
+  // If a throw expression we need to return dead as lvalue.
+  if (auto *ThrowExpr = dyn_cast(live->IgnoreParens())) {
+EmitCXXThrowExpr(ThrowExpr);
+llvm::Type *Ty =
+llvm::PointerType::getUnqual(ConvertType(dead->getType()));
+return MakeAddrLValue(
+Address(llvm::UndefValue::get(Ty), CharUnits::One()),
+dead->getType());
+  }
   return EmitLValue(live);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen updated this revision to Diff 255276.
sdesmalen added a comment.

Changed `unsigned` -> `uint64_t` for `encodeMemoryElementType` and 
`encodeMergeType`.


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

https://reviews.llvm.org/D76617

Files:
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/Basic/arm_sve.td
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -65,9 +65,6 @@
 applyModifier(CharMod);
   }
 
-  /// Return the value in SVETypeFlags for this type.
-  unsigned getTypeFlags() const;
-
   bool isPointer() const { return Pointer; }
   bool isVoidPointer() const { return Pointer && Void; }
   bool isSigned() const { return Signed; }
@@ -138,36 +135,22 @@
   /// The architectural #ifdef guard.
   std::string Guard;
 
+  // The merge suffix such as _m, _x or _z.
+  std::string MergeSuffix;
+
   /// The types of return value [0] and parameters [1..].
   std::vector Types;
 
   /// The "base type", which is VarType('d', BaseTypeSpec).
   SVEType BaseType;
 
-  unsigned Flags;
+  uint64_t Flags;
 
 public:
-  /// The type of predication.
-  enum MergeType {
-MergeNone,
-MergeAny,
-MergeOp1,
-MergeZero,
-MergeAnyExp,
-MergeZeroExp,
-MergeInvalid
-  } Merge;
-
-  Intrinsic(StringRef Name, StringRef Proto, int64_t MT, StringRef LLVMName,
-unsigned Flags, TypeSpec BT, ClassKind Class, SVEEmitter &Emitter,
-StringRef Guard)
-  : Name(Name.str()), LLVMName(LLVMName), Proto(Proto.str()),
-BaseTypeSpec(BT), Class(Class), Guard(Guard.str()), BaseType(BT, 'd'),
-Flags(Flags), Merge(MergeType(MT)) {
-// Types[0] is the return value.
-for (unsigned I = 0; I < Proto.size(); ++I)
-  Types.emplace_back(BaseTypeSpec, Proto[I]);
-  }
+  Intrinsic(StringRef Name, StringRef Proto, uint64_t MergeTy,
+StringRef MergeSuffix, uint64_t MemoryElementTy, StringRef LLVMName,
+uint64_t Flags, TypeSpec BT, ClassKind Class, SVEEmitter &Emitter,
+StringRef Guard);
 
   ~Intrinsic()=default;
 
@@ -179,14 +162,13 @@
 
   StringRef getGuard() const { return Guard; }
   ClassKind getClassKind() const { return Class; }
-  MergeType getMergeType() const { return Merge; }
 
   SVEType getReturnType() const { return Types[0]; }
   ArrayRef getTypes() const { return Types; }
   SVEType getParamType(unsigned I) const { return Types[I + 1]; }
   unsigned getNumParams() const { return Proto.size() - 1; }
 
-  unsigned getFlags() const { return Flags; }
+  uint64_t getFlags() const { return Flags; }
   bool isFlagSet(uint64_t Flag) const { return Flags & Flag;}
 
   /// Return the type string for a BUILTIN() macro in Builtins.def.
@@ -209,7 +191,7 @@
   void emitIntrinsic(raw_ostream &OS) const;
 
 private:
-  std::string getMergeSuffix() const;
+  std::string getMergeSuffix() const { return MergeSuffix; }
   std::string mangleName(ClassKind LocalCK) const;
   std::string replaceTemplatedArgs(std::string Name, TypeSpec TS,
std::string Proto) const;
@@ -221,8 +203,8 @@
   llvm::StringMap EltTypes;
   llvm::StringMap MemEltTypes;
   llvm::StringMap FlagTypes;
+  llvm::StringMap MergeTypes;
 
-  unsigned getTypeFlags(const SVEType &T);
 public:
   SVEEmitter(RecordKeeper &R) : Records(R) {
 for (auto *RV : Records.getAllDerivedDefinitions("EltType"))
@@ -231,8 +213,42 @@
   MemEltTypes[RV->getNameInitAsString()] = RV->getValueAsInt("Value");
 for (auto *RV : Records.getAllDerivedDefinitions("FlagType"))
   FlagTypes[RV->getNameInitAsString()] = RV->getValueAsInt("Value");
+for (auto *RV : Records.getAllDerivedDefinitions("MergeType"))
+  MergeTypes[RV->getNameInitAsString()] = RV->getValueAsInt("Value");
+  }
+
+  // Returns the SVETypeFlags for a given value and mask.
+  uint64_t encodeFlag(uint64_t V, StringRef MaskName) const {
+auto It = FlagTypes.find(MaskName);
+if (It != FlagTypes.end()) {
+  uint64_t Mask = It->getValue();
+  unsigned Shift = llvm::countTrailingZeros(Mask);
+  return (V << Shift) & Mask;
+}
+llvm_unreachable("Unsupported flag");
+  }
+
+  // Returns the SVETypeFlags for the given element type.
+  uint64_t encodeEltType(StringRef EltName) {
+auto It = EltTypes.find(EltName);
+if (It != EltTypes.end())
+  return encodeFlag(It->getValue(), "EltTypeMask");
+llvm_unreachable("Unsupported EltType");
+  }
+
+  // Returns the SVETypeFlags for the given memory element type.
+  uint64_t encodeMemoryElementType(uint64_t MT) {
+return encodeFlag(MT, "MemEltTypeMask");
   }
 
+  // Returns the SVETypeFlags for the given merge type.
+  uint64_t encodeMergeType(uint64_t MT) {
+return encodeFlag(MT, "MergeTypeMask");
+  }
+
+  // Returns the SVETypeFlags value for the given SVEType.
+  ui

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

[Paula Toth via Phabricator via cfe-commits]

PaulkaToast added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp:21
+  Finder->addMatcher(
+  decl(hasParent(translationUnitDecl()), unless(linkageSpecDecl()))
+  .bind("child_of_translation_unit"),

aaron.ballman wrote:
> This skips linkage spec declarations, but are there other declarations that 
> should be similarly skipped? For instance `static_assert` declarations?
I believe that linkage is the only exception needed, static_asserts and all 
other declarations should also be within the namespace. 



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp:33-34
+
+  if (isa(MatchedDecl)) {
+const auto *NS = cast(MatchedDecl);
+if (NS->getName() != RequiredNamespace) {

aaron.ballman wrote:
> Instead of doing an `isa<>` followed by a `cast<>`, the more common pattern 
> is to do:
> ```
> if (const auto *NS = dyn_cast(MatchedDecl)) {
> ```
Ah thank you this looks much nicer!



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp:42
+  diag(MatchedDecl->getLocation(),
+   "Please wrap implentation in '%0' namespace.")
+  << RequiredNamespace;

aaron.ballman wrote:
> They also aren't grammatically correct sentences, so the capital P and period 
> should both go. While this definitely gets points for politeness, I think a 
> more typical diagnostic might be: `declaration must be declared within the 
> '%0' namespace`
ah, alright, good call. (:



Comment at: 
clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h:35
+private:
+  std::string RequiredNamespace;
+};

aaron.ballman wrote:
> njames93 wrote:
> > This can be made const
> Will there only ever be a single namespace? Or should this be a list (for 
> instance, a main namespace and a details namespace)?
There will only be this one namespace.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-implementation-in-namespace.rst:32-35
+.. option:: RequiredNamespace
+
+The namespace that llvm-libc implementations must be wrapped in. The 
default
+is `__llvm_libc`.

aaron.ballman wrote:
> Given that this check is specific to llvm-libc, why is the option needed at 
> all?
I was concerned that maybe there would be a desire to make the check 
generalized, however it does seem quite specific so I will take your advice. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76818



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

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

[Paula Toth via Phabricator via cfe-commits]

PaulkaToast updated this revision to Diff 255277.
PaulkaToast marked 13 inline comments as done.
PaulkaToast added a comment.

Addressed njames's and aaron's comments. (:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76818

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h
  clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/llvmlibc-implementation-in-namespace.rst
  
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s llvmlibc-implementation-in-namespace %t
+
+#define MACRO_A "defining macros outside namespace is valid"
+
+class ClassB;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the '__llvm_libc' namespace
+struct StructC {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration must be declared within the '__llvm_libc' namespace
+char *VarD = MACRO_A;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration must be declared within the '__llvm_libc' namespace
+typedef int typeE;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: declaration must be declared within the '__llvm_libc' namespace
+void funcF() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration must be declared within the '__llvm_libc' namespace
+
+namespace namespaceG {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: '__llvm_libc' needs to be the outermost namespace
+namespace __llvm_libc{
+namespace namespaceH {
+class ClassB;
+} // namespace namespaceH
+struct StructC {};
+} // namespace __llvm_libc
+char *VarD = MACRO_A;
+typedef int typeE;
+void funcF() {}
+} // namespace namespaceG
+
+// Wrapped in correct namespace.
+namespace __llvm_libc {
+// Namespaces within __llvim_libc namespace allowed.
+namespace namespaceI {
+class ClassB;
+} // namespace namespaceI
+struct StructC {};
+char *VarD = MACRO_A;
+typedef int typeE;
+void funcF() {}
+extern "C" void extern_funcJ() {}
+} // namespace __llvm_libc
Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-implementation-in-namespace.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-implementation-in-namespace.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - llvmlibc-implementation-in-namespace
+
+llvmlibc-implementation-in-namespace
+
+
+Checks that all declarations in the llvm-libc implementation are within the
+correct namespace.
+
+.. code-block:: c++
+
+// Correct: implementation inside the correct namespace.
+namespace __llvm_libc {
+void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+// Namespaces within __llvm_libc namespace are allowed.
+namespace inner{
+int localVar = 0;
+}
+// Functions with C linkage are allowed.
+extern "C" void str_fuzz(){}
+}
+
+// Incorrect: implementation not in a namespace.
+void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+
+// Incorrect: outer most namespace is not correct.
+namespace something_else {
+void LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {}
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-prefer-isa-or-dyn-cast-in-conditionals `_, "Yes"
`llvm-prefer-register-over-unsigned `_, "Yes"
`llvm-twine-local `_, "Yes"
+   `llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
`misc-misplaced-const `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
+- New :doc:`llvmlibc-implementation-in-namespace
+  ` check.
+
+  Checks all llvm-libc implementation is within the correct namespace.
+
 - New :doc:`llvmlibc-restrict-system-libc-headers
   ` check.
 
Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyM

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen updated this revision to Diff 255278.
sdesmalen marked 3 inline comments as done.
sdesmalen added a comment.

- Renamed `ImmCheckPredicatePattern` -> `ImmCheck0_31`
- Updated tests.


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

https://reviews.llvm.org/D76678

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/Basic/arm_sve.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_qdech.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_qdech.c
  clang/utils/TableGen/SveEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -95,6 +95,7 @@
 void EmitSveBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitSveBuiltinCG(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitSveTypeFlags(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitSveRangeChecks(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
 void EmitMveHeader(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitMveBuiltinDef(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -74,6 +74,7 @@
   GenArmSveBuiltins,
   GenArmSveBuiltinCG,
   GenArmSveTypeFlags,
+  GenArmSveRangeChecks,
   GenArmCdeHeader,
   GenArmCdeBuiltinDef,
   GenArmCdeBuiltinSema,
@@ -197,6 +198,8 @@
"Generate arm_sve_builtin_cg_map.inc for clang"),
 clEnumValN(GenArmSveTypeFlags, "gen-arm-sve-typeflags",
"Generate arm_sve_typeflags.inc for clang"),
+clEnumValN(GenArmSveRangeChecks, "gen-arm-sve-sema-rangechecks",
+   "Generate arm_sve_sema_rangechecks.inc for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",
"Generate arm_mve.h for clang"),
 clEnumValN(GenArmMveBuiltinDef, "gen-arm-mve-builtin-def",
@@ -390,6 +393,9 @@
   case GenArmSveTypeFlags:
 EmitSveTypeFlags(Records, OS);
 break;
+  case GenArmSveRangeChecks:
+EmitSveRangeChecks(Records, OS);
+break;
   case GenArmCdeHeader:
 EmitCdeHeader(Records, OS);
 break;
Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -46,6 +46,22 @@
 
 namespace {
 
+class ImmCheck {
+  unsigned Arg;
+  unsigned Kind;
+  unsigned ElementSizeInBits;
+
+public:
+  ImmCheck(unsigned Arg, unsigned Kind, unsigned ElementSizeInBits = 0)
+  : Arg(Arg), Kind(Kind), ElementSizeInBits(ElementSizeInBits) {}
+  ImmCheck(const ImmCheck &Other) = default;
+  ~ImmCheck() = default;
+
+  unsigned getArg() const { return Arg; }
+  unsigned getKind() const { return Kind; }
+  unsigned getElementSizeInBits() const { return ElementSizeInBits; }
+};
+
 class SVEType {
   TypeSpec TS;
   bool Float, Signed, Immediate, Void, Constant, Pointer;
@@ -146,11 +162,13 @@
 
   uint64_t Flags;
 
+  SmallVector ImmChecks;
+
 public:
   Intrinsic(StringRef Name, StringRef Proto, uint64_t MergeTy,
 StringRef MergeSuffix, uint64_t MemoryElementTy, StringRef LLVMName,
-uint64_t Flags, TypeSpec BT, ClassKind Class, SVEEmitter &Emitter,
-StringRef Guard);
+uint64_t Flags, ArrayRef ImmChecks, TypeSpec BT,
+ClassKind Class, SVEEmitter &Emitter, StringRef Guard);
 
   ~Intrinsic()=default;
 
@@ -171,6 +189,8 @@
   uint64_t getFlags() const { return Flags; }
   bool isFlagSet(uint64_t Flag) const { return Flags & Flag;}
 
+  ArrayRef getImmChecks() const { return ImmChecks; }
+
   /// Return the type string for a BUILTIN() macro in Builtins.def.
   std::string getBuiltinTypeStr();
 
@@ -204,6 +224,7 @@
   llvm::StringMap MemEltTypes;
   llvm::StringMap FlagTypes;
   llvm::StringMap MergeTypes;
+  llvm::StringMap ImmCheckTypes;
 
 public:
   SVEEmitter(RecordKeeper &R) : Records(R) {
@@ -215,6 +236,16 @@
   FlagTypes[RV->getNameInitAsString()] = RV->getValueAsInt("Value");
 for (auto *RV : Records.getAllDerivedDefinitions("MergeType"))
   MergeTypes[RV->getNameInitAsString()] = RV->getValueAsInt("Value");
+for (auto *RV : Records.getAllDerivedDefinitions("ImmCheckType"))
+  ImmCheckTypes[RV->getNameInitAsString()] = RV->getValueAsInt("Value");
+  }
+
+  /// Returns the enum value for the immcheck type
+  unsigned getEnumValueForImmCheck(StringRef C) const {
+auto It = ImmCheckTypes.find(C);
+if (It 

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

[Eugene Leviant via Phabricator via cfe-commits]

evgeny777 added a comment.

This needs to be rebased




Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1784
+  ImportSummary->getTypeIdSummary(cast(TypeId)->getString());
+  if (!TidSummary)
+RemoveTypeTestAssumes();

tejohnson wrote:
> Here is the fix for the issue with the multi-stage clang bootstrap test 
> failures described in D75201. I also restructured this code to try to make it 
> clearer (rather than an early continue when we don't need to remove type test 
> assumes, make it an explicit removal when needed).
> 
> Added new test llvm/test/ThinLTO/X86/type_test_noindircall.ll for this.
Can you please also test the case when LTO unit is split?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242



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

[PATCH] D76617: [SveEmitter] Fix encoding/decoding of SVETypeFlags

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added a comment.

LGTM

This patch now implements a bit more than the original commit message would 
suggest. Could you please update? Thanks for working on this!


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

https://reviews.llvm.org/D76617



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

[PATCH] D76680: [SveEmitter] Add immediate checks for lanes and complex imms

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen updated this revision to Diff 255282.
sdesmalen added a comment.

- Updated tests.


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

https://reviews.llvm.org/D76680

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/arm_sve.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_mla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -455,9 +455,19 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'e':
+Signed = false;
+ElementBitwidth /= 2;
+break;
   case 'h':
 ElementBitwidth /= 2;
 break;
+  case 'q':
+ElementBitwidth /= 4;
+break;
+  case 'o':
+ElementBitwidth *= 4;
+break;
   case 'P':
 Signed = true;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svfloat16_t test_svmla_lane_f16(svfloat16_t op1, svfloat16_t op2, svfloat16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f16,,)(op1, op2, op3, 8);
+}
+
+svfloat32_t test_svmla_lane_f32(svfloat32_t op1, svfloat32_t op2, svfloat32_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f32,,)(op1, op2, op3, -1);
+}
+
+svfloat64_t test_svmla_lane_f64(svfloat64_t op1, svfloat64_t op2, svfloat64_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f64,,)(op1, op2, op3, 2);
+}
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svint32_t test_svdot_lane_s32(svint32_t op1, svint8_t op2, svint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s32,,)(op1, op2, op3, -1);
+}
+
+svint32_t test_svdot_lane_s32_1(svint32_t op1, svint8_t op2, svint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s32,,)(op1, op2, op3, 4);
+}
+
+svint64_t test_svdot_lane_s64(svint64_t op1, svint16_t op2, svint16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s64,,)(op1, op2, op3, -1);
+}
+
+svint64_t test_svdot_lane_s64_1(svint64_t op1, svint16_t op2, svint16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s64,,)(op1, op2, op3, 2);
+}
+
+svuint32_t test_svdot_lane_u32(svuint32_t op1, svuint8_t op2, svuint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_u32,,)(op1, op2, op3, 4);
+}
+
+svuint64_t test_svdot_lane_u64(svuint64_t op1, svuint16_t op2, svuint

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 255279.
baloghadamsoftware added a comment.

More progress...


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

https://reviews.llvm.org/D77229

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/iterator-modeling.cpp

Index: clang/test/Analysis/iterator-modeling.cpp
===
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -28,7 +28,7 @@
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
 
-void begin(const std::vector &v) {
+/*void begin(const std::vector &v) {
   auto i = v.begin();
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i) == &v); // expected-warning{{TRUE}}
@@ -1773,7 +1773,7 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i3)); // expected-warning{{$i1 + 2}} FIXME: Should be $i1 + 1
   // clang_analyzer_express(clang_analyzer_iterator_position(i5)); FIXME: expect warning $i1 + 1
   clang_analyzer_express(clang_analyzer_iterator_position(i4)); // expected-warning{{$FL.end()}}
-}
+}*/
 
 struct simple_iterator_base {
   simple_iterator_base();
@@ -1812,7 +1812,7 @@
   }
 }
 
-void iter_diff(std::vector &V) {
+/*void iter_diff(std::vector &V) {
   auto i0 = V.begin(), i1 = V.end();
   ptrdiff_t len = i1 - i0; // no-crash
 }
@@ -1880,3 +1880,4 @@
 // CHECK-NEXT: "i1 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
 // CHECK-NEXT:   ]}
 }
+*/
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -109,6 +109,96 @@
   return LValue;
 }
 
+Optional ExprEngine::retrieveFromConstructionContext(
+ProgramStateRef State, const LocationContext *LCtx,
+const ConstructionContext *CC) const {
+  if (CC) {
+switch (CC->getKind()) {
+case ConstructionContext::CXX17ElidedCopyVariableKind:
+case ConstructionContext::SimpleVariableKind: {
+  const auto *DSCC = cast(CC);
+  const auto *DS = DSCC->getDeclStmt();
+  return getObjectUnderConstruction(State, DS, LCtx);
+}
+case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
+case ConstructionContext::SimpleConstructorInitializerKind: {
+  const auto *ICC = cast(CC);
+  const auto *Init = ICC->getCXXCtorInitializer();
+  return getObjectUnderConstruction(State, Init, LCtx);
+}
+case ConstructionContext::SimpleReturnedValueKind:
+case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
+  const StackFrameContext *SFC = LCtx->getStackFrame();
+  if (const LocationContext *CallerLCtx = SFC->getParent()) {
+auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+   .getAs();
+if (!RTC) {
+  // We were unable to find the correct construction context for the
+  // call in the parent stack frame. This is equivalent to not being
+  // able to find construction context at all.
+  break;
+}
+if (isa(CallerLCtx)) {
+  // Unwrap block invocation contexts. They're mostly part of
+  // the current stack frame.
+  CallerLCtx = CallerLCtx->getParent();
+  assert(!isa(CallerLCtx));
+}
+return retrieveFromConstructionContext(
+  State, CallerLCtx, RTC->getConstructionContext());
+  }
+  break;
+}
+case ConstructionContext::ElidedTemporaryObjectKind: {
+  assert(AMgr.getAnalyzerOptions().ShouldElideConstructors);
+  const auto *TCC = cast(CC);
+  Optional RetVal = retrieveFromConstructionContext(
+  State, LCtx, TCC->getConstructionContextAfterElision());
+  if (RetVal.hasValue())
+return RetVal;
+  
+  LLVM_FALLTHROUGH;
+}
+case ConstructionContext::SimpleTemporaryObjectKind: {
+  const auto *TCC = cast(CC);
+  const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
+  Optional RetVal;
+  if (BTE) {
+RetVal = getObjectUnderConstruction(State, BTE, LCtx);
+if (RetVal.hasValue())
+  return RetVal;
+  }
+  
+  const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
+  if (MTE)
+RetVal = getObjectUnderConstruction(State, MTE, LCtx);
+
+  return RetVal;
+}
+case

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks for the patch!

this is quite similar to handling `kw_virtual` even handling multiple `static` 
keywords. Could you rather factor the `search for kw, generate an edit to 
delete specifier` part into a `DelKeyword` lambda similar to `DelAttr` and call 
it with kw_static and kw_virtual?

p.s. you can make use of `tok::getKeywordSpelling` for error messages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77534



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

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189
+  if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+if (const ElementRegion *TheElementRegion =
+MRegion->getAs()) {

NoQ wrote:
> The sequence of `FieldRegion`s and `ElementRegion`s on top of a base region 
> may be arbitrary: `var.a[0].b[1][2].c.d[3]` etc.
> 
> I'd rather unwrap those regions one-by-one in a loop and look at the 
> alignment of each layer.
Alternatively, just decompose the whole region into base region and offset and 
see if base region has the necessary alignment and the offset is divisible by 
the necessary alignment.


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

https://reviews.llvm.org/D76996



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

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh, Ádám via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:127
+  const auto *Init = ICC->getCXXCtorInitializer();
+  return getObjectUnderConstruction(State, Init, LCtx);
+}

This does not find anything in the map for base initializers.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:230
   const auto *Init = ICC->getCXXCtorInitializer();
   assert(Init->isAnyMemberInitializer());
   const CXXMethodDecl *CurCtor = cast(LCtx->getDecl());

This assertion seems to be unfounded. What to do with base initializers? 
However, in our case it seems that this function should never be executed 
because to the objects under construction are already in the map. However, I do 
not know how to handle base initializers.



Comment at: clang/test/Analysis/iterator-modeling.cpp:1791
 
 struct simple_derived_iterator: public simple_iterator_base {
   int& operator*();

We are crashing here (assertion) on calling the base initializer.


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

https://reviews.llvm.org/D77229



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

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Uh-oh, it's annoying indeed that you can't easily obtain the current CFG 
element from inside a `CallEvent`.

Given that every `CallEvent` is in fact associated with a `CFGElement` (but not 
every `CallEvent` is associated with an `Expr` or have a `Decl`), can we modify 
the `CallEvent` structure to store a `CFGElementRef` //instead// of 
`llvm::PointerUnion Origin;`? That might be some 
work but it should increase our sanity by a lot.


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

https://reviews.llvm.org/D77229



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

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:1888
+// Check valididty of memory ordering as per C11 / C++11's memody model.
+if (ord < static_cast(llvm::AtomicOrderingCABI::acquire) ||
+  ord > static_cast(llvm::AtomicOrderingCABI::seq_cst)) {

I think I'd write this as a switch over the enum instead of a ranged compare.

It'll codegen to the same thing, but we'll get warnings if more values are 
introduced to the enum and things will keep working (here, anyway) if the 
values are reordered.



Comment at: clang/test/CodeGenHIP/builtin_memory_fence.cpp:9
+  // CHECK: fence syncscope("workgroup") seq_cst
+  __builtin_memory_fence(__ATOMIC_SEQ_CST,  "workgroup");
+  

saiislam wrote:
> sameerds wrote:
> > Orderings like `__ATOMIC_SEQ_CST` are defined for C/C++ memory models. They 
> > should not be used with the new builtin because this new builtin does not 
> > follow any specific language model. For user convenience, the right thing 
> > to do is to introduce new tokens in the Clang preprocessor, similar to the 
> > `__ATOMIC_*` tokens. The convenient shortcut is to just tell the user to 
> > supply numerical values by looking at the LLVM source code.
> > 
> > From llvm/Support/AtomicOrdering.h, note how the numerical value for 
> > `__ATOMIC_SEQ_CST` is 5, but the numerical value for the LLVM 
> > SequentiallyConsistent ordering is 7. The numerical value 5 refers to the 
> > LLVM ordering "release". So, if the implementation were correct, this line 
> > should result in the following unexpected LLVM IR:
> >   fence syncscope("workgroup") release
> As you pointed out, the range of acquire to sequentiallly consistent memory 
> orders for llvm::AtomicOrdering is [4, 7], while for llvm::AtomicOrderingCABI 
> is [2, 5]. Enums of C ABI was taken to ensure easy of use for the users who 
> are familiar with C/C++ standard memory model. It allows them to use macros 
> like __ATOMIC_ACQUIRE etc.
> Clang CodeGen of the builtin internally maps C ABI ordering to llvm atomic 
> ordering.
What language, implemented in clang, do you have in mind that reusing the 
existing __ATOMIC_* macros would be incorrect for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75917



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

[PATCH] D75068: libclang: Add static build support for Windows

[Cristian Adam via Phabricator via cfe-commits]

cristian.adam added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75068



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

[PATCH] D77532: Fix a typo in clang/lib/Frontend/FrontendAction.cpp

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! Do you need someone to commit on your behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77532



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

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 255288.
saiislam added a comment.

Changes:

1. Moved builtin definition with rest of atomic builtins
2. Updated validity checking of memory order using exact mathches instead of 
range matching
3. Added a sucessful test case which passes arbitrary string scope
4. Corrected formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75917

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenHIP/builtin_memory_fence.cpp
  clang/test/Sema/builtins.c

Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -322,13 +322,13 @@
 }
 
 void test_memory_fence_errors() {
-  __builtin_memory_fence(__ATOMIC_SEQ_CST + 1, "workgroup");  // expected-warning {{memory order argument to atomic operation is invalid}}
+  __builtin_memory_fence(__ATOMIC_SEQ_CST + 1, "workgroup"); // expected-warning {{memory order argument to atomic operation is invalid}}
 
-  __builtin_memory_fence(__ATOMIC_ACQUIRE - 1, "workgroup");  // expected-warning {{memory order argument to atomic operation is invalid}}
+  __builtin_memory_fence(__ATOMIC_ACQUIRE - 1, "workgroup"); // expected-warning {{memory order argument to atomic operation is invalid}}
 
-  __builtin_memory_fence(4);  // expected-error {{too few arguments to function call, expected 2}}
+  __builtin_memory_fence(4); // expected-error {{too few arguments to function call, expected 2}}
 
-  __builtin_memory_fence(4, 4, 4);  // expected-error {{too many arguments to function call, expected 2}}
+  __builtin_memory_fence(4, 4, 4); // expected-error {{too many arguments to function call, expected 2}}
 
-  __builtin_memory_fence(3.14, "");  // expected-warning {{implicit conversion from 'double' to 'unsigned int' changes value from 3.14 to 3}}
+  __builtin_memory_fence(3.14, ""); // expected-warning {{implicit conversion from 'double' to 'unsigned int' changes value from 3.14 to 3}}
 }
Index: clang/test/CodeGenHIP/builtin_memory_fence.cpp
===
--- clang/test/CodeGenHIP/builtin_memory_fence.cpp
+++ clang/test/CodeGenHIP/builtin_memory_fence.cpp
@@ -19,4 +19,7 @@
 
 // CHECK: fence syncscope("workgroup") release
   __builtin_memory_fence(3, "workgroup");
+
+  // CHECK: fence syncscope("foobar") release
+  __builtin_memory_fence(3, "foobar");
 }
\ No newline at end of file
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1869,8 +1869,7 @@
   ? "__builtin_return_address"
   : "__builtin_frame_address")
   << TheCall->getSourceRange();
-  }
-break;
+  } break;
 
   case Builtin::BI__builtin_memory_fence: {
 ExprResult Arg = TheCall->getArg(0);
@@ -1878,22 +1877,27 @@
 Expr::EvalResult ArgResult;
 
 if(!ArgExpr->EvaluateAsInt(ArgResult, Context)) {
-  Diag(ArgExpr->getExprLoc(), diag::err_typecheck_expect_int) <<
-   ArgExpr->getType();
+  Diag(ArgExpr->getExprLoc(), diag::err_typecheck_expect_int)
+<< ArgExpr->getType();
   return ExprError();
 }
 int ord = ArgResult.Val.getInt().getZExtValue();
 
 // Check valididty of memory ordering as per C11 / C++11's memody model.
-if (ord < static_cast(llvm::AtomicOrderingCABI::acquire) ||
-  ord > static_cast(llvm::AtomicOrderingCABI::seq_cst)) {
-  Diag(ArgExpr->getBeginLoc(),
-  diag::warn_atomic_op_has_invalid_memory_order)
-  << ArgExpr->getSourceRange();
-  return ExprError();
-}
+switch (static_cast(ord)) {
+  case llvm::AtomicOrderingCABI::acquire:
+  case llvm::AtomicOrderingCABI::release:
+  case llvm::AtomicOrderingCABI::acq_rel:
+  case llvm::AtomicOrderingCABI::seq_cst:
+break;
+  default: {
+Diag(ArgExpr->getBeginLoc(),
+diag::warn_atomic_op_has_invalid_memory_order)
+  << ArgExpr->getSourceRange();
+return ExprError();
+  }
 }
-break;
+} break;
   }
 
   // Since the target specific builtins for each arch overlap, only check those
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13624,7 +13624,7 @@
 Value *Order = EmitScalarExpr(E->getArg(0));
 Value *Scope = EmitScalarExpr(E->getArg(1));
 
-if ( isa(Order)) {
+if (isa(Order)) {
   int ord = cast(Order)->getZExtValue();
 
   // Map C11/C++11 memory ordering to LLVM memory ordering
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/cla

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

[Sjoerd Meijer via Phabricator via cfe-commits]

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

LGTM, please wait a day in case Eli has more comments.


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

https://reviews.llvm.org/D76678



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

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D77229#1963485 , @NoQ wrote:

> Uh-oh, it's annoying indeed that you can't easily obtain the current CFG 
> element from inside a `CallEvent`.
>
> Given that every `CallEvent` is in fact associated with a `CFGElement` (but 
> not every `CallEvent` is associated with an `Expr` or have a `Decl`), can we 
> modify the `CallEvent` structure to store a `CFGElementRef` //instead// of 
> `llvm::PointerUnion Origin;`? That might be some 
> work but it should increase our sanity by a lot.


Yes, that seems a good idea. For now I solved this problem, but storing 
something instead of searching for it linearily sound better in regards of 
performance. However my current problem is worse, I have no idea at all, how to 
solve it.


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

https://reviews.llvm.org/D77229



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

[PATCH] D75364: [clang-format] Handle macros in function params and return value

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

A sample of snippets that this misformats:

- inside other macro:

  EXPECT_EQ(Calc()* bar, baz);

- in member function call:

  BENCHMARK(BM_Foo)->ThreadRange(1, NumCPUs()* 2);

- this causes javascript diffs:

  - const foo = (bar == 'rtl' ? -1 : 1) * (blah || blah || 0);
  + const foo =(bar == 'rtl' ? -1 : 1) * (blah || blah || 0);

- binary operators with casts:

  a = (int)(b()* c);
  rectaa->SetSize((int)(GetWidth(foo)* bar),
  (int)(GetHeight(foo)* bar));
  int f() {
...
  - return static_cast(param) * (a * b + c);
  + return static_cast(param)*(a * b + c);
  }



- some arithmetic expressions:

  float  = 43 * log(sin(a/4) *b);

Test:

  % cat test.cc   
  EXPECT_EQ(Calc() * bar, baz);
  
  BENCHMARK(BM_Foo)->ThreadRange(1, NumCPUs() * 2);
  
  a = (int)(b() * c);
  
  rectaa->SetSize((int)(GetWidth(foo) * bar),
  (int)(GetHeight(foo) * bar));
  
  int f() { return static_cast(param) * (a * b + c); }
  
  float  = 43 * log(sin(a / 4) * b);
  % bin/clang-format -style=google test.cc
  EXPECT_EQ(Calc()* bar, baz);
  
  BENCHMARK(BM_Foo)->ThreadRange(1, NumCPUs()* 2);
  
  a = (int)(b()* c);
  
  rectaa->SetSize((int)(GetWidth(foo)* bar),
  (int)(GetHeight(foo)* bar));
  
  int f() { return static_cast(param)*(a * b + c); }
  
  float  = 43 * log(sin(a / 4)* b);
  % cat test.js
  const foo = (bar == 'rtl' ? -1 : 1) * (blah || blah || 0);
  % bin/clang-format -style=google test.js
  const foo =(bar == 'rtl' ? -1 : 1) * (blah || blah || 0);


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

https://reviews.llvm.org/D75364



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

[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai created this revision.
nemanjai added reviewers: hfinkel, PowerPC.
Herald added subscribers: cfe-commits, shchenz, kbarton.
Herald added a project: clang.

We currently emit incorrect codegen for this constraint because we set it as a 
constraint that allows registers. This will cause the value to be copied to the 
stack and that address to be passed as the address. This is not what we want.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=42762


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77542

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/test/CodeGen/ppc64-inline-asm.c


Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -37,3 +37,16 @@
 // CHECK-LABEL: double @test_fmax(double %x, double %y)
 // CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", 
"=^ws,^ws,^ws"(double %x, double %y)
 }
+
+void testZ(void *addr) {
+  asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)addr) : "memory");
+// CHECK-LABEL: void @testZ(i8* %addr)
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %addr)
+}
+
+void testZwOff(void *addr, long long off) {
+  asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)(addr + off)) : 
"memory");
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHEC: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHEC: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* 
%[[VAL]])
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -276,11 +276,12 @@
   break;
 case 'Q': // Memory operand that is an offset from a register (it is
   // usually better to use `m' or `es' in asm statements)
+  Info.setAllowsRegister();
+  LLVM_FALLTHROUGH;
 case 'Z': // Memory operand that is an indexed or indirect from a
   // register (it is usually better to use `m' or `es' in
   // asm statements)
   Info.setAllowsMemory();
-  Info.setAllowsRegister();
   break;
 case 'R': // AIX TOC entry
 case 'a': // Address operand that is an indexed or indirect from a


Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -37,3 +37,16 @@
 // CHECK-LABEL: double @test_fmax(double %x, double %y)
 // CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ws,^ws,^ws"(double %x, double %y)
 }
+
+void testZ(void *addr) {
+  asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)addr) : "memory");
+// CHECK-LABEL: void @testZ(i8* %addr)
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %addr)
+}
+
+void testZwOff(void *addr, long long off) {
+  asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)(addr + off)) : "memory");
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHEC: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHEC: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %[[VAL]])
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -276,11 +276,12 @@
   break;
 case 'Q': // Memory operand that is an offset from a register (it is
   // usually better to use `m' or `es' in asm statements)
+  Info.setAllowsRegister();
+  LLVM_FALLTHROUGH;
 case 'Z': // Memory operand that is an indexed or indirect from a
   // register (it is usually better to use `m' or `es' in
   // asm statements)
   Info.setAllowsMemory();
-  Info.setAllowsRegister();
   break;
 case 'R': // AIX TOC entry
 case 'a': // Address operand that is an indexed or indirect from a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

In D76678#1963576 , @SjoerdMeijer 
wrote:

> LGTM, please wait a day in case Eli has more comments.


Will do, thanks Sjoerd!


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

https://reviews.llvm.org/D76678



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

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 255298.
njames93 added a comment.

- Moved keyword removing logic into a lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77534

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2142,6 +2142,17 @@
 };)cpp",
   "void B::foo()   {}\n",
   },
+  {
+  R"cpp(
+struct A {
+  static void fo^o() {}
+};)cpp",
+  R"cpp(
+struct A {
+  static void foo() ;
+};)cpp",
+  " void A::foo() {}\n",
+  },
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
@@ -2236,6 +2247,24 @@
 STUPID_MACRO(sizeof sizeof int) void foo() ;
   };)cpp",
" void A::foo() {}\n"},
+  {R"cpp(#define STAT static
+  struct A {
+STAT void f^oo() {}
+  };)cpp",
+   R"cpp(#define STAT static
+  struct A {
+STAT void foo() ;
+  };)cpp",
+   " void A::foo() {}\n"},
+  {R"cpp(#define STUPID_MACRO(X) static
+  struct A {
+STUPID_MACRO(sizeof sizeof int) void f^oo() {}
+  };)cpp",
+   R"cpp(#define STUPID_MACRO(X) static
+  struct A {
+STUPID_MACRO(sizeof sizeof int) void foo() ;
+  };)cpp",
+   " void A::foo() {}\n"},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Test);
@@ -2360,8 +2389,8 @@
   struct A {
 VIRT fo^o() {}
   };)cpp",
-  "fail: define outline: Can't move out of line as function has a "
-  "macro `virtual` specifier."},
+  "fail: define outline: can't move outline as can't remove `virtual` "
+  "keyword."},
   {
   R"cpp(
   #define OVERFINAL final override
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -239,21 +239,36 @@
   DelAttr(FD->getAttr());
   DelAttr(FD->getAttr());
 
-  if (FD->isVirtualAsWritten()) {
-SourceRange SpecRange{FD->getBeginLoc(), FD->getLocation()};
-bool HasErrors = true;
-
-// Clang allows duplicating virtual specifiers so check for multiple
-// occurances.
-for (const auto &Tok : TokBuf.expandedTokens(SpecRange)) {
-  if (Tok.kind() != tok::kw_virtual)
+  auto DeleteKeyword = [&](tok::TokenKind Kind, SourceRange FromRange,
+   bool AllowMultiple = false) {
+bool FoundAny = false;
+for (const auto &Tok : TokBuf.expandedTokens(FromRange)) {
+  if (Tok.kind() != Kind)
 continue;
+  if (FoundAny && !AllowMultiple) {
+Errors = llvm::joinErrors(
+std::move(Errors),
+llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+(llvm::StringRef("define outline: can't move outline as "
+ "multiple `") +
+ tok::getKeywordSpelling(Kind) + "` keywords exist.")
+.str()));
+break;
+  }
+  FoundAny = true;
   auto Spelling = TokBuf.spelledForExpanded(llvm::makeArrayRef(Tok));
   if (!Spelling) {
-HasErrors = true;
+Errors = llvm::joinErrors(
+std::move(Errors),
+llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+(StringRef(
+ "define outline: can't move outline as can't remove `") +
+ tok::getKeywordSpelling(Kind) + "` keyword.")
+.str()));
 break;
   }
-  HasErrors = false;
   CharSourceRange DelRange =
   syntax::Token::range(SM, Spelling->front(), Spelling->back())
   .toCharRange(SM);
@@ -261,14 +276,22 @@
   DeclarationCleanups.add(tooling::Replacement(SM, DelRange, "")))
 Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
 }
-if (HasErrors) {
+if (!FoundAny)
   Errors = llvm::joinErrors(
   std::move(Errors),
-  llvm::createStringError(llvm::inconvertibleErrorCode(),
-  "define outline: Can't move out of line as "
-  "function has a macro `virtual` specifier."));
-}
-  }
+  llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  (StringRef(
+   "define outline: can't move outline as couldn't find `") +
+

[PATCH] D77532: Fix a typo in clang/lib/Frontend/FrontendAction.cpp

[BoYao Zhang via Phabricator via cfe-commits]

rZhBoYao added a comment.

In D77532#1963551 , @aaron.ballman 
wrote:

> LGTM! Do you need someone to commit on your behalf?


Yes, much appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77532



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: clang/test/Analysis/stream-error.c:1
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream 
-analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.StreamTester 
-analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
 

Szelethus wrote:
> `core` isn't enabled! :o
Is it needed? It was probably not set (and is not) in the original test in 
`stream.c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 255301.
mibintc added a comment.

I beleive this patch responds to all @rjmccall 's review comments, except I 
don't know how to program a solution to his StmtVisitor remark. I'll add more 
info about that.  This patch optionally allocates the trailing storage in 
BinaryOperator. I verified that it was optionally allocated by adding an assert 
when trailing storage was allocated; the only lit tests that failed were the 
floating point pragma tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/ReachableCode.cpp
  clang/lib/Analysis/ThreadSafetyCommon.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Index/IndexBody.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr.c
  clang/test/AST/dump.cpp
  clang/test/Import/compound-assign-op/test.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -435,10 +435,6 @@
 K = CXCursor_BinaryOperator;
 break;
 
-  case Stmt::CompoundAssignOperatorClass:
-K = CXCursor_CompoundAssignOperator;
-break;
-
   case Stmt::ConditionalOperatorClass:
 K = CXCursor_ConditionalOperator;
 break;
Index: clang/test/Import/compound-assign-op/test.cpp
===
--- clang/test/Import/compound-assign-op/test.cpp
+++ clang/test/Import/compound-assign-op/test.cpp
@@ -2,42 +2,42 @@
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '+='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '-='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '*='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '/='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '&='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '^='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '<<='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '>>='
 
 void expr() {
Index: clang/test/AST/dump.cpp
===
--- clang/test/AST/dump.cpp
+++ clang/test/AST/dump.cpp
@@ -14,14 +14,14 @@
 #pragma omp declare reduction(fun : float : omp_out += omp_in) initializer(omp_priv = omp_orig + 15)
 
 // CHECK:  |-OMPDeclareReductionDecl {{.+}}  col:35 operator+ 'int' combiner 0x{{.+}}
-// CHECK-NEXT: | |-CompoundAssignOperator {{.+}}  'int' lvalue '*=' ComputeLHSTy='int' ComputeResultTy='int'
+// CHECK-NEXT: | |-Bi

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

If I change StmtVisitor to send compound assignment opcodes to BinaryOperator, 
then 12 clang LIT tests fail, all related to compound assignment mishandling, 
i.e. this patch

- a/clang/include/clang/AST/StmtVisitor.h

+++ b/clang/include/clang/AST/StmtVisitor.h
@@ -143,7 +143,7 @@ public:

  // methods, fall back on VisitCompoundAssignOperator.

#define CAO_FALLBACK(NAME) \

  RetTy VisitBin##NAME(PTR(BinaryOperator) S, ParamTys... P) { \

- DISPATCH(BinAssign, BinaryOperator);   \

+DISPATCH(BinaryOperator, BinaryOperator);  
 \

  }
  CAO_FALLBACK(MulAssign) CAO_FALLBACK(DivAssign) CAO_FALLBACK(RemAssign)
  CAO_FALLBACK(AddAssign) CAO_FALLBACK(SubAssign) CAO_FALLBACK(ShlAssign)

These clang lit tests fail,
Failing Tests (12):

  Clang :: CXX/drs/dr2xx.cpp
  Clang :: CXX/expr/expr.const/p2-0x.cpp
  Clang :: CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
  Clang :: CodeGenCXX/const-init-cxx1y.cpp
  Clang :: CodeGenCXX/const-init-cxx2a.cpp
  Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
  Clang :: Sema/warn-unsequenced.c
  Clang :: SemaCXX/constant-expression-cxx1y.cpp
  Clang :: SemaCXX/constant-expression-cxx2a.cpp
  Clang :: SemaCXX/decomposed-condition.cpp
  Clang :: SemaCXX/integer-overflow.cpp
  Clang :: SemaCXX/warn-unsequenced.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384



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

[PATCH] D77545: Represent FP options in AST by special Expression node

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall, andrew.w.kaylor, efriedma, 
mibintc, kpn, sammccall, hokein.
Herald added a subscriber: martong.
Herald added a project: clang.

The patch D76599  proposed a new statement 
node to represent pragmas that
modifies floating point environment. These nodes are used to implement
FP environment tracking and control in a function. Such node modify FP
environment in enclosing compound statement thus organizing FP
environment in hierarchical manner.

That solution however cannot be applied to initializers of global
variables and some other objects. They represent expressions outside
function bodies and thus cannot be embedded into compound statements.

Expression node introduced here (FPEnvironmentExpr) is to represent
FP environment in such cases. It contains a field of type FPOptions,
which specifies FP environment in which subexpression of
FPEnvironmentExpr should be executed. The patch implements FP
environment in the following cases:

- Initializers of global variables, like:

  float v1 = 3,1415926535;

- Inline initializers of fields:

  struct C1 { float f1 = 1,4142135623; };

- Arguments of base constructors in constructors:

  struct C3 : public C2 { C3(double x, double y) : C2(x + y) {} };

- Default function arguments:

  void func_01(float x = 1.1);

In contrast to the solution for compound statements in D76599 
, this
solution does not keep reference to the source code construct that set
the FP environment, namely the file scope pragma. It may cause problems
for AST consumers like source analyzers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77545

Files:
  clang/include/clang/AST/EvaluatedExprVisitor.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ConstantEmitter.h
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/AST/ast-dump-pragma.cpp
  clang/test/CodeGen/fp-contract-pragma.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -336,6 +336,7 @@
   case Stmt::ObjCBoxedExprClass:
   case Stmt::ObjCSubscriptRefExprClass:
   case Stmt::RecoveryExprClass:
+  case Stmt::FPEnvironmentExprClass:
 K = CXCursor_UnexposedExpr;
 break;
 
Index: clang/test/CodeGen/fp-contract-pragma.cpp
===
--- clang/test/CodeGen/fp-contract-pragma.cpp
+++ clang/test/CodeGen/fp-contract-pragma.cpp
@@ -89,3 +89,15 @@
   #pragma STDC FP_CONTRACT ON
   return c - a * b;
 }
+
+
+#pragma STDC FP_CONTRACT ON
+struct C1 {
+  C1(float);
+};
+struct C2 : public C1 {
+  C2(float a, float b, float c);
+};
+C2::C2(float a, float b, float c) : C1(a*b+c) {}
+// CHECK: _ZN2C2C2Efff
+// CHECK: call float @llvm.fmuladd.f32
Index: clang/test/AST/ast-dump-pragma.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-pragma.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck --check-prefixes=CHECK,NOOPT %s
+
+#pragma STDC FP_CONTRACT ON
+
+float var_01 = 1.1;
+
+// CHECK: VarDecl {{.*}} var_01
+// CHECK-NEXT: FPEnvironmentExpr {{.*}} contract(on)
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: FloatingLiteral
+
+struct C1 {
+  float f1 = 1.1;
+};
+
+// CHECK: CXXRecordDecl {{.*}} struct C1
+// CHECK: FieldDecl {{.*}} f1
+// CHECK-NEXT: FPEnvironmentExpr {{.*}} contract(on)
+// CHECK-NEXT: ImplicitCastExpr
+// CHECK-NEXT: FloatingLiteral
+
+struct C2 {
+  C2(float);
+};
+
+struct C3 : public C2 {
+  C3(double x) : C2(x) {}
+};
+
+// CHECK: CXXRecordDecl {{.*}} struct C3
+// CHECK: CXXConstructorDecl {{.*}} C3 'void (double)'
+// CHECK: CXXCtorInitializer 'C2'
+// CHECK-NEXT: FPEnvironmentExpr {{.*}} contract(on)
+// CHECK-NEXT: CXXConstructExpr {{.*}

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:394
+ return true;
+  }
+

rjmccall wrote:
> sepavloff wrote:
> > rjmccall wrote:
> > > sepavloff wrote:
> > > > erichkeane wrote:
> > > > > rjmccall wrote:
> > > > > > erichkeane wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > rjmccall wrote:
> > > > > > > > > erichkeane wrote:
> > > > > > > > > > rjmccall wrote:
> > > > > > > > > > > The problem with having both functions that take 
> > > > > > > > > > > `ASTContext`s and functions that don't is that it's easy 
> > > > > > > > > > > to mix them, so they either need to have the same 
> > > > > > > > > > > behavior (in which case it's pointless to have an 
> > > > > > > > > > > overload that takes the `ASTContext`) or you're making 
> > > > > > > > > > > something really error-prone.
> > > > > > > > > > > 
> > > > > > > > > > > I would feel a lot more confident that you were designing 
> > > > > > > > > > > and using these APIs correctly if you actually took 
> > > > > > > > > > > advantage of the ability to not store trailing FPOptions 
> > > > > > > > > > > in some case, like when they match the global settings in 
> > > > > > > > > > > the ASTContext.  That way you'll actually be verifying 
> > > > > > > > > > > that everything behaves correctly if nodes don't store 
> > > > > > > > > > > FPOptions.  If you do that, I think you'll see my point 
> > > > > > > > > > > about not having all these easily-confusable functions 
> > > > > > > > > > > that do or do not take `ASTContext`s..
> > > > > > > > > > I think I disagree with @rjmccall that these 
> > > > > > > > > > requiresTrailingStorage should be here at all.  I think we 
> > > > > > > > > > should store in the AST ANY programmer opinion, even if 
> > > > > > > > > > they match the global setting.  It seems to me that this 
> > > > > > > > > > would be more tolerant of any global-setting rewrites that 
> > > > > > > > > > modules/et-al introduce, as well as make the AST Print 
> > > > > > > > > > consistent.  Always storing FPOptions when the user has 
> > > > > > > > > > explicitly overriding it also better captures the 
> > > > > > > > > > programmer's intent.
> > > > > > > > > I covered this elsewhere in the review.  If you want to have 
> > > > > > > > > that tolerance — and I think you should — then expressions 
> > > > > > > > > should store (and Sema should track) the active pragma state, 
> > > > > > > > > which can be most easily expressed as a pair of an FPOptions 
> > > > > > > > > and a mask to apply to the global FPOptions.  When you enter 
> > > > > > > > > a pragma, you clear the relevant bits from the global 
> > > > > > > > > FPOptions mask.
> > > > > > > > > 
> > > > > > > > > But the whole point of putting this stuff in trailing storage 
> > > > > > > > > is so that you can make FPOptions as big as you need without 
> > > > > > > > > actually inflating the AST size for a million nodes that 
> > > > > > > > > don't care in the slightest about FPOptions.
> > > > > > > > > But the whole point of putting this stuff in trailing storage 
> > > > > > > > > is so that you can make FPOptions as big as you need without 
> > > > > > > > > actually inflating the AST size for a million nodes that 
> > > > > > > > > don't care in the slightest about FPOptions.
> > > > > > > > 
> > > > > > > > I meant to say: for a million nodes that don't care in the 
> > > > > > > > slightest about FPOptions, as well as for a million more nodes 
> > > > > > > > that aren't using pragma overrides.
> > > > > > > Right, I get the intent, and I completely agree with that.  My 
> > > > > > > point was EVERY Expr that is affected by a #pragma should store 
> > > > > > > it.  Though, after looking at your Macro concern above, I'm less 
> > > > > > > compelled.
> > > > > > > 
> > > > > > > I guess was suggesting that the logic for 
> > > > > > > "requiresTrailingStorage" should just be "modified by a pragma" 
> > > > > > > instead of "FPOptions != The global setting".
> > > > > > Well, "modified by a pragma" still wouldn't make the AST agnostic 
> > > > > > to global settings, since the pragmas don't override everything in 
> > > > > > FPOptions at once.  But I agree that would achieve the most 
> > > > > > important goal, which is to stop inflating the AST when pragmas 
> > > > > > *aren't* in effect, which is approximately 100% of the time.  In 
> > > > > > order to do that, though, we'll need to start tracking pragmas, 
> > > > > > which we should do but which can wait for a follow-up patch.  In 
> > > > > > the meantime, I don't think you're ever going to get the interfaces 
> > > > > > right for queries like `BinaryOperator::getFPOptions` unless you 
> > > > > > actually stop relying on the fact that you're unconditionally 
> > > > > > storing `FPOptions`.  You need to passing around ASTContexts for 
> > > > > > that.  That's why I'm suggesting using an exact match with the 
> > > > > > g

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Another problem: parameters can also be `LazyCompoundVal`s, but 
`CallEvent::getParameterLocation()` does not handle non-inlined calls.


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

https://reviews.llvm.org/D77229



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

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

[Louis Dionne via Phabricator via cfe-commits]

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

Thanks for looking into this!




Comment at: clang/test/SemaObjC/type-traits-is-pointer.mm:10
+void test_is_pointer() {
+assert_is_pointer();
+

You can just use `static_assert` directly here.



Comment at: libcxx/include/type_traits:901
+// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
+#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+

Doesn't `__has_keyword` return a number that can be compared against? `#if 
__has_keyword(__is_pointer) > some-number` would be better if feasible, because 
it would handle Clang, AppleClang and any other potential derivative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77519



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

Re: [clang] 8527c1e - Added constraints on cl-options.cu test

[Hans Wennborg via cfe-commits]

I'm seeing this failure when trying to build the Windows package for
http://llvm.org/builds (yes, it's been a while). Not sure why it
hasn't been on the bots (maybe the Windows bots don't build the nvptx
target). Anyway, the error comes from "nocudalib" not being a valid
clang-cl option. Is it supposed to be?

FAIL: Clang :: Driver/cl-options.cu (5411 of 17056)
 TEST 'Clang :: Driver/cl-options.cu' FAILED

Script:
--
: 'RUN: at line 11';
c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe
--driver-mode=cl -### -nocudalib -nocudainc --
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
2>&1 | c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe
-check-prefix=GS-default
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
: 'RUN: at line 18';
c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe
--driver-mode=cl /c /GX -### -nocudalib -nocudainc --
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
2>&1 | c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe
-check-prefix=GX
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
: 'RUN: at line 26';
c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe
--driver-mode=cl /c /Gd -### -nocudalib -nocudainc --
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
2>&1 | c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe
-check-prefix=Gd
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 11"
$ "c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe"
"--driver-mode=cl" "-###" "-nocudalib" "-nocudainc" "--"
"C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu"
$ "c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe"
"-check-prefix=GS-default"
"C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu"
# command stderr:
C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu:12:16:
error: GS-default: expected string not found in input
// GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
   ^
:1:1: note: scanning from here
clang: warning: unknown argument ignored in clang-cl: '-nocudalib'
[-Wunknown-argument]
^

On Thu, Mar 12, 2020 at 12:07 AM Artem Belevich via cfe-commits
 wrote:
>
>
> Author: Artem Belevich
> Date: 2020-03-11T16:06:09-07:00
> New Revision: 8527c1ed66c63db0590cd69320ba0bf8fad59b87
>
> URL: 
> https://github.com/llvm/llvm-project/commit/8527c1ed66c63db0590cd69320ba0bf8fad59b87
> DIFF: 
> https://github.com/llvm/llvm-project/commit/8527c1ed66c63db0590cd69320ba0bf8fad59b87.diff
>
> LOG: Added constraints on cl-options.cu test
>
> Added:
>
>
> Modified:
> clang/test/Driver/cl-options.cu
>
> Removed:
>
>
>
> 
> diff  --git a/clang/test/Driver/cl-options.cu 
> b/clang/test/Driver/cl-options.cu
> index 7597970af160..2fd393e06d2d 100644
> --- a/clang/test/Driver/cl-options.cu
> +++ b/clang/test/Driver/cl-options.cu
> @@ -3,6 +3,10 @@
>  // Note: %s must be preceded by --, otherwise it may be interpreted as a
>  // command-line option, e.g. on Mac where %s is commonly under /Users.
>
> +// REQUIRES: clang-driver
> +// REQUIRES: x86-registered-target
> +// REQUIRES: nvptx-registered-target
> +
>  // -stack-protector should not be passed to device-side CUDA compilation
>  // RUN: %clang_cl -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
> -check-prefix=GS-default %s
>  // GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77540: [PATCH] [ARM]: Armv8.6-a Matrix Mul Asm and Intrinsics Support

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson added a comment.

build failures related to linting, unit tests passing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77540



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

[PATCH] D77028: Speed up deferred diagnostic emitter

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 255315.
yaxunl marked an inline comment as done.
yaxunl retitled this revision from "[NFC] Refactor DeferredDiagsEmitter and 
skip redundant visit" to "Speed up deferred diagnostic emitter".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Revised by John's comments. Skip visited functions and check error limit.


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

https://reviews.llvm.org/D77028

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/test/CodeGenCUDA/deferred-diag.cu
  clang/test/SemaCUDA/deferred-diags-limit.cu
  clang/test/SemaCUDA/deferred-diags.cu

Index: clang/test/SemaCUDA/deferred-diags.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/deferred-diags.cu
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fcuda-is-device -fsyntax-only -verify %s
+
+#include "Inputs/cuda.h"
+
+// Error, instantiated on device.
+inline __host__ __device__ void hasInvalid() {
+  throw NULL;
+  // expected-error@-1 2{{cannot use 'throw' in __host__ __device__ function}}
+}
+
+static __device__ void use0() {
+  hasInvalid(); // expected-note {{called by 'use0'}}
+  hasInvalid(); // expected-note {{called by 'use0'}}
+}
+
+// To avoid excessive diagnostic messages, deferred diagnostics are only
+// emitted the first time a function is called.
+static __device__ void use1() {
+  use0(); // expected-note 2{{called by 'use1'}}
+  use0();
+}
+
+static __device__ void use2() {
+  use1(); // expected-note 2{{called by 'use2'}}
+  use1();
+}
+
+static __device__ void use3() {
+  use2(); // expected-note 2{{called by 'use3'}}
+  use2();
+}
+
+__global__ void use4() {
+  use3(); // expected-note 2{{called by 'use4'}}
+  use3();
+}
Index: clang/test/SemaCUDA/deferred-diags-limit.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/deferred-diags-limit.cu
@@ -0,0 +1,20 @@
+// RUN: not %clang_cc1 -fcxx-exceptions -fcuda-is-device -fsyntax-only \
+// RUN:   -ferror-limit 2 2>&1 %s | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: cannot use 'throw' in __host__ __device__ function
+// CHECK: cannot use 'throw' in __host__ __device__ function
+// CHECK-NOT: cannot use 'throw' in __host__ __device__ function
+// CHECK: too many errors emitted, stopping now
+
+inline __host__ __device__ void hasInvalid() {
+  throw NULL;
+}
+
+__global__ void use0() {
+  hasInvalid();
+  hasInvalid();
+  hasInvalid();
+  hasInvalid();
+}
Index: clang/test/CodeGenCUDA/deferred-diag.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/deferred-diag.cu
@@ -0,0 +1,25 @@
+// RUN: not %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu \
+// RUN:   -fcuda-is-device -emit-llvm -o - %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Check no crash due to deferred diagnostics.
+
+#include "Inputs/cuda.h"
+
+// CHECK: error: invalid output constraint '=h' in asm
+// CHECK-NOT: core dump
+inline __host__ __device__ int foo() {
+  short h;
+  __asm__("dont care" : "=h"(h) : "f"(0.0), "d"(0.0), "h"(0), "r"(0), "l"(0));
+  return 0;
+}
+
+void host_fun() {
+  foo();
+}
+
+__global__ void kernel() {
+  foo();
+}
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1440,59 +1440,72 @@
 static void emitCallStackNotes(Sema &S, FunctionDecl *FD) {
   auto FnIt = S.DeviceKnownEmittedFns.find(FD);
   while (FnIt != S.DeviceKnownEmittedFns.end()) {
+// Respect error limit.
+if (S.Diags.hasFatalErrorOccurred())
+  return;
 DiagnosticBuilder Builder(
 S.Diags.Report(FnIt->second.Loc, diag::note_called_by));
 Builder << FnIt->second.FD;
-Builder.setForceEmit();
-
 FnIt = S.DeviceKnownEmittedFns.find(FnIt->second.FD);
   }
 }
 
-// Emit any deferred diagnostics for FD and erase them from the map in which
-// they're stored.
-void Sema::emitDeferredDiags(FunctionDecl *FD, bool ShowCallStack) {
-  auto It = DeviceDeferredDiags.find(FD);
-  if (It == DeviceDeferredDiags.end())
-return;
-  bool HasWarningOrError = false;
-  bool FirstDiag = true;
-  for (PartialDiagnosticAt &PDAt : It->second) {
-const SourceLocation &Loc = PDAt.first;
-const PartialDiagnostic &PD = PDAt.second;
-HasWarningOrError |= getDiagnostics().getDiagnosticLevel(
- PD.getDiagID(), Loc) >= DiagnosticsEngine::Warning;
-{
-  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
-  Builder.setForceEmit();
-  PD.Emit(Builder);
-}
-
-// Emit the note on the first diagnostic in case too many diagnostics cause
-// the note not emitted.
-if (FirstDiag && HasWarningOrError && ShowCallStack) {
-  em

[PATCH] D77548: clang-format: [JS] handle pseudo-keywords.

[Martin Probst via Phabricator via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: krasimir.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The previous change in https://reviews.llvm.org/D77311 attempted to
detect more C++ keywords. However it also precisely detected all
JavaScript keywords. That's generally correct, but many JavaScripy
keywords, e.g. `get`, are so-called pseudo-keywords. They can be used in
positions where a keyword would never be legal, e.g. in a dotted
expression:

  x.type;  // type is a pseudo-keyword, but can be used here.
  x.get;   // same for get etc.

This change introduces an additional parameter to
`IsJavaScriptIdentifier`, allowing clients to toggle whether they want
to allow `IdentifierName` tokens, i.e. pseudo-keywords.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77548

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2412,6 +2412,11 @@
   verifyFormat("using!;");
   verifyFormat("virtual!;");
   verifyFormat("wchar_t!;");
+
+  // Positive tests:
+  verifyFormat("x.type!;");
+  verifyFormat("x.get!;");
+  verifyFormat("x.set!;");
 }
 
 TEST_F(FormatTestJS, NullPropagatingOperator) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1522,9 +1522,11 @@
 if (Style.Language == FormatStyle::LK_JavaScript) {
   if (Current.is(tok::exclaim)) {
 if (Current.Previous &&
-(Keywords.IsJavaScriptIdentifier(*Current.Previous) ||
- Current.Previous->isOneOf(tok::kw_namespace, tok::r_paren,
-   tok::r_square, tok::r_brace) ||
+(Keywords.IsJavaScriptIdentifier(
+ *Current.Previous, /* AcceptIdentifierName= */ true) ||
+ Current.Previous->isOneOf(
+ tok::kw_namespace, tok::r_paren, tok::r_square, tok::r_brace,
+ Keywords.kw_type, Keywords.kw_get, Keywords.kw_set) ||
  Current.Previous->Tok.isLiteral())) {
   Current.Type = TT_JsNonNullAssertion;
   return;
@@ -3071,7 +3073,9 @@
   return false;
 // In tagged template literals ("html`bar baz`"), there is no space between
 // the tag identifier and the template string.
-if (Keywords.IsJavaScriptIdentifier(Left) && Right.is(TT_TemplateString))
+if (Keywords.IsJavaScriptIdentifier(Left,
+/* AcceptIdentifierName= */ false) &&
+Right.is(TT_TemplateString))
   return false;
 if (Right.is(tok::star) &&
 Left.isOneOf(Keywords.kw_function, Keywords.kw_yield))
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -909,7 +909,11 @@
 
   /// Returns \c true if \p Tok is a true JavaScript identifier, returns
   /// \c false if it is a keyword or a pseudo keyword.
-  bool IsJavaScriptIdentifier(const FormatToken &Tok) const {
+  /// If \c AcceptIdentifierName is true, returns true not only for keywords,
+  // but also for IdentifierName tokens (aka pseudo-keywords), such as
+  // ``yield``.
+  bool IsJavaScriptIdentifier(const FormatToken &Tok,
+  bool AcceptIdentifierName = true) const {
 // Based on the list of JavaScript & TypeScript keywords here:
 // 
https://github.com/microsoft/TypeScript/blob/master/src/compiler/scanner.ts#L74
 switch (Tok.Tok.getKind()) {
@@ -946,11 +950,14 @@
 case tok::kw_while:
   // These are JS keywords that are lexed by LLVM/clang as keywords.
   return false;
-case tok::identifier:
+case tok::identifier:  {
   // For identifiers, make sure they are true identifiers, excluding the
   // JavaScript pseudo-keywords (not lexed by LLVM/clang as keywords).
-  return JsExtraKeywords.find(Tok.Tok.getIdentifierInfo()) ==
- JsExtraKeywords.end();
+bool IsPseudoKeyword =
+  JsExtraKeywords.find(Tok.Tok.getIdentifierInfo()) !=
+  JsExtraKeywords.end();
+  return AcceptIdentifierName || !IsPseudoKeyword;
+}
 default:
   // Other keywords are handled in the switch below, to avoid problems due
   // to duplicate case labels when using the #include trick.


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2412,6 +2412,11 @@
   verifyFormat("using!;");
   verifyFormat("virtual!;");
   verifyFormat("wchar_t!;");
+
+  // Positive tests:
+  verif

[clang-tools-extra] 9e1af17 - [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-04-06T09:01:43-05:00
New Revision: 9e1af172eec9a06bffac337057a2452b88466288

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

LOG: [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee

Added: 


Modified: 
clang-tools-extra/clang-reorder-fields/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt 
b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
index 9c75d785cc9a..c357d0a3cfbf 100644
--- a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
+++ b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
@@ -1,4 +1,7 @@
-set(LLVM_LINK_COMPONENTS support)
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  support
+)
 
 add_clang_library(clangReorderFields
   ReorderFieldsAction.cpp



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

[clang] f8e1fc2 - Make clang/test/Driver/cl-options.cu pass in 32-bit builds

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2020-04-06T16:04:43+02:00
New Revision: f8e1fc20cb3984faf62cdd947e4d1a408a9000d8

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

LOG: Make clang/test/Driver/cl-options.cu pass in 32-bit builds

Added: 


Modified: 
clang/test/Driver/cl-options.cu

Removed: 




diff  --git a/clang/test/Driver/cl-options.cu b/clang/test/Driver/cl-options.cu
index 2fd393e06d2d..e29d7cc1b8f8 100644
--- a/clang/test/Driver/cl-options.cu
+++ b/clang/test/Driver/cl-options.cu
@@ -9,14 +9,14 @@
 
 // -stack-protector should not be passed to device-side CUDA compilation
 // RUN: %clang_cl -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
-check-prefix=GS-default %s
-// GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// GS-default: "-cc1" "-triple" "nvptx{{(64)?}}-nvidia-cuda"
 // GS-default-NOT: "-stack-protector"
 // GS-default: "-cc1" "-triple"
 // GS-default: "-stack-protector" "2"
 
 // -exceptions should be passed to device-side compilation.
 // RUN: %clang_cl /c /GX -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
-check-prefix=GX %s
-// GX: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// GX: "-cc1" "-triple" "nvptx{{(64)?}}-nvidia-cuda"
 // GX-NOT: "-fcxx-exceptions"
 // GX-NOT: "-fexceptions"
 // GX: "-cc1" "-triple"
@@ -24,7 +24,7 @@
 
 // /Gd should not override default calling convention on device side.
 // RUN: %clang_cl /c /Gd -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
-check-prefix=Gd %s
-// Gd: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// Gd: "-cc1" "-triple" "nvptx{{(64)?}}-nvidia-cuda"
 // Gd-NOT: "-fcxx-exceptions"
 // Gd-NOT: "-fdefault-calling-conv=cdecl"
 // Gd: "-cc1" "-triple"



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

Re: [clang] 8527c1e - Added constraints on cl-options.cu test

[Hans Wennborg via cfe-commits]

Oh no, the warning is a red herring. The problem is I'm doing a 32-bit
build and the triple is nvptx-nvidia-cuda, not nvptx64-nvidia-cuda.

f8e1fc20cb3 should fix.

On Mon, Apr 6, 2020 at 3:54 PM Hans Wennborg  wrote:
>
> I'm seeing this failure when trying to build the Windows package for
> http://llvm.org/builds (yes, it's been a while). Not sure why it
> hasn't been on the bots (maybe the Windows bots don't build the nvptx
> target). Anyway, the error comes from "nocudalib" not being a valid
> clang-cl option. Is it supposed to be?
>
> FAIL: Clang :: Driver/cl-options.cu (5411 of 17056)
>  TEST 'Clang :: Driver/cl-options.cu' FAILED
> 
> Script:
> --
> : 'RUN: at line 11';
> c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe
> --driver-mode=cl -### -nocudalib -nocudainc --
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
> 2>&1 | c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe
> -check-prefix=GS-default
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
> : 'RUN: at line 18';
> c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe
> --driver-mode=cl /c /GX -### -nocudalib -nocudainc --
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
> 2>&1 | c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe
> -check-prefix=GX
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
> : 'RUN: at line 26';
> c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe
> --driver-mode=cl /c /Gd -### -nocudalib -nocudainc --
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
> 2>&1 | c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe
> -check-prefix=Gd
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu
> --
> Exit Code: 1
>
> Command Output (stdout):
> --
> $ ":" "RUN: at line 11"
> $ "c:\src\llvm_package_64c23127\build32_stage0\bin\clang.exe"
> "--driver-mode=cl" "-###" "-nocudalib" "-nocudainc" "--"
> "C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu"
> $ "c:\src\llvm_package_64c23127\build32_stage0\bin\filecheck.exe"
> "-check-prefix=GS-default"
> "C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu"
> # command stderr:
> C:\src\llvm_package_64c23127\llvm-project\clang\test\Driver\cl-options.cu:12:16:
> error: GS-default: expected string not found in input
> // GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
>^
> :1:1: note: scanning from here
> clang: warning: unknown argument ignored in clang-cl: '-nocudalib'
> [-Wunknown-argument]
> ^
>
> On Thu, Mar 12, 2020 at 12:07 AM Artem Belevich via cfe-commits
>  wrote:
> >
> >
> > Author: Artem Belevich
> > Date: 2020-03-11T16:06:09-07:00
> > New Revision: 8527c1ed66c63db0590cd69320ba0bf8fad59b87
> >
> > URL: 
> > https://github.com/llvm/llvm-project/commit/8527c1ed66c63db0590cd69320ba0bf8fad59b87
> > DIFF: 
> > https://github.com/llvm/llvm-project/commit/8527c1ed66c63db0590cd69320ba0bf8fad59b87.diff
> >
> > LOG: Added constraints on cl-options.cu test
> >
> > Added:
> >
> >
> > Modified:
> > clang/test/Driver/cl-options.cu
> >
> > Removed:
> >
> >
> >
> > 
> > diff  --git a/clang/test/Driver/cl-options.cu 
> > b/clang/test/Driver/cl-options.cu
> > index 7597970af160..2fd393e06d2d 100644
> > --- a/clang/test/Driver/cl-options.cu
> > +++ b/clang/test/Driver/cl-options.cu
> > @@ -3,6 +3,10 @@
> >  // Note: %s must be preceded by --, otherwise it may be interpreted as a
> >  // command-line option, e.g. on Mac where %s is commonly under /Users.
> >
> > +// REQUIRES: clang-driver
> > +// REQUIRES: x86-registered-target
> > +// REQUIRES: nvptx-registered-target
> > +
> >  // -stack-protector should not be passed to device-side CUDA compilation
> >  // RUN: %clang_cl -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
> > -check-prefix=GS-default %s
> >  // GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
> >
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] 9e1af17 - [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee

[Roman Lebedev via cfe-commits]

This seems suspicious.
Does clang-reorder-fields actually explicitly needs something from
FrontendOpenMP?
If not, it looks like there dependency is missing elsewhere, or
there's wrong layering.

On Mon, Apr 6, 2020 at 5:03 PM Johannes Doerfert via cfe-commits
 wrote:
>
>
> Author: Johannes Doerfert
> Date: 2020-04-06T09:01:43-05:00
> New Revision: 9e1af172eec9a06bffac337057a2452b88466288
>
> URL: 
> https://github.com/llvm/llvm-project/commit/9e1af172eec9a06bffac337057a2452b88466288
> DIFF: 
> https://github.com/llvm/llvm-project/commit/9e1af172eec9a06bffac337057a2452b88466288.diff
>
> LOG: [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee
>
> Added:
>
>
> Modified:
> clang-tools-extra/clang-reorder-fields/CMakeLists.txt
>
> Removed:
>
>
>
> 
> diff  --git a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt 
> b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
> index 9c75d785cc9a..c357d0a3cfbf 100644
> --- a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
> +++ b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
> @@ -1,4 +1,7 @@
> -set(LLVM_LINK_COMPONENTS support)
> +set(LLVM_LINK_COMPONENTS
> +  FrontendOpenMP
> +  support
> +)
>
>  add_clang_library(clangReorderFields
>ReorderFieldsAction.cpp
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] 9e1af17 - [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee

[Johannes Doerfert via cfe-commits]

On 4/6/20 9:06 AM, Roman Lebedev wrote:

This seems suspicious.


Agreed, especially since this is also not the only place.

I was hoping to unblock the builders with this.



Does clang-reorder-fields actually explicitly needs something from
FrontendOpenMP?
If not, it looks like there dependency is missing elsewhere, or
there's wrong layering.


The root cause is the use of `isAllowedClauseForDirective` in ASTMatchers.h.



On Mon, Apr 6, 2020 at 5:03 PM Johannes Doerfert via cfe-commits
 wrote:


Author: Johannes Doerfert
Date: 2020-04-06T09:01:43-05:00
New Revision: 9e1af172eec9a06bffac337057a2452b88466288

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

LOG: [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee

Added:


Modified:
 clang-tools-extra/clang-reorder-fields/CMakeLists.txt

Removed:




diff  --git a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt 
b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
index 9c75d785cc9a..c357d0a3cfbf 100644
--- a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
+++ b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
@@ -1,4 +1,7 @@
-set(LLVM_LINK_COMPONENTS support)
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  support
+)

  add_clang_library(clangReorderFields
ReorderFieldsAction.cpp



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

Re: [clang-tools-extra] 9e1af17 - [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee

[Roman Lebedev via cfe-commits]

On Mon, Apr 6, 2020 at 5:23 PM Johannes Doerfert  wrote:
>
> On 4/6/20 9:06 AM, Roman Lebedev wrote:
>
> This seems suspicious.
>
>
> Agreed, especially since this is also not the only place.
>
> I was hoping to unblock the builders with this.
>
>
> Does clang-reorder-fields actually explicitly needs something from
> FrontendOpenMP?
> If not, it looks like there dependency is missing elsewhere, or
> there's wrong layering.
>
> The root cause is the use of `isAllowedClauseForDirective` in ASTMatchers.h.
What is the target encompases that include?
I'd guess that is the one that is missing this dep.

> On Mon, Apr 6, 2020 at 5:03 PM Johannes Doerfert via cfe-commits
>  wrote:
>
> Author: Johannes Doerfert
> Date: 2020-04-06T09:01:43-05:00
> New Revision: 9e1af172eec9a06bffac337057a2452b88466288
>
> URL: 
> https://github.com/llvm/llvm-project/commit/9e1af172eec9a06bffac337057a2452b88466288
> DIFF: 
> https://github.com/llvm/llvm-project/commit/9e1af172eec9a06bffac337057a2452b88466288.diff
>
> LOG: [OpenMP][FIX] Add missing cmake dependence needed after 931c0cd713ee
>
> Added:
>
>
> Modified:
> clang-tools-extra/clang-reorder-fields/CMakeLists.txt
>
> Removed:
>
>
>
> 
> diff  --git a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt 
> b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
> index 9c75d785cc9a..c357d0a3cfbf 100644
> --- a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
> +++ b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
> @@ -1,4 +1,7 @@
> -set(LLVM_LINK_COMPONENTS support)
> +set(LLVM_LINK_COMPONENTS
> +  FrontendOpenMP
> +  support
> +)
>
>  add_clang_library(clangReorderFields
>ReorderFieldsAction.cpp
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

It seems like the macro's in StmtVisitor.h only work if CAO and BinaryOperator 
are separate classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384



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

[PATCH] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 255321.
ZarkoCA added a comment.

Rebase and ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76130

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
  llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll

Index: llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
@@ -0,0 +1,357 @@
+; RUN: llc -O2 -mtriple powerpc64-ibm-aix-xcoff -stop-after=machine-cp -verify-machineinstrs < %s | \
+; RUN: FileCheck --check-prefixes=CHECK,64BIT %s
+
+; RUN: llc -O2 -verify-machineinstrs -mcpu=pwr4 -mattr=-altivec \
+; RUN: -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefixes=CHECKASM,ASM64 %s
+
+  define i32 @int_va_arg(i32 %a, ...) local_unnamed_addr  {
+  entry:
+%arg1 = alloca i8*, align 8
+%arg2 = alloca i8*, align 8
+%0 = bitcast i8** %arg1 to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0) 
+%1 = bitcast i8** %arg2 to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %1)
+call void @llvm.va_start(i8* nonnull %0)
+call void @llvm.va_copy(i8* nonnull %1, i8* nonnull %0)
+%2 = va_arg i8** %arg1, i32
+%add = add nsw i32 %2, %a
+%3 = va_arg i8** %arg2, i32
+%mul = shl i32 %3, 1
+%add3 = add nsw i32 %add, %mul
+call void @llvm.va_end(i8* nonnull %0)
+call void @llvm.va_end(i8* nonnull %1)
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %1) 
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0) 
+ret i32 %add3
+  }
+
+  declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) 
+  declare void @llvm.va_start(i8*) 
+  declare void @llvm.va_copy(i8*, i8*) 
+  declare void @llvm.va_end(i8*) 
+  declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) 
+  
+; 64BIT-LABEL:   name:int_va_arg
+; 64BIT-LABEL:   liveins:
+; 64BIT-DAG: - { reg: '$x3', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x4', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x5', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x6', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x7', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x8', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x9', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x10', virtual-reg: '' }
+
+; 64BIT-LABEL:   fixedStack:
+; 64BIT-DAG: - { id: 0, type: default, offset: 56, size: 8
+
+; 64BIT-LABEL:   stack:
+; 64BIT-DAG: - { id: 0, name: arg1, type: default, offset: 0, size: 8
+; 64BIT-DAG: - { id: 1, name: arg2, type: default, offset: 0, size: 8 
+
+; 64BIT-LABEL:   body: |
+; 64BIT-DAG: bb.0.entry:
+; 64BIT-DAG: liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
+; 64BIT-DAG: STD killed renamable $x4, 0, %fixed-stack.0 :: (store 8 into %fixed-stack.0)
+; 64BIT-DAG: STD killed renamable $x5, 8, %fixed-stack.0 :: (store 8 into %fixed-stack.0 + 8)
+; 64BIT-DAG: STD killed renamable $x6, 16, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x7, 24, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x8, 32, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x9, 40, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x10, 48, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: renamable $x11 = ADDI8 %fixed-stack.0, 0
+; 64BIT-DAG: STD renamable $x11, 0, %stack.1.arg2 :: (store 8 into %ir.1)
+; 64BIT-DAG: renamable $x4 = LD 0, %stack.1.arg2 :: (load 8 from %ir.arg2)
+; 64BIT-DAG: renamable $x7 = ADDI8 renamable $x4, 4
+; 64BIT-DAG: renamable $x5 = ADDI8 %fixed-stack.0, 4
+; 64BIT-DAG: renamable $r6 = LWZ 0, %fixed-stack.0 :: (load 4 from %fixed-stack.0, align 8)
+; 64BIT-DAG: STD killed renamable $x11, 0, %stack.0.arg1 :: (store 8 into %ir.0)
+; 64BIT-DAG: STD killed renamable $x5, 0, %stack.0.arg1 :: (store 8 into %ir.arg1)
+; 64BIT-DAG: STD killed renamable $x7, 0, %stack.1.arg2 :: (store 8 into %ir.arg2)
+; 64BIT-DAG: renamable $r4 = LWZ 0, killed renamable $x4 :: (load 4)
+; 64BIT-DAG: renamable $r3 = nsw ADD4 killed renamable $r6, renamable $r3, implicit killed $x3
+; 64BIT-DAG: renamable $r4 = RLWINM killed renamable $r4, 1, 0, 30
+; 64BIT-DAG: renamable $r3 = nsw ADD4 killed renamable $r3, killed renamable $r4, implicit-def $x3
+; 64BIT-DAG: BLR8 implicit $lr8, implicit $rm, implicit $x3
+
+; ASM64-LABEL:   .int_va_arg:
+; ASM64-DAG: std 4, 56(1)
+; ASM64-DAG: addi 4, 1, 56
+; ASM64-DAG: std 4, -16(1)
+; ASM64-DAG: std 4, -8(1)
+; ASM64-DAG: ld 4, -16(1)
+; ASM64-DAG: std 5, 64(1)
+; ASM64-DAG: addi 5, 1, 60
+; ASM64-DAG: std 5, -8(1)
+; ASM64-DAG: addi 5, 4, 4
+; ASM64-DAG: std 6, 72(1)
+; ASM64-DAG: std 7, 80(1)
+; ASM64-DAG: std 8

[PATCH] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko updated this revision to Diff 255326.
hlopko marked 4 inline comments as done.
hlopko added a comment.

Adressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77209

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -1215,6 +1215,9 @@
 |   `-}
 `-}
)txt");
+}
+
+TEST_F(SyntaxTreeTest, ModifiableNodes) {
   // All nodes can be mutated.
   expectTreeDumpEqual(
   R"cpp(
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -57,6 +57,7 @@
 using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::Not;
 using ::testing::Pointee;
@@ -185,10 +186,14 @@
   template 
   llvm::ArrayRef findSubrange(llvm::ArrayRef Subrange,
  llvm::ArrayRef Range, Eq F) {
-for (auto Begin = Range.begin(); Begin < Range.end(); ++Begin) {
+assert(Subrange.size() >= 1);
+if (Range.size() < Subrange.size())
+  return llvm::makeArrayRef(Range.end(), Range.end());
+for (auto Begin = Range.begin(), Last = Range.end() - Subrange.size();
+ Begin <= Last; ++Begin) {
   auto It = Begin;
-  for (auto ItSub = Subrange.begin();
-   ItSub != Subrange.end() && It != Range.end(); ++ItSub, ++It) {
+  for (auto ItSub = Subrange.begin(); ItSub != Subrange.end();
+   ++ItSub, ++It) {
 if (!F(*ItSub, *It))
   goto continue_outer;
   }
@@ -889,4 +894,111 @@
   ASSERT_EQ(Code.points().size(), 8u);
 }
 
+TEST_F(TokenBufferTest, ExpandedBySpelled) {
+  recordTokens(R"cpp(
+a1 a2 a3 b1 b2
+  )cpp");
+  // Sanity check: expanded and spelled tokens are stored separately.
+  EXPECT_THAT(findExpanded("a1 a2"), Not(SameRange(findSpelled("a1 a2";
+  // Searching for subranges of expanded tokens should give the corresponding
+  // spelled ones.
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("a1 a2 a3 b1 b2")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3 b1 b2";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("a1 a2 a3")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("b1 b2")),
+  ElementsAre(SameRange(findExpanded("b1 b2";
+
+  // Test search on simple macro expansions.
+  recordTokens(R"cpp(
+#define A a1 a2 a3
+#define B b1 b2
+
+A split B
+  )cpp");
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("A split B")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3 split b1 b2";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("A split").drop_back()),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("split B").drop_front()),
+  ElementsAre(SameRange(findExpanded("b1 b2";
+
+  // Ranges not fully covering macro expansions should fail.
+  recordTokens(R"cpp(
+#define ID(x) x
+
+ID(a)
+  )cpp");
+  // Spelled don't cover entire mapping (missing ID token) -> empty result
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("( a )")), IsEmpty());
+  // Spelled don't cover entire mapping (missing ) token) -> empty result
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( a")), IsEmpty());
+
+  // Recursive macro invocations.
+  recordTokens(R"cpp(
+#define ID(x) x
+#define B b1 b2
+
+ID(ID(ID(a1) a2 a3)) split ID(B)
+  )cpp");
+
+  EXPECT_THAT(
+  Buffer.expandedForSpelled(findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) )")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3";
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( B )")),
+  ElementsAre(SameRange(findExpanded("b1 b2";
+  EXPECT_THAT(Buffer.expandedForSpelled(
+  findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) ) split ID ( B )")),
+  ElementsAre(SameRange(findExpanded("a1 a2 a3 split b1 b2";
+  // FIXME: these should succeed, but we do not support macro arguments yet.
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("a1")), IsEmpty());
+  EXPECT_THAT(Buffer.expandedForSpelled(findSpelled("ID ( a1 ) a2")),
+  IsEmpty());
+
+  // Empty macro expansions.
+  recordTokens(R"cpp(
+#define EMPTY
+#define ID(X) X
+
+EMPTY EMPTY ID(1 2 3) EMPTY EMPTY split1
+EMPTY EMPTY ID(4 5 6) split2
+ID(7 8 9) EMPTY EMPTY
+  )cpp");
+  //

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 255323.
ZarkoCA added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-vararg.c
  clang/test/CodeGen/aix32-dwarf-error.c

Index: clang/test/CodeGen/aix32-dwarf-error.c
===
--- /dev/null
+++ clang/test/CodeGen/aix32-dwarf-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -emit-llvm %s
+
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+  __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table); //expected-error {{cannot compile this __builtin_init_dwarf_reg_size_table yet}}
+  return __builtin_dwarf_sp_column();
+}
Index: clang/test/CodeGen/aix-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vararg.c
@@ -0,0 +1,39 @@
+// REQUIRES: powerpc-registered-target
+// REQUIRES: asserts
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | FileCheck %s --check-prefix=32BIT
+
+void aix_varg(int a, ...) {
+  __builtin_va_list arg;
+  __builtin_va_start(arg, a);
+  __builtin_va_list arg2;
+  __builtin_va_arg(arg, int);
+  __builtin_va_copy(arg2, arg);
+  __builtin_va_end(arg);
+  __builtin_va_end(arg2);
+}
+
+  // 32BIT:   define void @aix_varg(i32 %a, ...) #0 {
+  // 32BIT-NEXT:  entry:
+  // 32BIT-NEXT:%a.addr = alloca i32, align 4
+  // 32BIT-NEXT:%arg = alloca i8*, align 4
+  // 32BIT-NEXT:%arg2 = alloca i8*, align 4
+  // 32BIT-NEXT:store i32 %a, i32* %a.addr, align 4
+  // 32BIT-NEXT:%arg1 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_start(i8* %arg1)
+  // 32BIT-NEXT:%argp.cur = load i8*, i8** %arg, align 4
+  // 32BIT-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
+  // 32BIT-NEXT:store i8* %argp.next, i8** %arg, align 4
+  // 32BIT-NEXT:%0 = bitcast i8* %argp.cur to i32*
+  // 32BIT-NEXT:%1 = load i32, i32* %0, align 4
+  // 32BIT-NEXT:%2 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:%3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_copy(i8* %2, i8* %3)
+  // 32BIT-NEXT:%arg3 = bitcast i8** %arg to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg3)
+  // 32BIT-NEXT:%arg24 = bitcast i8** %arg2 to i8*
+  // 32BIT-NEXT:call void @llvm.va_end(i8* %arg24)
+  // 32BIT-NEXT:ret void
+  // 32BIT-NEXT:  }
+  // 32BIT: declare void @llvm.va_start(i8*) #1
+  // 32BIT: declare void @llvm.va_copy(i8*, i8*) #1
+  // 32BIT: declare void @llvm.va_end(i8*) #1
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4173,14 +4173,15 @@
 
 // PowerPC-32
 namespace {
-/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
-class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+/// PowerPC32ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF
+/// (SVR4), Darwin and AIX.
+class PowerPC32ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
 
   CharUnits getParamTypeAlignment(QualType Ty) const;
 
 public:
-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI)
+  PowerPC32ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI)
   : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -4190,7 +4191,7 @@
 class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
 public:
   PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI)
-  : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
 
   int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override {
 // This is recovered from gcc output.
@@ -4200,9 +4201,22 @@
   bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
llvm::Value *Address) const override;
 };
+
+class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo {
+public:
+  PPCAIX32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI)
+  : TargetCodeGenInfo(new PowerPC32ABIInfo(CGT, SoftFloatABI)) {}
+
+  int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override {
+return 1; // r1 is the dedicated stack pointer
+  }
+
+  bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
+   llvm::Value *Address) const override;
+};
 }
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
   if (const ComplexType *CTy = Ty->getAs())
 Ty = CTy->getElementTy

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

Thank you for working on this, I think it's very useful functionality! I'd 
appreciate seeing some unit test coverage for these changes.




Comment at: clang/lib/ASTMatchers/Dynamic/CMakeLists.txt:17
   Registry.cpp
+  Marshallers.cpp
 

We usually try to keep these lists in alphabetical order. Would you mind 
hoisting this up a bit (feel free to change VariantValue.cpp as a driveby if 
you want, or just leave it there)?



Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:10
+ llvm::StringRef DropPrefix = "", unsigned MaxEditDistance = 3) {
+  if (MaxEditDistance != -1u) {
+++MaxEditDistance;

Elide braces, and please use `~0U` instead of `-1u`.



Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:16-17
+if (Item.equals_lower(Search)) {
+  if (Item.equals(Search))
+return Item.str();
+  MaxEditDistance = 1;

Is this case possible to hit? I would imagine that if the search time matches 
anything in the allowed list, we would have not needed to call `getBestGuess()` 
in the first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77499



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

[PATCH] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

[Marcel Hlopko via Phabricator via cfe-commits]

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

Adressing comments.




Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:264
+  auto *FrontMapping = mappingStartingBeforeSpelled(File, &Spelled.front());
+  unsigned SpelledFrontI = &Spelled.front() - File.SpelledTokens.data();
+  unsigned ExpandedBegin;

gribozavr2 wrote:
> Or assert that SpelledFrontI is less than File.SpelledTokens.size().
I think the assert I added is good enough?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77209



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

[PATCH] D77551: [analyzer] Fix NSErrorChecker false positives

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, dcoughlin.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a project: clang.

NSErrorChecker used to suggest changing 'void' return type for
constructors and delete operators. This makes little sense because
return types of these functions could not be altered.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77551

Files:
  clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  clang/test/Analysis/SpecialFunctionsCFError.cpp


Index: clang/test/Analysis/SpecialFunctionsCFError.cpp
===
--- /dev/null
+++ clang/test/Analysis/SpecialFunctionsCFError.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.coreFoundation.CFError \
+// RUN:   -analyzer-store=region -verify %s
+
+typedef unsigned long size_t;
+struct __CFError {};
+typedef struct __CFError *CFErrorRef;
+void *malloc(size_t);
+
+class Foo {
+public:
+  Foo(CFErrorRef *error) {} // no-warning
+
+  void operator delete(void *pointer, CFErrorRef *error) { // no-warning
+return;
+  }
+
+  void operator delete[](void *pointer, CFErrorRef *error) { // no-warning
+return;
+  }
+
+  // Check that we report warnings for operators when it can be useful
+  void operator()(CFErrorRef *error) {} // expected-warning {{Function 
accepting CFErrorRef* should have a non-void return value to indicate whether 
or not an error occurred}}
+};
+
+// Check that global delete operator is not bothered as well
+void operator delete(void *pointer, CFErrorRef *error) { // no-warning
+  return;
+}
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -95,6 +95,15 @@
 };
 }
 
+static bool hasReservedReturnType(const FunctionDecl *D) {
+  if (isa(D))
+return true;
+
+  // operators delete and delete[] are required to have 'void' return type
+  auto OperatorKind = D->getOverloadedOperator();
+  return OperatorKind == OO_Delete || OperatorKind == OO_Array_Delete;
+}
+
 void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
 AnalysisManager &mgr,
 BugReporter &BR) const {
@@ -102,6 +111,8 @@
 return;
   if (!D->getReturnType()->isVoidType())
 return;
+  if (hasReservedReturnType(D))
+return;
 
   if (!II)
 II = &D->getASTContext().Idents.get("CFErrorRef");


Index: clang/test/Analysis/SpecialFunctionsCFError.cpp
===
--- /dev/null
+++ clang/test/Analysis/SpecialFunctionsCFError.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.coreFoundation.CFError \
+// RUN:   -analyzer-store=region -verify %s
+
+typedef unsigned long size_t;
+struct __CFError {};
+typedef struct __CFError *CFErrorRef;
+void *malloc(size_t);
+
+class Foo {
+public:
+  Foo(CFErrorRef *error) {} // no-warning
+
+  void operator delete(void *pointer, CFErrorRef *error) { // no-warning
+return;
+  }
+
+  void operator delete[](void *pointer, CFErrorRef *error) { // no-warning
+return;
+  }
+
+  // Check that we report warnings for operators when it can be useful
+  void operator()(CFErrorRef *error) {} // expected-warning {{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred}}
+};
+
+// Check that global delete operator is not bothered as well
+void operator delete(void *pointer, CFErrorRef *error) { // no-warning
+  return;
+}
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -95,6 +95,15 @@
 };
 }
 
+static bool hasReservedReturnType(const FunctionDecl *D) {
+  if (isa(D))
+return true;
+
+  // operators delete and delete[] are required to have 'void' return type
+  auto OperatorKind = D->getOverloadedOperator();
+  return OperatorKind == OO_Delete || OperatorKind == OO_Array_Delete;
+}
+
 void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
 AnalysisManager &mgr,
 BugReporter &BR) const {
@@ -102,6 +111,8 @@
 return;
   if (!D->getReturnType()->isVoidType())
 return;
+  if (hasReservedReturnType(D))
+return;
 
   if (!II)
 II = &D->getASTContext().Idents.get("CFErrorRef");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77548: clang-format: [JS] handle pseudo-keywords.

[Krasimir Georgiev via Phabricator via cfe-commits]

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

Looks good! The linter suggests a few formatting fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77548



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

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for tracking this down. Unfortunately I don't think removing the 
assertion is enough, the code doesn't handle this case correctly.




Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:487
 )"},
+  // Baseline case, bugz: https://bugs.llvm.org/show_bug.cgi?id=45428
+  {R"cpp(

This is the case that always worked, right? Not sure we need to add this test.



Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:505
+  // Causes mapping miss when invoking tryConsumeSpelledUntil
+  {R"cpp(
+#define NUM 42

From the linked bug, the varags isn't essential to this crash.
The linked example can be simplified a little further by removing foo:

```
#define N 1
#define ID(X) X
#define ID2 ID
ID2(N)
```

This makes it clearer that the trigger is using a macro in a arg to an 
"indirect" function macro (i.e. the function macro's name is expanded from 
another macro).



Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:519
+['#'_0, 'void'_34) => ['void'_0, 'void'_0)
+['M0'_40, 'NUM'_44) => ['{'_6, ';'_10)
+['NUM'_44, ')'_45) => [';'_10, ';'_10)

The problem with just removing the assertion is that this behavior is wrong: 
it's mapping `M0(,` onto `{ 42 ; }` - i.e. the whole M0 macro expansion. Then 
all the other mappings are empty. Compare to the baseline case above...

This should be `[M0_42, ;_46)] => [{_6, ;_10]` I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77507



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

[PATCH] D75917: Expose llvm fence instruction as clang intrinsic

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13651
+  llvm::getConstantStringInfo(Scope, scp);
+  SSID = getLLVMContext().getOrInsertSyncScopeID(scp);
+

saiislam wrote:
> sameerds wrote:
> > This seems to be creating a new ID for any arbitrary string passed as sync 
> > scope. This should be validated against LLVMContext::getSyncScopeNames(). 
> As the FE is not aware about specific target and implementation of sync scope 
> for that target, getSyncScopeNames() here returns llvm'sdefault sync scopes, 
> which only supports singlethreaded and system as valid scopes. Validity 
> checking of memory scope string is being intentionally left for the later 
> stages which deal with the generated IR.
That's pretty strange. At this point, Clang should know what the target is, and 
it should have a chance to update the list of sync scopes somewhere. @arsenm, 
do you see a way around this?



Comment at: clang/test/CodeGenHIP/builtin_memory_fence.cpp:9
+  // CHECK: fence syncscope("workgroup") seq_cst
+  __builtin_memory_fence(__ATOMIC_SEQ_CST,  "workgroup");
+  

JonChesterfield wrote:
> saiislam wrote:
> > sameerds wrote:
> > > Orderings like `__ATOMIC_SEQ_CST` are defined for C/C++ memory models. 
> > > They should not be used with the new builtin because this new builtin 
> > > does not follow any specific language model. For user convenience, the 
> > > right thing to do is to introduce new tokens in the Clang preprocessor, 
> > > similar to the `__ATOMIC_*` tokens. The convenient shortcut is to just 
> > > tell the user to supply numerical values by looking at the LLVM source 
> > > code.
> > > 
> > > From llvm/Support/AtomicOrdering.h, note how the numerical value for 
> > > `__ATOMIC_SEQ_CST` is 5, but the numerical value for the LLVM 
> > > SequentiallyConsistent ordering is 7. The numerical value 5 refers to the 
> > > LLVM ordering "release". So, if the implementation were correct, this 
> > > line should result in the following unexpected LLVM IR:
> > >   fence syncscope("workgroup") release
> > As you pointed out, the range of acquire to sequentiallly consistent memory 
> > orders for llvm::AtomicOrdering is [4, 7], while for 
> > llvm::AtomicOrderingCABI is [2, 5]. Enums of C ABI was taken to ensure easy 
> > of use for the users who are familiar with C/C++ standard memory model. It 
> > allows them to use macros like __ATOMIC_ACQUIRE etc.
> > Clang CodeGen of the builtin internally maps C ABI ordering to llvm atomic 
> > ordering.
> What language, implemented in clang, do you have in mind that reusing the 
> existing __ATOMIC_* macros would be incorrect for?
I think we agreed that this builtin exposes the LLVM fence exactly. That would 
mean it takes arguments defined by LLVM. If you are implementing something 
different from that, then it first needs to be specified properly. Perhaps you 
could say that this is a C ABI compatible builtin, that happens to take target 
specific scopes? That should cover OpenCL whose scope enum is designed to be 
compatible with C.

Whatever it is that you are trying to implement here, it definitely does not 
expose a raw LLVM fence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75917



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

[PATCH] D76612: [Matrix] Add draft specification for matrix support in Clang.

[Florian Hahn via Phabricator via cfe-commits]

fhahn updated this revision to Diff 255341.
fhahn added a comment.

Update standard conversion wording as suggested by @rjmccall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76612

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/MatrixSupport.rst

Index: clang/docs/MatrixSupport.rst
===
--- /dev/null
+++ clang/docs/MatrixSupport.rst
@@ -0,0 +1,305 @@
+==
+Matrix Support
+==
+
+.. contents::
+   :local:
+
+.. _matrixsupport:
+
+Clang provides a language extension that allows users to express high-level
+matrix math on the C/C++ level. The draft specification can be found
+:ref:`below `.
+
+Note that the implementation is currently in progress.
+
+.. _matrixsupport-draftspec:
+
+Draft Specification
+===
+
+Matrix Type Attribute
+-
+
+The *attribute-token* ``matrix_type`` is used to declare a matrix type. It shall
+appear at most once in each *attribute-list*. The attribute shall only appertain
+to a *typedef-name* of a typedef of a non-volatile type that is a *signed
+integer type*, an *unsigned integer type*, or a *floating-point type*. The
+element type must be unqualified and qualifiers are applied to the matrix type
+directly. An *attribute-argument-clause* must be present and it shall have the
+form:
+
+``(constant-expression, constant-expression)``
+
+Both *constant-expressions* shall be a positive non-zero integral constant
+expressions. The maximum of the product of the constants is implementation
+defined. If that implementation defined limit is exceeded, the program is
+ill-formed.
+
+An *attribute* of the form ``matrix_type(R, C)`` forms a matrix type with an
+element type of the cv-qualified type the attribute appertains to and *R* rows
+and *C* columns.
+
+If a declaration of a *typedef-name* has a ``matrix_type`` attribute, then all
+declaration of that *typedef-name* shall have a matrix_type attribute with the
+same element type, number of rows, and number of columns.
+
+Matrix Type
+---
+
+A matrix type has an underlying *element type*, a constant number of rows, and
+a constant number of columns. Matrix types with the same element type, rows,
+and columns are the same type. A value of a matrix type contains ``rows *
+columns`` values of the *element type* laid out in column-major order without
+padding in a way compatible with an array of at least that many elements of the
+underlying *element type*.
+
+A matrix type is a *scalar type* and its alignment is implementation defined.
+
+TODO: Does it make sense to allow M::element_type, M::rows, and M::columns
+where M is a matrix type? We don’t support this anywhere else, but it’s
+convenient. The alternative is using template deduction to extract this
+information. Also add spelling for C.
+
+TODO: Specify how matrix values are passed to functions.
+
+Future Work: Initialization syntax.
+
+Standard Conversions
+
+
+The standard conversions are extended as follows. Note that these conversions
+are intentionally not listed as satisfying the constraints for assignment,
+which is to say, they are only permitted as explicit casts, not as implicit
+conversions.
+
+For integral promotions, floating-point promotion, integral conversions,
+floating-point conversions, and floating-integral conversions: apply the rules
+to the underlying type of the matrix type. The resulting type is a matrix type
+with that underlying element type. If the number of rows or columns differ
+between the original and resulting type, the program is ill-formed. Otherwise
+the resulting value is as follows:
+
+* If the original value was of matrix type, each element is converted element
+  by element.
+* If the original value was not of matrix type, each element takes the value of
+  the original value.
+
+Arithmetic Conversions
+--
+
+The usual arithmetic conversions are extended as follows.
+
+Insert at the start:
+
+* If both operands are of matrix type, no arithmetic conversion is performed.
+* If one operand is of matrix type and the other operand is of an integer or
+  floating point type, convert the integer or floating point operand to the
+  underlying element type of the operand of matrix type.
+
+Matrix Type Element Access Operator
+---
+
+An expression of the form ``postfix-expression [expression][expression]`` where
+the ``postfix-expression`` is of matrix type is a matrix element access
+expression. ``expression`` shall not be a comma expression, and shall be a
+prvalue of unscoped enumeration or integral type. Given the expression
+``E1[E2][E3]`` the result is an lvalue of the same type as the underlying
+element type of the matrix that refers to the value at E2 row and E3 column in
+the matrix. The expression E1 is sequenced before E2 and E3. The expressions

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Oh dear, this seems like a design bug.

TokenBuffer only attempts to record "top-level" expansions, i.e. 
`FOO(BAR(BAZ))` will record the expansion of FOO, and say the tokens `FOO ( BAR 
( BAZ ) )` were expanded into `some other thing` in an essentially-opaque way.
The problem is this conflates two notions of "top-level"

- macro use is in main file (this is what the PPCallbacks checks)
- tokens emitted by the expansion are in the final expanded token stream (this 
is what makes the code correct, and I think what we're asserting)

The second seems more fundamental, so we should try patching the code to avoid 
the former. But I worry there are subtle assumptions of this scattered 
through...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77507



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

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss updated this revision to Diff 255347.
danielkiss added a reviewer: tamas.petz.
danielkiss added a comment.

Fix review comments from Tamas.


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

https://reviews.llvm.org/D75181

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aarch64-branch-protection-attr.c
  clang/test/CodeGenCXX/aarch64-branch-target_clang_call_terminate.cpp
  llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
  llvm/lib/Target/AArch64/AArch64BranchTargets.cpp
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-0.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-1.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-2.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-3.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-9.ll

Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-9.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-9.ll
@@ -0,0 +1,27 @@
+; RUN: llc -mtriple=aarch64-linux %s   -o - | \
+; RUN:   FileCheck %s --check-prefix=ASM
+; RUN: llc -mtriple=aarch64-linux %s -filetype=obj -o - | \
+; RUN:   llvm-readelf --notes | FileCheck %s --check-prefix=OBJ
+
+define dso_local i32 @f() #0 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @g() {
+entry:
+  ret i32 0
+}
+
+attributes #0 = { "branch-target-enforcement" }
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"branch-target-enforcement", i32 1}
+
+; Only the common atttribute (BTI)
+; ASM:	.word	3221225472
+; ASM-NEXT:	.word	4
+; ASM-NEXT	.word	1
+
+; OBJ: Properties: aarch64 feature: BTI
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll
@@ -13,6 +13,10 @@
 
 attributes #0 = { "branch-target-enforcement" }
 
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"branch-target-enforcement", i32 1}
+
 ; Declarations don't prevent setting BTI
 ; ASM:	.word	3221225472
 ; ASM-NEXT:	.word	4
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
@@ -13,10 +13,14 @@
   ret i32 0
 }
 
-attributes #0 = { "sign-return-address"="non-leaf" }
+attributes #0 = { "ignore-branch-target-enforcement" "sign-return-address"="non-leaf" }
 
 attributes #1 = { "branch-target-enforcement" }
 
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"branch-target-enforcement", i32 1}
+
 ; No common attribute, no note section
 ; ASM: warning: not setting BTI in feature flags
 ; ASM-NOT: .note.gnu.property
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
@@ -17,6 +17,11 @@
 
 attributes #1 = { "sign-return-address"="none" }
 
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"sign-return-address", !"non-leaf"}
+
+
 ; No common attribute, no note section
 ; ASM-NOT: .note.gnu.property
 ; OBJ-NOT: .note.gnu.property
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
@@ -15,7 +15,13 @@
 
 attributes #0 = { "branch-target-enforcement" "sign-return-address"="non-leaf" }
 
-attributes #1 = { "sign-return-address"="all" }
+attributes #1 = { "ignore-branch-target-enforcement" "sign-return-address"="all" }
+
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 4, !"branch-target-enforcement", i32 1}
+!1 = !{i32 4, !"sign-return-address", !"all"}
+
 
 ; Only the common atttribute (PAC)
 ; ASM: warning: not setting BTI in feature flags
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
@@ -17,6 +17,10 @@
 
 attributes #1 = { "branch-target-enforcement" }
 
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"branch-target-enforcement", i32 1}
+
 ; Only the common atttribute (BTI)
 ; ASM:	.word	3221225472
 ; ASM-NEXT:	.word	4
Index: llvm/test/CodeGen/AArch64/note-gnu-prop

[clang] ab1fad8 - [analyzer] StdLibraryFunctionsChecker: Add test for function with default parameter

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-04-06T17:08:58+02:00
New Revision: ab1fad8a3a8b8e3264c34448205061add013b8d7

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

LOG: [analyzer] StdLibraryFunctionsChecker: Add test for function with default 
parameter

Reviewers: Szelethus, baloghadamsoftware, gamesh411, steakhal, balazske

Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin, 
mikhail.ramalho, donat.nagy, dkrupp, Charusso, ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 
clang/test/Analysis/std-c-library-functions-arg-constraints.cpp

Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index f03696daab0b..6ca664a28351 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -960,6 +960,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ArgumentCondition(0U, OutOfRange, SingleValue(1)))
.ArgConstraint(
ArgumentCondition(0U, OutOfRange, 
SingleValue(2)))}},
+{"__defaultparam", Summaries{Summary(ArgTypes{Irrelevant, IntTy},
+ RetType{IntTy}, EvalCallAsPure)
+ .ArgConstraint(NotNull(ArgNo(0)))}},
 };
 for (auto &E : TestFunctionSummaryMap) {
   auto InsertRes =

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp 
b/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
new file mode 100644
index ..fed815f67a57
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux \
+// RUN:   -verify
+
+void clang_analyzer_eval(int);
+
+int __defaultparam(void *, int y = 3);
+
+void test_arg_constraint_on_fun_with_default_param() {
+  __defaultparam(nullptr); // \
+  // expected-warning{{Function argument constraint is not satisfied}}
+}



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

[clang] 9220150 - clang-format: [JS] handle pseudo-keywords.

[Martin Probst via cfe-commits]

Author: Martin Probst
Date: 2020-04-06T17:13:04+02:00
New Revision: 92201505cdecd8ba5795f5e33821c49dc57f0816

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

LOG: clang-format: [JS] handle pseudo-keywords.

Summary:
The previous change in https://reviews.llvm.org/D77311 attempted to
detect more C++ keywords. However it also precisely detected all
JavaScript keywords. That's generally correct, but many JavaScripy
keywords, e.g. `get`, are so-called pseudo-keywords. They can be used in
positions where a keyword would never be legal, e.g. in a dotted
expression:

x.type;  // type is a pseudo-keyword, but can be used here.
x.get;   // same for get etc.

This change introduces an additional parameter to
`IsJavaScriptIdentifier`, allowing clients to toggle whether they want
to allow `IdentifierName` tokens, i.e. pseudo-keywords.

Reviewers: krasimir

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestJS.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 48ec7602c21c..c67cf192ab1f 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -909,7 +909,11 @@ struct AdditionalKeywords {
 
   /// Returns \c true if \p Tok is a true JavaScript identifier, returns
   /// \c false if it is a keyword or a pseudo keyword.
-  bool IsJavaScriptIdentifier(const FormatToken &Tok) const {
+  /// If \c AcceptIdentifierName is true, returns true not only for keywords,
+  // but also for IdentifierName tokens (aka pseudo-keywords), such as
+  // ``yield``.
+  bool IsJavaScriptIdentifier(const FormatToken &Tok,
+  bool AcceptIdentifierName = true) const {
 // Based on the list of JavaScript & TypeScript keywords here:
 // 
https://github.com/microsoft/TypeScript/blob/master/src/compiler/scanner.ts#L74
 switch (Tok.Tok.getKind()) {
@@ -946,11 +950,14 @@ struct AdditionalKeywords {
 case tok::kw_while:
   // These are JS keywords that are lexed by LLVM/clang as keywords.
   return false;
-case tok::identifier:
+case tok::identifier: {
   // For identifiers, make sure they are true identifiers, excluding the
   // JavaScript pseudo-keywords (not lexed by LLVM/clang as keywords).
-  return JsExtraKeywords.find(Tok.Tok.getIdentifierInfo()) ==
- JsExtraKeywords.end();
+  bool IsPseudoKeyword =
+  JsExtraKeywords.find(Tok.Tok.getIdentifierInfo()) !=
+  JsExtraKeywords.end();
+  return AcceptIdentifierName || !IsPseudoKeyword;
+}
 default:
   // Other keywords are handled in the switch below, to avoid problems due
   // to duplicate case labels when using the #include trick.

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 029741c3dce7..8204623645a4 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1522,9 +1522,11 @@ class AnnotatingParser {
 if (Style.Language == FormatStyle::LK_JavaScript) {
   if (Current.is(tok::exclaim)) {
 if (Current.Previous &&
-(Keywords.IsJavaScriptIdentifier(*Current.Previous) ||
- Current.Previous->isOneOf(tok::kw_namespace, tok::r_paren,
-   tok::r_square, tok::r_brace) ||
+(Keywords.IsJavaScriptIdentifier(
+ *Current.Previous, /* AcceptIdentifierName= */ true) ||
+ Current.Previous->isOneOf(
+ tok::kw_namespace, tok::r_paren, tok::r_square, tok::r_brace,
+ Keywords.kw_type, Keywords.kw_get, Keywords.kw_set) ||
  Current.Previous->Tok.isLiteral())) {
   Current.Type = TT_JsNonNullAssertion;
   return;
@@ -3071,7 +3073,9 @@ bool TokenAnnotator::spaceRequiredBefore(const 
AnnotatedLine &Line,
   return false;
 // In tagged template literals ("html`bar baz`"), there is no space between
 // the tag identifier and the template string.
-if (Keywords.IsJavaScriptIdentifier(Left) && Right.is(TT_TemplateString))
+if (Keywords.IsJavaScriptIdentifier(Left,
+/* AcceptIdentifierName= */ false) &&
+Right.is(TT_TemplateString))
   return false;
 if (Right.is(tok::star) &&
 Left.isOneOf(Keywords.kw_function, Keywords.kw_yield))

diff  --git a/clang/unittests/Format/FormatTestJS.cpp 
b/clang/unittests/Format/FormatTestJS.cpp
index 3fd795c526b1..eadea35f051a 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -2412,6 +2412,11 @

[clang] 8f96139 - [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-04-06T17:34:08+02:00
New Revision: 8f961399739f539cb0b3c9ac68ca1b62c2a17a80

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

LOG: [analyzer] StdLibraryFunctionsChecker: match signature based on 
FunctionDecl

Summary:
Currently we match the summary signature based on the arguments in the CallExpr.
There are a few problems with this approach.
1) Variadic arguments are handled badly. Consider the below code:
 int foo(void *stream, const char *format, ...);
 void test_arg_constraint_on_variadic_fun() {
foo(0, "%d%d", 1, 2); // CallExpr
 }
   Here the call expression holds 4 arguments, whereas the function declaration
   has only 2 `ParmVarDecl`s. So there is no way to create a summary that
   matches the call expression, because the discrepancy in the number of
   arguments causes a mismatch.
2) The call expression does not handle the `restrict` type qualifier.
   In C99, fwrite's signature is the following:
 size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
   However, in a call expression, like below, the type of the argument does not
   have the restrict qualifier.
void test_fread_fwrite(FILE *fp, int *buf) {
  size_t x = fwrite(buf, sizeof(int), 10, fp);
}
   This can result in an unmatches signature, so the summary is not applied.
The solution is to match the summary against the referened callee
`FunctionDecl` that we can query from the `CallExpr`.

Further patches will continue with additional refactoring where I am going to
do a lookup during the checker initialization and the signature match will
happen there. That way, we will not check the signature during every call,
rather we will compare only two `FunctionDecl` pointers.

Reviewers: NoQ, Szelethus, gamesh411, baloghadamsoftware

Subscribers: whisperity, xazax.hun, kristof.beyls, szepet, rnkovacs, a.sidorin, 
mikhail.ramalho, donat.nagy, dkrupp, Charusso, steakhal, danielkiss, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 6ca664a28351..5e36938b613d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -268,7 +268,7 @@ class StdLibraryFunctionsChecker
 
 /// Try our best to figure out if the call expression is the call of
 /// *the* library function to which this specification applies.
-bool matchesCall(const CallExpr *CE) const;
+bool matchesCall(const FunctionDecl *FD) const;
   };
 
   // The same function (as in, function identifier) may have 
diff erent
@@ -316,7 +316,6 @@ class StdLibraryFunctionsChecker
 
 private:
   Optional findFunctionSummary(const FunctionDecl *FD,
-const CallExpr *CE,
 CheckerContext &C) const;
   Optional findFunctionSummary(const CallEvent &Call,
 CheckerContext &C) const;
@@ -532,13 +531,13 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent 
&Call,
 }
 
 bool StdLibraryFunctionsChecker::Summary::matchesCall(
-const CallExpr *CE) const {
+const FunctionDecl *FD) const {
   // Check number of arguments:
-  if (CE->getNumArgs() != ArgTys.size())
+  if (FD->param_size() != ArgTys.size())
 return false;
 
   // Check return type if relevant:
-  if (!RetTy.isNull() && RetTy != CE->getType().getCanonicalType())
+  if (!RetTy.isNull() && RetTy != FD->getReturnType().getCanonicalType())
 return false;
 
   // Check argument types when relevant:
@@ -550,8 +549,7 @@ bool StdLibraryFunctionsChecker::Summary::matchesCall(
 
 assertTypeSuitableForSummary(FormalT);
 
-QualType ActualT = StdLibraryFunctionsChecker::getArgType(CE, I);
-assert(ActualT.isCanonical());
+QualType ActualT = FD->getParamDecl(I)->getType().getCanonicalType();
 if (ActualT != FormalT)
   return false;
   }
@@ -561,12 +559,7 @@ bool StdLibraryFunctionsChecker::Summary::matchesCall(
 
 Optional
 StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
-const CallExpr *CE,
 CheckerContext &C) const {
-  // Note: we cannot always obtain FD from CE
-  // (eg. virtual call, or call by pointer).
-  assert(CE);
-
   if (!FD)
 return None;
 
@@ 

[PATCH] D77028: Speed up deferred diagnostic emitter

[John McCall via Phabricator via cfe-commits]

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

Thank you.  Minor request, but feel free to commit with that change.




Comment at: clang/lib/Sema/Sema.cpp:1558
+// visited before.
+if (Done.count(FD))
+  return;

`insert` returns whether it changed the set, so you can combine this check with 
the insertion unless the exact ordering is important.


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

https://reviews.llvm.org/D77028



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

[PATCH] D77393: [X86] Fix implicit sign conversion warnings in X86 headers.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@craig.topper Any comments? Hopefully we can add extra tests as time goes on 
(for instance I'm intending to add Wdocumentation test coverage for PR35949).




Comment at: clang/test/Headers/x86-header-warnings.c:12
+void test() {
+  // CHECK-NOT: warning:
+  _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_ON);

Add a LABEL check as well


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

https://reviews.llvm.org/D77393



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

[PATCH] D77520: Treat default values in LangOptions.def in the scope of enums

[Reid Kleckner via Phabricator via cfe-commits]

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

I don't think we can do this because GCC 5.1 won't support it.

To confirm, that's still the minimum supported version:
https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

And here's a godbolt example showing the error: https://gcc.godbolt.org/z/fYb58Q


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77520



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

[PATCH] D77058: [Clang] Add llvm.loop.unroll.disable to loops with -fno-unroll-loops.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

ping.

@tejohnson are you happy with this approach, given that it sounds good to 
@dexonsmith as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77058



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

[PATCH] D75453: [Driver][ARM] fix undefined behaviour when checking architecture version

[Jan Ole Hüser via Phabricator via cfe-commits]

j0le updated this revision to Diff 255354.
j0le added a comment.

I added a test case.
I hope the folder "clang/tests/Driver" is the right one.


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

https://reviews.llvm.org/D75453

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/windows-thumbv7em.cpp


Index: clang/test/Driver/windows-thumbv7em.cpp
===
--- /dev/null
+++ clang/test/Driver/windows-thumbv7em.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cpp --target=thumbv7em-none-windows-eabi-coff \
+// RUN: -mcpu=cortex-m7 -c %s -o /dev/null 2>&1 \
+// RUN: | FileCheck --allow-empty %s
+
+// CHECK-NOT: error: the target architecture 'thumbv7em' is not supported by 
the target 'thumbv7em-none-windows-eabi'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4091,9 +4091,10 @@
   if (Triple.isOSWindows() && (Triple.getArch() == llvm::Triple::arm ||
Triple.getArch() == llvm::Triple::thumb)) {
 unsigned Offset = Triple.getArch() == llvm::Triple::arm ? 4 : 6;
-unsigned Version;
-Triple.getArchName().substr(Offset).getAsInteger(10, Version);
-if (Version < 7)
+unsigned Version = 0;
+bool Failure =
+Triple.getArchName().substr(Offset).consumeInteger(10, Version);
+if (Failure || Version < 7)
   D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName()
 << TripleStr;
   }


Index: clang/test/Driver/windows-thumbv7em.cpp
===
--- /dev/null
+++ clang/test/Driver/windows-thumbv7em.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cpp --target=thumbv7em-none-windows-eabi-coff \
+// RUN: -mcpu=cortex-m7 -c %s -o /dev/null 2>&1 \
+// RUN: | FileCheck --allow-empty %s
+
+// CHECK-NOT: error: the target architecture 'thumbv7em' is not supported by the target 'thumbv7em-none-windows-eabi'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4091,9 +4091,10 @@
   if (Triple.isOSWindows() && (Triple.getArch() == llvm::Triple::arm ||
Triple.getArch() == llvm::Triple::thumb)) {
 unsigned Offset = Triple.getArch() == llvm::Triple::arm ? 4 : 6;
-unsigned Version;
-Triple.getArchName().substr(Offset).getAsInteger(10, Version);
-if (Version < 7)
+unsigned Version = 0;
+bool Failure =
+Triple.getArchName().substr(Offset).consumeInteger(10, Version);
+if (Failure || Version < 7)
   D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName()
 << TripleStr;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:264
+  auto *FrontMapping = mappingStartingBeforeSpelled(File, &Spelled.front());
+  unsigned SpelledFrontI = &Spelled.front() - File.SpelledTokens.data();
+  unsigned ExpandedBegin;

hlopko wrote:
> gribozavr2 wrote:
> > Or assert that SpelledFrontI is less than File.SpelledTokens.size().
> I think the assert I added is good enough?
The assertion that I suggested is stronger, because it would prevent an 
out-of-bounds read from even happening, and would not rely on 
`isBeforeInTranslationUnit` returning false on garbage data.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:601
 assert(Result.ExpandedTokens.back().kind() == tok::eof);
+for (auto &pair : Result.Files) {
+  auto &mappings = pair.second.Mappings;

I'd suggest moving this loop to the very bottom of this function. It is not 
related to the expanded token processing, which happens here (above we have 
assertions related to expanded tokens, and below we have the 
`processExpandedToken` call).

Also, `fillGapsAtEndOfFiles` below modifies mappings, it would be good if 
modifications were covered by the assertion as well.



Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:602
+for (auto &pair : Result.Files) {
+  auto &mappings = pair.second.Mappings;
+  assert(std::is_sorted(

We'd get an "unused variable" warning here when assertions are disabled. Please 
wrap the whole loop in `#ifndef NDEBUG`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77209



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

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

[Jason Liu via Phabricator via cfe-commits]

jasonliu added a comment.

Thanks for splitting the test case. I think it helps a lot for reading and 
maintainability.




Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1490
   continue;
 GlobalValue::VisibilityTypes V = F.getVisibility();
+

This line is not used before XCOFF specialized code, we should move it down. 



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1495
+  if (F.isIntrinsic())
+continue;
+

If I understand correctly, we do not want to touch how we interact with 
visibility in this patch. 
If that's the case, we don't want to `continue` here and in line 1512 since the 
early `continue` will change the code flow for visibility. 
Suggestion:
if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) {
and remove continue in line 1512. 



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1498
+  // Get the function entry point symbol.
+  Name = OutContext.getOrCreateSymbol("." + Name->getName());
+  if (cast(Name)->hasContainingCsect())

nit: I don't think it's a good idea here to assign back to `Name` here, as 
`Name` will also get used in  `emitVisibility` below and we want to keep it as 
it is. Let's create a new MCSymbol* here.  



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500
+  if (cast(Name)->hasContainingCsect())
+emitLinkage(&F, Name);
+

1. We need to rebase here, as it is called `hasRepresentedCsectSet()` instead 
of `hasContainingCsect()` now.
2. I'm slightly worried here to rely on `hasRepresentedCsectSet()` to check if 
a linkage should be emitted. This is based on the assumption that we will not 
ever change our implementation for `.bl foo` to `.bl foo[PR]`. But in 
https://reviews.llvm.org/D77080#inline-706207, we discussed about this 
possibility. So this assumption might not be true in the future. However, I'm 
not sure if there is another way to check if this function have been called 
directly. 
So if there is another way to check, we should pursue the alternative instead. 
If there is not, then we need to add an assert here, like 
`assert(Name->getName().equals(cast(Name)->getUnqualifiedName())`
 to make sure we don't get a qualname here. 
3. 
Have a comment here and tell people what we are doing here.
For example, 
// If there is a direct call to external function, then we need to emit linkage 
for its function entry point. 



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1503
+  if (F.hasAddressTaken()) {
+// Indirect call reference of the extern function.
+// for example c source code as :

Comment need to mention what we are trying to do here: 
// If address is taken from an extern function, we need to emit linkage for its 
function descriptor symbol.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1504
+// Indirect call reference of the extern function.
+// for example c source code as :
+// extern int bar_ext();

nit: for -> For,
and we don't need space between `as` and  `:`



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1510
+Name = Csect->getQualNameSymbol();
+emitLinkage(&F, Name);
+  }

nit: We don't need to assign it back to `Name`.
emitLinkage(&F, Csect->getQualNameSymbol());



Comment at: llvm/test/CodeGen/PowerPC/aix-WeakODRLinkage.ll:10
+  ret void
+}
+

Do we also want to test WeakAnyLinkage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932



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