[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344692.
mgartmann added a comment.

Revert `ReleaseNotes.rst` to its initial content in a try to fix the pre-build 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be us

[PATCH] D102280: [analyzer] Engine: fix crash with SEH __leave keyword

[Balázs Benics via Phabricator via cfe-commits]

steakhal added reviewers: NoQ, vsavchenko, steakhal.
steakhal added a comment.

Please, add these reviewers for your upcoming [analyzer] patches.
Inline a couple of nits.
Nice to see some fixes for visual c++ stuff.




Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:352
 
+  case Stmt::SEHLeaveStmtClass:
   case Stmt::ContinueStmtClass:

You should probably extend the `ExprEngine.cpp:1312` in a similar fashion.



Comment at: clang/test/Analysis/misc-ms-leave.cpp:12
+  __try {
+__leave;
+  } __except (filter()) {

If it crashed previously at this statement, please put there a `// no-crash` 
comment.



Comment at: clang/test/Analysis/misc-ms-leave.cpp:16
+  int *p = 0;
+  int x = *p; // expected-warning {{Dereference of null pointer (loaded from 
variable 'p')}}
+}

I guess you could simply use the `clang_analyzer_warnIfReached()` here. 
https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102280

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

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344711.
mgartmann added a comment.

Re-add description for this check in `ReleaseNotes.rst`.
Adjust `AvoidStdIoOutsideMainCheck.cpp` third matcher to call `hasAnyName()` 
with a vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

I honestly don't see a reason why it's not a part of `forFunction`.  
`forFunction` matches C++ methods and lambdas, Obj-C methods and blocks don't 
seem that much more special to have an extended matcher just for those.
I really don't think that it will break someone's workflow and that someone 
really relied on the fact that `forFunction` doesn't match those.
Other than that it's a great patch IMO!


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

https://reviews.llvm.org/D102213

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

[PATCH] D102303: [ASTMatchers] Fix formatting around forFunction().

[Valeriy Savchenko via Phabricator via cfe-commits]

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

It always pleases me when more code becomes compliant to our style guides. 
Thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D102303

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

[PATCH] D102294: [clang-tidy] bugprone-infinite-loop: React to ObjC ivars and messages in condition.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Tests look thorough, it should only reduce the number of warnings and should 
only affect Obj-C. So, I'd say that it's safe and sound! Great job!




Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:64-66
+  } else if (isa(Cond) || isa(Cond) ||
+ isa(Cond) || isa(Cond) ||
+ isa(Cond)) {

Situations like this make me think that variadic `isa` is not such a bad idea 😅


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

https://reviews.llvm.org/D102294

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

[clang] cbd93ce - Revert "[PowerPC] [Clang] Enable float128 feature on VSX targets"

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2021-05-12T16:51:52+08:00
New Revision: cbd93cee9bf014402a7405479ba21f6f3340a126

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

LOG: Revert "[PowerPC] [Clang] Enable float128 feature on VSX targets"

This commit brought build break in some f128 related tests. But that's
not the root cause. There exists some differences between Clang and
GCC's definition for 128-bit float types on PPC, so macros/functions in
glibc may not work with clang -mfloat128 well. We need to handle this
carefully and reland it.

Added: 


Modified: 
clang/lib/Basic/Targets/PPC.cpp
clang/test/Driver/ppc-f128-support-check.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 21a196ab01435..cd8cc1aed39ea 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -328,12 +328,6 @@ bool PPCTargetInfo::initFeatureMap(
 .Case("pwr9", true)
 .Case("pwr8", true)
 .Default(false);
-  Features["float128"] = llvm::StringSwitch(CPU)
- .Case("ppc64le", true)
- .Case("pwr9", true)
- .Case("pwr8", true)
- .Case("pwr7", true)
- .Default(false);
 
   // ROP Protect is off by default.
   Features["rop-protect"] = false;
@@ -362,9 +356,9 @@ bool PPCTargetInfo::initFeatureMap(
   if (!ppcUserFeaturesCheck(Diags, FeaturesVec))
 return false;
 
-  if (!(ArchDefs & ArchDefinePwr7) && (ArchDefs & ArchDefinePpcgr) &&
+  if (!(ArchDefs & ArchDefinePwr9) && (ArchDefs & ArchDefinePpcgr) &&
   llvm::find(FeaturesVec, "+float128") != FeaturesVec.end()) {
-// We have __float128 on PPC but not pre-VSX targets.
+// We have __float128 on PPC but not power 9 and above.
 Diags.Report(diag::err_opt_not_valid_with_opt) << "-mfloat128" << CPU;
 return false;
   }

diff  --git a/clang/test/Driver/ppc-f128-support-check.c 
b/clang/test/Driver/ppc-f128-support-check.c
index 616d641f54b96..24748905612ff 100644
--- a/clang/test/Driver/ppc-f128-support-check.c
+++ b/clang/test/Driver/ppc-f128-support-check.c
@@ -2,17 +2,13 @@
 // RUN:   -mcpu=pwr9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=power9 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr7 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=HASF128
 
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr6 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
+// RUN:   -mcpu=pwr8 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
+// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
+// RUN:   -mcpu=pwr7 -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mno-vsx -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
+// RUN:   -mfloat128 %s 2>&1 | FileCheck %s --check-prefix=NOF128
 
 #ifdef __FLOAT128__
 static_assert(false, "__float128 enabled");



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

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344726.
mgartmann added a comment.

Remove any parentheses and slashes from the check's section in 
`ReleaseNotes.rst` in order to try to fix the build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CH

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 344731.
mgartmann added a comment.

Remove text from `ReleaseNotes.rst` to narrow down the cause for the failing 
build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99646

Files:
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp
  clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-avoid-std-io-outside-main.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-avoid-std-io-outside-main.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-io-outside-main %t
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+
+struct Ostream {
+  Ostream &operator<<(string Message);
+};
+
+struct Istream {
+  Istream &operator>>(string Message);
+};
+
+Ostream cout{}, wcout{}, cerr{}, wcerr{};
+Istream cin{}, wcin{};
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+} // namespace std
+
+int printf(const char *Format, ...);
+int vprintf(const char *const, ...);
+int puts(const char *Str);
+int putchar(int Character);
+int scanf(const char *Format, ...);
+int getchar(void);
+char *gets(char *Str);
+
+namespace arbitrary_namespace {
+std::Ostream cout{};
+std::Istream cin{};
+} // namespace arbitrary_namespace
+
+void anyNonMainFunction() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcout << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cerr << "This should trigger the check";
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcerr << "This should trigger the check";
+
+  arbitrary_namespace::cout << "This should not trigger the check"; // OK
+
+  std::string Foo{"bar"};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::cin >> Foo;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: predefined standard stream objects should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::wcin >> Foo;
+
+  arbitrary_namespace::cin >> Foo; // OK
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::printf("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  printf("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::puts("This should trigger the check");
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  puts("This should trigger the check");
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::putchar('m');
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  putchar('m');
+
+  char Input[5];
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::scanf("%s", Input);
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  scanf("%s", Input);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: cstdio functions should not be used outside the main function [misc-avoid-std-io-outside-main]
+  std::getchar();
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: wa

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Great job on the patch! Thanks!




Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:166-167
 
-static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
+static bool isCapturedByReference(const VarDecl *VD, ExplodedNode *N,
+  const DeclRefExpr *DR) {
+  assert(DR->refersToEnclosingVariableOrCapture());

One thought here.
I think it's better to not have implicit connections between parameters that 
are not obvious to the user of the function (or to the person who is going to 
modify this function in the future).  Here we always have `VD = 
DR->getDecl()->getCanonicalDecl()`. So, IMO, the interface of this function 
will be much cleaner if we accept only a `DeclRefExpr`.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186
+  return FD->getType()->isReferenceType();
+} else {
+  assert(false && "Unknown captured variable");
+}

But actually, it's a bit different with this one. I don't know exact details 
and rules how clang sema fills in the class for lambda.
According to [[ https://en.cppreference.com/w/cpp/language/lambda | 
cppreference ]]:
> For the entities that are captured by reference (with the default capture [&] 
> or when using the character &, e.g. [&a, &b, &c]), it is unspecified if 
> additional data members are declared in the closure type

It can be pretty much specified in clang, that's true, but it looks like in 
`DeadStoreChecker` we have a very similar situation and we do not assume that 
captured variable always have a corresponding field.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:188-191
+  } else {
+assert(false &&
+   "Captured variable should only be seen while evaluating a lambda");
+  }

There is a general rule in LLVM's style guide of trying to minimize nestedness 
of `if's, `forms and so on.
In this particular situation this assertion can be more local: `assert(MD && 
MD->getParent()->isLambda() & ...);`.
This way it is more obvious what it checks and we avoid confusion while reading 
this function because `if (condition)` automatically means that `!condition` is 
something that might happen.
The same applies to another assertion here.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:202-203
 
-  const bool isParm = isa(VD);
+  const bool IsEntryValue =
+  isa(VD) || DR->refersToEnclosingVariableOrCapture();
   // Reference parameters are assumed as escaped variables.

I think this whole section is less obvious now and requires one good solid 
comment explaining what are the different situations here, what is "entry" and 
so on.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:204
+  isa(VD) || DR->refersToEnclosingVariableOrCapture();
   // Reference parameters are assumed as escaped variables.
+  if ((DR->refersToEnclosingVariableOrCapture() &&

I think that this comment has to be updated now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D99260: [analyzer] Fix false positives in inner pointer checker (PR49628)

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D99260#2751967 , @steakhal wrote:

> In D99260#2704102 , @NoQ wrote:
>
>> In https://bugs.llvm.org/show_bug.cgi?id=45786 the godbolt link shows that 
>> there are still problems with `addressof` (yes, their "trunk" clang is fresh 
>> enough). They seem to have `__addressof` instead of `addressof` so maybe we 
>> should cover that case real quick. Or maybe outright suppress reference 
>> invalidation on double-underscore functions because the checker generally 
>> relies on the number of standard functions that don't invalidate references 
>> while taking a non-const reference being very limited but this limit 
>> definitely doesn't take double-underscore functions into account.
>
> What is the status quo of this issue? @vsavchenko

Ah, yep. Just got back from my vacation!
I'm not sure about shooting off double underscore functions because one never 
knows what weird coding conventions people have in their project (all TU-local 
static functions should be named like this for better visibility at their call 
sites, for example). Being more specific and dealing with `std::__` can be 
better, but I think a quick hack specifically for `std::__addressof` is better 
atm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99260

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

[PATCH] D102322: [clang-tidy] Add '-target' CLI option to override target triple

[Georgy Komarov via Phabricator via cfe-commits]

jubnzv created this revision.
jubnzv added a project: clang-tools-extra.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
jubnzv requested review of this revision.

The added option allows the user to specify the target for which //clang-tidy// 
checks will be executed. This is necessary for users who run the tool on 
project that compiles for a different target platform.

This option also makes it easier to test of //clang-tidy// and allows to run 
platform-specific tests on any host system.
This will fix the failed tests when they are run on a Windows host, but the 
toolchain is targeted a non-Windows platform. The problem is described here: 
https://reviews.llvm.org/D101259#2739466.


https://reviews.llvm.org/D102322

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
@@ -2,9 +2,7 @@
 // Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the
 // built-in va_list on Windows systems.
 
-// REQUIRES: system-windows
-
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t
+// RUN: %check_clang_tidy -target="x86_64-windows" %s 
cppcoreguidelines-pro-type-vararg %t
 
 void test_ms_va_list(int a, ...) {
   __builtin_ms_va_list ap;
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -256,6 +256,12 @@
 )"),
   cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt TargetTriple("target", cl::desc(R"(
+Override target triple for clang-tidy. If not set, the host target will be
+used.)"),
+ cl::init(""),
+ cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -488,11 +494,20 @@
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
+  if (!TargetTriple.empty()) {
+TargetTriple = Triple::normalize(TargetTriple);
+auto T = Triple(TargetTriple);
+if (T.getArch() == Triple::UnknownArch) {
+  llvm::errs() << "Error: unknown target triple '" << TargetTriple << 
"'\n";
+  return 1;
+}
+  }
+
   ClangTidyContext Context(std::move(OwningOptionsProvider),
AllowEnablingAnalyzerAlphaCheckers);
   std::vector Errors =
   runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
-   FixNotes, EnableCheckProfile, ProfilePrefix);
+   FixNotes, EnableCheckProfile, TargetTriple, ProfilePrefix);
   bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
return E.DiagLevel == ClangTidyError::Error;
  }) != Errors.end();
Index: clang-tools-extra/clang-tidy/ClangTidy.h
===
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -80,6 +80,7 @@
  ArrayRef InputFiles,
  llvm::IntrusiveRefCntPtr BaseFS,
  bool ApplyAnyFix, bool EnableCheckProfile = false,
+ const std::string &TargetTriple = "",
  llvm::StringRef StoreCheckProfile = StringRef());
 
 /// Controls what kind of fixes clang-tidy is allowed to apply.
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -515,15 +515,18 @@
  ArrayRef InputFiles,
  llvm::IntrusiveRefCntPtr BaseFS,
  bool ApplyAnyFix, bool EnableCheckProfile,
+ const std::string &TargetTriple,
  llvm::StringRef StoreCheckProfile) {
   ClangTool Tool(Compilations, InputFiles,
  std::make_shared(), BaseFS);
 
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
-  [&Context](const CommandLineArguments &Args, StringRef Filename) {
+  [&Context, &TargetTriple](const CommandLineArguments &Args, StringRef 
Filename) {
 ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
 CommandLineArguments AdjustedArgs = Args;
+if (!TargetTriple.empty())
+  AdjustedArgs.push_back("--target=" + TargetTriple);
 if (Opts.ExtraArgsBefore) {
   auto I = AdjustedArgs.begin();
 

[PATCH] D102322: [clang-tidy] Add '-target' CLI option to override target triple

[Georgy Komarov via Phabricator via cfe-commits]

jubnzv updated this revision to Diff 344758.

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

https://reviews.llvm.org/D102322

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
@@ -2,9 +2,7 @@
 // Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the
 // built-in va_list on Windows systems.
 
-// REQUIRES: system-windows
-
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t
+// RUN: %check_clang_tidy -target="x86_64-windows" %s cppcoreguidelines-pro-type-vararg %t
 
 void test_ms_va_list(int a, ...) {
   __builtin_ms_va_list ap;
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -256,6 +256,12 @@
 )"),
   cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt TargetTriple("target", cl::desc(R"(
+Override target triple for clang-tidy. If not set, the host target will be
+used.)"),
+ cl::init(""),
+ cl::cat(ClangTidyCategory));
+
 namespace clang {
 namespace tidy {
 
@@ -488,11 +494,20 @@
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
+  if (!TargetTriple.empty()) {
+TargetTriple = Triple::normalize(TargetTriple);
+auto T = Triple(TargetTriple);
+if (T.getArch() == Triple::UnknownArch) {
+  llvm::errs() << "Error: unknown target triple '" << TargetTriple << "'\n";
+  return 1;
+}
+  }
+
   ClangTidyContext Context(std::move(OwningOptionsProvider),
AllowEnablingAnalyzerAlphaCheckers);
   std::vector Errors =
   runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
-   FixNotes, EnableCheckProfile, ProfilePrefix);
+   FixNotes, EnableCheckProfile, TargetTriple, ProfilePrefix);
   bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
return E.DiagLevel == ClangTidyError::Error;
  }) != Errors.end();
Index: clang-tools-extra/clang-tidy/ClangTidy.h
===
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -80,6 +80,7 @@
  ArrayRef InputFiles,
  llvm::IntrusiveRefCntPtr BaseFS,
  bool ApplyAnyFix, bool EnableCheckProfile = false,
+ const std::string &TargetTriple = "",
  llvm::StringRef StoreCheckProfile = StringRef());
 
 /// Controls what kind of fixes clang-tidy is allowed to apply.
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -509,21 +509,23 @@
   return Factory.getCheckOptions();
 }
 
-std::vector
-runClangTidy(clang::tidy::ClangTidyContext &Context,
- const CompilationDatabase &Compilations,
- ArrayRef InputFiles,
- llvm::IntrusiveRefCntPtr BaseFS,
- bool ApplyAnyFix, bool EnableCheckProfile,
- llvm::StringRef StoreCheckProfile) {
+std::vector runClangTidy(
+clang::tidy::ClangTidyContext &Context,
+const CompilationDatabase &Compilations, ArrayRef InputFiles,
+llvm::IntrusiveRefCntPtr BaseFS,
+bool ApplyAnyFix, bool EnableCheckProfile, const std::string &TargetTriple,
+llvm::StringRef StoreCheckProfile) {
   ClangTool Tool(Compilations, InputFiles,
  std::make_shared(), BaseFS);
 
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
-  [&Context](const CommandLineArguments &Args, StringRef Filename) {
+  [&Context, &TargetTriple](const CommandLineArguments &Args,
+StringRef Filename) {
 ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
 CommandLineArguments AdjustedArgs = Args;
+if (!TargetTriple.empty())
+  AdjustedArgs.push_back("--target=" + TargetTriple);
 if (Opts.ExtraArgsBefore) {
   auto I = AdjustedArgs.begin();
   if (I != AdjustedArgs.end() && !StringRef(*I).startswith("-"))
___
cfe-commits mailing list
cf

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal updated this revision to Diff 344763.
pdhaliwal added a comment.

Fixed the if-else logic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102065

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/amdgpu-openmp-toolchain.c


Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -6,7 +6,7 @@
 // verify the tools invocations
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-x" 
"c"{{.*}}
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-x" 
"ir"{{.*}}
-// CHECK: clang{{.*}}"-cc1"{{.*}}"-triple" 
"amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx906" "-fcuda-is-device" 
"-mlink-builtin-bitcode"{{.*}}libomptarget-amdgcn-gfx906.bc"{{.*}}
+// CHECK: clang{{.*}}"-cc1"{{.*}}"-triple" 
"amdgcn-amd-amdhsa"{{.*}}"-emit-llvm-bc"{{.*}}"-target-cpu" "gfx906" 
"-fcuda-is-device"{{.*}}"-mlink-builtin-bitcode"{{.*}}libomptarget-amdgcn-gfx906.bc"{{.*}}
 // CHECK: llvm-link{{.*}}"-o" 
"{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-linked-{{.*}}.bc"
 // CHECK: llc{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-linked-{{.*}}.bc" 
"-mtriple=amdgcn-amd-amdhsa" "-mcpu=gfx906" "-filetype=obj" 
"-o"{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-{{.*}}.o"
 // CHECK: lld{{.*}}"-flavor" "gnu" "--no-undefined" "-shared" 
"-o"{{.*}}amdgpu-openmp-toolchain-{{.*}}.out" 
"{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-{{.*}}.o"
@@ -71,3 +71,6 @@
 // CHECK-C: "x86_64-unknown-linux-gnu" - "clang"
 // CHECK-C: "x86_64-unknown-linux-gnu" - "clang::as"
 // CHECK-C: "x86_64-unknown-linux-gnu" - "offload bundler"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa 
-march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
+// CHECK-EMIT-LLVM-IR: clang{{.*}}"-cc1"{{.*}}"-triple" 
"amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4394,7 +4394,13 @@
   CmdArgs.push_back("-emit-llvm");
 } else if (JA.getType() == types::TY_LLVM_BC ||
JA.getType() == types::TY_LTO_BC) {
-  CmdArgs.push_back("-emit-llvm-bc");
+  // Emit textual llvm IR for AMDGPU offloading for -emit-llvm -S
+  if (Triple.isAMDGCN() && IsOpenMPDevice && Args.hasArg(options::OPT_S) &&
+  Args.hasArg(options::OPT_emit_llvm)) {
+CmdArgs.push_back("-emit-llvm");
+  } else {
+CmdArgs.push_back("-emit-llvm-bc");
+  }
 } else if (JA.getType() == types::TY_IFS ||
JA.getType() == types::TY_IFS_CPP) {
   StringRef ArgStr =


Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -6,7 +6,7 @@
 // verify the tools invocations
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-x" "c"{{.*}}
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-x" "ir"{{.*}}
-// CHECK: clang{{.*}}"-cc1"{{.*}}"-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx906" "-fcuda-is-device" "-mlink-builtin-bitcode"{{.*}}libomptarget-amdgcn-gfx906.bc"{{.*}}
+// CHECK: clang{{.*}}"-cc1"{{.*}}"-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm-bc"{{.*}}"-target-cpu" "gfx906" "-fcuda-is-device"{{.*}}"-mlink-builtin-bitcode"{{.*}}libomptarget-amdgcn-gfx906.bc"{{.*}}
 // CHECK: llvm-link{{.*}}"-o" "{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-linked-{{.*}}.bc"
 // CHECK: llc{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-linked-{{.*}}.bc" "-mtriple=amdgcn-amd-amdhsa" "-mcpu=gfx906" "-filetype=obj" "-o"{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-{{.*}}.o"
 // CHECK: lld{{.*}}"-flavor" "gnu" "--no-undefined" "-shared" "-o"{{.*}}amdgpu-openmp-toolchain-{{.*}}.out" "{{.*}}amdgpu-openmp-toolchain-{{.*}}-gfx906-{{.*}}.o"
@@ -71,3 +71,6 @@
 // CHECK-C: "x86_64-unknown-linux-gnu" - "clang"
 // CHECK-C: "x86_64-unknown-linux-gnu" - "clang::as"
 // CHECK-C: "x86_64-unknown-linux-gnu" - "offload bundler"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
+// CHECK-EMIT-LLVM-IR: clang{{.*}}"-cc1"{{.*}}"-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4394,7 +4394,13 @@
   CmdArgs.p

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal added a comment.

Tested on gfx906. The libomptarget tests are working as expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102065

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

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Seems reasonable.  I'd like to see a test for autoupgrade; not sure if you need 
to make any code changes for that.




Comment at: llvm/docs/LangRef.rst:13282
+to one of the ``__powi*`` functions in compiler-rt. Not all targets support
 all types however.
 

Maybe make this a bit more explicit: move the description of the exponent into 
a separate paragraph, and explicitly state "generally, the only exponent 
supported is the C type `int`".



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp:581
RTLIB::POWI_F128,
RTLIB::POWI_PPCF128);
   if (!TLI.getLibcallName(LC)) {

This is missing a diagnostic for the exponent.  We don't want to silently 
miscompile if someone uses an exponent that isn't supported by the target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99439

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

[PATCH] D92815: [PowerPC] [Clang] Enable float128 feature on VSX targets

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

Looks like this is causing failures at 
https://lab.llvm.org/buildbot/#/builders/76/builds/2422
Please revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92815

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

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

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

xbolva00 added a comment.

What about IR backward compatibility?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99439

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

[PATCH] D99439: Update @llvm.powi to handle different int sizes for the exponent

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

xbolva00 added inline comments.



Comment at: llvm/test/Transforms/InstCombine/pow_fp_int16.ll:3
 
-; PR42190
+; Test case was copied from pow_fp_int.ll but adjusted for 16-bit int.
+; Assuming that we can't generate test checks for the same reason (PR42740).

Precommit? And we dont need full copy of existings tests - 2-3 tests for 16bit 
int are anough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99439

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

[PATCH] D92815: [PowerPC] [Clang] Enable float128 feature on VSX targets

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

In D92815#2753684 , @nemanjai wrote:

> Looks like this is causing failures at 
> https://lab.llvm.org/buildbot/#/builders/76/builds/2422
> Please revert.

Yes. Already reverted in 
https://reviews.llvm.org/rGcbd93cee9bf014402a7405479ba21f6f3340a126.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92815

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:147
+/// where N = size(LHS), M = size(RHS)
+RangeSet unite(RangeSet Original, RangeSet RHS);
+/// Create a new set by uniting given range set with the given range.

`LHS`



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:247
 RangeSet intersect(const ContainerType &LHS, const ContainerType &RHS);
+/// NOTE: This function relies that all values in the containers are
+/// persistent (created via BasicValueFactory::getValue). User shall

nit: "...on the fact that..."



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:250
+/// guarantee this.
+ContainerType unite(const ContainerType &LHS, const ContainerType &RHS);
 

`ContainerType` is basically a mutable version of `RangeSet`, so there is only 
one reason to return it - you believe that the users might want to modify it 
after they called this `unite`.  But as long as this `unite` is just a 
generalized version of user-facing `unites, it can totally return `RangeSet`.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:112
+RangeSet RangeSet::Factory::add(RangeSet LHS, RangeSet RHS) {
+  ContainerType Result;
+  std::merge(LHS.begin(), LHS.end(), RHS.begin(), RHS.end(),

Let's reserve some place here.  Because `LHS` and `RHS` don't have 
intersections, the result always has `size(LHS) + size(RHS)` elements



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:221
+  Result.append(B, E);
+
+  return Result;

Oof, I don't know about this algorithm.  I mean it does its job.  But IMO it 
lacks a good description of what are the invariants and what are the different 
situations we are looking for.
Aaaand you kind of re-check certain conditions multiple times.  One example 
here is the check for `Min` and `Max`. Those situations are super rare, but we 
check for them on every single iteration. `std::min` and `std::max` are 
additional comparisons.  As I mentioned before, constant factor is the key here 
and less comparisons we do is way more important than doing binary search at 
some point.
Just make a benchmark if you don't believe me (with google-benchmark, for 
example).  The version with less comparisons will dominate one with more on 
`RangeSet` under 20 (and they'll be even smaller in practice).


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

https://reviews.llvm.org/D99797

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

[PATCH] D102313: [docs] Fix documentation for bugprone-dangling-handle

[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, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102313

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

[PATCH] D102303: [ASTMatchers] Fix formatting around forFunction().

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thanks for the cleanup!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D102303

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko Thanka for the suggestions! I'll take them into account and update 
the patch.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:250
+/// guarantee this.
+ContainerType unite(const ContainerType &LHS, const ContainerType &RHS);
 

vsavchenko wrote:
> `ContainerType` is basically a mutable version of `RangeSet`, so there is 
> only one reason to return it - you believe that the users might want to 
> modify it after they called this `unite`.  But as long as this `unite` is 
> just a generalized version of user-facing `unites, it can totally return 
> `RangeSet`.
I'm going to use raw ContainerType in further patches. So this is exactly what 
I want.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:221
+  Result.append(B, E);
+
+  return Result;

vsavchenko wrote:
> Oof, I don't know about this algorithm.  I mean it does its job.  But IMO it 
> lacks a good description of what are the invariants and what are the 
> different situations we are looking for.
> Aaaand you kind of re-check certain conditions multiple times.  One example 
> here is the check for `Min` and `Max`. Those situations are super rare, but 
> we check for them on every single iteration. `std::min` and `std::max` are 
> additional comparisons.  As I mentioned before, constant factor is the key 
> here and less comparisons we do is way more important than doing binary 
> search at some point.
> Just make a benchmark if you don't believe me (with google-benchmark, for 
> example).  The version with less comparisons will dominate one with more on 
> `RangeSet` under 20 (and they'll be even smaller in practice).
I'll investigate the whole algorithm once more and reduce comparisons.


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

https://reviews.llvm.org/D99797

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

[PATCH] D101519: [C++4OpenCL] Fix reinterpret_cast of vectors

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:2333
+  if (DestType->isExtVectorType() || SrcType->isExtVectorType()) {
+// FIXME: Allow for reinterpret cast between 3 and 4 element vectors
+if (Self.areVectorTypesSameSize(SrcType, DestType)) {

Just a small nit - add `.` at the end.



Comment at: clang/lib/Sema/SemaExpr.cpp:7347
 
+bool Sema::areVectorTypesSameSize(QualType srcTy, QualType destTy) {
+  assert(destTy->isVectorType() || srcTy->isVectorType());

Btw I understand that this function is copied over from the old code that has 
wrong formatting but since we are rewriting this now let's convert it to the 
current style:
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

In partucular variable name should start from the upper case letter. 



Comment at: clang/test/SemaOpenCLCXX/reinterpret-cast.clcpp:14
+
+  reserve_id_t r_id1;
+  auto r_id2 = reinterpret_cast(r_id1); // 
expected-error{{reinterpret_cast from 'reserve_id_t' to 'reserve_id_t' is not 
allowed}}

Let's add a comment here that it is testing casting OpenCL types to itself.

Btw I assume casting vector to the same vector type will work? Maybe we can add 
a test case for this too?

At the top of the function we can add a comment about teting the vector type.


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

https://reviews.llvm.org/D101519

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

By checking the line coverage of the `LoopUnrolling.cpp` test file, looks like 
all lines are covered you touched.

There are only two return statements uncovered though: L200, L251.
We should consider extending this test file to cover them as well in a 
follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[clang-tools-extra] 1633250 - [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

[Nathan James via cfe-commits]

Author: Hana Joo
Date: 2021-05-12T12:57:21+01:00
New Revision: 163325086c35b3984c5e6f7a2adb6022003fcd84

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

LOG: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init 
rule

The `IgnoreArray` flag was not used before while running the rule. Fixes [[ 
https://bugs.llvm.org/show_bug.cgi?id=47288 | b/47288 ]]

Reviewed By: njames93

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp

Modified: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
index f4468367ffa3..43812fe17a1c 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -402,6 +402,8 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
   forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+if (IgnoreArrays && F->getType()->isArrayType())
+  return;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
new file mode 100644
index ..1b526089b2fb
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-pro-type-member-init %t \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-pro-type-member-init.IgnoreArrays, value: 
true} ]}"
+
+typedef int TypedefArray[4];
+using UsingArray = int[4];
+
+struct HasArrayMember {
+  HasArrayMember() {}
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: 
Number
+  UsingArray U;
+  TypedefArray T;
+  int RawArray[4];
+  int Number;
+};



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

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG163325086c35: [clang-tidy] Enable the use of IgnoreArray 
flag in pro-type-member-init rule (authored by h-joo, committed by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101239

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-pro-type-member-init %t \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-pro-type-member-init.IgnoreArrays, value: 
true} ]}"
+
+typedef int TypedefArray[4];
+using UsingArray = int[4];
+
+struct HasArrayMember {
+  HasArrayMember() {}
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: 
Number
+  UsingArray U;
+  TypedefArray T;
+  int RawArray[4];
+  int Number;
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -402,6 +402,8 @@
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
   forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+if (IgnoreArrays && F->getType()->isArrayType())
+  return;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-pro-type-member-init %t \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-pro-type-member-init.IgnoreArrays, value: true} ]}"
+
+typedef int TypedefArray[4];
+using UsingArray = int[4];
+
+struct HasArrayMember {
+  HasArrayMember() {}
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: Number
+  UsingArray U;
+  TypedefArray T;
+  int RawArray[4];
+  int Number;
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -402,6 +402,8 @@
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
   forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+if (IgnoreArrays && F->getType()->isArrayType())
+  return;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102216: [Clang][docs]JSONLinkDatabase

[Mirzazyanov Gleb via Phabricator via cfe-commits]

Glebuska updated this revision to Diff 344773.
Glebuska added a comment.

Updating D102216 : 
[Clang][docs]JSONLinkDatabase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102216

Files:
  clang/docs/JSONLinkDatabase.rst

Index: clang/docs/JSONLinkDatabase.rst
===
--- /dev/null
+++ clang/docs/JSONLinkDatabase.rst
@@ -0,0 +1,142 @@
+==
+JSON Compilation Database Format Specification
+==
+
+This document describes a format for specifying how to replay single link commands independently of the build system.
+
+Background
+==
+
+Currently, there is `compilation database format `_ , which specified by Clang. Compilation commands are actively used for syntax analysis of code, 
+but there are cases, when the compilation commands cannot provide all the necessary information, for example, look into simple C/C++ code:  
+
+ ::
+
+a.cpp:
+
+int f(int a) {
+return g(a);
+ }
+
+x.cpp:
+
+int g(int a) {
+  return a + 1;
+}
+
+y.cpp:
+
+int g(int a) {
+  return a - 1;
+}
+
+
+A tool may need to know which function is actually called, but this information is only available at linking step as soon as there may be several functions with same name in different units.  
+Using of link database is providing more information for advance software analysis.
+More examples:
+
+1) Unit testing tools which use code analysis need to understand which function is called in order to get correct test result.
+2) IDEs need it to understand which file to go when you click "Go to definition" on a function call.
+
+Format
+
+
+A link database is a JSON file, which consist of an array of “command objects”, where each command object specifies one way a target. 
+The contracts for each field in the command object are:
+
+- **command**: The link command executed. The field contains all the flags and files needed to create a target or library. As result of executing all commands, it's a full-build project.
+- **directory** The working directory of the link's command. All paths specified in the command or file fields must be either absolute or relative to this directory.
+- **files** An array which described all the object files, dynamic and static libraries needed for linking.
+- **arguments** The link command executed as list of strings. Either arguments or command is required.
+- **output** The name of the output created by this linking step. This field is optional. It can be used to distinguish different processing modes of the same input.
+
+Using link commands from file can give 3 different types of output:  
+- executable file 
+- static library
+- dynamic library
+
+Example:
+
+ :: 
+
+
+[   
+	{
+	  "command" : "/usr/bin/ar qc libcmstd.a CMakeFiles/cmstd.dir/cm/bits/fs_path.cxx.o CMakeFiles/cmstd.dir/cm/bits/string_view.cxx.o",
+  "directory" : "/home/myuser/exampleProject/build/Utilities/std",
+	  "files" : 
+	  [
+	  "/home/myuser/exampleProject/build/Utilities/std/CMakeFiles/cmstd.dir/cm/bits/fs_path.cxx.o", 
+	  "/home/myuser/exampleProject/build/Utilities/std/CMakeFiles/cmstd.dir/cm/bits/string_view.cxx.o"
+	  ]
+	},
+	{
+	  "command" : "/usr/bin/cc  -Wcast-align -Werror-implicit-function-declaration -Wchar-subscripts -Wall -W -Wpointer-arith -Wwrite-strings -Wformat-security -Wmissing-format-attribute -fno-common -Wundef CMakeFiles/pseudonl_purify.dir/ret0.c.o -o purify",
+  "directory" : "/home/myuser/exampleProject/build/Tests/CMakeLib/PseudoMemcheck/NoLog",
+	  "files" : 
+	  [
+  "/home/myuser/exampleProject/build/Tests/CMakeLib/PseudoMemcheck/NoLog/CMakeFiles/pseudonl_purify.dir/ret0.c.o"
+	  ]
+	},
+	...
+]
+
+
+
+
+
+
+This example demonstrates command **ar** to create **dynamic library libcmstd.a** and **cc** command to create **purify target**  
+
+In addition to the commands responsible for linking, link database can contain commands specific to the build system:
+Example for CMake build system:
+
+ 1) CMake command for deleting library
+
+ ::
+
+[   
+...
+	{
+	  "command" : "/usr/local/bin/cmake -E rm -f CMakeFiles/library.dir/lib.a",
+  "directory" : "/home/myuser/exampleProject/build/Utilities/std",
+	  "files" : 
+	  [
+	  "/home/myuser/exampleP

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/AST/StmtCXX.h:356-359
 Expr *Allocate = nullptr;
 Expr *Deallocate = nullptr;
+Expr *AlignedAllocate = nullptr;
+Expr *AlignedDeallocate = nullptr;

Can't we merge these?



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:445-483
+void emitDynamicAlignedDealloc(CodeGenFunction &CGF,
+   llvm::BasicBlock *AlignedFreeBB,
+   llvm::CallInst *CoroFree) {
+  llvm::CallInst *Dealloc = nullptr;
+  for (llvm::User *U : CoroFree->users()) {
+if (auto *CI = dyn_cast(U))
+  if (CI->getParent() == CGF.Builder.GetInsertBlock())

This code would only work if we use `::operator new(size_t, align_val_t)`, 
which is implemented in another patch. I would suggest to move this into that 
one.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:528-547
 CGF.EmitBlock(FreeBB);
 CGF.EmitStmt(Deallocate);
-
-auto *AfterFreeBB = CGF.createBasicBlock("after.coro.free");
-CGF.EmitBlock(AfterFreeBB);
+CGF.Builder.CreateBr(AfterFreeBB);
 
 // We should have captured coro.free from the emission of deallocate.
 auto *CoroFree = CGF.CurCoro.Data->LastCoroFree;
+CGF.CurCoro.Data->LastCoroFreeUsedForDealloc = true;

It looks like it would emit a `deallocate` first, and emit an 
`alignedDeallocate`, which is very odd. Although I can find that the second 
`deallocate` wouldn't be emitted due to the check `LastCoroFreeUsedForDealloc`, 
it is still very odd to me. If the second `deallocate` wouldn't come up all the 
way, what's the reason we need to write `emit(deallocate)` twice?



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:700
+  auto *AlignedAllocateCall = EmitScalarExpr(S.getAlignedAllocate());
+  bool HasAlignArg = hasAlignArg(cast(AlignedAllocateCall));
+

Since `hasAlignArg` is called only once, I suggested to make it a lambda here 
which would make the code more easy to read.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:702-704
+  if (!HasAlignArg)
+overAllocateFrame(*this, cast(AlignedAllocateCall),
+  /*IsAlloc*/ true);

I recommend to add a detailed comment here to tell the story why we need to 
over allocate the frame. It is really hard to understand for people who are new 
to this code. Otherwise, I think they need to use `git blame` to find the 
commit id and this review page to figure the reasons out.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:706-728
+  if (auto *RetOnAllocFailure = S.getReturnStmtOnAllocFailure()) {
+auto *Cond = Builder.CreateICmpNE(AlignedAllocateCall, NullPtr);
+if (HasAlignArg) {
+  Builder.CreateCondBr(Cond, InitBB, RetOnFailureBB);
+} else {
+  AlignAllocBB2 = createBasicBlock("coro.alloc.align2");
+  Builder.CreateCondBr(Cond, AlignAllocBB2, RetOnFailureBB);

It may be better to organize it as:
```
if (!HasAlignArg) {
   if (auto *RetOnAllocFailure = S.getReturnStmtOnAllocFailure()) {
   auto *Cond = Builder.CreateICmpNE(AlignedAllocateCall, NullPtr);
   AlignAllocBB2 = createBasicBlock("coro.alloc.align2");
   Builder.CreateCondBr(Cond, AlignAllocBB2, RetOnFailureBB);
   EmitBlock(AlignAllocBB2);
   }
   auto *CoroAlign = Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::coro_align, SizeTy));
   ...
}
```



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:878
+if (CurCoro.Data && CurCoro.Data->LastCoroFreeUsedForDealloc)
+  return RValue::get(CurCoro.Data->LastCoroFree);
+

Is it possible that it would return a nullptr value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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

[PATCH] D102322: [clang-tidy] Add '-target' CLI option to override target triple

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

This change doesn't seem to accomplish anything. We can already set the target 
via the compile command.
This can be done in the compilation database directly or by using the 
`extra-arg` and `extra-arg-before` command line arguments.
If you're not using a compilation database, then you can pass the compile 
command to clang-tidy after an empty `--` argument.

For the test case, this is sufficient to fix the issue:

  // RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t -- 
--extra-arg=--target=x86_64-windows


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

https://reviews.llvm.org/D102322

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

[PATCH] D102248: [C++4OpenCL] Fix initialization of __constant constructors without arguments

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12643
+  if (CXXRecordDecl *RD = Var->getType()->getAsCXXRecordDecl()) {
+for (auto *ctor : RD->ctors()) {
+  if (ctor->isConstexpr() && ctor->getNumParams() == 0 &&

ctor -> Ctor



Comment at: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp:20
+kernel void k() {
+  __constant X cx1;
+  __constant X cx2(1);

Btw, even though your change doesn't modify this I would suggest to add a 
CodeGen test that checks the initialization of objects in IR is adequate.



Comment at: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp:36
+  Z() __local = default;
+  Z() __global = default;
+  constexpr Z() __constant : z(0) {}

We seem to be missing the coverage with __constant and `= default`.



Comment at: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp:42
+  int w;
+  constexpr W() __constant = default; // expected-error {{defaulted definition 
of default constructor is not constexpr}}
+};

I find the wording of the error a bit odd, but it is standard C++ though... 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102248

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

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D102213#2753388 , @vsavchenko 
wrote:

> I honestly don't see a reason why it's not a part of `forFunction`.  
> `forFunction` matches C++ methods and lambdas, Obj-C methods and blocks don't 
> seem that much more special to have an extended matcher just for those.
> I really don't think that it will break someone's workflow and that someone 
> really relied on the fact that `forFunction` doesn't match those.

I think this is a good observation -- if you modify `forFunction()`, do you see 
any test failures that look unexpected when running all the clang-tidy tests?


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

https://reviews.llvm.org/D102213

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

[clang-tools-extra] 4c59ab3 - [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-05-12T13:18:41+01:00
New Revision: 4c59ab34f7bda74296e42ef7ea8d83828cb45558

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

LOG: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers

There should be a follow up to this for changing the traversal mode, but some 
of the tests don't like that.

Reviewed By: steveire

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 8a6f7da36196..44a19b1f824a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -90,11 +90,8 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
   const auto IntegerCallExpr = ignoringParenImpCasts(
   callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
unless(isInTemplateInstantiation(;
-  const auto SizeOfExpr = expr(anyOf(
-  sizeOfExpr(
-  has(hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type",
-  sizeOfExpr(has(expr(hasType(
-  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
+  const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
+  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
   const auto SizeOfZero =
   sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0);
 
@@ -184,9 +181,8 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
 Finder->addMatcher(
 binaryOperator(matchers::isRelationalOperator(),
hasOperands(ignoringParenImpCasts(SizeOfExpr),
-   ignoringParenImpCasts(anyOf(
-   integerLiteral(equals(0)),
-   
integerLiteral(isBiggerThan(0x8))
+   ignoringParenImpCasts(integerLiteral(anyOf(
+   equals(0), isBiggerThan(0x8))
 .bind("sizeof-compare-constant"),
 this);
   }
@@ -207,18 +203,15 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
   const auto ElemType =
   arrayType(hasElementType(recordType().bind("elem-type")));
   const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
-  const auto NumType = hasCanonicalType(
-  type(anyOf(ElemType, ElemPtrType, type())).bind("num-type"));
-  const auto DenomType = hasCanonicalType(type().bind("denom-type"));
 
   Finder->addMatcher(
-  binaryOperator(hasOperatorName("/"),
- hasLHS(expr(ignoringParenImpCasts(
- anyOf(sizeOfExpr(has(NumType)),
-   sizeOfExpr(has(expr(hasType(NumType,
- hasRHS(expr(ignoringParenImpCasts(
- anyOf(sizeOfExpr(has(DenomType)),
-   sizeOfExpr(has(expr(hasType(DenomType)
+  binaryOperator(
+  hasOperatorName("/"),
+  hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
+  hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
+   .bind("num-type")),
+  hasRHS(ignoringParenImpCasts(sizeOfExpr(
+  
hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))
   .bind("sizeof-divide-expr"),
   this);
 



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

[PATCH] D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c59ab34f7bd: [clang-tidy][NFC] Simplify a lot of 
bugprone-sizeof-expression matchers (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101614

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp


Index: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -90,11 +90,8 @@
   const auto IntegerCallExpr = ignoringParenImpCasts(
   callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
unless(isInTemplateInstantiation(;
-  const auto SizeOfExpr = expr(anyOf(
-  sizeOfExpr(
-  has(hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type",
-  sizeOfExpr(has(expr(hasType(
-  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
+  const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
+  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
   const auto SizeOfZero =
   sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0);
 
@@ -184,9 +181,8 @@
 Finder->addMatcher(
 binaryOperator(matchers::isRelationalOperator(),
hasOperands(ignoringParenImpCasts(SizeOfExpr),
-   ignoringParenImpCasts(anyOf(
-   integerLiteral(equals(0)),
-   
integerLiteral(isBiggerThan(0x8))
+   ignoringParenImpCasts(integerLiteral(anyOf(
+   equals(0), isBiggerThan(0x8))
 .bind("sizeof-compare-constant"),
 this);
   }
@@ -207,18 +203,15 @@
   const auto ElemType =
   arrayType(hasElementType(recordType().bind("elem-type")));
   const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
-  const auto NumType = hasCanonicalType(
-  type(anyOf(ElemType, ElemPtrType, type())).bind("num-type"));
-  const auto DenomType = hasCanonicalType(type().bind("denom-type"));
 
   Finder->addMatcher(
-  binaryOperator(hasOperatorName("/"),
- hasLHS(expr(ignoringParenImpCasts(
- anyOf(sizeOfExpr(has(NumType)),
-   sizeOfExpr(has(expr(hasType(NumType,
- hasRHS(expr(ignoringParenImpCasts(
- anyOf(sizeOfExpr(has(DenomType)),
-   sizeOfExpr(has(expr(hasType(DenomType)
+  binaryOperator(
+  hasOperatorName("/"),
+  hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
+  hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
+   .bind("num-type")),
+  hasRHS(ignoringParenImpCasts(sizeOfExpr(
+  
hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))
   .bind("sizeof-divide-expr"),
   this);
 


Index: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -90,11 +90,8 @@
   const auto IntegerCallExpr = ignoringParenImpCasts(
   callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
unless(isInTemplateInstantiation(;
-  const auto SizeOfExpr = expr(anyOf(
-  sizeOfExpr(
-  has(hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type",
-  sizeOfExpr(has(expr(hasType(
-  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
+  const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
+  hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type";
   const auto SizeOfZero =
   sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0);
 
@@ -184,9 +181,8 @@
 Finder->addMatcher(
 binaryOperator(matchers::isRelationalOperator(),
hasOperands(ignoringParenImpCasts(SizeOfExpr),
-   ignoringParenImpCasts(anyOf(
-   integerLiteral(equals(0)),
-   integerLiteral(isBiggerThan(0x8))
+   ignoringParenImpCasts(integerLiteral(anyOf(
+   equals(0), isBiggerThan(0x8))
 .bind("sizeof-compare-constant"),
 this);
   }
@@ -207,18 +203,15 @@
   const auto ElemType =
   arrayType(hasElementType(recordType().bind("elem-type")));
   const auto ElemPtrType = pointerType(pointee(typ

[PATCH] D102306: Add gfx1034

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

Can you also check for updates in:
clang/lib/Basic/Cuda.cpp
openmp/libomptarget/plugins/amdgpu/impl/get_elf_mach_gfx_name.cpp
llvm/test/CodeGen/AMDGPU/directive-amdgcn-target.ll
llvm/test/tools/llvm-objdump/ELF/AMDGPU/subtarget.ll
llvm/test/tools/llvm-readobj/ELF/amdgpu-elf-headers.test




Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:4489
   case CudaArch::GFX1033:
+  case CudaArch::GFX1034:
   case CudaArch::UNUSED:

I think lint is complaining about trailing whitespace here -- please check the 
whole patch for it.



Comment at: llvm/test/Object/AMDGPU/elf-header-flags-mach.yaml:334
 
+# ELF-AMDGCN-GFX1034:   EF_AMDGPU_MACH_AMDGCN_GFX1034 (0x3E)
+# YAML-AMDGCN-GFX1034:  Flags: [ EF_AMDGPU_MACH_AMDGCN_GFX1034 ]

You also need to add RUN lines at the top of this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102306

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

dblaikie wrote:
> hoy wrote:
> > dblaikie wrote:
> > > hoy wrote:
> > > > dblaikie wrote:
> > > > > aaron.ballman wrote:
> > > > > > dblaikie wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > dblaikie wrote:
> > > > > > > > > hoy wrote:
> > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > > Does this need to be down here? 
> > > > > > > > > > > > > > > > > > > > > > > > Or would the code be a well 
> > > > > > > > > > > > > > > > > > > > > > > > exercised if it was up next to 
> > > > > > > > > > > > > > > > > > > > > > > > the go declaration above?
> > > > > > > > > > > > > > > > > > > > > > > Yes, it needs to be here. 
> > > > > > > > > > > > > > > > > > > > > > > Otherwise it will just like the 
> > > > > > > > > > > > > > > > > > > > > > > function `bar` above that doesn't 
> > > > > > > > > > > > > > > > > > > > > > > get a uniquefied name. I think 
> > > > > > > > > > > > > > > > > > > > > > > moving the definition up to right 
> > > > > > > > > > > > > > > > > > > > > > > after the declaration hides the 
> > > > > > > > > > > > > > > > > > > > > > > declaration.
> > > > > > > > > > > > > > > > > > > > > > Not sure I follow - do you mean 
> > > > > > > > > > > > > > > > > > > > > > that if the go declaration and go 
> > > > > > > > > > > > > > > > > > > > > > definition were next to each other, 
> > > > > > > > > > > > > > > > > > > > > > this test would (mechanically 
> > > > > > > > > > > > > > > > > > > > > > speaking) not validate what the 
> > > > > > > > > > > > > > > > > > > > > > patch? Or that it would be less 
> > > > > > > > > > > > > > > > > > > > > > legible, but still mechanically 
> > > > > > > > > > > > > > > > > > > > > > correct?
> > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > I think it would be (assuming it's 
> > > > > > > > > > > > > > > > > > > > > > still mechanically correct) more 
> > > > > > > > > > > > > > > > > > > > > > legible to put the declaration next 
> > > > > > > > > > > > > > > > > > > > > > to the definition - the comment 
> > > > > > > > > > > > > > > > > > > > > > describes why the declaration is 
> > > > > > > > > > > > > > > > > > > > > > significant/why the definition is 
> > > > > > > > > > > > > > > > > > > > > > weird, and seeing all that together 
> > > > > > > > > > > > > > > > > > > > > > would be clearer to me than 
> > > > > > > > > > > > > > > > > > > > > > spreading it out/having to look 
> > > > > > > > > > > > > > > > > > > > > > further away to see what's going on.
> > > > > > > > > > > > > > > > > > > > > When the `go` declaration and `go` 
> > > > > > > > > > > > > > > > > > > > > definition were next to each other, 
> > > > > > > > > > > > > > > > > > > > > the go function won't get a uniqufied 
> > > > > > > > > > > > > > > > > > > > > name at all. The declaration will be 
> > > > > > > > > > > > > > > > > > > > > overwritten by the definition. Only 
> > > > > > > > > > > > > > > > > > > > > when the declaration is seen by 
> > > > > > > > > > > > > > > > > > > > > others, such the callsite in `baz`, 
> > > > > > > > > > > > > > > > > > > > > the declaration makes a difference by 
> > > > > > > > > > > > > > > > > > > > > having the callsite use a uniqufied 
> > > > > > > > > > > > > > > > > > > > > name.
> > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > Ah! Interesting, good to know. 
> > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > Is that worth supporting, I wonder? I 
> > > > > > > > > > > > > > > > > > > > guess it falls out for free/without 
> > > > > > > > > > > > > > > > > > > > significant additional complexity. I 
> > > > > > > > > > > > > > > > > > > > worry about the subtlety of the 
> > > > > > > > > > > > > > > > > > > > additional declaration changing the 
> > > > > > > > > > > > > > > > > > > > behavior here... might be a bit 
> > > > > > > > > > > > > > > > > > > > surprising/subtle. But maybe no nice 
> > > > > > > > > > > > > > > > > > > > way to avoid it either.
> > > > > > > > > > > > > > > > > > > It would be ideal if user never 

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:250
+/// guarantee this.
+ContainerType unite(const ContainerType &LHS, const ContainerType &RHS);
 

ASDenysPetrov wrote:
> vsavchenko wrote:
> > `ContainerType` is basically a mutable version of `RangeSet`, so there is 
> > only one reason to return it - you believe that the users might want to 
> > modify it after they called this `unite`.  But as long as this `unite` is 
> > just a generalized version of user-facing `unites, it can totally return 
> > `RangeSet`.
> I'm going to use raw ContainerType in further patches. So this is exactly 
> what I want.
Oh, I see. OK then :)


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

https://reviews.llvm.org/D99797

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

[PATCH] D100976: [OpenCL] Simplify use of C11 atomic types

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 344781.
Anastasia added a comment.

Added  a comment in the test.


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

https://reviews.llvm.org/D100976

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/Parser/opencl-atomics-cl20.cl

Index: clang/test/Parser/opencl-atomics-cl20.cl
===
--- clang/test/Parser/opencl-atomics-cl20.cl
+++ clang/test/Parser/opencl-atomics-cl20.cl
@@ -1,66 +1,78 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20
-// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -DCL20 -DEXT -Wpedantic-core-features
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -cl-ext=-cl_khr_int64_base_atomics
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CLC++
+// RUN: %clang_cc1 %s -triple spir64-unknown-unknown -verify -fsyntax-only -cl-std=CL2.0 -cl-ext=-cl_khr_int64_base_atomics
 
-#ifdef EXT
-#pragma OPENCL EXTENSION cl_khr_int64_base_atomics:enable
-#pragma OPENCL EXTENSION cl_khr_int64_extended_atomics:enable
-#pragma OPENCL EXTENSION cl_khr_fp64:enable
-#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
-// expected-warning@-2{{OpenCL extension 'cl_khr_fp64' is core feature or supported optional core feature - ignoring}}
-#endif
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#define LANG_VER_OK
 #endif
 
 void atomic_types_test() {
 // OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+
+// Non-optional types
   atomic_int i;
   atomic_uint ui;
+  atomic_float f;
+  atomic_flag fl;
+#if !defined(LANG_VER_OK)
+// expected-error@-5 {{use of undeclared identifier 'atomic_int'}}
+// expected-error@-5 {{use of undeclared identifier 'atomic_uint'}}
+// expected-error@-5 {{use of undeclared identifier 'atomic_float'}}
+// expected-error@-5 {{use of undeclared identifier 'atomic_flag'}}
+#endif
+
+// Optional types
   atomic_long l;
   atomic_ulong ul;
-  atomic_float f;
   atomic_double d;
-  atomic_flag fl;
+  atomic_size_t s;
   atomic_intptr_t ip;
   atomic_uintptr_t uip;
-  atomic_size_t s;
   atomic_ptrdiff_t pd;
+// Optional type identifiers are not added in earlier version or if at least
+// one of the extensions is not supported. Here we check with
+// `cl_khr_int64_base_atomics` only.
+#if !defined(LANG_VER_OK) || !defined(cl_khr_int64_base_atomics)
+// expected-error@-8 {{use of undeclared identifier 'atomic_long'}}
+// expected-error@-8 {{use of undeclared identifier 'atomic_ulong'}}
+// expected-error@-8 {{use of undeclared identifier 'atomic_double'}}
+#if defined(LANG_VER_OK)
+// expected-error@-12 {{expected ';' after expression}}
+// expected-error@-13 {{use of undeclared identifier 'l'}}
+// expected-error@-13 {{expected ';' after expression}}
+// expected-error@-14 {{use of undeclared identifier 'ul'}}
+#endif
+#if !defined(LANG_VER_OK) || defined(__SPIR64__)
+// expected-error@-15 {{use of undeclared identifier 'atomic_size_t'}}
+// expected-error@-13 {{use of undeclared identifier 'atomic_ptrdiff_t'}}
+#if !defined(LANG_VER_OK)
+// expected-error@-17 {{use of undeclared identifier 'atomic_intptr_t'}}
+// expected-error@-17 {{use of undeclared identifier 'atomic_uintptr_t'}}
+#else
+// expected-error@-21 {{expected ';' after expression}}
+// expected-error@-22 {{use of undeclared identifier 's'}}
+// expected-error@-22 {{unknown type name 'atomic_intptr_t'; did you mean 'atomic_int'?}}
+// expected-note@* {{'atomic_int' declared here}}
+// expected-error@-23 {{unknown type name 'atomic_uintptr_t'; did you mean 'atomic_uint'?}}
+// expected-note@* {{'atomic_uint' declared here}}
+#endif
+#endif
+#endif
+
 // OpenCL v2.0 s6.13.11.8, _Atomic type specifier and _Atomic type qualifier
 // are not supported by OpenCL.
-  _Atomic int i; // expected-error {{use of undeclared identifier '_Atomic'}}
-}
-#ifndef CL20
-// expected-error@-16 {{use of undeclared identifier 'atomic_int'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_uint'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_long'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_ulong'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_float'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_double'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_flag'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_intptr_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_uintptr_t'}}
-// expected-error@-16 {{use of undeclared identifier 'atomic_size_t'}}
-// expected-er

[PATCH] D100976: [OpenCL] Simplify use of C11 atomic types

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: clang/test/Parser/opencl-atomics-cl20.cl:12
 void atomic_types_test() {
-// OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+  // OpenCL v2.0 s6.13.11.6 defines supported atomic types.
+

mantognini wrote:
> nit: there are a few tabs in this files you may want to remove before 
> submitting.
There were no tabs or anything weird in the test, but now that I have 
re-created the patch it doesn't show anything. I wonder why it was showing 
something earlier...


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

https://reviews.llvm.org/D100976

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

[PATCH] D101843: [OpenCL] Add clang extension for bitfields

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 344784.
Anastasia added a comment.

Added suggestions from Sven.


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

https://reviews.llvm.org/D101843

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Misc/amdgcn.languageOptsOpenCL.cl
  clang/test/Misc/nvptx.languageOptsOpenCL.cl
  clang/test/Misc/r600.languageOptsOpenCL.cl
  clang/test/SemaOpenCL/unsupported.cl

Index: clang/test/SemaOpenCL/unsupported.cl
===
--- clang/test/SemaOpenCL/unsupported.cl
+++ clang/test/SemaOpenCL/unsupported.cl
@@ -1,7 +1,15 @@
 // RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -DBITFIELDS_EXT
 
-struct {
-  int a : 1; // expected-error {{bit-fields are not supported in OpenCL}}
+#ifdef BITFIELDS_EXT
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+#endif
+
+struct test {
+  int a : 1;
+#ifndef BITFIELDS_EXT
+// expected-error@-2 {{bit-fields are not supported in OpenCL}}
+#endif
 };
 
 void no_vla(int n) {
Index: clang/test/Misc/r600.languageOptsOpenCL.cl
===
--- clang/test/Misc/r600.languageOptsOpenCL.cl
+++ clang/test/Misc/r600.languageOptsOpenCL.cl
@@ -34,6 +34,11 @@
 #endif
 #pragma OPENCL EXTENSION __cl_clang_variadic_functions : enable
 
+#ifndef __cl_clang_bitfields
+#error "Missing __cl_clang_bitfields define"
+#endif
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+
 #ifdef cl_khr_fp16
 #error "Incorrect cl_khr_fp16 define"
 #endif
Index: clang/test/Misc/nvptx.languageOptsOpenCL.cl
===
--- clang/test/Misc/nvptx.languageOptsOpenCL.cl
+++ clang/test/Misc/nvptx.languageOptsOpenCL.cl
@@ -28,6 +28,11 @@
 #endif
 #pragma OPENCL EXTENSION __cl_clang_variadic_functions : enable
 
+#ifndef __cl_clang_bitfields
+#error "Missing __cl_clang_bitfields define"
+#endif
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+
 #ifdef cl_khr_fp16
 #error "Incorrect cl_khr_fp16 define"
 #endif
Index: clang/test/Misc/amdgcn.languageOptsOpenCL.cl
===
--- clang/test/Misc/amdgcn.languageOptsOpenCL.cl
+++ clang/test/Misc/amdgcn.languageOptsOpenCL.cl
@@ -24,6 +24,11 @@
 #endif
 #pragma OPENCL EXTENSION __cl_clang_variadic_functions : enable
 
+#ifndef __cl_clang_bitfields
+#error "Missing __cl_clang_bitfields define"
+#endif
+#pragma OPENCL EXTENSION __cl_clang_bitfields : enable
+
 #ifndef cl_khr_fp16
 #error "Missing cl_khr_fp16 define"
 #endif
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16759,8 +16759,10 @@
   Record->setInvalidDecl();
   InvalidDecl = true;
 }
-// OpenCL v1.2 s6.9.c: bitfields are not supported.
-if (BitWidth) {
+// OpenCL v1.2 s6.9.c: bitfields are not supported, unless Clang extension
+// is enabled.
+if (BitWidth && !getOpenCLOptions().isAvailableOption(
+"__cl_clang_bitfields", LangOpts)) {
   Diag(Loc, diag::err_opencl_bitfields);
   InvalidDecl = true;
 }
Index: clang/lib/Basic/Targets/NVPTX.h
===
--- clang/lib/Basic/Targets/NVPTX.h
+++ clang/lib/Basic/Targets/NVPTX.h
@@ -130,6 +130,7 @@
 Opts["cl_clang_storage_class_specifiers"] = true;
 Opts["__cl_clang_function_pointers"] = true;
 Opts["__cl_clang_variadic_functions"] = true;
+Opts["__cl_clang_bitfields"] = true;
 
 Opts["cl_khr_fp64"] = true;
 Opts["cl_khr_byte_addressable_store"] = true;
Index: clang/lib/Basic/Targets/AMDGPU.h
===
--- clang/lib/Basic/Targets/AMDGPU.h
+++ clang/lib/Basic/Targets/AMDGPU.h
@@ -287,6 +287,7 @@
 Opts["cl_clang_storage_class_specifiers"] = true;
 Opts["__cl_clang_variadic_functions"] = true;
 Opts["__cl_clang_function_pointers"] = true;
+Opts["__cl_clang_bitfields"] = true;
 
 bool IsAMDGCN = isAMDGCN(getTriple());
 
Index: clang/include/clang/Basic/OpenCLExtensions.def
===
--- clang/include/clang/Basic/OpenCLExtensions.def
+++ clang/include/clang/Basic/OpenCLExtensions.def
@@ -87,6 +87,7 @@
 OPENCL_EXTENSION(cl_clang_storage_class_specifiers, true, 100)
 OPENCL_EXTENSION(__cl_clang_function_pointers, true, 100)
 OPENCL_EXTENSION(__cl_clang_variadic_functions, true, 100)
+OPENCL_EXTENSION(__cl_clang_bitfields, true, 100)
 
 // AMD OpenCL extensions
 OPENCL_EXTENSION(cl_amd_media_ops, true, 100)
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/Languag

[PATCH] D101843: [OpenCL] Add clang extension for bitfields

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:16798
 }
 // OpenCL v1.2 s6.9.c: bitfields are not supported.
+if (BitWidth && !getOpenCLOptions().isAvailableOption(

svenvh wrote:
> 
Slightly modified to avoid repeating the extension name that can be read from 
the source code.


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

https://reviews.llvm.org/D101843

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

[PATCH] D102064: Parse vector bool when stdbool.h and altivec.h are included

[Zarko Todorovski via Phabricator via cfe-commits]

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

- Rename test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102064

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/altivec-zvector-bool.c

Index: clang/test/Parser/altivec-zvector-bool.c
===
--- /dev/null
+++ clang/test/Parser/altivec-zvector-bool.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff \
+// RUN:-target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff \
+// RUN:-target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=powerpc-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only %s
+// RUN: %clang_cc1 -triple=s390x-linux-gnu -target-cpu arch11 \
+// RUN:-fzvector -fsyntax-only %s
+// RUN: %clang_cc1 -triple=s390x-ibm-zos -target-cpu arch11 \
+// RUN:-fzvector -fsyntax-only %s
+
+__vector bool char bc;
+__vector bool short bsh;
+__vector bool short int bshi;
+__vector bool int bi;
+__vector _Bool char Bc;
+__vector _Bool short Bsh;
+__vector _Bool short int Bshi;
+__vector _Bool int Bi;
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -503,10 +503,12 @@
 
   Ident_vector = nullptr;
   Ident_bool = nullptr;
+  Ident_Bool = nullptr;
   Ident_pixel = nullptr;
   if (getLangOpts().AltiVec || getLangOpts().ZVector) {
 Ident_vector = &PP.getIdentifierTable().get("vector");
 Ident_bool = &PP.getIdentifierTable().get("bool");
+Ident_Bool = &PP.getIdentifierTable().get("_Bool");
   }
   if (getLangOpts().AltiVec)
 Ident_pixel = &PP.getIdentifierTable().get("pixel");
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -7334,6 +7334,7 @@
   case tok::kw_float:
   case tok::kw_double:
   case tok::kw_bool:
+  case tok::kw__Bool:
   case tok::kw___bool:
   case tok::kw___pixel:
 Tok.setKind(tok::kw___vector);
@@ -7343,7 +7344,8 @@
   Tok.setKind(tok::kw___vector);
   return true;
 }
-if (Next.getIdentifierInfo() == Ident_bool) {
+if (Next.getIdentifierInfo() == Ident_bool ||
+Next.getIdentifierInfo() == Ident_Bool) {
   Tok.setKind(tok::kw___vector);
   return true;
 }
@@ -7368,6 +7370,7 @@
 case tok::kw_float:
 case tok::kw_double:
 case tok::kw_bool:
+case tok::kw__Bool:
 case tok::kw___bool:
 case tok::kw___pixel:
   isInvalid = DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID, Policy);
@@ -7377,8 +7380,10 @@
 isInvalid = DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID,Policy);
 return true;
   }
-  if (Next.getIdentifierInfo() == Ident_bool) {
-isInvalid = DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID,Policy);
+  if (Next.getIdentifierInfo() == Ident_bool ||
+  Next.getIdentifierInfo() == Ident_Bool) {
+isInvalid =
+DS.SetTypeAltiVecVector(true, Loc, PrevSpec, DiagID, Policy);
 return true;
   }
   break;
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -118,10 +118,12 @@
   /// Ident_super - IdentifierInfo for "super", to support fast
   /// comparison.
   IdentifierInfo *Ident_super;
-  /// Ident_vector, Ident_bool - cached IdentifierInfos for "vector" and
-  /// "bool" fast comparison.  Only present if AltiVec or ZVector are enabled.
+  /// Ident_vector, Ident_bool, Ident_Bool - cached IdentifierInfos for "vector"
+  /// and "bool" fast comparison.  Only present if AltiVec or ZVector are
+  /// enabled.
   IdentifierInfo *Ident_vector;
   IdentifierInfo *Ident_bool;
+  IdentifierInfo *Ident_Bool;
   /// Ident_pixel - cached IdentifierInfos for "pixel" fast comparison.
   /// Only present if AltiVec enabled.
   IdentifierInfo *Ident_pixel;
@@ -879,6 +881,7 @@
 
 if (Tok.getIdentifierInfo() != Ident_vector &&
 Tok.getIdentifierInfo() != Ident_bool &&
+Tok.getIdentifierInfo() != Ident_Bool &&
 (!getLangOpts().AltiVec || Tok.getIdentifierInfo() != Ident_pixel))
   return false;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102251: Suppress Deferred Diagnostics in discarded statements.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/Stmt.cpp:1000-1003
+  if (Optional Result =
+  const_cast(this)->getNondiscardedCase(Ctx))
+return *Result;
+  return None;

rsmith wrote:
> Can this simply be
> ```
>   return const_cast(this)->getNondiscardedCase(Ctx));
> ```
> ? I would expect that if `T` implicitly converts to `U` then `Optional` 
> implicitly converts to `Optional`.
that does not seem to be the case unfortunately.  `llvm::Optional` doesn't have 
any such conversion functions.  Its only constructors and or `operator=` just 
take `Optional`.  It is missing most of the constructors here: 
https://en.cppreference.com/w/cpp/utility/optional/optional



Comment at: clang/lib/Sema/Sema.cpp:1545
 class DeferredDiagnosticsEmitter
 : public UsedDeclVisitor {
 public:

rsmith wrote:
> I wonder if there are other `UsedDeclVisitor`s that would want this behavior, 
> largely because of [basic.def.odr]p10: "Every program shall contain exactly 
> one definition of every non-inline function or variable that is odr-used in 
> that program outside of a discarded statement (8.5.2)". It seems worth 
> checking, and maybe moving this functionality to an optional behavior in the 
> base class, but I don't want to hold up this patch on that.
I've actually implemented this 2x (another time in a different class, that 
inherits from a different visitor, I believe `CallGraph`) in our downstream.  
The issue in many cases is that we lack access to `ASTContext` in those.

HOWEVER, I note that `EvaluatedExprVisitorBase` actually has access to an 
`ASTContext`, so I can put that in there controlled by an inherited function 
(like we do with some of the other visitors).

If it ends up being low-touch enough, I'll move it as an opt-in behavior for 
this patch this morning. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102251

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

[PATCH] D99683: [HIP] Support ThinLTO

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D99683#2744764 , @yaxunl wrote:

> In D99683#2683308 , @tejohnson wrote:
>
>> To do what I suggested in the prior comment, you'd probably want to add a 
>> new index-wide flag (since we don't read IR in the thin link). See for 
>> example how EnableSplitLTOUnit is set and used. You could add a flag like 
>> ForceImportAll or something like that. Then you don't necessarily even need 
>> to bump up the importing threshold or add the new import-noinline flag. Just 
>> key off of that in the importer to try to force import everything. If 
>> something cannot be imported, fail with a clear error.
>
> will do

I noticed you implemented with an internal error rather than a flag in the 
index. I think this is ok for now, especially if the support will eventually be 
removed because the linker will support external functions as noted in your 
TODO (note in order to do this in the index you would need to set up the flag 
during the compile step, not the linker invocation as you are doing here, which 
has some advantages if this will persist longer term). I have a suggestion 
about the error detection below, so that you can report errors earlier along 
with the failure reason.




Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:496
 dbgs() << "ignored! No qualifying callee with summary found.\n");
 continue;
   }

Probably better to issue an error here with the import failure reason?


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

https://reviews.llvm.org/D99683

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann created this revision.
mgartmann added reviewers: aaron.ballman, njames93, alexfh.
mgartmann added a project: clang-tools-extra.
Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai.
mgartmann requested review of this revision.

Finds base classes and structs whose destructor is neither public and virtual 
nor protected and non-virtual.
A base class's destructor should be specified in one of these ways to prevent 
undefined behaviour.

Fixes are available for user-declared and implicit destructors that are either 
public
and non-virtual or protected and virtual.

This check implements C.35 

 from the CppCoreGuidelines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-base-class-destructor.cpp
@@ -0,0 +1,131 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-base-class-destructor %t
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'PrivateVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct(){};
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'ProtectedVirtualBaseStruct' is protected and virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct(){};
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'PrivateNonVirtualBaseStruct' is private and prevents using the type. Consider making it public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:8: warning: destructor of struct 'PublicNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct(){};
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct(){};
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+3]]:8: warning: destructor of struct 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of struct 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual. It should either be public and virtual or protected and non-virtual [cppcoreguidelines-virtual-base-class-destructor]
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct(){};
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:7: warning: destructor of class 'PrivateVirtualBaseClass' is private and prevents using the type. Consider making it public and virtual or prot

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

[Paul C. Anagnostopoulos via Phabricator via cfe-commits]

Paul-C-Anagnostopoulos updated this revision to Diff 344809.
Paul-C-Anagnostopoulos added a comment.

The arm_*.inc files do not change with this revision.

I removed the commented-out code.


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

https://reviews.llvm.org/D102238

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


Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -575,7 +575,8 @@
 }
 
 foreach half = [ "b", "t" ] in {
-  defvar halfconst = !if(!eq(half, "b"), 0, 1);
+  defvar halfconst = !ne(half, "b");
+  defvar halfconst = !if(!eq(half, "b"), 0, 1);
 
   let params = [f32], pnt = PNT_None in {
 def vcvt#half#q_f16: Intrinsic<
@@ -1153,8 +1154,9 @@
 
 multiclass DyadicImmShift {
-  defvar intparams = !if(!eq(!cast(outtype), !cast(Vector)),
- [Vector], [outtype, Vector]);
+  defvar intparams = !if(!eq(outtype, Vector), [Vector], [outtype, Vector]);
+  defvar intparams = !if(!eq(!cast(outtype), 
!cast(Vector)),
+ [Vector], [outtype, Vector]);
 
   def q_n: Intrinsic<
   outtype, (args outtype:$a, Vector:$b, imm:$sh),
@@ -1532,9 +1534,10 @@
   //
   // So this foldl expression implements what you'd write in Python as
   // [srctype for srctype in T.All if srctype != desttype]
-  let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
-  !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype])))
-  in {
+  let params = !filter(srctype, T.All, !ne(srctype, desttype)) in {
+  let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
+  
!if(!eq(!cast(desttype),!cast(srctype)),[],[srctype])))
+  in {
 def "vreinterpretq_" # desttype: Intrinsic<
 VecOf, (args Vector:$x), (vreinterpret $x, VecOf)>;
   }
@@ -1576,8 +1579,11 @@
   defvar is_dest_float = !eq(desttype.kind, "f");
   defvar is_dest_unsigned = !eq(desttype.kind, "u");
   // First immediate operand of the LLVM intrinsic
-  defvar unsigned_flag = !if(is_dest_float, (unsignedflag Scalar),
- !if(is_dest_unsigned, V.True, V.False));
+  defvar unsigned_flag = !cond(is_dest_float : (unsignedflag Scalar),
+   is_dest_unsigned : V.True,
+   true : V.False);
+  defvar unsigned_flag = !if(is_dest_float, (unsignedflag Scalar),
+ !if(is_dest_unsigned, V.True, V.False));
   // For float->int conversions _n and _x_n intrinsics are not polymorphic
   // because the signedness of the destination type cannot be inferred.
   defvar pnt_nx = !if(is_dest_float, PNT_2Type, PNT_None);


Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -575,7 +575,8 @@
 }
 
 foreach half = [ "b", "t" ] in {
-  defvar halfconst = !if(!eq(half, "b"), 0, 1);
+  defvar halfconst = !ne(half, "b");
+  defvar halfconst = !if(!eq(half, "b"), 0, 1);
 
   let params = [f32], pnt = PNT_None in {
 def vcvt#half#q_f16: Intrinsic<
@@ -1153,8 +1154,9 @@
 
 multiclass DyadicImmShift {
-  defvar intparams = !if(!eq(!cast(outtype), !cast(Vector)),
- [Vector], [outtype, Vector]);
+  defvar intparams = !if(!eq(outtype, Vector), [Vector], [outtype, Vector]);
+  defvar intparams = !if(!eq(!cast(outtype), !cast(Vector)),
+ [Vector], [outtype, Vector]);
 
   def q_n: Intrinsic<
   outtype, (args outtype:$a, Vector:$b, imm:$sh),
@@ -1532,9 +1534,10 @@
   //
   // So this foldl expression implements what you'd write in Python as
   // [srctype for srctype in T.All if srctype != desttype]
-  let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
-  !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype])))
-  in {
+  let params = !filter(srctype, T.All, !ne(srctype, desttype)) in {
+  let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
+  !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype])))
+  in {
 def "vreinterpretq_" # desttype: Intrinsic<
 VecOf, (args Vector:$x), (vreinterpret $x, VecOf)>;
   }
@@ -1576,8 +1579,11 @@
   defvar is_dest_float = !eq(desttype.kind, "f");
   defvar is_dest_unsigned = !eq(desttype.kind, "u");
   // First immediate operand of the LLVM intrinsic
-  defvar unsigned_flag = !if(is_dest_float, (unsignedflag Scalar),
- !if(is_dest_unsigned, V.True, V.False));
+  defvar unsigned_flag = !cond(is_dest_float : (unsignedflag Scalar),
+   is_dest_unsigned : V.True,
+   true : V.False);
+  defvar unsigned_flag = !if(is_dest_float, (unsignedflag Scalar),
+ !if(is_dest_unsigned, V.True, V.False));
   // For float->int

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: 
openmp/libomptarget/deviceRTLs/common/generated_microtask_cases.gen:1
+case 0:
+((void (*)(kmp_int32 *, kmp_int32 *

This is not very pretty. Why do we need runtime dispatch to a function pointer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95976

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

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

[Paul C. Anagnostopoulos via Phabricator via cfe-commits]

Paul-C-Anagnostopoulos updated this revision to Diff 344810.
Paul-C-Anagnostopoulos added a comment.

One must amend the commit when one makes changes to the files.


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

https://reviews.llvm.org/D102238

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


Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -575,7 +575,7 @@
 }
 
 foreach half = [ "b", "t" ] in {
-  defvar halfconst = !if(!eq(half, "b"), 0, 1);
+  defvar halfconst = !ne(half, "b");
 
   let params = [f32], pnt = PNT_None in {
 def vcvt#half#q_f16: Intrinsic<
@@ -1153,8 +1153,7 @@
 
 multiclass DyadicImmShift {
-  defvar intparams = !if(!eq(!cast(outtype), !cast(Vector)),
- [Vector], [outtype, Vector]);
+  defvar intparams = !if(!eq(outtype, Vector), [Vector], [outtype, Vector]);
 
   def q_n: Intrinsic<
   outtype, (args outtype:$a, Vector:$b, imm:$sh),
@@ -1532,9 +1531,7 @@
   //
   // So this foldl expression implements what you'd write in Python as
   // [srctype for srctype in T.All if srctype != desttype]
-  let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
-  !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype])))
-  in {
+  let params = !filter(srctype, T.All, !ne(srctype, desttype)) in {
 def "vreinterpretq_" # desttype: Intrinsic<
 VecOf, (args Vector:$x), (vreinterpret $x, VecOf)>;
   }
@@ -1576,8 +1573,9 @@
   defvar is_dest_float = !eq(desttype.kind, "f");
   defvar is_dest_unsigned = !eq(desttype.kind, "u");
   // First immediate operand of the LLVM intrinsic
-  defvar unsigned_flag = !if(is_dest_float, (unsignedflag Scalar),
- !if(is_dest_unsigned, V.True, V.False));
+  defvar unsigned_flag = !cond(is_dest_float: (unsignedflag Scalar),
+   is_dest_unsigned: V.True,
+   true: V.False);
   // For float->int conversions _n and _x_n intrinsics are not polymorphic
   // because the signedness of the destination type cannot be inferred.
   defvar pnt_nx = !if(is_dest_float, PNT_2Type, PNT_None);


Index: clang/include/clang/Basic/arm_mve.td
===
--- clang/include/clang/Basic/arm_mve.td
+++ clang/include/clang/Basic/arm_mve.td
@@ -575,7 +575,7 @@
 }
 
 foreach half = [ "b", "t" ] in {
-  defvar halfconst = !if(!eq(half, "b"), 0, 1);
+  defvar halfconst = !ne(half, "b");
 
   let params = [f32], pnt = PNT_None in {
 def vcvt#half#q_f16: Intrinsic<
@@ -1153,8 +1153,7 @@
 
 multiclass DyadicImmShift {
-  defvar intparams = !if(!eq(!cast(outtype), !cast(Vector)),
- [Vector], [outtype, Vector]);
+  defvar intparams = !if(!eq(outtype, Vector), [Vector], [outtype, Vector]);
 
   def q_n: Intrinsic<
   outtype, (args outtype:$a, Vector:$b, imm:$sh),
@@ -1532,9 +1531,7 @@
   //
   // So this foldl expression implements what you'd write in Python as
   // [srctype for srctype in T.All if srctype != desttype]
-  let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
-  !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype])))
-  in {
+  let params = !filter(srctype, T.All, !ne(srctype, desttype)) in {
 def "vreinterpretq_" # desttype: Intrinsic<
 VecOf, (args Vector:$x), (vreinterpret $x, VecOf)>;
   }
@@ -1576,8 +1573,9 @@
   defvar is_dest_float = !eq(desttype.kind, "f");
   defvar is_dest_unsigned = !eq(desttype.kind, "u");
   // First immediate operand of the LLVM intrinsic
-  defvar unsigned_flag = !if(is_dest_float, (unsignedflag Scalar),
- !if(is_dest_unsigned, V.True, V.False));
+  defvar unsigned_flag = !cond(is_dest_float: (unsignedflag Scalar),
+   is_dest_unsigned: V.True,
+   true: V.False);
   // For float->int conversions _n and _x_n intrinsics are not polymorphic
   // because the signedness of the destination type cannot be inferred.
   defvar pnt_nx = !if(is_dest_float, PNT_2Type, PNT_None);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.



Comment at: clang/include/clang/Basic/arm_mve.td:1532
   //
   // So this foldl expression implements what you'd write in Python as
   // [srctype for srctype in T.All if srctype != desttype]

If you've removed my unreadable `foldl` then you can also safely remove the 
comment in which I had to explain what it means :-)


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

https://reviews.llvm.org/D102238

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

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Felix Berger via Phabricator via cfe-commits]

flx added a comment.

Could someone take a look at this patch? Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97567

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

[PATCH] D99683: [HIP] Support ThinLTO

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:496
 dbgs() << "ignored! No qualifying callee with summary found.\n");
 continue;
   }

tejohnson wrote:
> Probably better to issue an error here with the import failure reason?
My understanding is that the import failure reason is only available if 
PrintImportFailures is enabled. Also it can only print the GUID and can not 
print the failed callee name since it is not available, therefore the 
information is cryptic. It seems to me the current error msg at line 1332 is 
more suitable for common users. For compiler developers, they can enable 
PrintImportFailures and see the reason of failed imports.


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

https://reviews.llvm.org/D99683

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:38
+
+  const auto Destructor = MatchedClassOrStruct->getDestructor();
+

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:77
+const LangOptions &LangOpts) const {
+  auto VirtualBeginLoc = Destructor.getBeginLoc();
+  auto VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:78
+  auto VirtualBeginLoc = Destructor.getBeginLoc();
+  auto VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
+  Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:83
+  /// virtual is included.
+  auto StartOfNextToken = Lexer::findNextToken(VirtualEndLoc, SM, LangOpts)
+  .getValue()

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:96
+const SourceManager &SourceManager) const {
+  auto DestructorString = std::string("");
+  SourceLocation Loc;

Why not just `std::string DestructorString`?



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:101
+
+  auto ParentIndentation =
+  SourceManager.getExpansionColumnNumber(StructOrClass.getBeginLoc()) -

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:105
+
+  auto AccessSpecDecl = getPublicASDecl(StructOrClass);
+

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:9
+
+This check implements `C.35 
`_
 from the CppCoreGuidelines.
+

Links to documentation usually placed at the end. Please also try to follow 80 
characters line length limit



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:16
+  struct Foo {  // NOK, protected destructor should not be virtual
+ ~~~
+virtual void f();

Is it really necessary? Same below.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-base-class-destructor.rst:55
+
+.. option:: IndentationWidth
+

Shouldn't `Clang-format` take care about such things?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

[Paul C. Anagnostopoulos via Phabricator via cfe-commits]

Paul-C-Anagnostopoulos added inline comments.



Comment at: clang/include/clang/Basic/arm_mve.td:1532
   //
   // So this foldl expression implements what you'd write in Python as
   // [srctype for srctype in T.All if srctype != desttype]

simon_tatham wrote:
> If you've removed my unreadable `foldl` then you can also safely remove the 
> comment in which I had to explain what it means :-)
Will do!


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

https://reviews.llvm.org/D102238

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

[clang] 892c56e - [clang][AVR] Redefine some types to be compatible with avr-gcc

[Ben Shi via cfe-commits]

Author: Ben Shi
Date: 2021-05-12T22:05:26+08:00
New Revision: 892c56eabe250acaeb761eaddf783f47d95f7f4d

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

LOG: [clang][AVR] Redefine some types to be compatible with avr-gcc

Reviewed By: dylanmckay

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

Added: 


Modified: 
clang/lib/Basic/Targets/AVR.cpp
clang/test/CodeGen/builtins.cpp
clang/test/Preprocessor/init.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index e87b7338c4d6..86fda19869a8 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -309,6 +309,8 @@ void AVRTargetInfo::getTargetDefines(const LangOptions 
&Opts,
   Builder.defineMacro("__AVR__");
   Builder.defineMacro("__ELF__");
   Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
+  Builder.defineMacro("__UINT16_TYPE__", "unsigned int");
+  Builder.defineMacro("__INT16_TYPE__", "int");
 
   if (!this->CPU.empty()) {
 auto It = llvm::find_if(

diff  --git a/clang/test/CodeGen/builtins.cpp b/clang/test/CodeGen/builtins.cpp
index 6ba67ecbfc7e..575a8a11ca5b 100644
--- a/clang/test/CodeGen/builtins.cpp
+++ b/clang/test/CodeGen/builtins.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -ffreestanding -verify 
%s
 // RUN: %clang_cc1 -std=c++11 -triple i686-pc-linux-gnu -ffreestanding -verify 
%s
-// RUN: %clang_cc1 -std=c++11 -triple avr-unknown-unknown -ffreestanding 
-verify %s
 
 // expected-no-diagnostics
 

diff  --git a/clang/test/Preprocessor/init.c b/clang/test/Preprocessor/init.c
index be60eb6d9cf6..20ab2c05596b 100644
--- a/clang/test/Preprocessor/init.c
+++ b/clang/test/Preprocessor/init.c
@@ -1942,7 +1942,7 @@
 // AVR:#define __GXX_ABI_VERSION 1002
 // AVR:#define __INT16_C_SUFFIX__
 // AVR:#define __INT16_MAX__ 32767
-// AVR:#define __INT16_TYPE__ short
+// AVR:#define __INT16_TYPE__ int
 // AVR:#define __INT32_C_SUFFIX__ L
 // AVR:#define __INT32_MAX__ 2147483647L
 // AVR:#define __INT32_TYPE__ long int
@@ -2017,7 +2017,7 @@
 // AVR:#define __SIZE_TYPE__ unsigned int
 // AVR:#define __STDC__ 1
 // AVR:#define __UINT16_MAX__ 65535U
-// AVR:#define __UINT16_TYPE__ unsigned short
+// AVR:#define __UINT16_TYPE__ unsigned int
 // AVR:#define __UINT32_C_SUFFIX__ UL
 // AVR:#define __UINT32_MAX__ 4294967295UL
 // AVR:#define __UINT32_TYPE__ long unsigned int



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

[PATCH] D100701: [clang][AVR] Redefine some types to be compatible with avr-gcc

[Ben Shi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG892c56eabe25: [clang][AVR] Redefine some types to be 
compatible with avr-gcc (authored by benshi001).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100701

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/test/CodeGen/builtins.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1942,7 +1942,7 @@
 // AVR:#define __GXX_ABI_VERSION 1002
 // AVR:#define __INT16_C_SUFFIX__
 // AVR:#define __INT16_MAX__ 32767
-// AVR:#define __INT16_TYPE__ short
+// AVR:#define __INT16_TYPE__ int
 // AVR:#define __INT32_C_SUFFIX__ L
 // AVR:#define __INT32_MAX__ 2147483647L
 // AVR:#define __INT32_TYPE__ long int
@@ -2017,7 +2017,7 @@
 // AVR:#define __SIZE_TYPE__ unsigned int
 // AVR:#define __STDC__ 1
 // AVR:#define __UINT16_MAX__ 65535U
-// AVR:#define __UINT16_TYPE__ unsigned short
+// AVR:#define __UINT16_TYPE__ unsigned int
 // AVR:#define __UINT32_C_SUFFIX__ UL
 // AVR:#define __UINT32_MAX__ 4294967295UL
 // AVR:#define __UINT32_TYPE__ long unsigned int
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -ffreestanding -verify 
%s
 // RUN: %clang_cc1 -std=c++11 -triple i686-pc-linux-gnu -ffreestanding -verify 
%s
-// RUN: %clang_cc1 -std=c++11 -triple avr-unknown-unknown -ffreestanding 
-verify %s
 
 // expected-no-diagnostics
 
Index: clang/lib/Basic/Targets/AVR.cpp
===
--- clang/lib/Basic/Targets/AVR.cpp
+++ clang/lib/Basic/Targets/AVR.cpp
@@ -309,6 +309,8 @@
   Builder.defineMacro("__AVR__");
   Builder.defineMacro("__ELF__");
   Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
+  Builder.defineMacro("__UINT16_TYPE__", "unsigned int");
+  Builder.defineMacro("__INT16_TYPE__", "int");
 
   if (!this->CPU.empty()) {
 auto It = llvm::find_if(


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1942,7 +1942,7 @@
 // AVR:#define __GXX_ABI_VERSION 1002
 // AVR:#define __INT16_C_SUFFIX__
 // AVR:#define __INT16_MAX__ 32767
-// AVR:#define __INT16_TYPE__ short
+// AVR:#define __INT16_TYPE__ int
 // AVR:#define __INT32_C_SUFFIX__ L
 // AVR:#define __INT32_MAX__ 2147483647L
 // AVR:#define __INT32_TYPE__ long int
@@ -2017,7 +2017,7 @@
 // AVR:#define __SIZE_TYPE__ unsigned int
 // AVR:#define __STDC__ 1
 // AVR:#define __UINT16_MAX__ 65535U
-// AVR:#define __UINT16_TYPE__ unsigned short
+// AVR:#define __UINT16_TYPE__ unsigned int
 // AVR:#define __UINT32_C_SUFFIX__ UL
 // AVR:#define __UINT32_MAX__ 4294967295UL
 // AVR:#define __UINT32_TYPE__ long unsigned int
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -ffreestanding -verify %s
 // RUN: %clang_cc1 -std=c++11 -triple i686-pc-linux-gnu -ffreestanding -verify %s
-// RUN: %clang_cc1 -std=c++11 -triple avr-unknown-unknown -ffreestanding -verify %s
 
 // expected-no-diagnostics
 
Index: clang/lib/Basic/Targets/AVR.cpp
===
--- clang/lib/Basic/Targets/AVR.cpp
+++ clang/lib/Basic/Targets/AVR.cpp
@@ -309,6 +309,8 @@
   Builder.defineMacro("__AVR__");
   Builder.defineMacro("__ELF__");
   Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
+  Builder.defineMacro("__UINT16_TYPE__", "unsigned int");
+  Builder.defineMacro("__INT16_TYPE__", "int");
 
   if (!this->CPU.empty()) {
 auto It = llvm::find_if(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102248: [C++4OpenCL] Fix initialization of __constant constructors without arguments

[Ole Strohm via Phabricator via cfe-commits]

olestrohm added inline comments.



Comment at: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp:36
+  Z() __local = default;
+  Z() __global = default;
+  constexpr Z() __constant : z(0) {}

Anastasia wrote:
> We seem to be missing the coverage with __constant and `= default`.
This is because of the struct W below, you can't default a constexpr 
constructor, but __constant constructors need to be constexpr. Though it is 
currently possible to create them, they just can't actually be used. Or maybe 
just combine Z and W?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102248

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

[PATCH] D102251: Suppress Deferred Diagnostics in discarded statements.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 344813.
erichkeane added a comment.

Moved `IfStmt`/discarded case logic to ExprEvaluatorBase per @rsmith 's 
suggestion.  This is the 'highest' in the tree that has a `Sema`/`ASTContext` 
reference, so anything 'higher' in the tree would be higher touch. @rsmith: 
WDYT?


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

https://reviews.llvm.org/D102251

Files:
  clang/include/clang/AST/EvaluatedExprVisitor.h
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Sema/Sema.cpp
  clang/test/SemaCUDA/deferred-diags.cu

Index: clang/test/SemaCUDA/deferred-diags.cu
===
--- clang/test/SemaCUDA/deferred-diags.cu
+++ clang/test/SemaCUDA/deferred-diags.cu
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fcuda-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fcxx-exceptions -fcuda-is-device -fsyntax-only -std=c++17 -verify %s
 
 #include "Inputs/cuda.h"
 
@@ -8,29 +8,55 @@
   // expected-error@-1 2{{cannot use 'throw' in __host__ __device__ function}}
 }
 
+inline __host__ __device__ void hasInvalid2() {
+  throw NULL;
+  // expected-error@-1 2{{cannot use 'throw' in __host__ __device__ function}}
+}
+
+inline __host__ __device__ void hasInvalidDiscarded() {
+  // This is only used in the discarded statements below, so this should not diagnose.
+  throw NULL;
+}
+
 static __device__ void use0() {
   hasInvalid(); // expected-note {{called by 'use0'}}
   hasInvalid(); // expected-note {{called by 'use0'}}
+
+  if constexpr (true) {
+hasInvalid2(); // expected-note {{called by 'use0'}}
+  } else {
+hasInvalidDiscarded();
+  }
+
+  if constexpr (false) {
+hasInvalidDiscarded();
+  } else {
+hasInvalid2(); // expected-note {{called by 'use0'}}
+  }
+
+  if constexpr (false) {
+hasInvalidDiscarded();
+  }
 }
 
 // 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(); // expected-note 4{{called by 'use1'}}
   use0();
 }
 
 static __device__ void use2() {
-  use1(); // expected-note 2{{called by 'use2'}}
+  use1(); // expected-note 4{{called by 'use2'}}
   use1();
 }
 
 static __device__ void use3() {
-  use2(); // expected-note 2{{called by 'use3'}}
+  use2(); // expected-note 4{{called by 'use3'}}
   use2();
 }
 
 __global__ void use4() {
-  use3(); // expected-note 2{{called by 'use4'}}
+  use3(); // expected-note 4{{called by 'use4'}}
   use3();
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1569,6 +1569,8 @@
   DeferredDiagnosticsEmitter(Sema &S)
   : Inherited(S), ShouldEmitRootNode(false), InOMPDeviceContext(0) {}
 
+  bool shouldVisitDiscardedCase() const { return false; }
+
   void VisitOMPTargetDirective(OMPTargetDirective *Node) {
 ++InOMPDeviceContext;
 Inherited::VisitOMPTargetDirective(Node);
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -989,12 +989,20 @@
   return isa(getCond());
 }
 
-Optional IfStmt::getNondiscardedCase(const ASTContext &Ctx) const {
+Optional IfStmt::getNondiscardedCase(const ASTContext &Ctx) {
   if (!isConstexpr() || getCond()->isValueDependent())
 return None;
   return !getCond()->EvaluateKnownConstInt(Ctx) ? getElse() : getThen();
 }
 
+Optional
+IfStmt::getNondiscardedCase(const ASTContext &Ctx) const {
+  if (Optional Result =
+  const_cast(this)->getNondiscardedCase(Ctx))
+return *Result;
+  return None;
+}
+
 ForStmt::ForStmt(const ASTContext &C, Stmt *Init, Expr *Cond, VarDecl *condVar,
  Expr *Inc, Stmt *Body, SourceLocation FL, SourceLocation LP,
  SourceLocation RP)
Index: clang/include/clang/AST/Stmt.h
===
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -2080,6 +2080,7 @@
   /// If this is an 'if constexpr', determine which substatement will be taken.
   /// Otherwise, or if the condition is value-dependent, returns None.
   Optional getNondiscardedCase(const ASTContext &Ctx) const;
+  Optional getNondiscardedCase(const ASTContext &Ctx);
 
   bool isObjCAvailabilityCheck() const;
 
Index: clang/include/clang/AST/EvaluatedExprVisitor.h
===
--- clang/include/clang/AST/EvaluatedExprVisitor.h
+++ clang/include/clang/AST/EvaluatedExprVisitor.h
@@ -32,6 +32,9 @@
   const ASTContext &Context;
 
 public:
+  // Return whether this visitor should recurse into discarded statements for a
+  // 'constexpr-if'.
+  bool shouldVisitDiscardedCase() const { return true; }
 #define PTR(CLASS) typename Ptr::type
 
   explicit EvaluatedExprVi

[PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

[Philipp Krones via Phabricator via cfe-commits]

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



Comment at: clang/tools/driver/cc1as_main.cpp:407
 
-  MOFI->initMCObjectFileInfo(Ctx, PIC);
+  // FIXME: This is not pretty. MCContext has a ptr to MCObjectFileInfo and
+  // MCObjectFileInfo needs a MCContext reference in order to initialize 
itself.

MaskRay wrote:
> Can we fix the FIXME now?
Not quite. The construction is still in the order `MCContext` -> `MOFI` -> 
"continue construction of `MCContext` by setting the `MOFI`".  The order just 
changed from "dummy `MOFI`" -> `MCContext` -> "init dummy `MOFI`".



Comment at: llvm/include/llvm/MC/MCObjectFileInfo.h:255
 
+  virtual unsigned getTextSectionAlignment() const { return 4; }
   MCSection *getTextSection() const { return TextSection; }

MaskRay wrote:
> This should be moved to D102052
Will do.



Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:57
 
-  MOFI.reset(new MCObjectFileInfo);
-  MC.reset(
-  new MCContext(TheTriple, MAI.get(), MRI.get(), MOFI.get(), MSTI.get()));
-  MOFI->initMCObjectFileInfo(*MC, /*PIC=*/false);
+  MC.reset(new MCContext(TheTriple, MAI.get(), MRI.get(), /*MOFI=*/nullptr,
+ MSTI.get()));

MaskRay wrote:
> flip1995 wrote:
> > MaskRay wrote:
> > > The argument is almost always `/*MOFI=*/nullptr`. Doesn't this regress a 
> > > bit?
> > I agree. I think the best thing to do here would be to just remove this 
> > argument?
> I think so. Since MOFI is guaranteed to be constructed after MCContext now. 
Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101921

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

[PATCH] D99683: [HIP] Support ThinLTO

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:496
 dbgs() << "ignored! No qualifying callee with summary found.\n");
 continue;
   }

yaxunl wrote:
> tejohnson wrote:
> > Probably better to issue an error here with the import failure reason?
> My understanding is that the import failure reason is only available if 
> PrintImportFailures is enabled. Also it can only print the GUID and can not 
> print the failed callee name since it is not available, therefore the 
> information is cryptic. It seems to me the current error msg at line 1332 is 
> more suitable for common users. For compiler developers, they can enable 
> PrintImportFailures and see the reason of failed imports.
selectCallee always sets the Reason. And we have the name in addition to the 
GUID in normal circumstances (linking from modules). It would only not be 
available in certain debugging situations (e.g. linking from an existing 
combined module with llvm-lto). Also, by failing here, you don't need to wait 
until the LTO backends to issue the error, so it fails a little earlier.


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

https://reviews.llvm.org/D99683

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

[PATCH] D102244: [llvm][AsmPrinter] Restore source location to register clobber warning

[David Spickett via Phabricator via cfe-commits]

DavidSpickett updated this revision to Diff 344821.
DavidSpickett added a comment.
Herald added a subscriber: dexonsmith.

- Report via LLVMContext


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102244

Files:
  clang/test/Misc/inline-asm-clobber-warning.c
  llvm/include/llvm/IR/LLVMContext.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/IR/LLVMContext.cpp


Index: llvm/lib/IR/LLVMContext.cpp
===
--- llvm/lib/IR/LLVMContext.cpp
+++ llvm/lib/IR/LLVMContext.cpp
@@ -252,6 +252,14 @@
   diagnose(DiagnosticInfoInlineAsm(LocCookie, ErrorStr));
 }
 
+void LLVMContext::emitWarning(unsigned LocCookie, const Twine &WarningStr) {
+  diagnose(DiagnosticInfoInlineAsm(LocCookie, WarningStr, DS_Warning));
+}
+
+void LLVMContext::emitNote(unsigned LocCookie, const Twine &NoteStr) {
+  diagnose(DiagnosticInfoInlineAsm(LocCookie, NoteStr, DS_Note));
+}
+
 
//===--===//
 // Metadata Kind Uniquing
 
//===--===//
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -527,11 +527,6 @@
   }
 
   if (!RestrRegs.empty()) {
-unsigned BufNum = addInlineAsmDiagBuffer(OS.str(), LocMD);
-auto &SrcMgr = *MMI->getContext().getInlineSourceManager();
-SMLoc Loc = SMLoc::getFromPointer(
-SrcMgr.getMemoryBuffer(BufNum)->getBuffer().begin());
-
 std::string Msg = "inline asm clobber list contains reserved registers: ";
 ListSeparator LS;
 for (const Register &RR : RestrRegs) {
@@ -542,8 +537,8 @@
 "Reserved registers on the clobber list may not be "
 "preserved across the asm statement, and clobbering them may "
 "lead to undefined behaviour.";
-SrcMgr.PrintMessage(Loc, SourceMgr::DK_Warning, Msg);
-SrcMgr.PrintMessage(Loc, SourceMgr::DK_Note, Note);
+MMI->getModule()->getContext().emitWarning(LocCookie, Msg.c_str());
+MMI->getModule()->getContext().emitNote(LocCookie, Note);
   }
 
   emitInlineAsm(OS.str(), getSubtargetInfo(), TM.Options.MCOptions, LocMD,
Index: llvm/include/llvm/IR/LLVMContext.h
===
--- llvm/include/llvm/IR/LLVMContext.h
+++ llvm/include/llvm/IR/LLVMContext.h
@@ -294,6 +294,16 @@
   void emitError(const Instruction *I, const Twine &ErrorStr);
   void emitError(const Twine &ErrorStr);
 
+  /// emitWarning - Emit a warning message to the currently installed warning
+  /// handler with location information. Message will be implicitly prefixed
+  /// with "warning: " and should not end with a ".".
+  void emitWarning(unsigned LocCookie, const Twine &WarningStr);
+
+  /// emitNote - Emit a note to the currently installed note handler with
+  /// location information. Same rules as emitWarning except the prefix will
+  /// be "note: ".
+  void emitNote(unsigned LocCookie, const Twine &NoteStr);
+
   /// Access the object which can disable optional passes and individual
   /// optimizations at compile time.
   OptPassGate &getOptPassGate() const;
Index: clang/test/Misc/inline-asm-clobber-warning.c
===
--- /dev/null
+++ clang/test/Misc/inline-asm-clobber-warning.c
@@ -0,0 +1,18 @@
+/// This test checks that the warning includes the location in the C source
+/// file that contains the inline asm. Instead of saying  for both.
+/// Although this warning is emitted in llvm it cannot be tested from IR as
+/// it does not have that location information at that stage.
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv7-m -c %s -o /dev/null \
+// RUN:   2>&1 | FileCheck %s
+
+// REQUIRES: arm-registered-target
+
+void bar(void) {
+__asm__ __volatile__ ( "nop" : : : "sp");
+}
+
+// CHECK:  inline-asm-clobber-warning.c:12:28: warning: inline asm clobber 
list contains reserved registers: SP [-Winline-asm]
+// CHECK-NEXT: __asm__ __volatile__ ( "nop" : : : "sp");
+// CHECK-NEXT:^
+// CHECK-NEXT: inline-asm-clobber-warning.c:12:28: note: Reserved registers on 
the clobber list may not be preserved across the asm statement, and clobbering 
them may lead to undefined behaviour.


Index: llvm/lib/IR/LLVMContext.cpp
===
--- llvm/lib/IR/LLVMContext.cpp
+++ llvm/lib/IR/LLVMContext.cpp
@@ -252,6 +252,14 @@
   diagnose(DiagnosticInfoInlineAsm(LocCookie, ErrorStr));
 }
 
+void LLVMContext::emitWarning(unsigned LocCookie, const Twine &WarningStr) {
+  diagnose(DiagnosticInfoInlineAsm(LocCookie, WarningStr, DS_Warning));
+}
+
+void LLVMContext::emitNote(unsigned Loc

[PATCH] D102168: Use an allow list on reserved macro identifiers

[serge via Phabricator via cfe-commits]

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

Minor nits as suggested by reviewers + extend the list.


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

https://reviews.llvm.org/D102168

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/macro-reserved.c


Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -62,3 +62,6 @@
 #undef X__Y
 
 int x;
+
+#define _GNU_SOURCE  // no-warning
+#define __STDC_FORMAT_MACROS // no-warning
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -121,8 +121,49 @@
 
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
-  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved)
+  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved) {
+// list from:
+// - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+// - 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+// - man 7 feature_test_macros
+// The list must be sorted for correct binary search.
+static constexpr StringRef ReservedMacro[] = {
+"_ATFILE_SOURCE",
+"_BSD_SOURCE",
+"_CRT_NONSTDC_NO_WARNINGS",
+"_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+"_CRT_SECURE_NO_WARNINGS",
+"_FILE_OFFSET_BITS",
+"_FORTIFY_SOURCE",
+"_GLIBCXX_ASSERTIONS",
+"_GLIBCXX_CONCEPT_CHECKS",
+"_GLIBCXX_DEBUG",
+"_GLIBCXX_DEBUG_PEDANTIC",
+"_GLIBCXX_PARALLEL",
+"_GLIBCXX_PARALLEL_ASSERTIONS",
+"_GLIBCXX_SANITIZE_VECTOR",
+"_GLIBCXX_USE_CXX11_ABI",
+"_GLIBCXX_USE_DEPRECATED",
+"_GNU_SOURCE",
+"_ISOC11_SOURCE",
+"_ISOC95_SOURCE",
+"_ISOC99_SOURCE",
+"_LARGEFILE64_SOURCE",
+"_POSIX_C_SOURCE",
+"_REENTRANT",
+"_SVID_SOURCE",
+"_THREAD_SAFE",
+"_XOPEN_SOURCE",
+"_XOPEN_SOURCE_EXTENDED",
+"__STDCPP_WANT_MATH_SPEC_FUNCS__",
+"__STDC_FORMAT_MACROS",
+};
+if (std::binary_search(ReservedMacro.begin(), ReservedMacro.end(),
+   II->getName()))
+  return MD_NoWarn;
+
 return MD_ReservedMacro;
+  }
   StringRef Text = II->getName();
   if (II->isKeyword(Lang))
 return MD_KeywordDef;


Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -62,3 +62,6 @@
 #undef X__Y
 
 int x;
+
+#define _GNU_SOURCE  // no-warning
+#define __STDC_FORMAT_MACROS // no-warning
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -121,8 +121,49 @@
 
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
-  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved)
+  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved) {
+// list from:
+// - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+// - https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+// - man 7 feature_test_macros
+// The list must be sorted for correct binary search.
+static constexpr StringRef ReservedMacro[] = {
+"_ATFILE_SOURCE",
+"_BSD_SOURCE",
+"_CRT_NONSTDC_NO_WARNINGS",
+"_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+"_CRT_SECURE_NO_WARNINGS",
+"_FILE_OFFSET_BITS",
+"_FORTIFY_SOURCE",
+"_GLIBCXX_ASSERTIONS",
+"_GLIBCXX_CONCEPT_CHECKS",
+"_GLIBCXX_DEBUG",
+"_GLIBCXX_DEBUG_PEDANTIC",
+"_GLIBCXX_PARALLEL",
+"_GLIBCXX_PARALLEL_ASSERTIONS",
+"_GLIBCXX_SANITIZE_VECTOR",
+"_GLIBCXX_USE_CXX11_ABI",
+"_GLIBCXX_USE_DEPRECATED",
+"_GNU_SOURCE",
+"_ISOC11_SOURCE",
+"_ISOC95_SOURCE",
+"_ISOC99_SOURCE",
+"_LARGEFILE64_SOURCE",
+"_POSIX_C_SOURCE",
+"_REENTRANT",
+"_SVID_SOURCE",
+"_THREAD_SAFE",
+"_XOPEN_SOURCE",
+"_XOPEN_SOURCE_EXTENDED",
+"__STDCPP_WANT_MATH_SPEC_FUNCS__",
+"__STDC_FORMAT_MACROS",
+};
+if (std::binary_search(ReservedMacro.begin(), ReservedMacro.end(),
+   II->getName()))
+  return MD_NoWarn;
+
 return MD_ReservedMacro;
+  }
   StringRef Text = II->getName();
   if (II->isKe

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: 
openmp/libomptarget/deviceRTLs/common/generated_microtask_cases.gen:1
+case 0:
+((void (*)(kmp_int32 *, kmp_int32 *

JonChesterfield wrote:
> This is not very pretty. Why do we need runtime dispatch to a function 
> pointer?
because we have variadic functions right now. A patch to remove this is already 
underway:
 https://reviews.llvm.org/D102107


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95976

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Two pieces of feedback:

1. This fires in system headers on windows, e.g. like so:

  ../../third_party/wtl/include\./atlapp.h(336,12): error: performing pointer 
arithmetic on a null pointer has undefined behavior if the offset is nonzero 
[-Werror,-Wnull-pointer-arithmetic]
  uSize = LVGROUP_V5_SIZE;
  ^~~
  ..\..\third_party\depot_tools\win_toolchain\vs_files\20d5f2553f\Windows 
Kits\10\Include\10.0.19041.0\um\commctrl.h(4074,25): note: expanded from macro 
'LVGROUP_V5_SIZE'
  #define LVGROUP_V5_SIZE CCSIZEOF_STRUCT(LVGROUP, uAlign)
  ^~~~
  ..\..\third_party\depot_tools\win_toolchain\vs_files\20d5f2553f\Windows 
Kits\10\Include\10.0.19041.0\um\commctrl.h(262,90): note: expanded from macro 
'CCSIZEOF_STRUCT'
  #define CCSIZEOF_STRUCT(structname, member)  
(((int)((LPBYTE)(&((structname*)0)->member) - ((LPBYTE)((structname*)0 + 
sizeof(((structname*)0)->member))

   ^ ~~

Can we make this not fire for macros for system headers? And if this takes a 
while to implement, can we revert this until then?

2. This reusing the existing group means we now either have to disable the old, 
existing warning and regress, or we can't build. Can we please put this in a 
new group (Wnull-pointer-arithmetic-sub or what) that's a subgroup of 
Wnull-pointer-arithmetic but that can be turned off separately, to allow 
incremental roll-out of this?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[clang] 9857570 - [CUDA][HIP] Fix device template variables

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2021-05-12T11:13:29-04:00
New Revision: 98575708da9544ccab8939fece9c3d638a32f09f

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

LOG: [CUDA][HIP] Fix device template variables

Currently clang does not emit device template variables
instantiated only in host functions, however, nvcc is
able to do that:

https://godbolt.org/z/fneEfferY

This patch fixes this issue by refactoring and extending
the existing mechanism for emitting static device
var ODR-used by host only. Basically clang records
device variables ODR-used by host code and force
them to be emitted in device compilation. The existing
mechanism makes sure these device variables ODR-used
by host code are added to llvm.compiler-used, therefore
they are guaranteed not to be deleted.

It also fixes non-ODR-use of static device variable by host code
causing static device variable to be emitted and registered,
which should not.

Reviewed by: Artem Belevich

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CodeGenCUDA/device-stub.cu
clang/test/CodeGenCUDA/host-used-device-var.cu
clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
clang/test/CodeGenCUDA/static-device-var-rdc.cu

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index bef793831c6b2..6ebdca06d58ff 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1064,8 +1064,8 @@ class ASTContext : public RefCountedBase {
   // Implicitly-declared type 'struct _GUID'.
   mutable TagDecl *MSGuidTagDecl = nullptr;
 
-  /// Keep track of CUDA/HIP static device variables referenced by host code.
-  llvm::DenseSet CUDAStaticDeviceVarReferencedByHost;
+  /// Keep track of CUDA/HIP device-side variables ODR-used by host code.
+  llvm::DenseSet CUDADeviceVarODRUsedByHost;
 
   ASTContext(LangOptions &LOpts, SourceManager &SM, IdentifierTable &idents,
  SelectorTable &sels, Builtin::Context &builtins);

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 8941d563768d6..6eb8da7411237 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -11635,7 +11635,7 @@ bool ASTContext::mayExternalizeStaticVar(const Decl *D) 
const {
 bool ASTContext::shouldExternalizeStaticVar(const Decl *D) const {
   return mayExternalizeStaticVar(D) &&
  (D->hasAttr() ||
-  CUDAStaticDeviceVarReferencedByHost.count(cast(D)));
+  CUDADeviceVarODRUsedByHost.count(cast(D)));
 }
 
 StringRef ASTContext::getCUIDHash() const {

diff  --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index 1cd718d2e22fc..995b6a0b5fec6 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -1015,10 +1015,14 @@ void CGNVCUDARuntime::handleVarRegistration(const 
VarDecl *D,
 // Don't register a C++17 inline variable. The local symbol can be
 // discarded and referencing a discarded local symbol from outside the
 // comdat (__cuda_register_globals) is disallowed by the ELF spec.
-// TODO: Reject __device__ constexpr and __device__ inline in Sema.
+//
 // HIP managed variables need to be always recorded in device and host
 // compilations for transformation.
+//
+// HIP managed variables and variables in CUDADeviceVarODRUsedByHost are
+// added to llvm.compiler-used, therefore they are safe to be registered.
 if ((!D->hasExternalStorage() && !D->isInline()) ||
+CGM.getContext().CUDADeviceVarODRUsedByHost.contains(D) ||
 D->hasAttr()) {
   registerDeviceVar(D, GV, !D->hasDefinition(),
 D->hasAttr());

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index c61da1c980a2e..e3c843c8e9d37 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2362,8 +2362,8 @@ void CodeGenModule::EmitDeferred() {
   }
 
   // Emit CUDA/HIP static device variables referenced by host code only.
-  if (getLangOpts().CUDA)
-for (auto V : getContext().CUDAStaticDeviceVarReferencedByHost)
+  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice)
+for (const auto *V : getContext().CUDADeviceVarODRUsedByHost)
   DeferredDeclsToEmit.push_back(V);
 
   // Stop if we're out of both deferred vtables and deferred declarations.

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1390c17de9ece..719161fb9ba1e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cp

[PATCH] D102237: [CUDA][HIP] Fix non-ODR-use of static device variable

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
yaxunl marked 2 inline comments as done.
Closed by commit rG98575708da95: [CUDA][HIP] Fix device template variables 
(authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D102237?vs=344389&id=344831#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102237

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/device-stub.cu
  clang/test/CodeGenCUDA/host-used-device-var.cu
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
  clang/test/CodeGenCUDA/static-device-var-rdc.cu

Index: clang/test/CodeGenCUDA/static-device-var-rdc.cu
===
--- clang/test/CodeGenCUDA/static-device-var-rdc.cu
+++ clang/test/CodeGenCUDA/static-device-var-rdc.cu
@@ -2,19 +2,19 @@
 // REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
-// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
 // RUN:   -check-prefixes=DEV,INT-DEV %s
 
 // RUN: %clang_cc1 -triple x86_64-gnu-linux \
-// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
 // RUN:   -check-prefixes=HOST,INT-HOST %s
 
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
-// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s > %t.dev
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s > %t.dev
 // RUN: cat %t.dev | FileCheck -check-prefixes=DEV,EXT-DEV %s
 
 // RUN: %clang_cc1 -triple x86_64-gnu-linux -cuid=abc \
-// RUN:   -fgpu-rdc -emit-llvm -o - -x hip %s > %t.host
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s > %t.host
 // RUN: cat %t.host | FileCheck -check-prefixes=HOST,EXT-HOST %s
 
 // Check host and device compilations use the same postfixes for static
@@ -22,6 +22,25 @@
 
 // RUN: cat %t.dev %t.host | FileCheck -check-prefix=POSTFIX %s
 
+// Negative tests.
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefix=DEV-NEG %s
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefix=HOST-NEG %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s > %t.dev
+// RUN: cat %t.dev | FileCheck -check-prefix=DEV-NEG %s
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -cuid=abc \
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s > %t.host
+// RUN: cat %t.host | FileCheck -check-prefix=HOST-NEG %s
+
+
 #include "Inputs/cuda.h"
 
 // Test function scope static device variable, which should not be externalized.
@@ -61,9 +80,14 @@
 
 // Test static host variable, which should not be externalized nor registered.
 // HOST-DAG: @_ZL1z = internal global i32 0
-// DEV-NOT: @_ZL1z
+// DEV-NEG-NOT: @_ZL1z
 static int z;
 
+// Test non-ODR-use of static device variable is not emitted or registered.
+// DEV-NEG-NOT: @_ZL1u
+// HOST-NEG-NOT: @_ZL1u
+static __device__ int u;
+
 // Test static device variable in inline function, which should not be
 // externalized nor registered.
 // DEV-DAG: @_ZZ6devfunPPKiE1p = linkonce_odr addrspace(4) constant i32 2, comdat
@@ -77,6 +101,7 @@
   const static int w = 1;
   a[0] = x;
   a[1] = y;
+  a[2] = sizeof(u);
   b[0] = &w;
   b[1] = &x2;
   devfun(b);
@@ -88,10 +113,12 @@
   getDeviceSymbol(&x);
   getDeviceSymbol(&y);
   z = 123;
+  decltype(u) tmp;
 }
 
-// HOST: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
-// HOST: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
-// HOST-NOT: __hipRegisterVar({{.*}}@_ZL2x2
-// HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
-// HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p
+// HOST-DAG: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
+// HOST-DAG: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
+// HOST-NEG-NOT: __hipRegisterVar({{.*}}@_ZL2x2
+// HOST-NEG-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
+// HOST-NEG-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p
+// HOST-NEG-NOT: __hipRegisterVar({{.*}}@_ZL1u
Index: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
===
--- clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
+++ clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
@@ -2,12 +2,18 @@
 // REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -std=c++11 \
-// RUN:   -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=DEV %s
+//

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Whats the intended behaviour for derived classes and their destructors? Can 
test be added to demonstrate that behaviour?




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:44
+ "making it public and virtual or protected and non-virtual")
+<< (MatchedClassOrStruct->isClass() ? "class" : "struct")
+<< MatchedClassOrStruct;

Printing `class` or `struct` here isn't necessary, but if you really feel it 
should be included use `%select{class|struct}0` instead.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:51
+  // Implicit destructors are public and non-virtual for classes and structs.
+  std::string TypeAndVirtuality = "public and non-virtual";
+  FixItHint Fix;

This can no be made a bool, see below for the Diag.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:68
+  diag(MatchedClassOrStruct->getLocation(),
+   "destructor of %0 %1 is %2. It should either be public and virtual or "
+   "protected and non-virtual")





Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:70
+   "protected and non-virtual")
+  << (MatchedClassOrStruct->isClass() ? "class" : "struct")
+  << MatchedClassOrStruct << TypeAndVirtuality << Fix;

Again this can be removed.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:74
+
+CharSourceRange VirtualBaseClassDestructorCheck::getVirtualKeywordRange(
+const CXXDestructorDecl &Destructor, const SourceManager &SM,

This doesn't need to be part of the class and should just be a static function 
in this file.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:101
+
+  auto ParentIndentation =
+  SourceManager.getExpansionColumnNumber(StructOrClass.getBeginLoc()) -

Eugene.Zelenko wrote:
> Please don't use auto unless type is spelled in same statement or iterator.
Again, don't worry about indentation, clang-format does that.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:129
+
+AccessSpecDecl *VirtualBaseClassDestructorCheck::getPublicASDecl(
+const CXXRecordDecl &StructOrClass) const {

Can be made static again. Also should probably return a `const AccessSpecDecl *`



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:143-145
+std::string VirtualBaseClassDestructorCheck::indent(int Indentation) const {
+  return std::string().append(Indentation, ' ');
+}

As clang-format handles indentation, we can just remove this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D102248: [C++4OpenCL] Fix initialization of __constant constructors without arguments

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp:36
+  Z() __local = default;
+  Z() __global = default;
+  constexpr Z() __constant : z(0) {}

olestrohm wrote:
> Anastasia wrote:
> > We seem to be missing the coverage with __constant and `= default`.
> This is because of the struct W below, you can't default a constexpr 
> constructor, but __constant constructors need to be constexpr. Though it is 
> currently possible to create them, they just can't actually be used. Or maybe 
> just combine Z and W?
I see makes sense. I think it's ok to have separate structs. You could add a 
comment though to explain what you aim to test in each?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102248

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

[clang] 58d18dd - [OpenCL] Remove pragma requirement from Arm dot extension.

[Anastasia Stulova via cfe-commits]

Author: Anastasia Stulova
Date: 2021-05-12T16:25:33+01:00
New Revision: 58d18dde5cca3417e3d52670775c95d2f6fe9d05

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

LOG: [OpenCL] Remove pragma requirement from Arm dot extension.

This removed the pointless need for extension pragma since
it doesn't disable anything properly and it doesn't need to
enable anything that is not possible to disable.

The change doesn't break existing kernels since it allows to
compile more cases i.e. without pragma statements but the
pragma continues to be accepted.

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

Added: 


Modified: 
clang/lib/Headers/opencl-c.h
clang/test/CodeGenOpenCL/arm-integer-dot-product.cl
clang/test/SemaOpenCL/arm-integer-dot-product.cl

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index 62dbe459baa76..dcd325528f38b 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -16996,31 +16996,23 @@ uint16 __ovld amd_sadw(uint16 src0, uint16 src1, 
uint16 src2);
 #endif // cl_amd_media_ops2
 
 #if defined(cl_arm_integer_dot_product_int8)
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_int8 : begin
 uint __ovld arm_dot(uchar4 a, uchar4 b);
 int __ovld arm_dot(char4 a, char4 b);
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_int8 : end
 #endif // defined(cl_arm_integer_dot_product_int8)
 
 #if defined(cl_arm_integer_dot_product_accumulate_int8)
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int8 : begin
 uint __ovld arm_dot_acc(uchar4 a, uchar4 b, uint c);
 int __ovld arm_dot_acc(char4 a, char4 b, int c);
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int8 : end
 #endif // defined(cl_arm_integer_dot_product_accumulate_int8)
 
 #if defined(cl_arm_integer_dot_product_accumulate_int16)
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int16 : begin
 uint __ovld arm_dot_acc(ushort2 a, ushort2 b, uint c);
 int __ovld arm_dot_acc(short2 a, short2 b, int c);
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int16 : end
 #endif // defined(cl_arm_integer_dot_product_accumulate_int16)
 
 #if defined(cl_arm_integer_dot_product_accumulate_saturate_int8)
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_saturate_int8 : 
begin
 uint __ovld arm_dot_acc_sat(uchar4 a, uchar4 b, uint c);
 int __ovld arm_dot_acc_sat(char4 a, char4 b, int c);
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_saturate_int8 : 
end
 #endif // defined(cl_arm_integer_dot_product_accumulate_saturate_int8)
 
 // Disable any extensions we may have enabled previously.

diff  --git a/clang/test/CodeGenOpenCL/arm-integer-dot-product.cl 
b/clang/test/CodeGenOpenCL/arm-integer-dot-product.cl
index a4d28c7e6cf8f..c98615c5dd988 100644
--- a/clang/test/CodeGenOpenCL/arm-integer-dot-product.cl
+++ b/clang/test/CodeGenOpenCL/arm-integer-dot-product.cl
@@ -1,38 +1,39 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -finclude-default-header 
-fdeclare-opencl-builtins -cl-std=CL1.2 -emit-llvm -o - -O0 | FileCheck %s
 
+// Pragmas are only accepted for backward compatibility.
+
 #pragma OPENCL EXTENSION cl_arm_integer_dot_product_int8 : enable
+#pragma OPENCL EXTENSION cl_arm_integer_dot_product_int8 : disable
 void test_int8(uchar4 ua, uchar4 ub, char4 sa, char4 sb) {
 uint ur = arm_dot(ua, ub);
 // CHECK: call spir_func i32 @_Z7arm_dotDv4_hS_
 int sr = arm_dot(sa, sb);
 // CHECK: call spir_func i32 @_Z7arm_dotDv4_cS_
 }
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_int8 : disable
 
 #pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int8 : enable
+#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int8 : disable
 void test_accumulate_int8(uchar4 ua, uchar4 ub, uint uc, char4 sa, char4 sb, 
int c) {
 uint ur = arm_dot_acc(ua, ub, uc);
 // CHECK: call spir_func i32 @_Z11arm_dot_accDv4_hS_j
 int sr = arm_dot_acc(sa, sb, c);
 // CHECK: call spir_func i32 @_Z11arm_dot_accDv4_cS_i
 }
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int8 : disable
 
 #pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int16 : enable
+#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int16 : disable
 void test_accumulate_int16(ushort2 ua, ushort2 ub, uint uc, short2 sa, short2 
sb, int c) {
 uint ur = arm_dot_acc(ua, ub, uc);
 // CHECK: call spir_func i32 @_Z11arm_dot_accDv2_tS_j
 int sr = arm_dot_acc(sa, sb, c);
 // CHECK: call spir_func i32 @_Z11arm_dot_accDv2_sS_i
 }
-#pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_int16 : disable
 
 #pragma OPENCL EXTENSION cl_arm_integer_dot_product_accumulate_saturate_int8 : 
enable
+#pragma OPENCL EXTENSIO

[PATCH] D100985: [OpenCL] Remove pragma requirement for functions from Arm dot extension

[Anastasia Stulova via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG58d18dde5cca: [OpenCL] Remove pragma requirement from Arm 
dot extension. (authored by Anastasia).
Herald added a subscriber: ldrumm.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D100985?vs=339336&id=344834#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100985

Files:
  clang/lib/Headers/opencl-c.h
  clang/test/CodeGenOpenCL/arm-integer-dot-product.cl
  clang/test/SemaOpenCL/arm-integer-dot-product.cl

Index: clang/test/SemaOpenCL/arm-integer-dot-product.cl
===
--- clang/test/SemaOpenCL/arm-integer-dot-product.cl
+++ clang/test/SemaOpenCL/arm-integer-dot-product.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -finclude-default-header -verify -cl-std=CL1.2 -emit-llvm -o - -O0
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -finclude-default-header  -fdeclare-opencl-builtins -verify -cl-std=CL1.2 -emit-llvm -o - -cl-ext=-all
 
 void test_negative() {
 uchar4 ua8, ub8;
@@ -7,37 +7,13 @@
 short2 sa16, sb16;
 uint ur;
 int sr;
-ur = arm_dot(ua8, ub8); // expected-error{{no matching function for call to 'arm_dot'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_int8' to be enabled}}
-sr = arm_dot(sa8, sb8); // expected-error{{no matching function for call to 'arm_dot'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_int8' to be enabled}}
-ur = arm_dot_acc(ua8, ub8, ur); // expected-error{{no matching function for call to 'arm_dot_acc'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_accumulate_int8' to be enabled}}
-sr = arm_dot_acc(sa8, sb8, sr); // expected-error{{no matching function for call to 'arm_dot_acc'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_accumulate_int8' to be enabled}}
-ur = arm_dot_acc(ua16, ub16, ur); // expected-error{{no matching function for call to 'arm_dot_acc'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_accumulate_int16' to be enabled}}
-sr = arm_dot_acc(sa16, sb16, sr); // expected-error{{no matching function for call to 'arm_dot_acc'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_accumulate_int16' to be enabled}}
-ur = arm_dot_acc_sat(ua8, ub8, ur); // expected-error{{no matching function for call to 'arm_dot_acc_sat'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_accumulate_saturate_int8' to be enabled}}
-sr = arm_dot_acc_sat(sa8, sb8, sr); // expected-error{{no matching function for call to 'arm_dot_acc_sat'}}
-// expected-note@opencl-c.h:* {{candidate function not viable}}
-// expected-note@opencl-c.h:* {{candidate unavailable as it requires OpenCL extension 'cl_arm_integer_dot_product_accumulate_saturate_int8' to be enabled}}
+ur = arm_dot(ua8, ub8); // expected-error{{implicit declaration of function 'arm_dot' is invalid in OpenCL}}
+sr = arm_dot(sa8, sb8);
+ur = arm_dot_acc(ua8, ub8, ur); // expected-error{{implicit declaration of function 'arm_dot_acc' is invalid in OpenCL}} //expected-note{{'arm_dot_acc' declared here}}
+sr = arm_dot_acc(sa8, sb8, sr);
+ur = arm_dot_acc(ua16, ub16, ur);
+sr = arm_dot_acc(sa16, sb16, sr);
+ur = arm_dot_acc_sat(ua8, ub8, ur); // expected-error{{implicit declaration of function 

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Nathan James via Phabricator via cfe-commits]

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

LGTM, but with a very minor nit.




Comment at: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp:49
   declRefExpr(to(varDecl(equalsNode(&VarDecl.bind("declRef");
+  auto MemberExprOfVar = memberExpr(has(DeclRefToVar));
+  auto DeclRefToVarOrMemberExprOfVar =

Can you use `hasObjectExpression` instead of `has` here, Its a bit more 
explicit in what we are matching.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97567

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

[PATCH] D102288: [HWASan] Add -fsanitize=lam flag and enable HWASan to use it.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/include/clang/Basic/Sanitizers.def:55-59
+// Utilize Intel LAM in sanitizers.  Currently only used in combination with
+// -fsanitize=hwaddress.  This is an experimental flag which may be removed in
+// the future.
+// TODO: Use -mlam instead, if/when it is supported by clang.
+SANITIZER("lam", LAM)

vitalybuka wrote:
> if it's experimental, why not just "-fsanitize=hwaddress -mllvm 
> -havasan-lam=1" ?
Well, `-mllvm` indicates a flag for LLVM, but we need to change the Clang 
behavior to link with the LAM-enabled HWASan runtime.  It seems to me that we 
should use a flag directed to Clang for this.

Maybe it's possible to parse the `-mllvm` flag before the point where we need 
to choose a runtime (I'm not sure), but it seems simpler to do it this way.



Comment at: compiler-rt/test/hwasan/TestCases/Linux/vfork.c:7
-// Aliasing mode does not support stack tagging.
-// XFAIL: x86_64
 

xiangzhangllvm wrote:
> What does here XFAIL mean, do not test in x86_64 ?
It means the test will run in x86_64 and is expected to fail.

Under LAM the test actually passes, since manually tagging the upper bits in 
the stack pointers succeeds.  So we need to distinguish between x86 LAM mode 
and x86 aliasing mode, which we do with the new `pointer-tagging` feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102288

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

[PATCH] D102261: Introduce SYCL 2020 mode

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

My bad. My last comment:

  LangOptions::SYCL::Ver2017
  LangOptions::SYCL_2017


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

https://reviews.llvm.org/D102261

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

[PATCH] D102168: Use an allow list on reserved macro identifiers

[serge via Phabricator via cfe-commits]

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

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

https://reviews.llvm.org/D102168

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/macro-reserved.c


Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -62,3 +62,6 @@
 #undef X__Y
 
 int x;
+
+#define _GNU_SOURCE  // no-warning
+#define __STDC_FORMAT_MACROS // no-warning
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -121,8 +121,49 @@
 
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
-  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved)
+  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved) {
+// list from:
+// - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+// - 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+// - man 7 feature_test_macros
+// The list must be sorted for correct binary search.
+static constexpr StringRef ReservedMacro[] = {
+"_ATFILE_SOURCE",
+"_BSD_SOURCE",
+"_CRT_NONSTDC_NO_WARNINGS",
+"_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+"_CRT_SECURE_NO_WARNINGS",
+"_FILE_OFFSET_BITS",
+"_FORTIFY_SOURCE",
+"_GLIBCXX_ASSERTIONS",
+"_GLIBCXX_CONCEPT_CHECKS",
+"_GLIBCXX_DEBUG",
+"_GLIBCXX_DEBUG_PEDANTIC",
+"_GLIBCXX_PARALLEL",
+"_GLIBCXX_PARALLEL_ASSERTIONS",
+"_GLIBCXX_SANITIZE_VECTOR",
+"_GLIBCXX_USE_CXX11_ABI",
+"_GLIBCXX_USE_DEPRECATED",
+"_GNU_SOURCE",
+"_ISOC11_SOURCE",
+"_ISOC95_SOURCE",
+"_ISOC99_SOURCE",
+"_LARGEFILE64_SOURCE",
+"_POSIX_C_SOURCE",
+"_REENTRANT",
+"_SVID_SOURCE",
+"_THREAD_SAFE",
+"_XOPEN_SOURCE",
+"_XOPEN_SOURCE_EXTENDED",
+"__STDCPP_WANT_MATH_SPEC_FUNCS__",
+"__STDC_FORMAT_MACROS",
+};
+if (std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+   II->getName()))
+  return MD_NoWarn;
+
 return MD_ReservedMacro;
+  }
   StringRef Text = II->getName();
   if (II->isKeyword(Lang))
 return MD_KeywordDef;


Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -62,3 +62,6 @@
 #undef X__Y
 
 int x;
+
+#define _GNU_SOURCE  // no-warning
+#define __STDC_FORMAT_MACROS // no-warning
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -121,8 +121,49 @@
 
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
-  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved)
+  if (II->isReserved(Lang) != ReservedIdentifierStatus::NotReserved) {
+// list from:
+// - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+// - https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+// - man 7 feature_test_macros
+// The list must be sorted for correct binary search.
+static constexpr StringRef ReservedMacro[] = {
+"_ATFILE_SOURCE",
+"_BSD_SOURCE",
+"_CRT_NONSTDC_NO_WARNINGS",
+"_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+"_CRT_SECURE_NO_WARNINGS",
+"_FILE_OFFSET_BITS",
+"_FORTIFY_SOURCE",
+"_GLIBCXX_ASSERTIONS",
+"_GLIBCXX_CONCEPT_CHECKS",
+"_GLIBCXX_DEBUG",
+"_GLIBCXX_DEBUG_PEDANTIC",
+"_GLIBCXX_PARALLEL",
+"_GLIBCXX_PARALLEL_ASSERTIONS",
+"_GLIBCXX_SANITIZE_VECTOR",
+"_GLIBCXX_USE_CXX11_ABI",
+"_GLIBCXX_USE_DEPRECATED",
+"_GNU_SOURCE",
+"_ISOC11_SOURCE",
+"_ISOC95_SOURCE",
+"_ISOC99_SOURCE",
+"_LARGEFILE64_SOURCE",
+"_POSIX_C_SOURCE",
+"_REENTRANT",
+"_SVID_SOURCE",
+"_THREAD_SAFE",
+"_XOPEN_SOURCE",
+"_XOPEN_SOURCE_EXTENDED",
+"__STDCPP_WANT_MATH_SPEC_FUNCS__",
+"__STDC_FORMAT_MACROS",
+};
+if (std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+   II->getName()))
+  return MD_NoWarn;
+
 return MD_ReservedMacro;
+  }
   StringRef Text = II->getName();
   if (II->isKeyword(Lang))
 return MD_KeywordDef;
_

[PATCH] D102261: Introduce SYCL 2020 mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D102261#2754289 , @tschuett wrote:

> My bad. My last comment:
>
>   LangOptions::SYCL::Ver2017
>   LangOptions::SYCL_2017

To me, those convey the same amount of information, so the use of the scoped 
enum doesn't get us much (but it would mean we can't use `LangOptions::SYCL` 
for any other purpose in the future).


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

https://reviews.llvm.org/D102261

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

[clang-tools-extra] 5389a05 - [docs] Fix documentation for bugprone-dangling-handle

[Malcolm Parsons via cfe-commits]

Author: Malcolm Parsons
Date: 2021-05-12T17:20:15+01:00
New Revision: 5389a05836e74e3acab6dbda7e80ea43e3bc6304

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

LOG: [docs] Fix documentation for bugprone-dangling-handle

string_view isn't experimental anymore.
This check has always handled both forms.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
index 8c2c316c090da..701b67d77acaa 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
@@ -3,8 +3,7 @@
 bugprone-dangling-handle
 
 
-Detect dangling references in value handles like
-``std::experimental::string_view``.
+Detect dangling references in value handles like ``std::string_view``.
 These dangling references can be a result of constructing handles from 
temporary
 values, where the temporary is destroyed soon after the handle is created.
 
@@ -35,4 +34,5 @@ Options
 .. option:: HandleClasses
 
A semicolon-separated list of class names that should be treated as handles.
-   By default only ``std::experimental::basic_string_view`` is considered.
+   By default only ``std::basic_string_view`` and
+   ``std::experimental::basic_string_view`` are considered.



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

[PATCH] D102313: [docs] Fix documentation for bugprone-dangling-handle

[Malcolm Parsons via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5389a05836e7: [docs] Fix documentation for 
bugprone-dangling-handle (authored by malcolm.parsons).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102313

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
@@ -3,8 +3,7 @@
 bugprone-dangling-handle
 
 
-Detect dangling references in value handles like
-``std::experimental::string_view``.
+Detect dangling references in value handles like ``std::string_view``.
 These dangling references can be a result of constructing handles from 
temporary
 values, where the temporary is destroyed soon after the handle is created.
 
@@ -35,4 +34,5 @@
 .. option:: HandleClasses
 
A semicolon-separated list of class names that should be treated as handles.
-   By default only ``std::experimental::basic_string_view`` is considered.
+   By default only ``std::basic_string_view`` and
+   ``std::experimental::basic_string_view`` are considered.


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-dangling-handle.rst
@@ -3,8 +3,7 @@
 bugprone-dangling-handle
 
 
-Detect dangling references in value handles like
-``std::experimental::string_view``.
+Detect dangling references in value handles like ``std::string_view``.
 These dangling references can be a result of constructing handles from temporary
 values, where the temporary is destroyed soon after the handle is created.
 
@@ -35,4 +34,5 @@
 .. option:: HandleClasses
 
A semicolon-separated list of class names that should be treated as handles.
-   By default only ``std::experimental::basic_string_view`` is considered.
+   By default only ``std::basic_string_view`` and
+   ``std::experimental::basic_string_view`` are considered.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

aaron.ballman wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > dblaikie wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > hoy wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > > > Does this need to be down 
> > > > > > > > > > > > > > > > > > > > > > > > > here? Or would the code be a 
> > > > > > > > > > > > > > > > > > > > > > > > > well exercised if it was up 
> > > > > > > > > > > > > > > > > > > > > > > > > next to the go declaration 
> > > > > > > > > > > > > > > > > > > > > > > > > above?
> > > > > > > > > > > > > > > > > > > > > > > > Yes, it needs to be here. 
> > > > > > > > > > > > > > > > > > > > > > > > Otherwise it will just like the 
> > > > > > > > > > > > > > > > > > > > > > > > function `bar` above that 
> > > > > > > > > > > > > > > > > > > > > > > > doesn't get a uniquefied name. 
> > > > > > > > > > > > > > > > > > > > > > > > I think moving the definition 
> > > > > > > > > > > > > > > > > > > > > > > > up to right after the 
> > > > > > > > > > > > > > > > > > > > > > > > declaration hides the 
> > > > > > > > > > > > > > > > > > > > > > > > declaration.
> > > > > > > > > > > > > > > > > > > > > > > Not sure I follow - do you mean 
> > > > > > > > > > > > > > > > > > > > > > > that if the go declaration and go 
> > > > > > > > > > > > > > > > > > > > > > > definition were next to each 
> > > > > > > > > > > > > > > > > > > > > > > other, this test would 
> > > > > > > > > > > > > > > > > > > > > > > (mechanically speaking) not 
> > > > > > > > > > > > > > > > > > > > > > > validate what the patch? Or that 
> > > > > > > > > > > > > > > > > > > > > > > it would be less legible, but 
> > > > > > > > > > > > > > > > > > > > > > > still mechanically correct?
> > > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > > I think it would be (assuming 
> > > > > > > > > > > > > > > > > > > > > > > it's still mechanically correct) 
> > > > > > > > > > > > > > > > > > > > > > > more legible to put the 
> > > > > > > > > > > > > > > > > > > > > > > declaration next to the 
> > > > > > > > > > > > > > > > > > > > > > > definition - the comment 
> > > > > > > > > > > > > > > > > > > > > > > describes why the declaration is 
> > > > > > > > > > > > > > > > > > > > > > > significant/why the definition is 
> > > > > > > > > > > > > > > > > > > > > > > weird, and seeing all that 
> > > > > > > > > > > > > > > > > > > > > > > together would be clearer to me 
> > > > > > > > > > > > > > > > > > > > > > > than spreading it out/having to 
> > > > > > > > > > > > > > > > > > > > > > > look further away to see what's 
> > > > > > > > > > > > > > > > > > > > > > > going on.
> > > > > > > > > > > > > > > > > > > > > > When the `go` declaration and `go` 
> > > > > > > > > > > > > > > > > > > > > > definition were next to each other, 
> > > > > > > > > > > > > > > > > > > > > > the go function won't get a 
> > > > > > > > > > > > > > > > > > > > > > uniqufied name at all. The 
> > > > > > > > > > > > > > > > > > > > > > declaration will be overwritten by 
> > > > > > > > > > > > > > > > > > > > > > the definition. Only when the 
> > > > > > > > > > > > > > > > > > > > > > declaration is seen by others, such 
> > > > > > > > > > > > > > > > > > > > > > the callsite in `baz`, the 
> > > > > > > > > > > > > > > > > > > > > > declaration makes a difference by 
> > > > > > > > > > > > > > > > > > > > > > having the callsite use a uniqufied 
> > > > > > > > > > > > > > > > > > > > > > name.
> > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > Ah! Interesting, good to know. 
> > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > Is that worth supporting, I wonder? I 
> > > > > > > > > > > > > > > > > > > > > guess it falls out for free/without 
> > > > > > > > > > > > > > > > > > > > > significant additional complexity. I 
> > > > > > > > > > > > > > > > > > > > > w

[PATCH] D70401: [WIP][RISCV] Implement ilp32e ABI

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

In D70401#2733003 , @khchen wrote:

> Hi, I would like to add ilp32e ABI support in llvm
> Is there anyone working on this?
> It seem the one thing missed is ilp32e ABI should disallow D ISA extension.
> Is there anything else?

Nobody is currently working on this on the lowRISC side, so please do pick this 
up if you're interested. I can't quite recall the open issues I'm afraid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70401

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> aaron.ballman wrote:
> > dblaikie wrote:
> > > hoy wrote:
> > > > dblaikie wrote:
> > > > > hoy wrote:
> > > > > > dblaikie wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > dblaikie wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > > > > Does this need to be down 
> > > > > > > > > > > > > > > > > > > > > > > > > > here? Or would the code be 
> > > > > > > > > > > > > > > > > > > > > > > > > > a well exercised if it was 
> > > > > > > > > > > > > > > > > > > > > > > > > > up next to the go 
> > > > > > > > > > > > > > > > > > > > > > > > > > declaration above?
> > > > > > > > > > > > > > > > > > > > > > > > > Yes, it needs to be here. 
> > > > > > > > > > > > > > > > > > > > > > > > > Otherwise it will just like 
> > > > > > > > > > > > > > > > > > > > > > > > > the function `bar` above that 
> > > > > > > > > > > > > > > > > > > > > > > > > doesn't get a uniquefied 
> > > > > > > > > > > > > > > > > > > > > > > > > name. I think moving the 
> > > > > > > > > > > > > > > > > > > > > > > > > definition up to right after 
> > > > > > > > > > > > > > > > > > > > > > > > > the declaration hides the 
> > > > > > > > > > > > > > > > > > > > > > > > > declaration.
> > > > > > > > > > > > > > > > > > > > > > > > Not sure I follow - do you mean 
> > > > > > > > > > > > > > > > > > > > > > > > that if the go declaration and 
> > > > > > > > > > > > > > > > > > > > > > > > go definition were next to each 
> > > > > > > > > > > > > > > > > > > > > > > > other, this test would 
> > > > > > > > > > > > > > > > > > > > > > > > (mechanically speaking) not 
> > > > > > > > > > > > > > > > > > > > > > > > validate what the patch? Or 
> > > > > > > > > > > > > > > > > > > > > > > > that it would be less legible, 
> > > > > > > > > > > > > > > > > > > > > > > > but still mechanically correct?
> > > > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > > > I think it would be (assuming 
> > > > > > > > > > > > > > > > > > > > > > > > it's still mechanically 
> > > > > > > > > > > > > > > > > > > > > > > > correct) more legible to put 
> > > > > > > > > > > > > > > > > > > > > > > > the declaration next to the 
> > > > > > > > > > > > > > > > > > > > > > > > definition - the comment 
> > > > > > > > > > > > > > > > > > > > > > > > describes why the declaration 
> > > > > > > > > > > > > > > > > > > > > > > > is significant/why the 
> > > > > > > > > > > > > > > > > > > > > > > > definition is weird, and seeing 
> > > > > > > > > > > > > > > > > > > > > > > > all that together would be 
> > > > > > > > > > > > > > > > > > > > > > > > clearer to me than spreading it 
> > > > > > > > > > > > > > > > > > > > > > > > out/having to look further away 
> > > > > > > > > > > > > > > > > > > > > > > > to see what's going on.
> > > > > > > > > > > > > > > > > > > > > > > When the `go` declaration and 
> > > > > > > > > > > > > > > > > > > > > > > `go` definition were next to each 
> > > > > > > > > > > > > > > > > > > > > > > other, the go function won't get 
> > > > > > > > > > > > > > > > > > > > > > > a uniqufied name at all. The 
> > > > > > > > > > > > > > > > > > > > > > > declaration will be overwritten 
> > > > > > > > > > > > > > > > > > > > > > > by the definition. Only when the 
> > > > > > > > > > > > > > > > > > > > > > > declaration is seen by others, 
> > > > > > > > > > > > > > > > > > > > > > > such the callsite in `baz`, the 
> > > > > > > > > > > > > > > > > > > > > > > declaration makes a difference by 
> > > > > > > > > > > > > > > > > > > > > > > having the callsite use a 
> > > > > > > > > > > > > > > > > > > > > > > uniqufied name.
> > > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > Ah! Interesting, good to know. 
> > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > Is that worth supporting, I wonder? 
> > > > > > > > > > > > > > > > >

[PATCH] D102134: [docs]Updated the AMD GPU Attributes documentation

[Shivam Gupta via Phabricator via cfe-commits]

xgupta resigned from this revision.
xgupta added a comment.

I am really an idol reviewer for this patch -:) don't know anything about 
AMDGPU.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102134

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

@thakis, I have some questions about your comments.
Are you sure this is coming from a system header?  The path that you gave has 
third_party as a directory in the path.  If the warning were being triggered by 
code in a system header, I would have expected it to fire on something in the 
windows build bots but they appear to be clean.
I don't know if it is possible to suppress a warning based on whether the 
source is in a system header, or from a macro expansion that is defined in a 
system header; are you aware about whether or not this is possible?  If it is, 
I suspect it would already be in force as a general setting, again making me 
question whether this is a system header...and if it isn't a system header, 
that wouldn't help you in any case.
If I understand your second point correctly, you have a system that previously 
compiled cleanly with warnings being treated as errors and you are concerned 
that if you use the existing options to suppress this particular warning, you 
are concerned that something could creep in.  Therefore you are proposing a 
different warning group that would be a subgroup of the existing one so that if 
you set it, you would set both but you would still be able to set the specific 
one.  I don't know if this is possible or not.  Either way, I suspect that it 
would require using a different warning for the subtraction case, which would 
also require significantly changing these changes. Am I understanding 
correctly?  If so, this implies that you have access to a workaround for your 
problem, although it may not be the best solution.
I do not have access to a windows setup to test any of these proposed changes; 
in particular, given that I suspect that the affected files are specific to 
some third party vendor from which you have purchased code, I do not have means 
of investigating the actual problems/solutions.  If it would be helpful, I 
would be happy to review any changes that you might like to make to remedy the 
situation.
I will be on vacation for the next few days so please excuse my delayed 
responses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

aaron.ballman wrote:
> hoy wrote:
> > aaron.ballman wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > dblaikie wrote:
> > > > > > hoy wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > > > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > > > > > > > > > > > > > > Does this need to be down 
> > > > > > > > > > > > > > > > > > > > > > > > > > > here? Or would the code 
> > > > > > > > > > > > > > > > > > > > > > > > > > > be a well exercised if it 
> > > > > > > > > > > > > > > > > > > > > > > > > > > was up next to the go 
> > > > > > > > > > > > > > > > > > > > > > > > > > > declaration above?
> > > > > > > > > > > > > > > > > > > > > > > > > > Yes, it needs to be here. 
> > > > > > > > > > > > > > > > > > > > > > > > > > Otherwise it will just like 
> > > > > > > > > > > > > > > > > > > > > > > > > > the function `bar` above 
> > > > > > > > > > > > > > > > > > > > > > > > > > that doesn't get a 
> > > > > > > > > > > > > > > > > > > > > > > > > > uniquefied name. I think 
> > > > > > > > > > > > > > > > > > > > > > > > > > moving the definition up to 
> > > > > > > > > > > > > > > > > > > > > > > > > > right after the declaration 
> > > > > > > > > > > > > > > > > > > > > > > > > > hides the declaration.
> > > > > > > > > > > > > > > > > > > > > > > > > Not sure I follow - do you 
> > > > > > > > > > > > > > > > > > > > > > > > > mean that if the go 
> > > > > > > > > > > > > > > > > > > > > > > > > declaration and go definition 
> > > > > > > > > > > > > > > > > > > > > > > > > were next to each other, this 
> > > > > > > > > > > > > > > > > > > > > > > > > test would (mechanically 
> > > > > > > > > > > > > > > > > > > > > > > > > speaking) not validate what 
> > > > > > > > > > > > > > > > > > > > > > > > > the patch? Or that it would 
> > > > > > > > > > > > > > > > > > > > > > > > > be less legible, but still 
> > > > > > > > > > > > > > > > > > > > > > > > > mechanically correct?
> > > > > > > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > > > > > > I think it would be (assuming 
> > > > > > > > > > > > > > > > > > > > > > > > > it's still mechanically 
> > > > > > > > > > > > > > > > > > > > > > > > > correct) more legible to put 
> > > > > > > > > > > > > > > > > > > > > > > > > the declaration next to the 
> > > > > > > > > > > > > > > > > > > > > > > > > definition - the comment 
> > > > > > > > > > > > > > > > > > > > > > > > > describes why the declaration 
> > > > > > > > > > > > > > > > > > > > > > > > > is significant/why the 
> > > > > > > > > > > > > > > > > > > > > > > > > definition is weird, and 
> > > > > > > > > > > > > > > > > > > > > > > > > seeing all that together 
> > > > > > > > > > > > > > > > > > > > > > > > > would be clearer to me than 
> > > > > > > > > > > > > > > > > > > > > > > > > spreading it out/having to 
> > > > > > > > > > > > > > > > > > > > > > > > > look further away to see 
> > > > > > > > > > > > > > > > > > > > > > > > > what's going on.
> > > > > > > > > > > > > > > > > > > > > > > > When the `go` declaration and 
> > > > > > > > > > > > > > > > > > > > > > > > `go` definition were next to 
> > > > > > > > > > > > > > > > > > > > > > > > each other, the go function 
> > > > > > > > > > > > > > > > > > > > > > > > won't get a uniqufied name at 
> > > > > > > > > > > > > > > > > > > > > > > > all. The declaration will be 
> > > > > > > > > > > > > > > > > > > > > > > > overwritten by the definition. 
> > > > > > > > > > > > > > > > > > > > > > > > Only when the declaration is 
> > > > > > > > > > > > > > > > > > > > > > > > seen by others, such the 
> > > > > > > > > > > > > > > > > > > > > > > > callsite in `baz`, the 
> > > > > > > > > > > > > > > > > > > > > > > > declaration makes a difference 
> > > > > > > > > > > > > > > > > > > > > > > > by having the callsite use a 
> > > > > > > > > > > > > > > > > > > > > > > > uniqufied name.
> > > > > > > > > > > > >

[PATCH] D102213: [ASTMatchers] Add forCallable(), a generalization of forFunction().

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I totally agree that changing `forFunction()` to work correctly is the right 
solution but backwards compatibility breakage is very real because 
`forFunction()` accepts a `Matcher` whereas `forCallable()` 
accepts `Matcher` which is a smaller set of matchers. Eg., 
`forFunction(hasName())` is valid code bug `forCallable(hasName())` is not 
because `BlockDecl` isn't a `NamedDecl`. So, like, I suspect that not a lot of 
code expects this to be **//the//** function, but a lot of code expects it to 
be **//a//** function. See how much types did I have to change just to get the 
infinite loop checker to compile in D102214 .

If we're ok with such compatibility breakage then i'm all for it.


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

https://reviews.llvm.org/D102213

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

[PATCH] D102270: [CUDA][HIP] Fix device template variables

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:17145
 };
-if (Var && Var->hasGlobalStorage() && !IsEmittedOnDeviceSide(Var)) {
-  SemaRef.targetDiag(Loc, diag::err_ref_bad_target)
-  << /*host*/ 2 << /*variable*/ 1 << Var << Target;
+if (Var && Var->hasGlobalStorage()) {
+  if (!IsEmittedOnDeviceSide(Var)) {

Revisiting my comment from https://reviews.llvm.org/D102237#inline-967732

I think this can be combined with the `if (SemaRef.LangOpts.CUDA)` above.

Unless I'm missing something obvious, what we have now is
```
if (SemaRef.LangOpts.CUDA) {
  
  if (Var && Var->hasGlobalStorage()) {

  }
}
```

You've mentioned that they can't be combined because of an `else`, but there's 
no else in the `if (Var && Var->hasGlobalStorage()) ` as far as I can see, and 
`` could be moved inside it.



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

https://reviews.llvm.org/D102270

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I think the best method is to

In D86671#2750957 , @dougpuob wrote:

> Hi @njames93:
> Could you do me a favor? Because it is my first patch, something I'm not 
> sure. I'm confused about can I land this patch now? I read the "LLVM 
> Code-Review Policy and Practices" document, it said patches can be landed if 
> received a LGTM, but seems you are still reviewing.

If you have made significant changes (excluding what a reviewer asks when 
giving an LGTM) Its best to get those further changes also reviewed.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:348
+getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options,
+ClangTidyContext &Context) {
+  IdentifierNamingCheck::HungarianNotationOption HNOption;

Rather than passing the ClangTidyContext, Make this function a method of 
IdentifierNamingCheck. Then you call create the Diag without worrying about the 
Context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D101139: Create install targets for scan-build-py.

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/tools/scan-build-py/CMakeLists.txt:1-7
+set (BinFiles
+ "analyze-build"
+ "analyze-c++"
+ "analyze-cc"
+ "intercept-build"
+ "intercept-c++"
+ "intercept-cc")

Shouldn't this list also contain `scan-build`?



Comment at: clang/tools/scan-build-py/CMakeLists.txt:9
+
+set (LibScanbuilds
+ "__init__.py"





Comment at: clang/tools/scan-build-py/CMakeLists.txt:21
+# compiler.
+set (LibEars
+ "__init__.py"




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

https://reviews.llvm.org/D101139

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

[PATCH] D102337: [clang-tidy] Fix test that requires Windows platofrm

[Georgy Komarov via Phabricator via cfe-commits]

jubnzv created this revision.
jubnzv added reviewers: aaron.ballman, njames93.
jubnzv added a project: clang-tools-extra.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
jubnzv requested review of this revision.

This commit fixes the `cppcoreguidelines-pro-type-vararg` test when it  runs on 
a Windows host, but the toolchain is targeted a non-Windows platform.
See the description of the problem with this test here: 
https://reviews.llvm.org/D101259#2739466


https://reviews.llvm.org/D102337

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
@@ -2,9 +2,7 @@
 // Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the
 // built-in va_list on Windows systems.
 
-// REQUIRES: system-windows
-
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t -- 
--extra-arg=--target=x86_64-windows
 
 void test_ms_va_list(int a, ...) {
   __builtin_ms_va_list ap;


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
@@ -2,9 +2,7 @@
 // Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the
 // built-in va_list on Windows systems.
 
-// REQUIRES: system-windows
-
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t -- --extra-arg=--target=x86_64-windows
 
 void test_ms_va_list(int a, ...) {
   __builtin_ms_va_list ap;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3bf1aca - [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-05-12T10:34:31-07:00
New Revision: 3bf1acab5b454ad7fb2074b34663108b53620695

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

LOG: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

llvm-dev message: https://lists.llvm.org/pipermail/llvm-dev/2021-May/150465.html

In an ELF shared object, a default visibility defined symbol is preemptible by 
default.
This creates some missed optimization opportunities. 
-fno-semantic-interposition can optimize -fPIC:

* in Clang: avoid GOT/PLT cost for variable access/function calls to external 
linkage definition in the same TU
* in GCC: enable interprocedural optimizations (including inlining) and avoid 
PLT

See https://gist.github.com/MaskRay/2d4dfcfc897341163f734afb59f689c6 for more 
information.

-Bsymbolic-functions is more aggressive than -fvisibility-inlines-hidden 
(present since 2012) as it applies
to all function definitions.  It can

* avoid PLT for cross-TU function calls && reduce dynamic symbol lookup
* reduce dynamic symbol lookup for taking function addresses and optimize out 
GOT/TOC on x86-64/ppc64

With both options, the libLLVM.so and libclang-cpp.so performance should
be closer to PIE binary linking against `libLLVM*.a` and `libclang*.a`

(In a -DLLVM_TARGETS_TO_BUILD=X86 build, the number of JUMP_SLOT decreases from 
12716 to 1628, and the number of GLOB_DAT decreases from 1918 to 1313
The built clang with `-DLLVM_LINK_LLVM_DYLIB=on -DCLANG_LINK_CLANG_DYLIB=on` is 
significantly faster.
See the Linux kernel build result https://bugs.archlinux.org/task/70697
)

Some implication:

Interposing a subset of functions is no longer supported.
(This is fragile anyway and cannot really be supported. For Mach-O we don't use
`ld -interpose`, so interposition is not supported on Mach-O at all.)

Compiling a program which takes the address of any LLVM function with
`{gcc,clang} -fno-pic` and expects the address to equal to the address taken
from libLLVM.so or libclang-cpp.so is unsupported. I am fairly confident that
llvm-project shouldn't have different behaviors depending on such pointer
equality (as we've been using -fvisibility-inlines-hidden which applies to
inline functions for a long time), but if we accidentally do, users should be
aware that they should not make assumption on pointer equality in `-fno-pic`
mode.

Reviewed By: phosek

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

Added: 


Modified: 
clang/tools/clang-shlib/CMakeLists.txt
llvm/cmake/modules/HandleLLVMOptions.cmake
llvm/tools/llvm-shlib/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/clang-shlib/CMakeLists.txt 
b/clang/tools/clang-shlib/CMakeLists.txt
index 5949223fc8e34..73e806c495556 100644
--- a/clang/tools/clang-shlib/CMakeLists.txt
+++ b/clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,8 @@ add_clang_library(clang-cpp
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+# Optimize function calls and global variable access for default visibility
+# definitions to avoid PLT and reduce dynamic relocations.
+if (NOT APPLE)
+  target_link_options(clang-cpp PRIVATE LINKER:-Bsymbolic-functions)
+endif()

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake 
b/llvm/cmake/modules/HandleLLVMOptions.cmake
index a64644a08e599..81f46cf116cf6 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -305,6 +305,7 @@ if( LLVM_ENABLE_PIC )
 # On Windows all code is PIC. MinGW warns if -fPIC is used.
   else()
 add_flag_or_print_warning("-fPIC" FPIC)
+add_flag_if_supported("-fno-semantic-interposition" 
FNO_SEMANTIC_INTERPOSITION)
   endif()
   # GCC for MIPS can miscompile LLVM due to PR37701.
   if(CMAKE_COMPILER_IS_GNUCXX AND LLVM_NATIVE_ARCH STREQUAL "Mips" AND

diff  --git a/llvm/tools/llvm-shlib/CMakeLists.txt 
b/llvm/tools/llvm-shlib/CMakeLists.txt
index b0ee19049e6fa..7259ba2636581 100644
--- a/llvm/tools/llvm-shlib/CMakeLists.txt
+++ b/llvm/tools/llvm-shlib/CMakeLists.txt
@@ -50,6 +50,9 @@ if(LLVM_BUILD_LLVM_DYLIB)
   # Solaris ld does not accept global: *; so there is no way to version 
*all* global symbols
   set(LIB_NAMES 
-Wl,--version-script,${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map
 ${LIB_NAMES})
 endif()
+# Optimize function calls and global variable access for default visibility
+# definitions to avoid PLT and reduce dynamic relocations.
+target_link_options(LLVM PRIVATE LINKER:-Bsymbolic-functions)
   elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
 set(LIB_NAMES -Wl,-all_load ${LIB_NAMES})
   endif()



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

[PATCH] D102090: [CMake][ELF] Add -fno-semantic-interposition and -Bsymbolic-functions

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3bf1acab5b45: [CMake][ELF] Add -fno-semantic-interposition 
and -Bsymbolic-functions (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D102090?vs=343758&id=344866#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102090

Files:
  clang/tools/clang-shlib/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/tools/llvm-shlib/CMakeLists.txt


Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -50,6 +50,9 @@
   # Solaris ld does not accept global: *; so there is no way to version 
*all* global symbols
   set(LIB_NAMES 
-Wl,--version-script,${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map
 ${LIB_NAMES})
 endif()
+# Optimize function calls and global variable access for default visibility
+# definitions to avoid PLT and reduce dynamic relocations.
+target_link_options(LLVM PRIVATE LINKER:-Bsymbolic-functions)
   elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
 set(LIB_NAMES -Wl,-all_load ${LIB_NAMES})
   endif()
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -305,6 +305,7 @@
 # On Windows all code is PIC. MinGW warns if -fPIC is used.
   else()
 add_flag_or_print_warning("-fPIC" FPIC)
+add_flag_if_supported("-fno-semantic-interposition" 
FNO_SEMANTIC_INTERPOSITION)
   endif()
   # GCC for MIPS can miscompile LLVM due to PR37701.
   if(CMAKE_COMPILER_IS_GNUCXX AND LLVM_NATIVE_ARCH STREQUAL "Mips" AND
Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,8 @@
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+# Optimize function calls and global variable access for default visibility
+# definitions to avoid PLT and reduce dynamic relocations.
+if (NOT APPLE)
+  target_link_options(clang-cpp PRIVATE LINKER:-Bsymbolic-functions)
+endif()


Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -50,6 +50,9 @@
   # Solaris ld does not accept global: *; so there is no way to version *all* global symbols
   set(LIB_NAMES -Wl,--version-script,${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map ${LIB_NAMES})
 endif()
+# Optimize function calls and global variable access for default visibility
+# definitions to avoid PLT and reduce dynamic relocations.
+target_link_options(LLVM PRIVATE LINKER:-Bsymbolic-functions)
   elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
 set(LIB_NAMES -Wl,-all_load ${LIB_NAMES})
   endif()
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -305,6 +305,7 @@
 # On Windows all code is PIC. MinGW warns if -fPIC is used.
   else()
 add_flag_or_print_warning("-fPIC" FPIC)
+add_flag_if_supported("-fno-semantic-interposition" FNO_SEMANTIC_INTERPOSITION)
   endif()
   # GCC for MIPS can miscompile LLVM due to PR37701.
   if(CMAKE_COMPILER_IS_GNUCXX AND LLVM_NATIVE_ARCH STREQUAL "Mips" AND
Index: clang/tools/clang-shlib/CMakeLists.txt
===
--- clang/tools/clang-shlib/CMakeLists.txt
+++ clang/tools/clang-shlib/CMakeLists.txt
@@ -48,3 +48,8 @@
   ${_OBJECTS}
   LINK_LIBS
   ${_DEPS})
+# Optimize function calls and global variable access for default visibility
+# definitions to avoid PLT and reduce dynamic relocations.
+if (NOT APPLE)
+  target_link_options(clang-cpp PRIVATE LINKER:-Bsymbolic-functions)
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102322: [clang-tidy] Add '-target' CLI option to override target triple

[Georgy Komarov via Phabricator via cfe-commits]

jubnzv abandoned this revision.
jubnzv added a comment.

Thank you, I didn't know that it is possible to set the target triple using 
`--extra-arg`. I created a new revision with this fix: 
https://reviews.llvm.org/D102337.
For some reasons, the `--target=unknown-windows` doesn't work for me with the 
following error: `error: unknown target triple 
'unknown-unknown-windows-msvc19.11.0', please use -triple or -arch 
[clang-diagnostic-error]`.
But `--target=x86_64-windows` works fine.


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

https://reviews.llvm.org/D102322

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

[clang] 81f56a2 - [NFC][clang][Codegen] Split ThunkInfo into it's own header

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2021-05-12T20:39:54+03:00
New Revision: 81f56a2eb3797eb5be61d65a8f7d7e19456e67d1

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

LOG: [NFC][clang][Codegen] Split ThunkInfo into it's own header

Otherwise we'll have issues with forward definition of GlobalDecl.

Split off from https://reviews.llvm.org/D100388

Added: 
clang/include/clang/Basic/Thunk.h

Modified: 
clang/include/clang/AST/VTableBuilder.h
clang/include/clang/Basic/ABI.h
clang/lib/AST/ItaniumMangle.cpp

Removed: 




diff  --git a/clang/include/clang/AST/VTableBuilder.h 
b/clang/include/clang/AST/VTableBuilder.h
index 241dd13f903e5..e451f3f861b79 100644
--- a/clang/include/clang/AST/VTableBuilder.h
+++ b/clang/include/clang/AST/VTableBuilder.h
@@ -18,6 +18,7 @@
 #include "clang/AST/GlobalDecl.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/ABI.h"
+#include "clang/Basic/Thunk.h"
 #include "llvm/ADT/DenseMap.h"
 #include 
 #include 

diff  --git a/clang/include/clang/Basic/ABI.h b/clang/include/clang/Basic/ABI.h
index 2401ffa20494e..231bad799a42c 100644
--- a/clang/include/clang/Basic/ABI.h
+++ b/clang/include/clang/Basic/ABI.h
@@ -37,174 +37,6 @@ enum CXXDtorType {
 Dtor_Comdat///< The COMDAT used for dtors
 };
 
-/// A return adjustment.
-struct ReturnAdjustment {
-  /// The non-virtual adjustment from the derived object to its
-  /// nearest virtual base.
-  int64_t NonVirtual;
-
-  /// Holds the ABI-specific information about the virtual return
-  /// adjustment, if needed.
-  union VirtualAdjustment {
-// Itanium ABI
-struct {
-  /// The offset (in bytes), relative to the address point
-  /// of the virtual base class offset.
-  int64_t VBaseOffsetOffset;
-} Itanium;
-
-// Microsoft ABI
-struct {
-  /// The offset (in bytes) of the vbptr, relative to the beginning
-  /// of the derived class.
-  uint32_t VBPtrOffset;
-
-  /// Index of the virtual base in the vbtable.
-  uint32_t VBIndex;
-} Microsoft;
-
-VirtualAdjustment() {
-  memset(this, 0, sizeof(*this));
-}
-
-bool Equals(const VirtualAdjustment &Other) const {
-  return memcmp(this, &Other, sizeof(Other)) == 0;
-}
-
-bool isEmpty() const {
-  VirtualAdjustment Zero;
-  return Equals(Zero);
-}
-
-bool Less(const VirtualAdjustment &RHS) const {
-  return memcmp(this, &RHS, sizeof(RHS)) < 0;
-}
-  } Virtual;
-
-  ReturnAdjustment() : NonVirtual(0) {}
-
-  bool isEmpty() const { return !NonVirtual && Virtual.isEmpty(); }
-
-  friend bool operator==(const ReturnAdjustment &LHS,
- const ReturnAdjustment &RHS) {
-return LHS.NonVirtual == RHS.NonVirtual && LHS.Virtual.Equals(RHS.Virtual);
-  }
-
-  friend bool operator!=(const ReturnAdjustment &LHS, const ReturnAdjustment 
&RHS) {
-return !(LHS == RHS);
-  }
-
-  friend bool operator<(const ReturnAdjustment &LHS,
-const ReturnAdjustment &RHS) {
-if (LHS.NonVirtual < RHS.NonVirtual)
-  return true;
-
-return LHS.NonVirtual == RHS.NonVirtual && LHS.Virtual.Less(RHS.Virtual);
-  }
-};
-
-/// A \c this pointer adjustment.
-struct ThisAdjustment {
-  /// The non-virtual adjustment from the derived object to its
-  /// nearest virtual base.
-  int64_t NonVirtual;
-
-  /// Holds the ABI-specific information about the virtual this
-  /// adjustment, if needed.
-  union VirtualAdjustment {
-// Itanium ABI
-struct {
-  /// The offset (in bytes), relative to the address point,
-  /// of the virtual call offset.
-  int64_t VCallOffsetOffset;
-} Itanium;
-
-struct {
-  /// The offset of the vtordisp (in bytes), relative to the ECX.
-  int32_t VtordispOffset;
-
-  /// The offset of the vbptr of the derived class (in bytes),
-  /// relative to the ECX after vtordisp adjustment.
-  int32_t VBPtrOffset;
-
-  /// The offset (in bytes) of the vbase offset in the vbtable.
-  int32_t VBOffsetOffset;
-} Microsoft;
-
-VirtualAdjustment() {
-  memset(this, 0, sizeof(*this));
-}
-
-bool Equals(const VirtualAdjustment &Other) const {
-  return memcmp(this, &Other, sizeof(Other)) == 0;
-}
-
-bool isEmpty() const {
-  VirtualAdjustment Zero;
-  return Equals(Zero);
-}
-
-bool Less(const VirtualAdjustment &RHS) const {
-  return memcmp(this, &RHS, sizeof(RHS)) < 0;
-}
-  } Virtual;
-
-  ThisAdjustment() : NonVirtual(0) { }
-
-  bool isEmpty() const { return !NonVirtual && Virtual.isEmpty(); }
-
-  friend bool operator==(const ThisAdjustment &LHS,
- const ThisAdjustment &RHS) {
-return LHS.NonVirtual == RHS.NonVirtual && LHS.Virtual.Equals(RHS.Virtual);
-  }
-
-  friend boo

[clang] 2d84195 - [NFCI][clang][Codegen] CodeGenVTables::addVTableComponent(): use getGlobalDecl

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2021-05-12T20:39:54+03:00
New Revision: 2d84195d60b0cb5ea43b18ab8f6770a84bf32da4

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

LOG: [NFCI][clang][Codegen] CodeGenVTables::addVTableComponent(): use 
getGlobalDecl

It does the same thing.
Split off from https://reviews.llvm.org/D100388

Added: 


Modified: 
clang/lib/CodeGen/CGVTables.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index bef9a293b7ed5..a8e3a669353c3 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -727,22 +727,7 @@ void 
CodeGenVTables::addVTableComponent(ConstantArrayBuilder &builder,
   case VTableComponent::CK_FunctionPointer:
   case VTableComponent::CK_CompleteDtorPointer:
   case VTableComponent::CK_DeletingDtorPointer: {
-GlobalDecl GD;
-
-// Get the right global decl.
-switch (component.getKind()) {
-default:
-  llvm_unreachable("Unexpected vtable component kind");
-case VTableComponent::CK_FunctionPointer:
-  GD = component.getFunctionDecl();
-  break;
-case VTableComponent::CK_CompleteDtorPointer:
-  GD = GlobalDecl(component.getDestructorDecl(), Dtor_Complete);
-  break;
-case VTableComponent::CK_DeletingDtorPointer:
-  GD = GlobalDecl(component.getDestructorDecl(), Dtor_Deleting);
-  break;
-}
+GlobalDecl GD = component.getGlobalDecl();
 
 if (CGM.getLangOpts().CUDA) {
   // Emit NULL for methods we can't codegen on this



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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D98798#2754449 , @jamieschmeiser 
wrote:

> @thakis, I have some questions about your comments.
> Are you sure this is coming from a system header?  The path that you gave has 
> third_party as a directory in the path.

Yes. `third_party\depot_tools\win_toolchain\vs_files\20d5f2553f\Windows 
Kits\10\Include\10.0.19041.0\um\commctrl.h` is a hermetic system directory used 
via `/winsysrootthird_party\depot_tools\win_toolchain\vs_files\20d5f2553f`.

> If the warning were being triggered by code in a system header, I would have 
> expected it to fire on something in the windows build bots but they appear to 
> be clean.

I don't think LLVM uses a lot of code from the windows system headers. The 
compiler doesn't contain any UI :) (commctrl.h is a UI header).

> I don't know if it is possible to suppress a warning based on whether the 
> source is in a system header, or from a macro expansion that is defined in a 
> system header; are you aware about whether or not this is possible?



> If it is, I suspect it would already be in force as a general setting, again 
> making me question whether this is a system header...and if it isn't a system 
> header, that wouldn't help you in any case.
> If I understand your second point correctly, you have a system that 
> previously compiled cleanly with warnings being treated as errors and you are 
> concerned that if you use the existing options to suppress this particular 
> warning, you are concerned that something could creep in.  Therefore you are 
> proposing a different warning group that would be a subgroup of the existing 
> one so that if you set it, you would set both but you would still be able to 
> set the specific one.  I don't know if this is possible or not.  Either way, 
> I suspect that it would require using a different warning for the subtraction 
> case, which would also require significantly changing these changes. Am I 
> understanding correctly?  If so, this implies that you have access to a 
> workaround for your problem, although it may not be the best solution.
> I do not have access to a windows setup to test any of these proposed 
> changes; in particular, given that I suspect that the affected files are 
> specific to some third party vendor from which you have purchased code, I do 
> not have means of investigating the actual problems/solutions.  If it would 
> be helpful, I would be happy to review any changes that you might like to 
> make to remedy the situation.
> I will be on vacation for the next few days so please excuse my delayed 
> responses.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(sorry, accidentally hit cmd-enter, not actually done writing)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

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

xbolva00 added a comment.

You dont have to use -Wno-…

You can use pragma to disable this warning for certain location in the code.

-1 for revert just because of this reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(Now completed the reply, see phab.)

In D98798#2754635 , @xbolva00 wrote:

> You dont have to use -Wno-…
>
> You can use pragma to disable this warning for certain location in the code, 
> eg include of commctrl.h.

commctrl.h is included by other system headers too though.

I don't think requiring users to add pragmas in all files that transitively 
include a random subset of windows headers is great :) I'm fine with not 
reverting if a fix for this will happen soon, but it sounds like the author is 
away for a few days. Reverts are cheap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D102338: [Sema] Always search the full function scope context if a potential availability violation is encountered

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added reviewers: erik.pilkington, rsmith, arphaman.
logan-5 added a project: clang.
logan-5 requested review of this revision.
Herald added a subscriber: cfe-commits.

Always search the full function scope context if a potential availability 
violation is encountered. This fixes both 
https://bugs.llvm.org/show_bug.cgi?id=50309 and 
https://bugs.llvm.org/show_bug.cgi?id=50310.

Previously, lambdas inside functions would mark their own bodies for later 
analysis when encountering a potentially unavailable decl, without taking into 
consideration that the entire lambda itself might be correctly guarded inside 
an `@available` check. The same applied to inner class member functions. Blocks 
happened to work as expected already, since `Sema::getEnclosingFunction()` 
skips through block scopes.

This patch instead simply and conservatively marks the entire outermost 
function scope for search, and removes some special-case logic that prevented 
`DiagnoseUnguardedAvailabilityViolations` from traversing down into lambdas and 
nested functions. This correctly accounts for arbitrarily nested lambdas, inner 
classes, and blocks that may be inside appropriate `@available` checks at any 
ancestor level. It also treats all potential availability violations inside 
functions consistently, without being overly sensitive to the current 
DeclContext, which previously caused issues where e.g. nested struct members 
were warned about twice.

`DiagnoseUnguardedAvailabilityViolations` now has more work to do in some 
cases, particularly in functions with many (possibly deeply) nested lambdas and 
classes, but the big-O is the same, and the simplicity of the approach and the 
fact that it fixes at least two bugs feels like a strong win IMO.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102338

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/unguarded-availability.m

Index: clang/test/SemaObjC/unguarded-availability.m
===
--- clang/test/SemaObjC/unguarded-availability.m
+++ clang/test/SemaObjC/unguarded-availability.m
@@ -125,6 +125,14 @@
   (void) ^{
 func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   };
+
+  if (@available(macos 10.12, *))
+(void) ^{
+  func_10_12();
+  (void) ^{
+func_10_12();
+  };
+};
 }
 
 void test_params(int_10_12 x); // expected-warning {{'int_10_12' is only available on macOS 10.12 or newer}} expected-note{{annotate 'test_params' with an availability attribute to silence this warning}}
@@ -233,6 +241,36 @@
   (void)(^ {
 func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   });
+
+  if (@available(macos 10.12, *)) {
+void([]() {
+  func_10_12();
+  void([] () {
+func_10_12();
+  });
+  struct DontWarn {
+void f() {
+  func_10_12();
+}
+  };
+});
+  }
+
+  if (@available(macos 10.12, *)) {
+struct DontWarn {
+  void f() {
+func_10_12();
+void([] () {
+  func_10_12();
+});
+struct DontWarn2 {
+  void f() {
+func_10_12();
+  }
+};
+  }
+};
+  }
 }
 
 #endif
@@ -259,9 +297,14 @@
 @end
 
 void with_local_struct() {
-  struct local { // expected-note{{annotate 'local' with an availability attribute}}
-new_int x; // expected-warning{{'new_int' is only available}}
+  struct local { 
+new_int x; // expected-warning{{'new_int' is only available}} expected-note{{enclose 'new_int' in an @available check}}
   };
+  if (@available(macos 10.12, *)) {
+struct DontWarn {
+  new_int x;
+};
+  }
 }
 
 // rdar://33156429:
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19653,12 +19653,10 @@
   if (Spec != AvailSpecs.end())
 Version = Spec->getVersion();
 
-  // The use of `@available` in the enclosing function should be analyzed to
+  // The use of `@available` in the enclosing context should be analyzed to
   // warn when it's used inappropriately (i.e. not if(@available)).
-  if (getCurFunctionOrMethodDecl())
-getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
-  else if (getCurBlock() || getCurLambda())
-getCurFunction()->HasPotentialAvailabilityViolations = true;
+  if (FunctionScopeInfo *Context = getCurFunctionAvailabilityContext())
+Context->HasPotentialAvailabilityViolations = true;
 
   return new (Context)
   ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
Index: clang/lib/Sema/SemaAvailability.cpp
===
--- clang/lib/Sema/SemaAvai

[clang] 1470b85 - Remove AST inclusion from Basic include

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2021-05-12T19:51:21+02:00
New Revision: 1470b8587f6fdc357163c2258747b77ae9ad6d7a

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

LOG: Remove AST inclusion from Basic include

That's a cyclic dependency. NFC.

Added: 


Modified: 
clang/include/clang/Basic/Thunk.h

Removed: 




diff  --git a/clang/include/clang/Basic/Thunk.h 
b/clang/include/clang/Basic/Thunk.h
index 7f3c4b24ec6d..91088be6ae73 100644
--- a/clang/include/clang/Basic/Thunk.h
+++ b/clang/include/clang/Basic/Thunk.h
@@ -15,10 +15,13 @@
 #ifndef LLVM_CLANG_BASIC_THUNK_H
 #define LLVM_CLANG_BASIC_THUNK_H
 
-#include "clang/AST/GlobalDecl.h"
+#include 
+#include 
 
 namespace clang {
 
+class CXXMethodDecl;
+
 /// A return adjustment.
 struct ReturnAdjustment {
   /// The non-virtual adjustment from the derived object to its



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

[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

[Sylvain Audi via Phabricator via cfe-commits]

saudi created this revision.
saudi added reviewers: rnk, thakis, dexonsmith, Bigcheese.
saudi added a project: clang.
saudi requested review of this revision.
Herald added a subscriber: cfe-commits.

We noticed that the dependency collectors of `clang` may report duplicate paths 
under Windows, when the differences are only in casing or separators (`/` vs 
`\`)
This problem affects the correctness of the information output by 
`clang-scan-deps`, which in turn, affects our integration with the `Fastbuild` 
cache.

For separators, it can happen as clang internally computes the full path of a 
header from an `#include` directive using native separator.

Example with argument `-I C:\includes`:
 `#include `  -> internally represented in the `FileManager` 
as `C:\includes\subdir/header.h`
 `#include "header.h"` from another header in `subdir` folder  -> 
`C:\includes\subdir\header.h`

The same applies with casing discrepancies that can be found sometimes in 
headers from external SDKs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102339

Files:
  clang/lib/Frontend/DependencyFile.cpp
  clang/test/Frontend/dependency-gen-windows-duplicates.c


Index: clang/test/Frontend/dependency-gen-windows-duplicates.c
===
--- /dev/null
+++ clang/test/Frontend/dependency-gen-windows-duplicates.c
@@ -0,0 +1,27 @@
+// REQUIRES: system-windows
+
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/subdir
+// RUN: echo > %t.dir/subdir/x.h
+// RUN: cp %s %t.dir/test.c
+// RUN: cd %t.dir
+
+// RUN: %clang -MD -MF - %t.dir/test.c -fsyntax-only -I %t.dir/subdir | 
FileCheck --ignore-case %s
+// CHECK: test.o:
+// CHECK-NEXT: \test.c
+// CHECK-NEXT: \subdir\x.h
+// File x.h must appear only once (case insensitive check).
+// CHECK-NOT: x.h
+
+// Include x.h several times, with different casing and separators.
+// Since all paths are passed to clang as absolute, all dependencies are 
absolute paths.
+// We expect the output dependencies to contain only one line for file x.h
+
+// Test case sensitivity.
+#include "subdir/x.h"
+#include "SubDir/X.h"
+
+// Test separator sensitivity:
+// clang will internally concatenates x.h paths using the Windows native 
separator.
+#include 
+
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -141,7 +141,18 @@
 }
 
 bool DependencyCollector::addDependency(StringRef Filename) {
-  if (Seen.insert(Filename).second) {
+  StringRef SearchPath;
+#ifdef _WIN32
+  // Make the search insensitive to case and separators.
+  llvm::SmallString<256> TmpPath = Filename;
+  std::replace(TmpPath.begin(), TmpPath.end(), '/', '\\');
+  std::transform(TmpPath.begin(), TmpPath.end(), TmpPath.begin(), ::tolower);
+  SearchPath = TmpPath.str();
+#else
+  SearchPath = Filename;
+#endif
+
+  if (Seen.insert(SearchPath).second) {
 Dependencies.push_back(std::string(Filename));
 return true;
   }


Index: clang/test/Frontend/dependency-gen-windows-duplicates.c
===
--- /dev/null
+++ clang/test/Frontend/dependency-gen-windows-duplicates.c
@@ -0,0 +1,27 @@
+// REQUIRES: system-windows
+
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/subdir
+// RUN: echo > %t.dir/subdir/x.h
+// RUN: cp %s %t.dir/test.c
+// RUN: cd %t.dir
+
+// RUN: %clang -MD -MF - %t.dir/test.c -fsyntax-only -I %t.dir/subdir | FileCheck --ignore-case %s
+// CHECK: test.o:
+// CHECK-NEXT: \test.c
+// CHECK-NEXT: \subdir\x.h
+// File x.h must appear only once (case insensitive check).
+// CHECK-NOT: x.h
+
+// Include x.h several times, with different casing and separators.
+// Since all paths are passed to clang as absolute, all dependencies are absolute paths.
+// We expect the output dependencies to contain only one line for file x.h
+
+// Test case sensitivity.
+#include "subdir/x.h"
+#include "SubDir/X.h"
+
+// Test separator sensitivity:
+// clang will internally concatenates x.h paths using the Windows native separator.
+#include 
+
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -141,7 +141,18 @@
 }
 
 bool DependencyCollector::addDependency(StringRef Filename) {
-  if (Seen.insert(Filename).second) {
+  StringRef SearchPath;
+#ifdef _WIN32
+  // Make the search insensitive to case and separators.
+  llvm::SmallString<256> TmpPath = Filename;
+  std::replace(TmpPath.begin(), TmpPath.end(), '/', '\\');
+  std::transform(TmpPath.begin(), TmpPath.end(), TmpPath.begin(), ::tolower);
+  SearchPath = TmpPath.str();
+#else
+  SearchPath = Filename;
+#endif
+
+  if (Seen.insert(SearchPath).second) {
 Dependencies.push_back(std::string(Filename));
 return true;
   }
___