[PATCH] D144620: [clang][driver] Handle '-mrelax' and '-mno-relax' for AVR

[Ben Shi via Phabricator via cfe-commits]

benshi001 created this revision.
benshi001 added reviewers: MaskRay, aykevl.
Herald added subscribers: Jim, dylanmckay.
Herald added a project: All.
benshi001 requested review of this revision.
Herald added subscribers: cfe-commits, jacquesguan.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144620

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-ld.c
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -67,7 +67,7 @@
 // LLD-NOT: "-mavr5"
 
 // RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree 
-mmcu=atmega328 %s -T avr.lds 2>&1 | FileCheck --check-prefix=LDS0 %s
-// LDS0: "-T" "avr.lds" "-mavr5"
+// LDS0: "-T" "avr.lds" "--relax" "-mavr5"
 
 // RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree 
-mmcu=atmega328 %s -fuse-ld=%S/Inputs/basic_avr_tree/usr/bin/ld.lld -T avr.lds 
2>&1 | FileCheck --check-prefix=LDS1 %s
 // LDS1: "-T" "avr.lds"
Index: clang/test/Driver/avr-ld.c
===
--- clang/test/Driver/avr-ld.c
+++ clang/test/Driver/avr-ld.c
@@ -1,44 +1,47 @@
 // RUN: %clang -### --target=avr -mmcu=at90s2313 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKA %s
-// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-lat90s2313" {{.*}} "--end-group" "-mavr2"
+// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-lat90s2313" {{.*}} "--end-group" "--relax" "-mavr2"
+// KINKA-NOT: "--no-relax"
 
-// RUN: %clang -### --target=avr -mmcu=at90s8515 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKB %s
-// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" "--start-group" {{.*}} 
"-lat90s8515" {{.*}} "--end-group" "-mavr2"
+// RUN: %clang -### --target=avr -mmcu=at90s8515 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s -mrelax 2>&1 | FileCheck -check-prefix LINKB %s
+// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" "--start-group" {{.*}} 
"-lat90s8515" {{.*}} "--end-group" "--relax" "-mavr2"
+// LINKB-NOT: "--no-relax"
 
-// RUN: %clang -### --target=avr -mmcu=attiny13 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKC %s
-// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" "--start-group" {{.*}} "-lattiny13" {{.*}} "--end-group" 
"-mavr25"
+// RUN: %clang -### --target=avr -mmcu=attiny13 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s -mno-relax 2>&1 | FileCheck -check-prefix LINKC %s
+// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" "--start-group" {{.*}} "-lattiny13" {{.*}} "--end-group" 
"--no-relax" "-mavr25"
+// LINLC-NOT: "--relax"
 
 // RUN: %clang -### --target=avr -mmcu=attiny44 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKD %s
-// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-lattiny44" {{.*}} "--end-group" "-mavr25"
+// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-lattiny44" {{.*}} "--end-group" "--relax" "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=atmega103 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKE %s
-// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-latmega103" {{.*}} "--end-group" "-mavr31"
+// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-latmega103" {{.*}} "--end-group" "--relax" "-mavr31"
 
 // RUN: %clang -### --target=avr -mmcu=atmega8u2 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKF %s
-// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega8u2" {{.*}} "--end-group" "-mavr35"
+// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega8u2" {{.*}} "--end-group" "--relax" "-mavr35"
 
 // RUN: %clang -### --target=avr -mmcu=atmega48pa --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKG %s
-// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega48pa" {{.*}} "--end-group" "-mavr4"
+// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega48pa" {{.*}} "--end-group" "--relax" "-mavr4"
 
 // RUN: %clang -### --target=avr -mmcu=atmega328 --rtlib=libgcc --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKH %s
-// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega328" 

[PATCH] D144431: [clang-tidy] Fix readability-identifer-naming Hungarian CString options

[Carlos Galvez via Phabricator via cfe-commits]

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

Alright, sounds good, thanks for the clarification! Then I misunderstood the 
change in the tests and indeed it belonged together in this patch, sorry for 
the confusion. I have approved both patches so we can land them now. Can you 
land the patch or would you like me to do it for you? If so please provide name 
and email (same as your Github account) for attribution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144431

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

[PATCH] D144594: [clang-tidy] Fix bugprone-copy-constructor-init documentation

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 499733.
PiotrZSL added a comment.

Update doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144594

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
@@ -3,8 +3,8 @@
 bugprone-copy-constructor-init
 ==
 
-Finds copy constructors where the constructor doesn't call
-the copy constructor of the base class.
+Finds copy constructors where the constructor doesn't call the copy constructor
+of the base class.
 
 .. code-block:: c++
 
@@ -12,7 +12,10 @@
 public:
   Copyable() = default;
   Copyable(const Copyable &) = default;
+
+  int memberToBeCopied = 0;
 };
+
 class X2 : public Copyable {
   X2(const X2 ) {} // Copyable(other) is missing
 };
@@ -22,8 +25,25 @@
 
 .. code-block:: c++
 
-class X4 : public Copyable {
-  X4(const X4 ) : Copyable() {} // other is missing
+class X3 : public Copyable {
+  X3(const X3 ) : Copyable() {} // other is missing
 };
 
+Failure to properly initialize base class sub-objects during copy construction
+can result in undefined behavior, crashes, data corruption, or other unexpected
+outcomes. The check ensures that the copy constructor of a derived class
+properly calls the copy constructor of the base class, helping to prevent bugs
+and improve code quality.
+
+Limitations:
+
+* It won't generate warnings for empty classes, as there are no class members
+  (including base class sub-objects) to worry about.
+
+* It won't generate warnings for base classes that have copy constructor
+  private or deleted.
+
+* It won't generate warnings for base classes that are initialized using other
+  non-default constructor, as this could be intentional.
+
 The check also suggests a fix-its in some cases.


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
@@ -3,8 +3,8 @@
 bugprone-copy-constructor-init
 ==
 
-Finds copy constructors where the constructor doesn't call
-the copy constructor of the base class.
+Finds copy constructors where the constructor doesn't call the copy constructor
+of the base class.
 
 .. code-block:: c++
 
@@ -12,7 +12,10 @@
 public:
   Copyable() = default;
   Copyable(const Copyable &) = default;
+
+  int memberToBeCopied = 0;
 };
+
 class X2 : public Copyable {
   X2(const X2 ) {} // Copyable(other) is missing
 };
@@ -22,8 +25,25 @@
 
 .. code-block:: c++
 
-class X4 : public Copyable {
-  X4(const X4 ) : Copyable() {} // other is missing
+class X3 : public Copyable {
+  X3(const X3 ) : Copyable() {} // other is missing
 };
 
+Failure to properly initialize base class sub-objects during copy construction
+can result in undefined behavior, crashes, data corruption, or other unexpected
+outcomes. The check ensures that the copy constructor of a derived class
+properly calls the copy constructor of the base class, helping to prevent bugs
+and improve code quality.
+
+Limitations:
+
+* It won't generate warnings for empty classes, as there are no class members
+  (including base class sub-objects) to worry about.
+
+* It won't generate warnings for base classes that have copy constructor
+  private or deleted.
+
+* It won't generate warnings for base classes that are initialized using other
+  non-default constructor, as this could be intentional.
+
 The check also suggests a fix-its in some cases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144594: [clang-tidy] Fix bugprone-copy-constructor-init documentation

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 499732.
PiotrZSL added a comment.

Update doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144594

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
@@ -3,8 +3,8 @@
 bugprone-copy-constructor-init
 ==
 
-Finds copy constructors where the constructor doesn't call
-the copy constructor of the base class.
+Finds copy constructors where the constructor doesn't call the copy constructor
+of the base class.
 
 .. code-block:: c++
 
@@ -12,7 +12,10 @@
 public:
   Copyable() = default;
   Copyable(const Copyable &) = default;
+
+  int memberToBeCopied = 0;
 };
+
 class X2 : public Copyable {
   X2(const X2 ) {} // Copyable(other) is missing
 };
@@ -22,8 +25,25 @@
 
 .. code-block:: c++
 
-class X4 : public Copyable {
-  X4(const X4 ) : Copyable() {} // other is missing
+class X3 : public Copyable {
+  X3(const X3 ) : Copyable() {} // other is missing
 };
 
+Failure to properly initialize base class sub-objects during copy construction
+can result in undefined behavior, crashes, data corruption, or other unexpected
+outcomes. The check ensures that the copy constructor of a derived class
+properly calls or initializes the copy constructor of the base class, which
+helps prevent bugs and improves code quality.
+
+Limitations:
+
+* It won't generate warnings for empty classes, as there are no class members
+  (including base class sub-objects) to worry about.
+
+* It won't generate warnings for base classes that have copy constructor
+  private or deleted.
+
+* It won't generate warnings for base classes that are initialized using other
+  non-default constructor, as this could be intentional.
+
 The check also suggests a fix-its in some cases.


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
@@ -3,8 +3,8 @@
 bugprone-copy-constructor-init
 ==
 
-Finds copy constructors where the constructor doesn't call
-the copy constructor of the base class.
+Finds copy constructors where the constructor doesn't call the copy constructor
+of the base class.
 
 .. code-block:: c++
 
@@ -12,7 +12,10 @@
 public:
   Copyable() = default;
   Copyable(const Copyable &) = default;
+
+  int memberToBeCopied = 0;
 };
+
 class X2 : public Copyable {
   X2(const X2 ) {} // Copyable(other) is missing
 };
@@ -22,8 +25,25 @@
 
 .. code-block:: c++
 
-class X4 : public Copyable {
-  X4(const X4 ) : Copyable() {} // other is missing
+class X3 : public Copyable {
+  X3(const X3 ) : Copyable() {} // other is missing
 };
 
+Failure to properly initialize base class sub-objects during copy construction
+can result in undefined behavior, crashes, data corruption, or other unexpected
+outcomes. The check ensures that the copy constructor of a derived class
+properly calls or initializes the copy constructor of the base class, which
+helps prevent bugs and improves code quality.
+
+Limitations:
+
+* It won't generate warnings for empty classes, as there are no class members
+  (including base class sub-objects) to worry about.
+
+* It won't generate warnings for base classes that have copy constructor
+  private or deleted.
+
+* It won't generate warnings for base classes that are initialized using other
+  non-default constructor, as this could be intentional.
+
 The check also suggests a fix-its in some cases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6ed67cc - [Coroutines] Remove -fcoroutines-ts

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-02-23T14:40:58+08:00
New Revision: 6ed67ccba7e4699e9e42302f2f9b7653444258ba

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

LOG: [Coroutines] Remove -fcoroutines-ts

Since we decided to remove the support for `-fcoroutines-ts` in
clang/llvm17 and the clang16/llvm16 is branched. So we're going to
remove the `-fcoroutines-ts` option.

Added: 
clang/test/Parser/cxx20-coroutines.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/StmtNodes.td
clang/include/clang/Basic/TokenKinds.def
clang/include/clang/Driver/Options.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/StmtPrinter.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Sema/TreeTransform.h
clang/test/AST/Inputs/std-coroutine-exp-namespace.h
clang/test/AST/Inputs/std-coroutine.h
clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
clang/test/CodeGenCoroutines/coro-builtins-err.c
clang/test/CodeGenCoroutines/coro-builtins.c
clang/test/CodeGenCoroutines/coro-gro2.cpp
clang/test/CodeGenCoroutines/coro-params.cpp
clang/test/CoverageMapping/coroutine.cpp
clang/test/Driver/coroutines.c
clang/test/Driver/coroutines.cpp
clang/test/Index/coroutines.cpp
clang/test/Lexer/coroutines.cpp
clang/test/Lexer/cxx-features.cpp
clang/test/Modules/requires-coroutines.mm
clang/test/PCH/coroutines.cpp
clang/test/SemaCXX/coroutine-builtins.cpp
clang/test/SemaCXX/thread-safety-coro.cpp

Removed: 
clang/test/Parser/cxx1z-coroutines.cpp
clang/test/SemaCXX/Inputs/std-coroutine-exp-namespace.h



diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c89bbc0d1786..be3ea5ff63cde 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -115,6 +115,8 @@ Removed Compiler Flags
 -
 - The deprecated flag `-fmodules-ts` is removed. Please use ``-std=c++20``
   or higher to use standard C++ modules instead.
+- The deprecated flag `-fcoroutines-ts` is removed. Please use ``-std=c++20``
+  or higher to use standard C++ coroutines instead.
 
 Attribute Changes in Clang
 --

diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index b70cf3aec5d6c..45010f19b69b2 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -978,7 +978,7 @@ class alignas(void *) Stmt {
 SourceLocation RequiresKWLoc;
   };
 
-  //===--- C++ Coroutines TS bitfields classes ---===//
+  //===--- C++ Coroutines bitfields classes ---===//
 
   class CoawaitExprBitfields {
 friend class CoawaitExpr;
@@ -1082,7 +1082,7 @@ class alignas(void *) Stmt {
 LambdaExprBitfields LambdaExprBits;
 RequiresExprBitfields RequiresExprBits;
 
-// C++ Coroutines TS expressions
+// C++ Coroutines expressions
 CoawaitExprBitfields CoawaitBits;
 
 // Obj-C Expressions

diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 77fb1e00585a0..4c922650e100f 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -637,11 +637,6 @@ def warn_drv_libstdcxx_not_found : Warning<
   "command line to use the libc++ standard library instead">,
   InGroup>;
 
-def warn_deperecated_fcoroutines_ts_flag : Warning<
-  "the '-fcoroutines-ts' flag is deprecated and it will be removed in Clang 
17; "
-  "use '-std=c++20' or higher to use standard C++ coroutines instead">,
-  InGroup;
-
 def err_drv_cannot_mix_options : Error<"cannot specify '%1' along with '%0'">;
 
 def err_drv_invalid_object_mode : Error<

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 17fdcffa2d427..d56aba34ac0a3 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -60,10 +60,8 @@ def CompoundTokenSplit : DiagGroup<"compound-token-split",
 CompoundTokenSplitBySpace]>;
 def CoroutineMissingUnhandledException :
   DiagGroup<"coroutine-missing-unhandled-exception">;
-def DeprecatedExperimentalCoroutine :
-  DiagGroup<"deprecated-experimental-coroutine">;
 def DeprecatedCoroutine :
-  DiagGroup<"deprecated-coroutine", [DeprecatedExperimentalCoroutine]>;
+  DiagGroup<"deprecated-coroutine">;
 def AlwaysInlineCoroutine :
   DiagGroup<"always-inline-coroutine">;
 def CoroNonAlignedAllocationFunction :

diff  --git a/clang/include/clang/Basic/StmtNodes.td 
b/clang/include/clang/Basic/StmtNodes.td
index 

[PATCH] D144616: Skip generating this[:1] map info for non-member variable.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 created this revision.
jyu2 added reviewers: ABataev, mikerice, jdoerfert.
jyu2 added a project: OpenMP.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1.
Herald added a project: clang.

This fix runtime problem due to generate this[:1] map info for non member 
variable.

To fix this check VD, if VD is not null, it is not member from current or base 
classes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144616

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_map_member_expr_codegen.cpp
  openmp/libomptarget/test/mapping/target_map_for_member_data.cpp

Index: openmp/libomptarget/test/mapping/target_map_for_member_data.cpp
===
--- openmp/libomptarget/test/mapping/target_map_for_member_data.cpp
+++ openmp/libomptarget/test/mapping/target_map_for_member_data.cpp
@@ -55,6 +55,26 @@
 { res = A + B; }
   }
 };
+struct descriptor {
+  int A;
+  int C;
+};
+
+class BASE {};
+
+class C : public BASE {
+public:
+  void bar(descriptor ) {
+auto Asize = 4;
+auto Csize = 4;
+
+#pragma omp target data map(to : d.A) map(from : d.C)
+{
+#pragma omp target teams firstprivate(Csize)
+  d.C = 1;
+}
+  }
+};
 
 int main(int argc, char *argv[]) {
   B b(2, 3);
@@ -65,4 +85,10 @@
   c.run();
   // CHECK: 5
   printf("c.res = %d \n", c.res);
+
+  descriptor d;
+  C z;
+  z.bar(d);
+  // CHECK 1
+  printf("%d\n", d.C);
 }
Index: clang/test/OpenMP/target_map_member_expr_codegen.cpp
===
--- clang/test/OpenMP/target_map_member_expr_codegen.cpp
+++ clang/test/OpenMP/target_map_member_expr_codegen.cpp
@@ -1,20 +1,11 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
 // RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=x86_64-pc-linux-gnu  \
 // RUN:  -x c++ -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - \
 // RUN:   | FileCheck %s
 
 // expected-no-diagnostics
 
-// CHECK: @.offload_sizes = private unnamed_addr constant [4 x i64] [i64 12, i64 4, i64 4, i64 4]
-// CHECK-NOT: @.offload_sizes = private unnamed_addr constant [4 x i64] [i64 0, i64 4, i64 4, i64 4]
 
-// CHECK-LABEL: define {{[^@]+}}@_Z3foov(
-// CHECK-NEXT:  entry:
-// CHECK-NEXT:[[B:%.*]] = alloca [[CLASS_B:%.*]], align 4
-// CHECK-NEXT:call void @_ZN1BC1Eii(ptr noundef nonnull align 4 dereferenceable(12) [[B]], i32 noundef 2, i32 noundef 3)
-// CHECK-NEXT:call void @_ZN1B3runEv(ptr noundef nonnull align 4 dereferenceable(12) [[B]])
-// CHECK-NEXT:ret void
-//
 class A {
 protected:
   int X;
@@ -29,28 +20,91 @@
   using A::Y;
 public:
   int res;
-// CHECK-LABEL: define {{[^@]+}}@_ZN1BC1Eii(
+  B (int x, int y) : A(x,y), res{0} {}
+  void run (void) {
+  #pragma omp target
+ res = X + Y;
+  }
+};
+
+template
+struct descriptor
+{
+  T *A;
+  T *C;
+  T *C_ref;
+  unsigned M;
+  unsigned K;
+  unsigned N;
+};
+
+class BASE
+{
+};
+
+//template
+class C : public BASE
+{
+public:
+  void bar (descriptor )
+  {
+ auto Asize = d.M * d.K;
+ auto Csize = d.M * d.N;
+ #pragma omp target data map(to:d.A[0:Asize]) map(from:d.C[0:Csize])
+ {
+   #pragma omp target teams firstprivate(Csize)
+   for (int i = 0; i < Csize; ++i)
+  d.C[i] = 1;
+ }
+   }
+};
+
+void foo() {
+  B b(2, 3);
+  b.run();
+  C c;
+  descriptor d;
+  c.bar(d);
+}
+// CHECK: @.offload_sizes = private unnamed_addr constant [4 x i64] [i64 12, i64 4, i64 4, i64 4]
+// CHECK-NOT: @.offload_sizes = private unnamed_addr constant [4 x i64] [i64 0, i64 4, i64 4, i64 4]
+
+// CHECK-LABEL: define {{[^@]+}}@_Z3foov
+// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[B:%.*]] = alloca [[CLASS_B:%.*]], align 4
+// CHECK-NEXT:[[C:%.*]] = alloca [[CLASS_C:%.*]], align 1
+// CHECK-NEXT:[[D:%.*]] = alloca [[STRUCT_DESCRIPTOR:%.*]], align 8
+// CHECK-NEXT:call void @_ZN1BC1Eii(ptr noundef nonnull align 4 dereferenceable(12) [[B]], i32 noundef 2, i32 noundef 3)
+// CHECK-NEXT:call void @_ZN1B3runEv(ptr noundef nonnull align 4 dereferenceable(12) [[B]])
+// CHECK-NEXT:call void @_ZN1C3barER10descriptorIfE(ptr noundef nonnull align 1 dereferenceable(1) [[C]], ptr noundef nonnull align 8 dereferenceable(40) [[D]])
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@_ZN1BC1Eii
+// CHECK-SAME: (ptr noundef nonnull align 4 dereferenceable(12) [[THIS:%.*]], i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]]) unnamed_addr #[[ATTR1:[0-9]+]] comdat align 2 {
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[THIS_ADDR:%.*]] = alloca ptr, align 

[clang-tools-extra] 3fac87b - [NFC] Remove the use of '-fcoroutines-ts' in a test of clang-tidy

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-02-23T14:16:00+08:00
New Revision: 3fac87b67749ece892fa6e797236eddc22c69ff6

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

LOG: [NFC] Remove the use of '-fcoroutines-ts' in a test of clang-tidy

Close https://github.com/llvm/llvm-project/issues/60864.

We're going to remove the support for `-fcoroutines-ts` in clang. But we
found there are additional use of `-fcoroutines-ts` in clang-tidy. This
patch removes such uses.

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
index a6a4f6e84cc90..b506b4bbe0103 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
@@ -1,7 +1,7 @@
 // Remove UNSUPPORTED for powerpc64le when the problem introduced by
 // r288563 is resolved.
 // UNSUPPORTED: target=powerpc64le{{.*}}
-// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN: %check_clang_tidy -std=c++20 %s readability-identifier-naming %t -- \
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: readability-identifier-naming.AbstractClassCase, value: 
CamelCase}, \
 // RUN: {key: readability-identifier-naming.AbstractClassPrefix, value: 
'A'}, \
@@ -81,7 +81,7 @@
 // RUN: {key: readability-identifier-naming.LocalPointerPrefix, value: 
'l_'}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerCase, 
value: CamelCase}, \
 // RUN: {key: readability-identifier-naming.LocalConstantPointerPrefix, 
value: 'lc_'}, \
-// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro -std=c++17 
-fcoroutines-ts \
+// RUN:   ]}' -- -fno-delayed-template-parsing -Dbad_macro \
 // RUN:   -I%S/Inputs/identifier-naming \
 // RUN:   -isystem %S/Inputs/identifier-naming/system
 



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

[PATCH] D144544: [OpenMP] Improve LIT tests on composite target constructs

[Animesh Kumar via Phabricator via cfe-commits]

animeshk-amd added a comment.

In D144544#4144563 , @jdoerfert wrote:

> I guess this is fine. We should try to run V tests, this is really just 
> checking if we generate (some) code.

Thanks for accepting. 
Right. By running V tests, do you mean to suggest that these combinations of 
directives should instead be tested using execution tests which are usually run 
using `ninja check-openmp` ? For example, tests in the 
"openmp/libomptarget/test/" directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144544

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

[PATCH] D144544: [OpenMP] Improve LIT tests on composite target constructs

[Animesh Kumar 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 rG265ea1c7459d: [OpenMP] Improve LIT tests on composite target 
constructs (authored by animeshk-amd).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144544

Files:
  clang/test/OpenMP/target_task_affinity_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_reduction_codegen.cpp

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

[PATCH] D144613: [RISCV] Properly diagnose mixing RVV scalable vectors with GNU vectors.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: kito-cheng, reames, frasercrmck, eopXD, rogfer01, 
c-rhodes.
Herald added subscribers: luke, VincentWu, ctetreau, vkmr, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, 
shiva0217, niosHD, sabuasal, sunfish, simoncook, johnrusso, rbar, asb, 
kristof.beyls, arichardson, dschuff.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added subscribers: alextsao1999, pcwang-thead, aheejin.
Herald added a project: clang.

This case was being picked up by SVE code and printing an SVE
specific message.

This patch distinquishes RVV from SVE and provides a correct error
message for RVV.

The use of the generic isSizelessBuiltinType was also picking up
WebAssembly reference types which was probably an accident so I've
removed that.

I've named the test similar to SVE's test that contains this check.
Their test also tests the arm_sve_vector_bits attribute. I plan to
add something similar for RISC-V soon so I've adopted this naming.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144613

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-riscv-rvv-vector-bits.c

Index: clang/test/Sema/attr-riscv-rvv-vector-bits.c
===
--- /dev/null
+++ clang/test/Sema/attr-riscv-rvv-vector-bits.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +zve64x -ffreestanding -fsyntax-only -verify %s
+
+// TODO: Support for a arm_sve_vector_bits like attribute will come in the future.
+
+#include 
+
+#define N 64
+
+typedef __rvv_int8m1_t vint8m1_t;
+typedef __rvv_uint8m1_t vuint8m1_t;
+typedef __rvv_int16m1_t vint16m1_t;
+typedef __rvv_uint16m1_t vuint16m1_t;
+typedef __rvv_int32m1_t vint32m1_t;
+typedef __rvv_uint32m1_t vuint32m1_t;
+typedef __rvv_int64m1_t vint64m1_t;
+typedef __rvv_uint64m1_t vuint64m1_t;
+typedef __rvv_float32m1_t vfloat32m1_t;
+typedef __rvv_float64m1_t vfloat64m1_t;
+
+// GNU vector types
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
+typedef int16_t gnu_int16_t __attribute__((vector_size(N / 8)));
+typedef int32_t gnu_int32_t __attribute__((vector_size(N / 8)));
+typedef int64_t gnu_int64_t __attribute__((vector_size(N / 8)));
+
+typedef uint8_t gnu_uint8_t __attribute__((vector_size(N / 8)));
+typedef uint16_t gnu_uint16_t __attribute__((vector_size(N / 8)));
+typedef uint32_t gnu_uint32_t __attribute__((vector_size(N / 8)));
+typedef uint64_t gnu_uint64_t __attribute__((vector_size(N / 8)));
+
+typedef float gnu_float32_t __attribute__((vector_size(N / 8)));
+typedef double gnu_float64_t __attribute__((vector_size(N / 8)));
+
+
+void f(int c) {
+  vint8m1_t ss8;
+  gnu_int8_t gs8;
+
+  // Check conditional expressions where the result is ambiguous are
+  // ill-formed.
+  void *sel __attribute__((unused));
+
+  sel = c ? gs8 : ss8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+  sel = c ? ss8 : gs8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  // Check binary expressions where the result is ambiguous are ill-formed.
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  ss8 += gs8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  gs8 += ss8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  ss8 = ss8 == gs8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 == ss8; // expected-error {{cannot combine GNU and RVV vectors in expression, result is ambiguous}}
+
+  ss8 = ss8 & gs8; // expected-error {{invalid operands to binary expression ('vint8m1_t' (aka '__rvv_int8m1_t') and 'gnu_int8_t' (vector of 8 'int8_t' values))}}
+
+  gs8 = gs8 & ss8; // expected-error {{invalid operands to binary expression ('gnu_int8_t' (vector of 8 'int8_t' values) and 'vint8m1_t' (aka '__rvv_int8m1_t'))}}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10768,10 +10768,12 @@
 
   // Expressions containing GNU and SVE (fixed or sizeless) vectors are invalid
   // since the ambiguity can affect the ABI.
-  auto IsSveGnuConversion = [](QualType FirstType, QualType SecondType) {
+  auto IsSveRVVGnuConversion = [](QualType FirstType, QualType SecondType,
+  unsigned ) {
 const VectorType *FirstVecType = 

[PATCH] D144352: Do not emit exception diagnostics from coroutines and coroutine lambdas

[Deniz Evrenci via Phabricator via cfe-commits]

denizevrenci added a comment.

@NoQ I don't have commit access. Should I leave landing the diff to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144352

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

[PATCH] D144611: [PowerPC] Adding test coverage for vector compatibility warning

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo created this revision.
Herald added subscribers: shchenz, nemanjai.
Herald added a project: All.
maryammo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is to test D143210  patch to have the 
same vector
compatibility logic for error and warning diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144611

Files:
  clang/test/Parser/lax-conv.cpp


Index: clang/test/Parser/lax-conv.cpp
===
--- clang/test/Parser/lax-conv.cpp
+++ clang/test/Parser/lax-conv.cpp
@@ -2,6 +2,20 @@
 // RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature 
+altivec -target-feature +vsx -target-cpu pwr8 -fsyntax-only 
-verify=expected,novsx %s
 // RUN: %clang_cc1 -triple=powerpc64-ibm-aix -target-feature +altivec 
-target-feature +vsx -target-cpu pwr8 -fsyntax-only -verify=expected,aix %s
 
+vector bool short vbs;
+vector signed short vss;
+vector unsigned short vus;
+vector bool int vbi;
+vector signed int vsi;
+vector unsigned int vui;
+vector bool long long vbl;
+vector signed long long vsl;
+vector unsigned long long vul;
+vector bool char vbc;
+vector signed char vsc;
+vector unsigned char vuc;
+vector pixel vp;
+
 void dummy(vector unsigned int a);
 template  VEC __attribute__((noinline)) test(vector unsigned 
char a, vector unsigned char b) {
 return (VEC)(a * b);
@@ -65,3 +79,34 @@
   return dummy((vector unsigned int)(ArgExplicitConvAddSame1Full +
  ArgExplicitConvAddSame2Full));
 }
+void test_bool_compat(void) {
+  vbs = vss; // expected-warning {{Implicit conversion between vector types 
(''__vector short' (vector of 8 'short' values)' and ''__vector __bool unsigned 
short' (vector of 8 'unsigned short' values)') is deprecated. In the future, 
the behavior implied by '-fno-lax-vector-conversions' will be the default.}}
+  vbs = vus; // expected-warning {{Implicit conversion between vector types 
(''__vector unsigned short' (vector of 8 'unsigned short' values)' and 
''__vector __bool unsigned short' (vector of 8 'unsigned short' values)') is 
deprecated. In the future, the behavior implied by 
'-fno-lax-vector-conversions' will be the default.}}
+
+  vbi = vsi; // expected-warning {{Implicit conversion between vector types 
(''__vector int' (vector of 4 'int' values)' and ''__vector __bool unsigned 
int' (vector of 4 'unsigned int' values)') is deprecated. In the future, the 
behavior implied by '-fno-lax-vector-conversions' will be the default.}}
+  vbi = vui; // expected-warning {{Implicit conversion between vector types 
(''__vector unsigned int' (vector of 4 'unsigned int' values)' and ''__vector 
__bool unsigned int' (vector of 4 'unsigned int' values)') is deprecated. In 
the future, the behavior implied by '-fno-lax-vector-conversions' will be the 
default.}}
+
+  vbl = vsl; // expected-warning {{Implicit conversion between vector types 
(''__vector long long' (vector of 2 'long long' values)' and ''__vector __bool 
unsigned long long' (vector of 2 'unsigned long long' values)') is deprecated. 
In the future, the behavior implied by '-fno-lax-vector-conversions' will be 
the default.}}
+  vbl = vul; // expected-warning {{Implicit conversion between vector types 
(''__vector unsigned long long' (vector of 2 'unsigned long long' values)' and 
''__vector __bool unsigned long long' (vector of 2 'unsigned long long' 
values)') is deprecated. In the future, the behavior implied by 
'-fno-lax-vector-conversions' will be the default.}}
+
+  vbc = vsc; // expected-warning {{Implicit conversion between vector types 
(''__vector signed char' (vector of 16 'signed char' values)' and ''__vector 
__bool unsigned char' (vector of 16 'unsigned char' values)') is deprecated. In 
the future, the behavior implied by '-fno-lax-vector-conversions' will be the 
default.}}
+  vbc = vuc; // expected-warning {{Implicit conversion between vector types 
(''__vector unsigned char' (vector of 16 'unsigned char' values)' and 
''__vector __bool unsigned char' (vector of 16 'unsigned char' values)') is 
deprecated. In the future, the behavior implied by 
'-fno-lax-vector-conversions' will be the default.}}
+}
+
+void test_pixel_compat(void) {
+  vp = vbs; // expected-warning {{Implicit conversion between vector types 
(''__vector __bool unsigned short' (vector of 8 'unsigned short' values)' and 
''__vector __pixel ' (vector of 8 'unsigned short' values)') is deprecated. In 
the future, the behavior implied by '-fno-lax-vector-conversions' will be the 
default.}}
+  vp = vss; // expected-warning {{Implicit conversion between vector types 
(''__vector short' (vector of 8 'short' values)' and ''__vector __pixel ' 
(vector of 8 'unsigned short' values)') is deprecated. In the future, the 
behavior implied by '-fno-lax-vector-conversions' will be the default.}}
+  vp = vus; // expected-warning 

[PATCH] D144035: [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:110
+  const auto *MoveCall = Result.Nodes.getNodeAs("move-call");
+  if (!MoveCall) {
+diag(Param->getLocation(),

PiotrZSL wrote:
> consider style:
> if (MoveCall) return;
> 
>  
I left this as is as I recall seeing on other phabs a preference towards 
keeping the curly braces for expressions that (after taking into account line 
length style rules) span multiple lines. I think it helps my own readability in 
these situations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter marked 4 inline comments as done.
ccotter added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:64-74
+  hasAncestor(compoundStmt(hasParent(lambdaExpr(
+  has(cxxRecordDecl(
+  has(cxxMethodDecl(ToParam, hasName("operator()"),
+  optionally(hasDescendant(MoveCallMatcher)),
+  hasAncestor(cxxConstructorDecl(
+  ToParam, isDefinition(), unless(isMoveConstructor()),
+  optionally(hasDescendant(MoveCallMatcher,

PiotrZSL wrote:
> looks like lambda context is visible as both operator() and as body to 
> lambaExpr directly.
> this mean that it may match twice, once as functionDecl, and once as 
> lambdaExpr.
> You can merge functionDecl with cxxConstructorDecl, just do same trick like 
> with isMoveAssignmentOperator.
> 
> I would probably start with functionDecl, and then would try do some trick 
> like forEachParam, but we dont have such matcher
> You missing also isDefinition() in functionDecl even that you got hasBody.
I don't remember why I ever had lambda specific logic here, but it doesn't seem 
necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 499681.
ccotter marked 4 inline comments as done.
ccotter added a comment.

- More matcher simplification


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -0,0 +1,328 @@
+// RUN: %check_clang_tidy -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowAnySubExpr, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,CXX14 -std=c++14 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowAnySubExpr, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,NOSUBEXPR -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowAnySubExpr, value: false},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,UNNAMED -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowAnySubExpr, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: false},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,NONDEDUCED -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.AllowAnySubExpr, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreNonDeducedTemplateTypes, value: false}]}" -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template 
+struct remove_reference;
+
+template  struct remove_reference { typedef _Tp type; };
+template  struct remove_reference<_Tp&> { typedef _Tp type; };
+template  struct remove_reference<_Tp&&> { typedef _Tp type; };
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &(_Tp &&__t) noexcept;
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept;
+
+}
+// NOLINTEND
+
+struct Obj {
+  Obj();
+  Obj(const Obj&);
+  Obj& operator=(const Obj&);
+  Obj(Obj&&);
+  Obj& operator=(Obj&&);
+  void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void just_a_declaration(Obj&& o);
+
+void copies_object(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj copy = o;
+}
+
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) 

[PATCH] D144352: Do not emit exception diagnostics from coroutines and coroutine lambdas

[Artem Dergachev via Phabricator via cfe-commits]

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

Makes sense to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144352

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

[PATCH] D144035: [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

@pcc does this lgty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[Lang Hames via Phabricator via cfe-commits]

lhames added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3738
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be 
set.");
-
   Value *MulOp0 = MulOp->getOperand(0);

craig.topper wrote:
> kpn wrote:
> > If I'm reading this right it looks like the assert() wasn't needed before. 
> > Do we know why it was added in the first place?
> I don't. I've added @lhames who wrote the code originally, but's been 10 
> years.
It was probably included as a guard rail during bringup (from memory we didn't 
have any patterns in LLVM that matched both negations) and was never removed. I 
think it should be fine to remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

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

[PATCH] D144603: Add option to disable compiler launcher on external projects

[Haowei Wu via Phabricator via cfe-commits]

haowei created this revision.
haowei added a reviewer: phosek.
Herald added a subscriber: abrachet.
Herald added a project: All.
haowei requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

When using compiler caching program like ccache, there are cases that a user 
would like to disable using it when building external projects like the 2nd 
stage clang or compiler runtimes to avoid polluting the cache due to using a 
fresh from source toolchain. This patch adds 
"LLVM_DISABLE_COMPILER_LAUNCHER_FOR_EXT_PROJECT" option so the compiler 
launcher can be disabled in these cases.

I would appreciate it if someone can suggest a better name for this option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144603

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake


Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -302,6 +302,13 @@
 list(APPEND compiler_args -DCMAKE_ASM_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
   endif()
 
+  set(C_COMPILER_LAUNCHER ${CMAKE_C_COMPILER_LAUNCHER})
+  set(CXX_COMPILER_LAUNCHER ${CMAKE_CXX_COMPILER_LAUNCHER})
+  if (LLVM_DISABLE_COMPILER_LAUNCHER_FOR_EXT_PROJECT)
+set(C_COMPILER_LAUNCHER "")
+set(CXX_COMPILER_LAUNCHER "")
+  endif()
+
   ExternalProject_Add(${name}
 DEPENDS ${ARG_DEPENDS} llvm-config
 ${name}-clobber
@@ -327,8 +334,8 @@
-DPACKAGE_VERSION=${PACKAGE_VERSION}
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-   -DCMAKE_C_COMPILER_LAUNCHER=${CMAKE_C_COMPILER_LAUNCHER}
-   -DCMAKE_CXX_COMPILER_LAUNCHER=${CMAKE_CXX_COMPILER_LAUNCHER}
+   -DCMAKE_C_COMPILER_LAUNCHER=${C_COMPILER_LAUNCHER}
+   -DCMAKE_CXX_COMPILER_LAUNCHER=${CXX_COMPILER_LAUNCHER}
-DCMAKE_EXPORT_COMPILE_COMMANDS=1
${cmake_args}
${PASSTHROUGH_VARIABLES}
Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -16,6 +16,7 @@
 set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
 set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+set(LLVM_DISABLE_COMPILER_LAUNCHER_FOR_EXT_PROJECT ON CACHE BOOL "")
 
 # Passthrough stage1 flags to stage1.
 set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,7 @@
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_STATIC_LINK_CXX_STDLIB ON CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLVM_DISABLE_COMPILER_LAUNCHER_FOR_EXT_PROJECT ON CACHE BOOL "")
 
 if(WIN32)
   set(LLVM_USE_CRT_RELEASE "MT" CACHE STRING "")
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -661,14 +661,18 @@
 LLVM_VERSION_SUFFIX
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
-CMAKE_C_COMPILER_LAUNCHER
-CMAKE_CXX_COMPILER_LAUNCHER
 CMAKE_MAKE_PROGRAM
 CMAKE_OSX_ARCHITECTURES
 CMAKE_BUILD_TYPE
 LLVM_ENABLE_PROJECTS
 LLVM_ENABLE_RUNTIMES)
 
+  if (NOT LLVM_DISABLE_COMPILER_LAUNCHER_FOR_EXT_PROJECT)
+list(APPEND _BOOTSTRAP_DEFAULT_PASSTHROUGH
+  CMAKE_C_COMPILER_LAUNCHER
+  CMAKE_CXX_COMPILER_LAUNCHER)
+  endif()
+
   # We don't need to depend on compiler-rt/libcxx if we're building 
instrumented
   # because the next stage will use the same compiler used to build this stage.
   if(NOT LLVM_BUILD_INSTRUMENTED)


Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -302,6 +302,13 @@
 list(APPEND compiler_args -DCMAKE_ASM_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
   endif()
 
+  set(C_COMPILER_LAUNCHER ${CMAKE_C_COMPILER_LAUNCHER})
+  set(CXX_COMPILER_LAUNCHER ${CMAKE_CXX_COMPILER_LAUNCHER})
+  if (LLVM_DISABLE_COMPILER_LAUNCHER_FOR_EXT_PROJECT)
+set(C_COMPILER_LAUNCHER "")
+set(CXX_COMPILER_LAUNCHER "")
+  endif()
+
   ExternalProject_Add(${name}
 DEPENDS ${ARG_DEPENDS} llvm-config
 ${name}-clobber
@@ -327,8 +334,8 @@
-DPACKAGE_VERSION=${PACKAGE_VERSION}
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-   

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

> `[llvm] `

`[SCEV]` is a more common tag for ScalarEvolution patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D144035#4145662 , @MaskRay wrote:

>> Full context at https://github.com/llvm/llvm-project/issues/60668.
>
> Best to leave a simplified description in the summary so that a reader 
> doesn't have to go to the external link and summarize the discussions 
> themselves.
> And give a link to https://reviews.llvm.org/D66606
>
> This SCEV function is invoked by `FunctionPass Manager -> Loop Pass Manager 
> -> Induction Variable Users` in the codegen pipeline.

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 499668.
leonardchan marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

Files:
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by 
utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | 
FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint 
(ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 
0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: 
[-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7449,10 +7449,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7639,7 +7635,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint (ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: [-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7449,10 +7449,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7639,7 +7635,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

@JonChesterfield I don't think we should be putting any Fedora specific logic 
into clang's build system or the driver, if that's what you are suggesting.  
Fedora can always patch the compiler or install a config file to change the 
default behavior, even though this is something we try really hard to avoid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

I need this change to fix compilation in our environment which uses 
`grpc-1.42.0`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144599

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

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added reviewers: kbobyrev, kuganv.
Herald added subscribers: modimo, wenlei, kadircet, arphaman, mcrosier.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

It seems newer grpc / protobuf versions renamed
`Status::error_message()` and `Status::error_code()` to `message()`
and `code()` to prepare for replacement with `absl::Status` with the
same names.

As far as I can tell the new names are already available in the
grpc-1.36 version mentioned in the `README` file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144599

Files:
  clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,8 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(), 
static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,8 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(), static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

If fedora has a specific set of paths it rejects in DT_RUNPATH

- and libomptarget is on one of those by default
- and user executables find it there
- and we can detect we're building on fedora

then we can disable the rpath for fedora installs to system root, while keeping 
openmp binaries working when the toolchain is installed elsewhere.

Are those constraints satisfiable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144218: [Clang] [AVR] Fix USHRT_MAX for 16-bit int.

[Daniel Thornburgh via Phabricator via cfe-commits]

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



Comment at: clang/lib/Headers/limits.h:55
 #define UCHAR_MAX (__SCHAR_MAX__*2  +1)
-#define USHRT_MAX (__SHRT_MAX__ *2  +1)
+#define USHRT_MAX (__SHRT_MAX__ * 2U + 1U)
 #define UINT_MAX  (__INT_MAX__  *2U +1U)

aaron.ballman wrote:
> It's worth double-checking that this still gives the correct type for the 
> macro:
> 
> C2x 5.2.4.2.1p2: For all unsigned integer types for which  or 
>  define a macro with suffix _WIDTH holding its width N, there is a 
> macro with suffix _MAX holding the maximal value 2N − 1 that is representable 
> by the type and that has the same type as would an expression that is an 
> object of the corresponding type converted according to the integer 
> promotions. ...
Ah, thanks; it hadn't occurred to me that the type of the expression would be 
specified in the standard. It could be either `unsigned int` or `int`, 
depending on the target.

The most straightforward approach I could think of to produce the right type is:
1) Perform the arithmetic in `unsigned int` to produce the right value
2) Cast to `unsigned short` to produce the right type
3) Directly perform integer promotion using unary plus

The use of unary plus is a bit odd here, but it seems like the most direct way 
to express the logic, and the overall expression seems less fragile than the 
`#if` alternative. I've added a comment to explain this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144218

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

[PATCH] D144218: [Clang] [AVR] Fix USHRT_MAX for 16-bit int.

[Daniel Thornburgh via Phabricator via cfe-commits]

mysterymath updated this revision to Diff 499659.
mysterymath added a comment.

Corrected type of USHRT_MAX.
Added tests and release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144218

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Headers/limits.h
  clang/test/Headers/limits.cpp


Index: clang/test/Headers/limits.cpp
===
--- clang/test/Headers/limits.cpp
+++ clang/test/Headers/limits.cpp
@@ -3,6 +3,10 @@
 // RUN: %clang_cc1 -std=c++11 -ffreestanding -fsyntax-only -verify %s
 // RUN: %clang_cc1 -std=c17 -ffreestanding -fsyntax-only -verify -x c %s
 // RUN: %clang_cc1 -std=c2x -ffreestanding -fsyntax-only -verify -x c %s
+
+// Specifically test 16-bit int platforms.
+// RUN: %clang_cc1 -triple=avr -ffreestanding -fsyntax-only -verify -x c %s
+
 // expected-no-diagnostics
 
 #include 
@@ -24,6 +28,16 @@
 
 _Static_assert(UCHAR_MAX == (unsigned char)~0ULL, "");
 _Static_assert(USHRT_MAX == (unsigned short)~0ULL, "");
+
+#if __cplusplus
+#define EXPR_TYPE_IS(EXPR, TYP) __is_same(__typeof(EXPR), TYP)
+#else
+#define EXPR_TYPE_IS(EXPR, TYP) _Generic(EXPR, TYP: 1, default: 0)
+#endif
+_Static_assert(USHRT_MAX <= INT_MAX ?
+ EXPR_TYPE_IS(USHRT_MAX, int) :
+ EXPR_TYPE_IS(USHRT_MAX, unsigned int), "");
+
 _Static_assert(UINT_MAX == (unsigned int)~0ULL, "");
 _Static_assert(ULONG_MAX == (unsigned long)~0ULL, "");
 
Index: clang/lib/Headers/limits.h
===
--- clang/lib/Headers/limits.h
+++ clang/lib/Headers/limits.h
@@ -52,7 +52,10 @@
 #define LONG_MIN  (-__LONG_MAX__ -1L)
 
 #define UCHAR_MAX (__SCHAR_MAX__*2  +1)
-#define USHRT_MAX (__SHRT_MAX__ *2  +1)
+/* This isn't safe to compute in type int on 16-bit int platforms, so compute 
it
+ * in unsigned int, cast to unsigned short, and perform the regular integer
+ * promotion via unary plus. */
+#define USHRT_MAX (+(unsigned short)(__SHRT_MAX__ * 2U + 1U))
 #define UINT_MAX  (__INT_MAX__  *2U +1U)
 #define ULONG_MAX (__LONG_MAX__ *2UL+1UL)
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -40,6 +40,10 @@
 
 C/C++ Language Potentially Breaking Changes
 ---
+- The definition of ``USHRT_MAX`` in the freestanding  no longer
+  overflows on AVR (where ``sizeof(unsigned int) == sizeof(unsigned short)``).
+  The type of ``USHRT_MAX`` on AVR is now ``unsigned int`` instead of ``int``,
+  as required by the C standard.
 
 C++ Specific Potentially Breaking Changes
 -


Index: clang/test/Headers/limits.cpp
===
--- clang/test/Headers/limits.cpp
+++ clang/test/Headers/limits.cpp
@@ -3,6 +3,10 @@
 // RUN: %clang_cc1 -std=c++11 -ffreestanding -fsyntax-only -verify %s
 // RUN: %clang_cc1 -std=c17 -ffreestanding -fsyntax-only -verify -x c %s
 // RUN: %clang_cc1 -std=c2x -ffreestanding -fsyntax-only -verify -x c %s
+
+// Specifically test 16-bit int platforms.
+// RUN: %clang_cc1 -triple=avr -ffreestanding -fsyntax-only -verify -x c %s
+
 // expected-no-diagnostics
 
 #include 
@@ -24,6 +28,16 @@
 
 _Static_assert(UCHAR_MAX == (unsigned char)~0ULL, "");
 _Static_assert(USHRT_MAX == (unsigned short)~0ULL, "");
+
+#if __cplusplus
+#define EXPR_TYPE_IS(EXPR, TYP) __is_same(__typeof(EXPR), TYP)
+#else
+#define EXPR_TYPE_IS(EXPR, TYP) _Generic(EXPR, TYP: 1, default: 0)
+#endif
+_Static_assert(USHRT_MAX <= INT_MAX ?
+ EXPR_TYPE_IS(USHRT_MAX, int) :
+ EXPR_TYPE_IS(USHRT_MAX, unsigned int), "");
+
 _Static_assert(UINT_MAX == (unsigned int)~0ULL, "");
 _Static_assert(ULONG_MAX == (unsigned long)~0ULL, "");
 
Index: clang/lib/Headers/limits.h
===
--- clang/lib/Headers/limits.h
+++ clang/lib/Headers/limits.h
@@ -52,7 +52,10 @@
 #define LONG_MIN  (-__LONG_MAX__ -1L)
 
 #define UCHAR_MAX (__SCHAR_MAX__*2  +1)
-#define USHRT_MAX (__SHRT_MAX__ *2  +1)
+/* This isn't safe to compute in type int on 16-bit int platforms, so compute it
+ * in unsigned int, cast to unsigned short, and perform the regular integer
+ * promotion via unary plus. */
+#define USHRT_MAX (+(unsigned short)(__SHRT_MAX__ * 2U + 1U))
 #define UINT_MAX  (__INT_MAX__  *2U +1U)
 #define ULONG_MAX (__LONG_MAX__ *2UL+1UL)
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -40,6 +40,10 @@
 
 C/C++ Language Potentially Breaking Changes
 ---
+- The definition of ``USHRT_MAX`` in the freestanding  no longer
+  overflows on AVR (where ``sizeof(unsigned int) == 

[PATCH] D144595: [WIP] Experimenting building Windows runtimes when cross compiling LLVM toolchain

[Haowei Wu via Phabricator via cfe-commits]

haowei created this revision.
haowei added a reviewer: phosek.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
haowei requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This is a working in progress patch, and it needs clean up and refactor. It is  
mainly for understanding what are needed to build windows runtime when the LLVM 
is being cross compiled for a different architecture. And to discuss the 
direction of refactoring the cmake configurations for the clang bootstrap build 
as well as LLVM runtimes build.

The use case is to include the Windows runtime build when cross compiling LLVM 
for an arch different from the host (e.g. building aarch64 on an x64 machine). 
This will be needed in the future when cross compile LLVM Windows toolchain for 
aarch64 on an x64 machine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144595

Files:
  clang/CMakeLists.txt
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  llvm/runtimes/CMakeLists.txt

Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -77,6 +77,12 @@
 
   set_enable_per_target_runtime_dir()
 
+  message(NOTICE "builtin target for default")
+  #message(NOTICE "cmake args ${CMAKE_ARGS}")
+  message(NOTICE "cmake common args ${COMMON_CMAKE_ARGS}")
+  message(NOTICE "extra args:")
+  message(NOTICE ${BUILTINS_CMAKE_ARGS})
+
   llvm_ExternalProject_Add(builtins
${compiler_rt_path}/lib/builtins
DEPENDS ${ARG_DEPENDS}
@@ -111,6 +117,12 @@
 endif()
   endforeach()
 
+  message(NOTICE "builtin target for ${target}")
+  #message(NOTICE "cmake args ${CMAKE_ARGS}")
+  message(NOTICE "cmake common args ${COMMON_CMAKE_ARGS}")
+  message(NOTICE "extra args:")
+  message(NOTICE ${${target}_extra_args})
+
   llvm_ExternalProject_Add(builtins-${target}
${compiler_rt_path}/lib/builtins
DEPENDS ${ARG_DEPENDS}
@@ -221,6 +233,13 @@
 
   set_enable_per_target_runtime_dir()
 
+  message(NOTICE "runtime target for default")
+  #message(NOTICE "cmake args ${CMAKE_ARGS}")
+  message(NOTICE "cmake common args ${COMMON_CMAKE_ARGS}")
+  message(NOTICE "extra args:")
+  message(NOTICE ${RUNTIMES_CMAKE_ARGS})
+  message(NOTICE ${ARG_CMAKE_ARGS})
+
   llvm_ExternalProject_Add(runtimes
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${ARG_DEPENDS}
@@ -349,6 +368,12 @@
 
   set_enable_per_target_runtime_dir()
 
+  message(NOTICE "runtime target for ${target}")
+  #message(NOTICE "cmake args ${CMAKE_ARGS}")
+  message(NOTICE "cmake common args ${COMMON_CMAKE_ARGS}")
+  message(NOTICE "extra args:")
+  message(NOTICE ${${name}_extra_args})
+
   llvm_ExternalProject_Add(runtimes-${name}
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${${name}_deps}
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -165,6 +165,9 @@
 endforeach()
   endforeach()
 
+  message(NOTICE "extproj ARG_USE_TOOLCHAIN is ${ARG_USE_TOOLCHAIN}")
+  message(NOTICE "extproj CMAKE_CROSSCOMPILING is ${CMAKE_CROSSCOMPILING}")
+  message(NOTICE "extproj CLANG_IN_TOOLCHAIN is ${CLANG_IN_TOOLCHAIN}")
   if(ARG_USE_TOOLCHAIN AND NOT CMAKE_CROSSCOMPILING)
 if(CLANG_IN_TOOLCHAIN)
   if(is_msvc_target)
@@ -249,18 +252,40 @@
   endif()
 
   if(CMAKE_CROSSCOMPILING OR _cmake_system_name STREQUAL AIX)
-set(compiler_args -DCMAKE_ASM_COMPILER=${CMAKE_ASM_COMPILER}
-  -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
-  -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
-  -DCMAKE_LINKER=${CMAKE_LINKER}
-  -DCMAKE_AR=${CMAKE_AR}
-  -DCMAKE_RANLIB=${CMAKE_RANLIB}
-  -DCMAKE_NM=${CMAKE_NM}
-  -DCMAKE_OBJCOPY=${CMAKE_OBJCOPY}
-  -DCMAKE_OBJDUMP=${CMAKE_OBJDUMP}
-  -DCMAKE_STRIP=${CMAKE_STRIP}
-  -DCMAKE_READELF=${CMAKE_READELF})
+if (ARG_USE_TOOLCHAIN AND CLANG_IN_TOOLCHAIN AND is_msvc_target)
+  get_filename_component(HOST_TOOLCHAIN_DIR ${CMAKE_C_COMPILER} DIRECTORY)
+  set(compiler_args -DCMAKE_C_COMPILER=${HOST_TOOLCHAIN_DIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX}
+-DCMAKE_CXX_COMPILER=${HOST_TOOLCHAIN_DIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX}
+-DCMAKE_ASM_COMPILER=${HOST_TOOLCHAIN_DIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX})
+  if(lld IN_LIST TOOLCHAIN_TOOLS)
+list(APPEND compiler_args 

[PATCH] D144594: [clang-tidy] Fix bugprone-copy-constructor-init documentation

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst:37
+
+In summary check detects cases where the copy constructor of a derived class
+doesn't properly call or initialize the copy constructor of the base class,

English is not my native tongue, but may be `the check` is correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144594

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

[PATCH] D144522: [clang-tidy] Add readability-operators-representation check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL added a comment.
PiotrZSL updated this revision to Diff 499306.
Eugene.Zelenko added reviewers: aaron.ballman, carlosgalvezp.
PiotrZSL marked 4 inline comments as done.
PiotrZSL updated this revision to Diff 499622.
PiotrZSL updated this revision to Diff 499626.
PiotrZSL published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

TODO: Tests still need to be extended to cover conversion of all operators in 
both direction.


PiotrZSL added a comment.

list.rst correction


Eugene.Zelenko added a comment.

Actually https://github.com/llvm/llvm-project/issues/25569 is exactly about 
this :-)


PiotrZSL added a comment.

Update tests + change config


PiotrZSL added a comment.

This check replaces D31308  and D107294 
.


PiotrZSL added a comment.

Style fixes


PiotrZSL added a comment.

Ready for review




Comment at: 
clang-tools-extra/clang-tidy/readability/OperatorsRepresentationCheck.cpp:16
+#include 
+#include 
+#include 

`cstring`, please.



Comment at: 
clang-tools-extra/clang-tidy/readability/OperatorsRepresentationCheck.cpp:29
+
+  auto  = Context.getSourceManager();
+

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



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:154
+**Unlock the Power of Clarity - Enforce Alternative Token Representation with
+Clang-Tidy for Crisp and Consistent Code!**
+

`Clang-tidy`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst:159
+---
+
+

Excessive newline.


Check helps enforce consistent token representation for binary, unary and
overloaded operators in C++ code. The check supports both traditional and
alternative representations of operators.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144522

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/OperatorsRepresentationCheck.cpp
  clang-tools-extra/clang-tidy/readability/OperatorsRepresentationCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/operators-representation.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/operators-representation-to-alternative.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/operators-representation-to-traditional.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/operators-representation-to-traditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/operators-representation-to-traditional.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s readability-operators-representation %t -- -config="{CheckOptions: [\
+// RUN: {key: readability-operators-representation.BinaryOperators, value: '&&;&=;&;|;~;!;!=;||;|=;^;^='}, \
+// RUN: {key: readability-operators-representation.OverloadedOperators, value: '&&;&=;&;|;~;!;!=;||;|=;^;^='}]}" --
+
+void testAllTokensToAlternative(int a, int b) {
+int value = 0;
+
+value = a or b;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'or' is an alternative token spelling, consider using a traditional token '||' for consistency [readability-operators-representation]
+// CHECK-FIXES: {{^}}value = a || b;{{$}}
+
+value = a and b;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'and' is an alternative token spelling, consider using a traditional token '&&' for consistency [readability-operators-representation]
+// CHECK-FIXES: {{^}}value = a && b;{{$}}
+
+value = a bitor b;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'bitor' is an alternative token spelling, consider using a traditional token '|' for consistency [readability-operators-representation]
+// CHECK-FIXES: {{^}}value = a | b;{{$}}
+
+value = a bitand b;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'bitand' is an alternative token spelling, consider using a traditional token '&' for consistency [readability-operators-representation]
+// CHECK-FIXES: {{^}}value = a & b;{{$}}
+
+value = not a;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 'not' is an alternative token spelling, consider using a traditional token '!' for consistency [readability-operators-representation]
+// CHECK-FIXES: {{^}}value = ! a;{{$}}
+
+value = a xor b;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'xor' is an alternative token spelling, consider 

[PATCH] D144594: [clang-tidy] Fix bugprone-copy-constructor-init documentation

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added a subscriber: xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Correct example, and add information about limitations.

Fixes: https://github.com/llvm/llvm-project/issues/55572


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144594

Files:
  clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
@@ -3,8 +3,8 @@
 bugprone-copy-constructor-init
 ==
 
-Finds copy constructors where the constructor doesn't call
-the copy constructor of the base class.
+Finds copy constructors where the constructor doesn't call the copy constructor
+of the base class.
 
 .. code-block:: c++
 
@@ -12,7 +12,10 @@
 public:
   Copyable() = default;
   Copyable(const Copyable &) = default;
+
+  int memberToBeCopied = 0;
 };
+
 class X2 : public Copyable {
   X2(const X2 ) {} // Copyable(other) is missing
 };
@@ -22,8 +25,28 @@
 
 .. code-block:: c++
 
-class X4 : public Copyable {
-  X4(const X4 ) : Copyable() {} // other is missing
+class X3 : public Copyable {
+  X3(const X3 ) : Copyable() {} // other is missing
 };
 
+
+Uninitialized or improperly initialized base class sub-objects during copy
+construction can cause undefined behavior, crashes, data corruption, or other
+unexpected behavior.
+
+In summary check detects cases where the copy constructor of a derived class
+doesn't properly call or initialize the copy constructor of the base class,
+helping to prevent bugs and improve code quality.
+
+Limitations:
+
+* It won't generate warnings for empty classes, as there are no class members
+  (including base class sub-objects) to worry about.
+
+* It won't generate warnings for base classes that have copy constructor
+  private or deleted.
+
+* It won't generate warnings for base classes that are initialized using other
+  non-default constructor, as this could be intentional.
+
 The check also suggests a fix-its in some cases.


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/copy-constructor-init.rst
@@ -3,8 +3,8 @@
 bugprone-copy-constructor-init
 ==
 
-Finds copy constructors where the constructor doesn't call
-the copy constructor of the base class.
+Finds copy constructors where the constructor doesn't call the copy constructor
+of the base class.
 
 .. code-block:: c++
 
@@ -12,7 +12,10 @@
 public:
   Copyable() = default;
   Copyable(const Copyable &) = default;
+
+  int memberToBeCopied = 0;
 };
+
 class X2 : public Copyable {
   X2(const X2 ) {} // Copyable(other) is missing
 };
@@ -22,8 +25,28 @@
 
 .. code-block:: c++
 
-class X4 : public Copyable {
-  X4(const X4 ) : Copyable() {} // other is missing
+class X3 : public Copyable {
+  X3(const X3 ) : Copyable() {} // other is missing
 };
 
+
+Uninitialized or improperly initialized base class sub-objects during copy
+construction can cause undefined behavior, crashes, data corruption, or other
+unexpected behavior.
+
+In summary check detects cases where the copy constructor of a derived class
+doesn't properly call or initialize the copy constructor of the base class,
+helping to prevent bugs and improve code quality.
+
+Limitations:
+
+* It won't generate warnings for empty classes, as there are no class members
+  (including base class sub-objects) to worry about.
+
+* It won't generate warnings for base classes that have copy constructor
+  private or deleted.
+
+* It won't generate warnings for base classes that are initialized using other
+  non-default constructor, as this could be intentional.
+
 The check also suggests a fix-its in some cases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

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

Thank you all for your guidance!

Now I am starting to learn C2X N2997, this is a fruitful process of learning 
for me ;-)

Cheers
Zhouyi




Comment at: clang/lib/Sema/SemaDecl.cpp:19571-19573
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.

h-vetinari wrote:
> That comment is still relevant AFAICT, at least partially (the ISO C 
> restrictions are still there, GCC still extends it)
> That comment is still relevant AFAICT, at least partially (the ISO C 
> restrictions are still there, GCC still extends it)

Thank for reviewing the patch ;-)

I think clang (with -pedantic) has emit the warning in function 
Sema::CheckEnumConstant, which behave very similar to what GCC (with -pedantic) 
do.

Cheers
Zhouyi


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

https://reviews.llvm.org/D144157

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D143306#4145603 , @tstellar wrote:

> In D143306#4145432 , @MaskRay wrote:
>
>> This is the point. Specifying a driver option to use 
>> libc++/libc++abi/libunwind doesn't magically change `DT_RUNPATH`. This is 
>> exactly the behavior a user wants for a system Clang.
>> It does make users with a non-system Clang inconvenient but that's the point 
>> that such users should specify rpath by themselves.
>> openmp should not diverge from libc++/libc++abi/libunwind in this regard.
>
> To me this is a really strong argument in favor of this change.  Why does 
> libomp need to be different here?

Because lots of HPC users wouldn't use it otherwise. (In the sense that it 
doesn't work out-of-the-box and they give up.)

To make more technical arguments why this is different:

- libomp(target) is the only one of these runtimes that has multiple levels of 
dynamically loaded runtimes (at least that I know of).
- libomp(target) is the only one that is modified and distributed under the 
same name by ~4 "other compilers" (XL, icx, amdclang, cce) and as such appears 
on a system multiple times in incompatible but somewhat "same" looking versions.
- libomp(target) is build and used by application developers directly from our 
main/dev branch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144590: Fix shared memory allocation on AMDGPU

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

Please use poison values as place holders instead of undef values as we're 
trying to get rid of Undef. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144590

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

[PATCH] D144590: Fix shared memory allocation on AMDGPU

[Shilei Tian via Phabricator via cfe-commits]

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

I think this looks reasonable to me. @jdoerfert WDYT? I'm not sure if you need 
to fix some clang tests. Let's see if Buildbot is happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144590

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

[PATCH] D144217: [clang-tidy] Fix false-positive in readability-container-size-empty

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 499643.
PiotrZSL added a comment.

Removed include


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144217

Files:
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: -config="{CheckOptions: [{key: readability-container-size-empty.ExcludedComparisonTypes , value: '::std::array;::IgnoredDummyType'}]}" \
+// RUN: -- -fno-delayed-template-parsing
 
 namespace std {
 template  struct vector {
@@ -123,12 +125,12 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
   if (intSet == std::set())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
   if (s_func() == "")
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
@@ -450,7 +452,7 @@
 public:
   ConstructWithBoolField(const std::vector ) : B(C.size()) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:57: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}ConstructWithBoolField(const std::vector ) : B(!C.empty()) {}
 };
 
@@ -458,21 +460,21 @@
   std::vector C;
   bool B = C.size();
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}bool B = !C.empty();
 };
 
 int func(const std::vector ) {
   return C.size() ? 0 : 1;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}return !C.empty() ? 0 : 1;
 }
 
 constexpr Lazy L;
 static_assert(!L.size(), "");
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :101:18: note: method 'Lazy'::empty() defined here
+// CHECK-MESSAGES: :103:18: note: method 'Lazy'::empty() defined here
 // CHECK-FIXES: {{^}}static_assert(L.empty(), "");
 
 struct StructWithLazyNoexcept {
@@ -487,7 +489,7 @@
   if (v.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v == std::vector())
 ;
@@ -496,24 +498,24 @@
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(v);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(v);
 
   TemplatedContainer templated_container;
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: 

[PATCH] D144590: Fix shared memory allocation on AMDGPU

[Zane Fink via Phabricator via cfe-commits]

ZwFink added a comment.

Full details and reproducer at this GitHub issue: 
https://github.com/llvm/llvm-project/issues/60345


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144590

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

[PATCH] D144217: [clang-tidy] Fix false-positive in readability-container-size-empty

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 499642.
PiotrZSL added a comment.

Review fixes + Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144217

Files:
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: -config="{CheckOptions: [{key: readability-container-size-empty.ExcludedComparisonTypes , value: '::std::array;::IgnoredDummyType'}]}" \
+// RUN: -- -fno-delayed-template-parsing
 
 namespace std {
 template  struct vector {
@@ -123,12 +125,12 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
   if (intSet == std::set())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
   if (s_func() == "")
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
@@ -450,7 +452,7 @@
 public:
   ConstructWithBoolField(const std::vector ) : B(C.size()) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:57: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}ConstructWithBoolField(const std::vector ) : B(!C.empty()) {}
 };
 
@@ -458,21 +460,21 @@
   std::vector C;
   bool B = C.size();
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}bool B = !C.empty();
 };
 
 int func(const std::vector ) {
   return C.size() ? 0 : 1;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}return !C.empty() ? 0 : 1;
 }
 
 constexpr Lazy L;
 static_assert(!L.size(), "");
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :101:18: note: method 'Lazy'::empty() defined here
+// CHECK-MESSAGES: :103:18: note: method 'Lazy'::empty() defined here
 // CHECK-FIXES: {{^}}static_assert(L.empty(), "");
 
 struct StructWithLazyNoexcept {
@@ -487,7 +489,7 @@
   if (v.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v == std::vector())
 ;
@@ -496,24 +498,24 @@
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(v);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(v);
 
   TemplatedContainer templated_container;
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: 

[PATCH] D144590: Fix shared memory allocation on AMDGPU

[Zane Fink via Phabricator via cfe-commits]

ZwFink created this revision.
Herald added subscribers: nlopes, kosarev, tpr, dstuttard, yaxunl, kzhuravl.
Herald added a project: All.
ZwFink requested review of this revision.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144590

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp


Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3352,7 +3352,7 @@
 llvm::Type *VarTy = CGF.ConvertTypeForMem(VD->getType());
 auto *GV = new llvm::GlobalVariable(
 CGM.getModule(), VarTy, /*isConstant=*/false,
-llvm::GlobalValue::InternalLinkage, 
llvm::Constant::getNullValue(VarTy),
+llvm::GlobalValue::InternalLinkage, llvm::UndefValue::get(VarTy),
 VD->getName(),
 /*InsertBefore=*/nullptr, llvm::GlobalValue::NotThreadLocal,
 CGM.getContext().getTargetAddressSpace(AS));


Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3352,7 +3352,7 @@
 llvm::Type *VarTy = CGF.ConvertTypeForMem(VD->getType());
 auto *GV = new llvm::GlobalVariable(
 CGM.getModule(), VarTy, /*isConstant=*/false,
-llvm::GlobalValue::InternalLinkage, llvm::Constant::getNullValue(VarTy),
+llvm::GlobalValue::InternalLinkage, llvm::UndefValue::get(VarTy),
 VD->getName(),
 /*InsertBefore=*/nullptr, llvm::GlobalValue::NotThreadLocal,
 CGM.getContext().getTargetAddressSpace(AS));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144586: [PS4/PS5] Specify no or

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

That's correct, I do see  in our SDK.

I don't see a need for a release note; we're not actually removing anything 
that we used to support.


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

https://reviews.llvm.org/D144586

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

[clang] 437727d - [AST] Add 'break; ' to the last case in two switches. NFC

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2023-02-22T13:42:40-08:00
New Revision: 437727d5865af01cf1c901eff931f5113008831c

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

LOG: [AST] Add 'break;' to the last case in two switches. NFC

Makes it easier for the switch to be extended in the future.

Added: 


Modified: 
clang/lib/AST/TypePrinter.cpp

Removed: 




diff  --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index ed3837ef01d8..4bd45768395a 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -692,6 +692,7 @@ void TypePrinter::printVectorBefore(const VectorType *T, 
raw_ostream ) {
 // Multiply by 8 for the number of bits.
 OS << ") * 8))) ";
 printBefore(T->getElementType(), OS);
+break;
   }
 }
 
@@ -757,6 +758,7 @@ void TypePrinter::printDependentVectorBefore(
 }
 OS << "))) ";
 printBefore(T->getElementType(), OS);
+break;
   }
 }
 



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

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> Full context at https://github.com/llvm/llvm-project/issues/60668.

Best to leave a simplified description in the summary so that a reader doesn't 
have to go to the external link and summarize the discussions themselves.

This SCEV function is invoked by `FunctionPass Manager -> Loop Pass Manager -> 
Induction Variable Users` in the codegen pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144572: [C++20] Stop claiming full support for consteval (for the moment!)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D144572#4145614 , @cor3ntin wrote:

> @aaron.ballman is the plan to backport that to clang 16? I think it probably 
> should.
> We ought to try to improve the situation for clang 17

cxx_status.html is always updated from trunk, no? I don't think we need to 
bother with backporting if that's the case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144572

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

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

LG




Comment at: clang/test/CodeGenCXX/pr60668.cpp:1
+// RUN: %clang --target=aarch64-unknown-linux-gnu -O1 -fvisibility=hidden \
+// RUN:   -fsanitize=hwaddress -fsanitize=undefined -std=c++17 -fno-exceptions 
\

We try avoiding .cc to assembly test. Drop this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D143891: [Clang] Adjust triviality computation in QualType::isTrivialType to C++20 cases.

[Roy Jacobson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG000ec50ef511: [Clang] Adjust triviality computation in 
QualType::isTrivialType to C++20 cases. (authored by royjacobson).

Changed prior to commit:
  https://reviews.llvm.org/D143891?vs=496884=499635#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143891

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp


Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- clang/test/SemaCXX/constrained-special-member-functions.cpp
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -265,3 +265,37 @@
 static_assert(__is_trivially_constructible(D), "");
 
 }
+
+namespace GH60697 {
+
+template 
+struct X {
+X() requires false = default;
+};
+static_assert(!__is_trivial(X));
+
+template 
+struct S {
+S() requires(__is_trivially_constructible(T)) = default;
+
+S() requires(!__is_trivially_constructible(T) &&
+  __is_constructible(T)) {}
+
+T t;
+};
+
+struct D {
+D(int i) : i(i) {}
+int i;
+};
+static_assert(!__is_trivially_constructible(D));
+static_assert(!__is_constructible(D));
+static_assert(!__is_trivial(D));
+
+static_assert(!__is_trivially_constructible(S));
+static_assert(!__is_constructible(S));
+
+static_assert(__is_trivial(S));
+static_assert(!__is_trivial(S));
+
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2501,11 +2501,13 @@
 return true;
   if (const auto *RT = CanonicalType->getAs()) {
 if (const auto *ClassDecl = dyn_cast(RT->getDecl())) {
-  // C++11 [class]p6:
-  //   A trivial class is a class that has a default constructor,
-  //   has no non-trivial default constructors, and is trivially
-  //   copyable.
-  return ClassDecl->hasDefaultConstructor() &&
+  // C++20 [class]p6:
+  //   A trivial class is a class that is trivially copyable, and
+  // has one or more eligible default constructors such that each is
+  // trivial.
+  // FIXME: We should merge this definition of triviality into
+  // CXXRecordDecl::isTrivial. Currently it computes the wrong thing.
+  return ClassDecl->hasTrivialDefaultConstructor() &&
  !ClassDecl->hasNonTrivialDefaultConstructor() &&
  ClassDecl->isTriviallyCopyable();
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -58,6 +58,8 @@
 
 ABI Changes in This Version
 ---
+- ``__is_trivial`` has changed for a small category of classes with 
constrained default constructors (`#60697 
`_).
+  *FIXME: Remove this note if we've backported this change to the Clang 16 
branch.*
 
 What's New in Clang |release|?
 ==


Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- clang/test/SemaCXX/constrained-special-member-functions.cpp
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -265,3 +265,37 @@
 static_assert(__is_trivially_constructible(D), "");
 
 }
+
+namespace GH60697 {
+
+template 
+struct X {
+X() requires false = default;
+};
+static_assert(!__is_trivial(X));
+
+template 
+struct S {
+S() requires(__is_trivially_constructible(T)) = default;
+
+S() requires(!__is_trivially_constructible(T) &&
+  __is_constructible(T)) {}
+
+T t;
+};
+
+struct D {
+D(int i) : i(i) {}
+int i;
+};
+static_assert(!__is_trivially_constructible(D));
+static_assert(!__is_constructible(D));
+static_assert(!__is_trivial(D));
+
+static_assert(!__is_trivially_constructible(S));
+static_assert(!__is_constructible(S));
+
+static_assert(__is_trivial(S));
+static_assert(!__is_trivial(S));
+
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2501,11 +2501,13 @@
 return true;
   if (const auto *RT = CanonicalType->getAs()) {
 if (const auto *ClassDecl = dyn_cast(RT->getDecl())) {
-  // C++11 [class]p6:
-  //   A trivial class is a class that has a default constructor,
-  //   has no non-trivial default constructors, and is trivially
-  //   copyable.
-  return ClassDecl->hasDefaultConstructor() &&
+  // C++20 [class]p6:
+  //   A trivial class is a class that is trivially copyable, and
+  // has one or more eligible default constructors such that each is
+  // trivial.
+  // FIXME: We should merge 

[clang] 000ec50 - [Clang] Adjust triviality computation in QualType::isTrivialType to C++20 cases.

[Roy Jacobson via cfe-commits]

Author: Roy Jacobson
Date: 2023-02-22T23:27:09+02:00
New Revision: 000ec50ef511243ec46c249c06d7a82cedf6d538

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

LOG: [Clang] Adjust triviality computation in QualType::isTrivialType to C++20 
cases.

Up to C++20, hasDefaultConstructor and !hasNonTrivialDefaultConstructor 
together implied
hasTrivialDefaultConstructor. In C++20, a constructor might be ineligible and 
can set
hasDefaultConstructor without setting hasNonTrivialDefaultConstructor.

Fix this by querying hasTrivialDefaultConstructor instead of 
hasDefaultConstructor.

I'd like to backport this to Clang 16.
I only change isTrivialType and in a way that should only affect code that uses
constrained constructors, so I think this is relatively safe to backport.

Fixes https://github.com/llvm/llvm-project/issues/60697

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/AST/Type.cpp
clang/test/SemaCXX/constrained-special-member-functions.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 24ceac59a96a2..9c89bbc0d1786 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -58,6 +58,8 @@ C++ Specific Potentially Breaking Changes
 
 ABI Changes in This Version
 ---
+- ``__is_trivial`` has changed for a small category of classes with 
constrained default constructors (`#60697 
`_).
+  *FIXME: Remove this note if we've backported this change to the Clang 16 
branch.*
 
 What's New in Clang |release|?
 ==

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index f55cb8db0f728..04359c7d82cbc 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2501,11 +2501,13 @@ bool QualType::isTrivialType(const ASTContext ) 
const {
 return true;
   if (const auto *RT = CanonicalType->getAs()) {
 if (const auto *ClassDecl = dyn_cast(RT->getDecl())) {
-  // C++11 [class]p6:
-  //   A trivial class is a class that has a default constructor,
-  //   has no non-trivial default constructors, and is trivially
-  //   copyable.
-  return ClassDecl->hasDefaultConstructor() &&
+  // C++20 [class]p6:
+  //   A trivial class is a class that is trivially copyable, and
+  // has one or more eligible default constructors such that each is
+  // trivial.
+  // FIXME: We should merge this definition of triviality into
+  // CXXRecordDecl::isTrivial. Currently it computes the wrong thing.
+  return ClassDecl->hasTrivialDefaultConstructor() &&
  !ClassDecl->hasNonTrivialDefaultConstructor() &&
  ClassDecl->isTriviallyCopyable();
 }

diff  --git a/clang/test/SemaCXX/constrained-special-member-functions.cpp 
b/clang/test/SemaCXX/constrained-special-member-functions.cpp
index 69d50eb04c572..389f80d80e563 100644
--- a/clang/test/SemaCXX/constrained-special-member-functions.cpp
+++ b/clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -265,3 +265,37 @@ static_assert(__is_trivially_constructible(C), "");
 static_assert(__is_trivially_constructible(D), "");
 
 }
+
+namespace GH60697 {
+
+template 
+struct X {
+X() requires false = default;
+};
+static_assert(!__is_trivial(X));
+
+template 
+struct S {
+S() requires(__is_trivially_constructible(T)) = default;
+
+S() requires(!__is_trivially_constructible(T) &&
+  __is_constructible(T)) {}
+
+T t;
+};
+
+struct D {
+D(int i) : i(i) {}
+int i;
+};
+static_assert(!__is_trivially_constructible(D));
+static_assert(!__is_constructible(D));
+static_assert(!__is_trivial(D));
+
+static_assert(!__is_trivially_constructible(S));
+static_assert(!__is_constructible(S));
+
+static_assert(__is_trivial(S));
+static_assert(!__is_trivial(S));
+
+}



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

[PATCH] D144572: [C++20] Stop claiming full support for consteval (for the moment!)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@aaron.ballman is the plan to backport that to clang 16? I think it probably 
should.
We ought to try to improve the situation for clang 17


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144572

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D143306#4145432 , @MaskRay wrote:

> This is the point. Specifying a driver option to use 
> libc++/libc++abi/libunwind doesn't magically change `DT_RUNPATH`. This is 
> exactly the behavior a user wants for a system Clang.
> It does make users with a non-system Clang inconvenient but that's the point 
> that such users should specify rpath by themselves.
> openmp should not diverge from libc++/libc++abi/libunwind in this regard.

To me this is a really strong argument in favor of this change.  Why does 
libomp need to be different here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144572: [C++20] Stop claiming full support for consteval (for the moment!)

[Roy Jacobson via Phabricator via cfe-commits]

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

+1 from me, at least as long as we don't define the feature test macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144572

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

[PATCH] D144586: [PS4/PS5] Specify no or

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Just to double-check, no to threads but yes to atomics? (Checking whether you 
also need `__STDC_NO_ATOMICS__` or not.)

Should this have a release note?


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

https://reviews.llvm.org/D144586

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

[PATCH] D144218: [Clang] [AVR] Fix USHRT_MAX for 16-bit int.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The changes need some test coverage and a release note.




Comment at: clang/lib/Headers/limits.h:55
 #define UCHAR_MAX (__SCHAR_MAX__*2  +1)
-#define USHRT_MAX (__SHRT_MAX__ *2  +1)
+#define USHRT_MAX (__SHRT_MAX__ * 2U + 1U)
 #define UINT_MAX  (__INT_MAX__  *2U +1U)

It's worth double-checking that this still gives the correct type for the macro:

C2x 5.2.4.2.1p2: For all unsigned integer types for which  or 
 define a macro with suffix _WIDTH holding its width N, there is a 
macro with suffix _MAX holding the maximal value 2N − 1 that is representable 
by the type and that has the same type as would an expression that is an object 
of the corresponding type converted according to the integer promotions. ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144218

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

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/SemaCXX/lambda-expressions.cpp:675-676
+StringLiteral(const char ()[N])
+__attribute__((enable_if(__builtin_strlen(array) == 2,
+  "invalid string literal")));
+};

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > 1) This code already compiles today without issue 
> > > (https://godbolt.org/z/q6hPxoWq9), so are you sure this is testing what 
> > > needs to be tested?
> > > 
> > > 2) What about for non-GNU-style attributes ([[]] attributes that 
> > > appertain to the function type)? e.g.,
> > > ```
> > > template 
> > > void foo(const char ()[N]) [[clang::annotate_type("test", array)]];
> > > ```
> > Yes, this is something that was broken by this patch.
> > enable_if did create odr uses (incorrectly). I can add a test for 
> > annotate_type but I don't think it would run into the same issue at all. 
> So I'm not certain what's being tested then -- how are you validating that 
> `array` is no longer ODR used?
> 
> In terms of `annotate_type`, that attribute accepts a string literal followed 
> by a variable number of expression arguments, so I think it should have the 
> same issues as a GNU-style attribute accepting an expression argument. I was 
> mostly trying to wrap my head around whether `[[]]` and `__attribute__` are 
> expected to have different behaviors when written in that position.
Context https://reviews.llvm.org/D124351#4101611


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

maybe we should cherry pick into 16?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144537

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

[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

Thanks for doing this @rymiel, LGTM..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144537

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

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:19113
+// parameter appertaining to the same declaration as that attribute.
+if(auto* Parm = dyn_cast(Var); Parm && Parm->getDeclContext() 
== DC)
+  return true;

cor3ntin wrote:
> aaron.ballman wrote:
> > Formatting nit.
> > 
> > I have a hazy recollection that we sometimes have a weird declaration 
> > context for parameters where the parameter doesn't think it's associated 
> > with a function DC but instead with the translation unit DC. I seem to 
> > recall this having something to do with PCH/AST importing, but I can't seem 
> > to find any open issues about it. So this might be fine, but it may have 
> > some weird edge cases still.
> Yep, parameters are all created in the top level dc before being attached to 
> the function. but by the time they get to be captured by a lambda they are 
> attached to the function
> (default parameters initializers can't capture)
Phew, thanks for checking!



Comment at: clang/test/SemaCXX/lambda-expressions.cpp:675-676
+StringLiteral(const char ()[N])
+__attribute__((enable_if(__builtin_strlen(array) == 2,
+  "invalid string literal")));
+};

cor3ntin wrote:
> aaron.ballman wrote:
> > 1) This code already compiles today without issue 
> > (https://godbolt.org/z/q6hPxoWq9), so are you sure this is testing what 
> > needs to be tested?
> > 
> > 2) What about for non-GNU-style attributes ([[]] attributes that appertain 
> > to the function type)? e.g.,
> > ```
> > template 
> > void foo(const char ()[N]) [[clang::annotate_type("test", array)]];
> > ```
> Yes, this is something that was broken by this patch.
> enable_if did create odr uses (incorrectly). I can add a test for 
> annotate_type but I don't think it would run into the same issue at all. 
So I'm not certain what's being tested then -- how are you validating that 
`array` is no longer ODR used?

In terms of `annotate_type`, that attribute accepts a string literal followed 
by a variable number of expression arguments, so I think it should have the 
same issues as a GNU-style attribute accepting an expression argument. I was 
mostly trying to wrap my head around whether `[[]]` and `__attribute__` are 
expected to have different behaviors when written in that position.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D143306#4145482 , @jdoerfert wrote:

> @jhuber6 Can you look into the last part, creating such a clang.cfg as part 
> of the build/install process?
>
> The fact that rpath is always set is a difference but I doubt it's that bad. 
> We effectively point only to the llvm runtimes, which should be what users 
> want anyway.
> If this is really a problem, maybe we can/should separate the OpenMP runtimes 
> into a subfolder and rpath (-L) that one. Users get by default the OpenMP 
> runtimes associated with the clang, but we would not pollute anything else.

If we do this as part of LLVM, what's the different between the config file and 
just setting `rpath` anyway? I figured that MaskRay was suggesting that each 
user curates their own and the LLVM install itself doesn't stipulate the 
`rpath` since it would lead to the same behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144586: [PS4/PS5] Specify no or

[Paul Robinson via Phabricator via cfe-commits]

probinson created this revision.
probinson added a reviewer: aaron.ballman.
Herald added a project: All.
probinson requested review of this revision.

We've never provided these headers so set the preprocessor
toggles to reflect that.


https://reviews.llvm.org/D144586

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/C/C11/n1460.c


Index: clang/test/C/C11/n1460.c
===
--- clang/test/C/C11/n1460.c
+++ clang/test/C/C11/n1460.c
@@ -7,9 +7,15 @@
 // If we claim to not support the feature then we expect diagnostics when
 // using that feature. Otherwise, we expect no diagnostics.
 #ifdef __STDC_NO_COMPLEX__
-  // We do not have any targets which do not support complex, so we don't
-  // expect to get into this block.
-  #error "it's unexpected that we don't support complex"
+  // PS4/PS5 set this to indicate no  but still support the
+  // _Complex syntax.
+  #ifdef __SCE__
+#define HAS_COMPLEX
+  #else
+// We do not have any other targets which do not support complex, so we
+// don't expect to get into this block.
+#error "it's unexpected that we don't support complex"
+  #endif
   float _Complex fc;
   double _Complex dc;
   long double _Complex ldc;
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -535,6 +535,8 @@
 DefineStd(Builder, "unix", Opts);
 Builder.defineMacro("__ELF__");
 Builder.defineMacro("__SCE__");
+Builder.defineMacro("__STDC_NO_COMPLEX__");
+Builder.defineMacro("__STDC_NO_THREADS__");
   }
 
 public:


Index: clang/test/C/C11/n1460.c
===
--- clang/test/C/C11/n1460.c
+++ clang/test/C/C11/n1460.c
@@ -7,9 +7,15 @@
 // If we claim to not support the feature then we expect diagnostics when
 // using that feature. Otherwise, we expect no diagnostics.
 #ifdef __STDC_NO_COMPLEX__
-  // We do not have any targets which do not support complex, so we don't
-  // expect to get into this block.
-  #error "it's unexpected that we don't support complex"
+  // PS4/PS5 set this to indicate no  but still support the
+  // _Complex syntax.
+  #ifdef __SCE__
+#define HAS_COMPLEX
+  #else
+// We do not have any other targets which do not support complex, so we
+// don't expect to get into this block.
+#error "it's unexpected that we don't support complex"
+  #endif
   float _Complex fc;
   double _Complex dc;
   long double _Complex ldc;
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -535,6 +535,8 @@
 DefineStd(Builder, "unix", Opts);
 Builder.defineMacro("__ELF__");
 Builder.defineMacro("__SCE__");
+Builder.defineMacro("__STDC_NO_COMPLEX__");
+Builder.defineMacro("__STDC_NO_THREADS__");
   }
 
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D143306#4145465 , @MaskRay wrote:

> In D143306#4144541 , @jdoerfert 
> wrote:
>
>> I'm worried this makes use of LLVM on HPC machines even harder. That said, 
>> I'm open to suggestions and I am not well versed in all the ways we can make 
>> it work.
>> Our problem is that there are N `libomptarget.so` files and N 
>> `libomptarget.nvptx.so` files on a system, including in system directories 
>> and in directories you have on your LD_LIBRARY_PATH.
>> However, we want a clang to pick up its own versions of those files. The 
>> former is linked into clang, the latter is dynamically loaded with dlopen. 
>> That is, IIRC, roughly our use case.
>>
>>> I'd argue that such systems should specify -Wl,-rpath explicitly or in a 
>>> Clang configuration file.
>>
>> Could you work me through this, please. We can't install a config file in a 
>> user or system directory. So all we have is the clang install directory.
>> Should we not set this flag but then install a file (by default) that says 
>> `-Wl,-rpath=...`. Is that what you mean? If so, what's the difference for 
>> the user?
>> Or would we add `--offload-add-rpath` to the clang build if OpenMP offload 
>> is enabled?
>
> This part of https://clang.llvm.org/docs/UsersManual.html#configuration-files 
> is relevant "... is searched for sequentially in the directories".
>
> Say my `clang` executable is at `/tmp/opt/Rel/bin/clang`. I can just create 
> `/tmp/opt/Rel/bin/clang.cfg` with one line `-Wl,-rpath=/tmp/opt/Rel/lib` 
> (there is no magic expansion as in a shell).
> Invoking `/tmp/opt/Rel/bin/clang` will get this default option (without an 
> unused command line option warning) unless `--no-default-config` is specified.
> The difference is that the rpath is now the default, instead of only when 
> `-fopenmp=libomp` is specified. But such HPC users don't appear to be averse 
> to rpath as much as we do.
>
> It should be fairly trivial to adjust one's llvm-project build/install script 
> to create such a `clang.cfg` file.

@jhuber6 Can you look into the last part, creating such a clang.cfg as part of 
the build/install process?

The fact that rpath is always set is a difference but I doubt it's that bad. We 
effectively point only to the llvm runtimes, which should be what users want 
anyway.
If this is really a problem, maybe we can/should separate the OpenMP runtimes 
into a subfolder and rpath (-L) that one. Users get by default the OpenMP 
runtimes associated with the clang, but we would not pollute anything else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D143306#4144541 , @jdoerfert wrote:

> I'm worried this makes use of LLVM on HPC machines even harder. That said, 
> I'm open to suggestions and I am not well versed in all the ways we can make 
> it work.
> Our problem is that there are N `libomptarget.so` files and N 
> `libomptarget.nvptx.so` files on a system, including in system directories 
> and in directories you have on your LD_LIBRARY_PATH.
> However, we want a clang to pick up its own versions of those files. The 
> former is linked into clang, the latter is dynamically loaded with dlopen. 
> That is, IIRC, roughly our use case.
>
>> I'd argue that such systems should specify -Wl,-rpath explicitly or in a 
>> Clang configuration file.
>
> Could you work me through this, please. We can't install a config file in a 
> user or system directory. So all we have is the clang install directory.
> Should we not set this flag but then install a file (by default) that says 
> `-Wl,-rpath=...`. Is that what you mean? If so, what's the difference for the 
> user?
> Or would we add `--offload-add-rpath` to the clang build if OpenMP offload is 
> enabled?

This part of https://clang.llvm.org/docs/UsersManual.html#configuration-files 
is relevant "... is searched for sequentially in the directories".

Say my `clang` executable is at `/tmp/opt/Rel/bin/clang`. I can just create 
`/tmp/opt/Rel/bin/clang.cfg` with one line `-Wl,-rpath=/tmp/opt/Rel/lib` (there 
is no magic expansion as in a shell).
Invoking `/tmp/opt/Rel/bin/clang` will get this default option (without an 
unused command line option warning) unless `--no-default-config` is specified.
The difference is that the rpath is now the default, instead of only when 
`-fopenmp=libomp` is specified. But such HPC users don't appear to be averse to 
rpath as much as we do.

It should be fairly trivial to adjust one's llvm-project build/install script 
to create such a `clang.cfg` file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D143306#4144518 , @JonChesterfield 
wrote:

> Marking this as "no" because as far as I can tell it'll stop anyone running 
> openmp built from source which constitutes a severe regression and I am 
> struggling to find information on what Fedora are doing here. This link 
> https://fedoraproject.org/wiki/Changes/Broken_RPATH_will_fail_rpmbuild 
> suggests changing clang to not set rpath when it would be aiming at a "system 
> directory", which is unfortunately platform specific magic strings, would be 
> fine. That is, maybe Fedora is OK with setting RPATH as long as it isn't set 
> to /usr/lib64 or possibly other unspecified strings.
>
> The use case I want to preserve is people running clang -fopenmp from a local 
> install and without setting environment variables. That means the binary 
> needs to find the shared libraries from that local install and not unrelated 
> files with the same name that happen to be under /usr somewhere.

Perhaps we simply fundamentally disagree with each other. My opinion is that 
D118493  should be reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D143306#4144396 , @JonChesterfield 
wrote:

> So what is this configuration file? Joseph found a Gentoo blog post 
> https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/
>  and I don't have a clang.cfg file in my install dir

`clang.cfg` is not provided by default. You can create it for your needs. The 
official doc is https://clang.llvm.org/docs/UsersManual.html#configuration-files

> Tests in clang look like it's a text file that you write command line flags 
> into and hopefully clang applies them, so if we write `-Wl, rpath` etc in 
> that text file and successfully distribute it to users, it'll behave 
> identically to setting rpath from clang and fedora will still be unhappy (I 
> don't see how to scope a flag to only -fopenmp, so it might set rpath on 
> every executable clang builds instead of only openmp ones, which will break 
> some applications and fail the rpm build check on all of them)
>
> I don't see how this helps anything?

It's different. With the current status quo, `-fopenmp-implicit-rpath` is the 
default and affects all distributions which include users adverse to magic 
`DT_RUNPATH` tags.
If you specify `-Wl,-rpath=...` in your build (possibly HPC related), it just 
affects your system, and Fedora and others (like I) won't complain.

`clang -fuse-ld=lld -fopenmp=libgomp -fno-openmp-implicit-rpath a.c -o a`

> I'm trying to work out how this works for libc++ and the like and as far as I 
> can see it doesn't - I think the binaries I've built go looking for libc++ in 
> the system libraries and totally ignore the one built as part of the 
> toolchain. I never noticed this because I statically link everything, am I 
> missing something here?

This is the point. Specifying a driver option to use libc++/libc++abi/libunwind 
doesn't magically change `DT_RUNPATH`. This is exactly the behavior a user 
wants for a system Clang.
It does make users with a non-system Clang inconvenient but that's the point 
that such users should specify rpath by themselves.
openmp should not diverge from libc++/libc++abi/libunwind in this regard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144431: [clang-tidy] Fix readability-identifer-naming Hungarian CString options

[Alexis Murzeau via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy:115
 value:  On
-  - key:
readability-identifier-naming.HungarianNotation.Options.TreatStructAsClass
-value:  false
+  - key:
readability-identifier-naming.HungarianNotation.General.TreatStructAsClass
+value:  true

carlosgalvezp wrote:
> Sorry, maybe I wasn't clear. Isn't this line meant to stay, since this is 
> what the patch is fixing?
The patch only fixes reading 
`readability-identifier-naming.HungarianNotation.CString.*` options.

I've updated the test completely to make options parsing issues tested.
The test was not catching the issue with `HungarianNotation.CString.*` because 
all options were set to their default value in this `.clang-tidy` file, so the 
test couldn't really check that the option was really correctly read (and it 
turns out it wasn't for `HungarianNotation.CString.*`).

This one 
(`readability-identifier-naming.HungarianNotation.General.TreatStructAsClass`) 
is correctly read, but was just misnamed in this test file with `Options` 
instead of `General`.
I found that while updating this test.

Both the code and docs currently use 
`readability-identifier-naming.HungarianNotation.General.TreatStructAsClass`.


The minimal changes in this test that reproduce the exact issue that is fixed 
here are the changes on 
`readability-identifier-naming.HungarianNotation.CString.*` options.
But I found it weird to just test them without updating all other options as 
well, that's why I've updated all options instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144431

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

[PATCH] D144582: [include-cleaner] Always treat constructor calls as implicit

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Treating constructor calls when the type name isn't explicitly spelled
can cause spurious results, so turn them into implicit references.
This doesn't change the behaviour for constructor calls that explicitly spell
the type name, as we should see a reference through the typeloc.

Fixes https://github.com/llvm/llvm-project/issues/60812


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144582

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -111,6 +111,9 @@
   testWalk("struct $explicit^S {};", "^S *y;");
   testWalk("enum $explicit^E {};", "^E *y;");
   testWalk("struct $explicit^S { static int x; };", "int y = ^S::x;");
+  // One explicit call from the TypeLoc in constructor spelling, another
+  // implicit reference through the constructor call.
+  testWalk("struct $explicit^$implicit^S { static int x; };", "auto y = 
^S();");
 }
 
 TEST(WalkAST, Alias) {
@@ -241,7 +244,7 @@
 TEST(WalkAST, ConstructExprs) {
   testWalk("struct $implicit^S {};", "S ^t;");
   testWalk("struct $implicit^S { S(); };", "S ^t;");
-  testWalk("struct $explicit^S { S(int); };", "S ^t(42);");
+  testWalk("struct $implicit^S { S(int); };", "S ^t(42);");
   testWalk("struct $implicit^S { S(int); };", "S t = ^42;");
   testWalk("namespace ns { struct S{}; } using ns::$implicit^S;", "S ^t;");
 }
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -102,9 +102,12 @@
   }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+// Always treat consturctor calls as implicit. We'll have an explicit
+// reference for the constructor calls that mention the type-name (through
+// TypeLocs). This reference only matters for cases where there's no
+// explicit syntax at all or there're only braces.
 report(E->getLocation(), getMemberProvider(E->getType()),
-   E->getParenOrBraceRange().isValid() ? RefType::Explicit
-   : RefType::Implicit);
+   RefType::Implicit);
 return true;
   }
 


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -111,6 +111,9 @@
   testWalk("struct $explicit^S {};", "^S *y;");
   testWalk("enum $explicit^E {};", "^E *y;");
   testWalk("struct $explicit^S { static int x; };", "int y = ^S::x;");
+  // One explicit call from the TypeLoc in constructor spelling, another
+  // implicit reference through the constructor call.
+  testWalk("struct $explicit^$implicit^S { static int x; };", "auto y = ^S();");
 }
 
 TEST(WalkAST, Alias) {
@@ -241,7 +244,7 @@
 TEST(WalkAST, ConstructExprs) {
   testWalk("struct $implicit^S {};", "S ^t;");
   testWalk("struct $implicit^S { S(); };", "S ^t;");
-  testWalk("struct $explicit^S { S(int); };", "S ^t(42);");
+  testWalk("struct $implicit^S { S(int); };", "S ^t(42);");
   testWalk("struct $implicit^S { S(int); };", "S t = ^42;");
   testWalk("namespace ns { struct S{}; } using ns::$implicit^S;", "S ^t;");
 }
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -102,9 +102,12 @@
   }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
+// Always treat consturctor calls as implicit. We'll have an explicit
+// reference for the constructor calls that mention the type-name (through
+// TypeLocs). This reference only matters for cases where there's no
+// explicit syntax at all or there're only braces.
 report(E->getLocation(), getMemberProvider(E->getType()),
-   E->getParenOrBraceRange().isValid() ? RefType::Explicit
-   : RefType::Implicit);
+   RefType::Implicit);
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:19113
+// parameter appertaining to the same declaration as that attribute.
+if(auto* Parm = dyn_cast(Var); Parm && Parm->getDeclContext() 
== DC)
+  return true;

aaron.ballman wrote:
> Formatting nit.
> 
> I have a hazy recollection that we sometimes have a weird declaration context 
> for parameters where the parameter doesn't think it's associated with a 
> function DC but instead with the translation unit DC. I seem to recall this 
> having something to do with PCH/AST importing, but I can't seem to find any 
> open issues about it. So this might be fine, but it may have some weird edge 
> cases still.
Yep, parameters are all created in the top level dc before being attached to 
the function. but by the time they get to be captured by a lambda they are 
attached to the function
(default parameters initializers can't capture)



Comment at: clang/test/SemaCXX/lambda-expressions.cpp:675-676
+StringLiteral(const char ()[N])
+__attribute__((enable_if(__builtin_strlen(array) == 2,
+  "invalid string literal")));
+};

aaron.ballman wrote:
> 1) This code already compiles today without issue 
> (https://godbolt.org/z/q6hPxoWq9), so are you sure this is testing what needs 
> to be tested?
> 
> 2) What about for non-GNU-style attributes ([[]] attributes that appertain to 
> the function type)? e.g.,
> ```
> template 
> void foo(const char ()[N]) [[clang::annotate_type("test", array)]];
> ```
Yes, this is something that was broken by this patch.
enable_if did create odr uses (incorrectly). I can add a test for annotate_type 
but I don't think it would run into the same issue at all. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D144579: [include-cleaner] Check macros against stdlib database

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144579

Files:
  clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
  clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
@@ -122,9 +122,17 @@
 }
 
 TEST(LocateSymbol, Stdlib) {
-  LocateExample Test("namespace std { struct vector; }");
-  EXPECT_THAT(locateSymbol(Test.findDecl("vector")),
-  ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
+  {
+LocateExample Test("namespace std { struct vector; }");
+EXPECT_THAT(
+locateSymbol(Test.findDecl("vector")),
+ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
+  }
+  {
+LocateExample Test("#define assert(x)\nvoid foo() { assert(true); }");
+EXPECT_THAT(locateSymbol(Test.findMacro("assert")),
+ElementsAre(*tooling::stdlib::Symbol::named("", "assert")));
+  }
 }
 
 TEST(LocateSymbol, Macros) {
Index: clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
===
--- clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
+++ clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Types.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -53,6 +54,12 @@
   return Result;
 }
 
+std::vector> locateMacro(const Macro ) {
+  // FIXME: Should we also provide physical locations?
+  if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName()))
+return {{*SS, Hints::CompleteSymbol}};
+  return {{M.Definition, Hints::CompleteSymbol}};
+}
 } // namespace
 
 std::vector> locateSymbol(const Symbol ) {
@@ -60,7 +67,7 @@
   case Symbol::Declaration:
 return locateDecl(S.declaration());
   case Symbol::Macro:
-return {{S.macro().Definition, Hints::CompleteSymbol}};
+return locateMacro(S.macro());
   }
   llvm_unreachable("Unknown Symbol::Kind enum");
 }


Index: clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
@@ -122,9 +122,17 @@
 }
 
 TEST(LocateSymbol, Stdlib) {
-  LocateExample Test("namespace std { struct vector; }");
-  EXPECT_THAT(locateSymbol(Test.findDecl("vector")),
-  ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
+  {
+LocateExample Test("namespace std { struct vector; }");
+EXPECT_THAT(
+locateSymbol(Test.findDecl("vector")),
+ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
+  }
+  {
+LocateExample Test("#define assert(x)\nvoid foo() { assert(true); }");
+EXPECT_THAT(locateSymbol(Test.findMacro("assert")),
+ElementsAre(*tooling::stdlib::Symbol::named("", "assert")));
+  }
 }
 
 TEST(LocateSymbol, Macros) {
Index: clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
===
--- clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
+++ clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Types.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -53,6 +54,12 @@
   return Result;
 }
 
+std::vector> locateMacro(const Macro ) {
+  // FIXME: Should we also provide physical locations?
+  if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName()))
+return {{*SS, Hints::CompleteSymbol}};
+  return {{M.Definition, Hints::CompleteSymbol}};
+}
 } // namespace
 
 std::vector> locateSymbol(const Symbol ) {
@@ -60,7 +67,7 @@
   case Symbol::Declaration:
 return locateDecl(S.declaration());
   case Symbol::Macro:
-return {{S.macro().Definition, Hints::CompleteSymbol}};
+return locateMacro(S.macro());
   }
   llvm_unreachable("Unknown Symbol::Kind enum");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144537: [clang-format] Don't move qualifiers past pointers-to-member

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:284-285
+// However,  `const Bar::*` remains the same.
+while (Next && Next->isOneOf(tok::identifier, tok::coloncolon) &&
+   !Next->startsSequence(tok::coloncolon, tok::star)) {
   Next = Next->Next;

Or:
```
while (Next && (Next->is(tok::identifier) || (Next->is(tok::coloncolon) &&
   (!Next->Next || Next->Next->isNot(tok::star) {
```
Nevertheless, I'm okay if you don't make any change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144537

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

[clang] 2893d55 - [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2023-02-22T11:01:40-08:00
New Revision: 2893d55f8f61edb2c253b960cab1107ea6c163c2

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

LOG: [Serialization] Don't warn when a deserialized category is equivalent to 
an existing one.

A single class allows multiple categories to be defined for it. But if
two of such categories have the same name, we emit a warning. It's not a
hard error but a good indication of a potential mistake.

With modules, we can end up with the same category in different modules.
Diagnosing such a situation has little value as the categories in
different modules are equivalent and don't reflect the usage of the same
name for different purposes. When we deserialize a duplicate category,
compare it to an existing one and warn only when the new one is
different.

rdar://104582081

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

Added: 


Modified: 
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/Modules/compare-objc-interface.m
clang/test/Modules/hidden-duplicates.m
clang/test/Modules/objc-categories.m

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 8cb513eff13e0..b9bbc0ec9eb28 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -14,6 +14,7 @@
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/AttrIterator.h"
 #include "clang/AST/Decl.h"
@@ -4181,23 +4182,22 @@ namespace {
   // Check for duplicate categories.
   if (Cat->getDeclName()) {
 ObjCCategoryDecl * = NameCategoryMap[Cat->getDeclName()];
-if (Existing &&
-Reader.getOwningModuleFile(Existing)
-  != Reader.getOwningModuleFile(Cat)) {
-  // FIXME: We should not warn for duplicates in diamond:
-  //
-  //   MT //
-  //  /  \//
-  // ML  MR   //
-  //  \  ///
-  //   MB //
-  //
-  // If there are duplicates in ML/MR, there will be warning when
-  // creating MB *and* when importing MB. We should not warn when
-  // importing.
-  Reader.Diag(Cat->getLocation(), diag::warn_dup_category_def)
-<< Interface->getDeclName() << Cat->getDeclName();
-  Reader.Diag(Existing->getLocation(), diag::note_previous_definition);
+if (Existing && Reader.getOwningModuleFile(Existing) !=
+Reader.getOwningModuleFile(Cat)) {
+  llvm::DenseSet> NonEquivalentDecls;
+  StructuralEquivalenceContext Ctx(
+  Cat->getASTContext(), Existing->getASTContext(),
+  NonEquivalentDecls, StructuralEquivalenceKind::Default,
+  /*StrictTypeSpelling =*/false,
+  /*Complain =*/false,
+  /*ErrorOnTagTypeMismatch =*/true);
+  if (!Ctx.IsEquivalent(Cat, Existing)) {
+// Warn only if the categories with the same name are 
diff erent.
+Reader.Diag(Cat->getLocation(), diag::warn_dup_category_def)
+<< Interface->getDeclName() << Cat->getDeclName();
+Reader.Diag(Existing->getLocation(),
+diag::note_previous_definition);
+  }
 } else if (!Existing) {
   // Record this category.
   Existing = Cat;

diff  --git a/clang/test/Modules/compare-objc-interface.m 
b/clang/test/Modules/compare-objc-interface.m
index 17a03de3ce29b..cc3b5fe60b1f5 100644
--- a/clang/test/Modules/compare-objc-interface.m
+++ b/clang/test/Modules/compare-objc-interface.m
@@ -444,3 +444,54 @@ @interface CompareLastImplAttribute: NSObject
 // expected-error@first.h:* {{'CompareLastImplAttribute' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 
'lastImplAttribute' with 'direct' attribute}}
 // expected-note-re@second.h:* {{but in {{'Second'|definition here}} found 
property 'lastImplAttribute' with 
diff erent 'direct' attribute}}
 #endif
+
+#if defined(FIRST)
+@interface CompareMatchingCategories: NSObject @end
+@interface CompareMatchingCategories(Matching)
+- (int)testMethod;
+@end
+
+@interface CompareMismatchingCategories1: NSObject @end
+@interface CompareMismatchingCategories1(Category1)
+- (void)presentMethod;
+@end
+@interface CompareMismatchingCategories2: NSObject @end
+@interface CompareMismatchingCategories2(Category2)
+@end
+
+@interface CompareDifferentCategoryNames: NSObject @end
+@interface CompareDifferentCategoryNames(CategoryFirst)
+- 

[PATCH] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

[Volodymyr Sapsai 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 rG2893d55f8f61: [Serialization] Dont warn when a 
deserialized category is equivalent to an… (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144149

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/compare-objc-interface.m
  clang/test/Modules/hidden-duplicates.m
  clang/test/Modules/objc-categories.m

Index: clang/test/Modules/objc-categories.m
===
--- clang/test/Modules/objc-categories.m
+++ clang/test/Modules/objc-categories.m
@@ -8,8 +8,6 @@
 
 @import category_bottom;
 
-// expected-note@Inputs/category_left.h:14 {{previous definition}}
-// expected-warning@Inputs/category_right.h:12 {{duplicate definition of category}}
 // expected-note@Inputs/category_top.h:1 {{receiver is instance of class declared here}}
 
 @interface Foo(Source)
Index: clang/test/Modules/hidden-duplicates.m
===
--- clang/test/Modules/hidden-duplicates.m
+++ clang/test/Modules/hidden-duplicates.m
@@ -32,6 +32,7 @@
 
 @interface NSObject @end
 @class ForwardDeclaredInterfaceWithoutDefinition;
+@interface NSObject(CategoryForTesting) @end
 
 NSObject *interfaceDefinition(NSObject *o);
 NSObject *forwardDeclaredInterface(NSObject *o);
Index: clang/test/Modules/compare-objc-interface.m
===
--- clang/test/Modules/compare-objc-interface.m
+++ clang/test/Modules/compare-objc-interface.m
@@ -444,3 +444,54 @@
 // expected-error@first.h:* {{'CompareLastImplAttribute' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'lastImplAttribute' with 'direct' attribute}}
 // expected-note-re@second.h:* {{but in {{'Second'|definition here}} found property 'lastImplAttribute' with different 'direct' attribute}}
 #endif
+
+#if defined(FIRST)
+@interface CompareMatchingCategories: NSObject @end
+@interface CompareMatchingCategories(Matching)
+- (int)testMethod;
+@end
+
+@interface CompareMismatchingCategories1: NSObject @end
+@interface CompareMismatchingCategories1(Category1)
+- (void)presentMethod;
+@end
+@interface CompareMismatchingCategories2: NSObject @end
+@interface CompareMismatchingCategories2(Category2)
+@end
+
+@interface CompareDifferentCategoryNames: NSObject @end
+@interface CompareDifferentCategoryNames(CategoryFirst)
+- (void)firstMethod:(int)x;
+@end
+#elif defined(SECOND)
+@interface CompareMatchingCategories: NSObject @end
+@interface CompareMatchingCategories(Matching)
+- (int)testMethod;
+@end
+
+@interface CompareMismatchingCategories1: NSObject @end
+@interface CompareMismatchingCategories1(Category1)
+@end
+@interface CompareMismatchingCategories2: NSObject @end
+@interface CompareMismatchingCategories2(Category2)
+- (void)presentMethod;
+@end
+
+@interface CompareDifferentCategoryNames: NSObject @end
+@interface CompareDifferentCategoryNames(CategorySecond)
+- (void)secondMethod;
+@end
+#else
+CompareMatchingCategories *compareMatchingCategories; // no diagnostic
+CompareMismatchingCategories1 *compareMismatchingCategories1;
+#ifdef TEST_MODULAR
+// expected-warning@second.h:* {{duplicate definition of category 'Category1' on interface 'CompareMismatchingCategories1'}}
+// expected-note@first.h:* {{previous definition is here}}
+#endif
+CompareMismatchingCategories2 *compareMismatchingCategories2;
+#ifdef TEST_MODULAR
+// expected-warning@second.h:* {{duplicate definition of category 'Category2' on interface 'CompareMismatchingCategories2'}}
+// expected-note@first.h:* {{previous definition is here}}
+#endif
+CompareDifferentCategoryNames *compareDifferentCategoryNames; // no diagnostic
+#endif
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -14,6 +14,7 @@
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/AttrIterator.h"
 #include "clang/AST/Decl.h"
@@ -4181,23 +4182,22 @@
   // Check for duplicate categories.
   if (Cat->getDeclName()) {
 ObjCCategoryDecl * = NameCategoryMap[Cat->getDeclName()];
-if (Existing &&
-Reader.getOwningModuleFile(Existing)
-  != Reader.getOwningModuleFile(Cat)) {
-  // FIXME: We should not warn for duplicates in diamond:
-  //
-  //   MT //
-  //  /  \//
-  // ML  MR   //
-  //  \  ///
-  //   MB //
-  //
-  // 

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 499583.
TIFitis marked an inline comment as done.
TIFitis added a comment.

Addressed reviewer comments, fixed error in MLIR translation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/include/mlir/Target/LLVMIR/Dialect/OpenMPCommon.h
  mlir/lib/Target/LLVMIR/CMakeLists.txt
  mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMPCommon.cpp

Index: mlir/lib/Target/LLVMIR/Dialect/OpenMPCommon.cpp
===
--- /dev/null
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMPCommon.cpp
@@ -0,0 +1,41 @@
+//===- OpenMPCommon.cpp - Utils for translating MLIR dialect to LLVM IR===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines general utilities for MLIR Dialect translations to LLVM IR.
+//
+//===--===//
+
+#include "mlir/Target/LLVMIR/Dialect/OpenMPCommon.h"
+
+llvm::Constant *
+mlir::LLVM::createSourceLocStrFromLocation(Location loc,
+   llvm::OpenMPIRBuilder ,
+   StringRef name, uint32_t ) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();
+unsigned colNo = fileLoc.getColumn();
+return builder.getOrCreateSrcLocStr(name, fileName, lineNo, colNo, strLen);
+  }
+  std::string locStr;
+  llvm::raw_string_ostream locOS(locStr);
+  locOS << loc;
+  return builder.getOrCreateSrcLocStr(locOS.str(), strLen);
+}
+
+llvm::Constant *
+mlir::LLVM::createMappingInformation(Location loc,
+ llvm::OpenMPIRBuilder ) {
+  uint32_t strLen;
+  if (auto nameLoc = loc.dyn_cast()) {
+StringRef name = nameLoc.getName();
+return createSourceLocStrFromLocation(nameLoc.getChildLoc(), builder, name,
+  strLen);
+  }
+  return createSourceLocStrFromLocation(loc, builder, "unknown", strLen);
+}
Index: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -15,10 +15,12 @@
 #include "mlir/IR/IRMapping.h"
 #include "mlir/IR/Operation.h"
 #include "mlir/Support/LLVM.h"
+#include "mlir/Target/LLVMIR/Dialect/OpenMPCommon.h"
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/TypeSwitch.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/IRBuilder.h"
@@ -1351,6 +1353,191 @@
   return success();
 }
 
+/// Process MapOperands for Target Data directives.
+static LogicalResult processMapOperand(
+llvm::IRBuilderBase , LLVM::ModuleTranslation ,
+const SmallVector , const ArrayAttr ,
+SmallVector ,
+SmallVectorImpl ,
+struct llvm::OpenMPIRBuilder::MapperAllocas ) {
+  auto numMapOperands = mapOperands.size();
+  llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+  llvm::PointerType *i8PtrTy = builder.getInt8PtrTy();
+  llvm::ArrayType *arrI8PtrTy = llvm::ArrayType::get(i8PtrTy, numMapOperands);
+  llvm::IntegerType *i64Ty = builder.getInt64Ty();
+  llvm::ArrayType *arrI64Ty = llvm::ArrayType::get(i64Ty, numMapOperands);
+
+  unsigned index = 0;
+  for (const auto  : mapOperands) {
+const auto  = mapTypes[index];
+
+llvm::Value *mapOpValue = moduleTranslation.lookupValue(mapOp);
+llvm::Value *mapOpPtrBase;
+llvm::Value *mapOpPtr;
+llvm::Value *mapOpSize;
+
+if (mapOp.getType().isa()) {
+  mapOpPtrBase = mapOpValue;
+  mapOpPtr = mapOpValue;
+  mapOpSize = ompBuilder->getSizeInBytes(mapOpValue);
+} else {
+  return failure();
+}
+
+// Store base pointer extracted from operand into the i-th position of
+// argBase.
+llvm::Value *ptrBaseGEP = builder.CreateInBoundsGEP(
+arrI8PtrTy, mapperAllocas.ArgsBase,
+{builder.getInt32(0), builder.getInt32(index)});
+llvm::Value *ptrBaseCast = builder.CreateBitCast(
+

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis marked 4 inline comments as done.
TIFitis added a comment.

In D142914#4144475 , 
@kiranchandramohan wrote:

> Please add tests for the MLIR portion.

Can you please tell me where to add these?

> Could you also post the full LLVM IR for a construct with the map clause?

The following is a simple example, let me know if you'd like a more complex 
example with loop :)

**Flang:**

  subroutine openmp_target_data
  integer :: i
  !$omp target data map(tofrom: i)
  i = 99;
  !$omp end target data
  end subroutine openmp_target_data

**FIR:**

  func.func @_QPopenmp_target_data() {
%0 = fir.alloca i32 {bindc_name = "i", uniq_name = 
"_QFopenmp_target_dataEi"}
omp.target_data   map((tofrom -> %0 : !fir.ref)) {
  %c99_i32 = arith.constant 99 : i32
  fir.store %c99_i32 to %0 : !fir.ref
  omp.terminator
}
return
  }

**LLVMIR:**

  llvm.func @_QPopenmp_target_data() {
%0 = llvm.mlir.constant(1 : i64) : i64
%1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, 
operand_segment_sizes = array, uniq_name = 
"_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
omp.target_data   map((tofrom -> %1 : !llvm.ptr)) {
  %2 = llvm.mlir.constant(99 : i32) : i32
  llvm.store %2, %1 : !llvm.ptr
  omp.terminator
}
llvm.return
  }

**llvm IR .ll:**

  ; ModuleID = 'LLVMDialectModule'
  source_filename = "LLVMDialectModule"
  target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
  target triple = "aarch64-unknown-linux-gnu"
  
  %struct.ident_t = type { i32, i32, i32, i32, ptr }
  
  @0 = private unnamed_addr constant [26 x i8] c";test.mlir;unknown;4;10;;\00", 
align 1
  @1 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", 
align 1
  @2 = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 
22, ptr @1 }, align 8
  @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
  @.offload_mapnames = private constant [1 x ptr] [ptr @0]
  
  declare ptr @malloc(i64)
  
  declare void @free(ptr)
  
  define void @_QPopenmp_target_data() {
%1 = alloca [1 x ptr], align 8
%2 = alloca [1 x ptr], align 8
%3 = alloca [1 x i64], align 8
%4 = alloca i32, i64 1, align 4
br label %entry
  
  entry:; preds = %0
%5 = getelementptr inbounds [1 x ptr], ptr %1, i32 0, i32 0
store ptr %4, ptr %5, align 8
%6 = getelementptr inbounds [1 x ptr], ptr %2, i32 0, i32 0
store ptr %4, ptr %6, align 8
%7 = getelementptr inbounds [1 x i64], ptr %3, i32 0, i32 0
store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr 
%7, align 8
%8 = getelementptr inbounds [1 x ptr], ptr %1, i32 0, i32 0
%9 = getelementptr inbounds [1 x ptr], ptr %2, i32 0, i32 0
%10 = getelementptr inbounds [1 x i64], ptr %3, i32 0, i32 0
call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %8, 
ptr %9, ptr %10, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
br label %omp.data.region
  
  omp.data.region:  ; preds = %entry
store i32 99, ptr %4, align 4
br label %omp.region.cont
  
  omp.region.cont:  ; preds = %omp.data.region
%11 = getelementptr inbounds [1 x ptr], ptr %1, i32 0, i32 0
%12 = getelementptr inbounds [1 x ptr], ptr %2, i32 0, i32 0
%13 = getelementptr inbounds [1 x i64], ptr %3, i32 0, i32 0
call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %11, ptr 
%12, ptr %13, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
ret void
  }
  
  ; Function Attrs: nounwind
  declare void @__tgt_target_data_begin_mapper(ptr, i64, i32, ptr, ptr, ptr, 
ptr, ptr, ptr) #0
  
  ; Function Attrs: nounwind
  declare void @__tgt_target_data_end_mapper(ptr, i64, i32, ptr, ptr, ptr, ptr, 
ptr, ptr) #0
  
  attributes #0 = { nounwind }
  
  !llvm.module.flags = !{!0}
  
  !0 = !{i32 2, !"Debug Info Version", i32 3}

Cheers,
Akash




Comment at: mlir/lib/Target/LLVMIR/Dialect/Utils.cpp:1
+//===- Utils.cpp - General Utils for translating MLIR dialect to LLVM 
IR---===//
+//

kiranchandramohan wrote:
> Nit: I would prefer the OpenMPCommon.cpp name suggested in 
> https://discourse.llvm.org/t/rfc-adding-new-util-file-to-mlir-dialect-translation/68221.
>  
Would you also like me to move the file inside OpenMP ( 
`mlir/lib/Target/LLVMIR/Dialect/OpenMP` ) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D144572: [C++20] Stop claiming full support for consteval (for the moment!)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

We've discussed this offline, and I think this is the right thing to do, but 
I'll let others comment before approving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144572

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

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:19113
+// parameter appertaining to the same declaration as that attribute.
+if(auto* Parm = dyn_cast(Var); Parm && Parm->getDeclContext() 
== DC)
+  return true;

Formatting nit.

I have a hazy recollection that we sometimes have a weird declaration context 
for parameters where the parameter doesn't think it's associated with a 
function DC but instead with the translation unit DC. I seem to recall this 
having something to do with PCH/AST importing, but I can't seem to find any 
open issues about it. So this might be fine, but it may have some weird edge 
cases still.



Comment at: clang/test/SemaCXX/lambda-expressions.cpp:675-676
+StringLiteral(const char ()[N])
+__attribute__((enable_if(__builtin_strlen(array) == 2,
+  "invalid string literal")));
+};

1) This code already compiles today without issue 
(https://godbolt.org/z/q6hPxoWq9), so are you sure this is testing what needs 
to be tested?

2) What about for non-GNU-style attributes ([[]] attributes that appertain to 
the function type)? e.g.,
```
template 
void foo(const char ()[N]) [[clang::annotate_type("test", array)]];
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/test/Driver/amdgpu-toolchain.c:7
 // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | 
FileCheck -check-prefix=DWARF_VER %s
 
 // AS_LINK: "-cc1as"

arsenm wrote:
> should add a test for thinlto? 
Only thing it would change is adding `-plugin-opt=thinlto`. Not sure if it's 
worth a test since it's probably checked by other uses of `addLTOOptions`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144505

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

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/test/Driver/amdgpu-toolchain.c:7
 // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | 
FileCheck -check-prefix=DWARF_VER %s
 
 // AS_LINK: "-cc1as"

should add a test for thinlto? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144505

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

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 499569.
Mordante added a comment.
Herald added a subscriber: mstorsjo.

Try to fix AIX CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144509

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build-py/tests/functional/exec/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  compiler-rt/lib/crt/CMakeLists.txt
  flang/CMakeLists.txt
  flang/lib/Decimal/CMakeLists.txt
  flang/runtime/CMakeLists.txt
  libc/CMakeLists.txt
  libc/examples/hello_world/CMakeLists.txt
  libclc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm-libgcc/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/CMake.rst
  llvm/docs/GettingStarted.rst
  llvm/docs/ReleaseNotes.rst
  mlir/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/cmake/DetectTestCompiler/CMakeLists.txt
  openmp/docs/SupportAndFAQ.rst
  openmp/libompd/src/CMakeLists.txt
  openmp/libomptarget/plugins/remote/src/CMakeLists.txt
  openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
  openmp/tools/Modules/FindOpenMPTarget.cmake
  openmp/tools/Modules/README.rst
  polly/CMakeLists.txt
  pstl/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -1,12 +1,5 @@
 # This file handles building LLVM runtime sub-projects.
-cmake_minimum_required(VERSION 3.13.4)
-if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
-  message(WARNING
-"Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
-"minimum version of CMake required to build LLVM will become 3.20.0, and "
-"using an older CMake will become an error. Please upgrade your CMake to "
-"at least 3.20.0 now to avoid issues in the future!")
-endif()
+cmake_minimum_required(VERSION 3.20.0)
 project(Runtimes C CXX ASM)
 
 # Add path for custom and the LLVM build's modules to the CMake module path.
Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===--===##
-cmake_minimum_required(VERSION 3.13.4)
+cmake_minimum_required(VERSION 3.20.0)
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,14 +1,7 @@
 # Check if this is a in tree build.
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
-  cmake_minimum_required(VERSION 3.13.4)
-  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
-message(WARNING
-  "Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
-  "minimum version of CMake required to build LLVM will become 3.20.0, and "
-  "using an older CMake will become an error. Please upgrade your CMake to "
-  "at least 3.20.0 now to avoid issues in the future!")
-  endif()
+  cmake_minimum_required(VERSION 3.20.0)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
Index: openmp/tools/Modules/README.rst
===
--- openmp/tools/Modules/README.rst
+++ openmp/tools/Modules/README.rst
@@ -26,7 +26,7 @@
 
 .. code-block:: cmake
 
-  cmake_minimum_required(VERSION 3.13.4)
+  cmake_minimum_required(VERSION 3.20.0)
   project(offloadTest VERSION 1.0 LANGUAGES CXX)
 
   list(APPEND CMAKE_MODULE_PATH "${PATH_TO_OPENMP_INSTALL}/lib/cmake/openmp")
@@ -37,7 +37,7 @@
   target_link_libraries(offload PRIVATE OpenMPTarget::OpenMPTarget_NVPTX)
   target_sources(offload PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src/Main.cpp)
 
-Using this module requires at least CMake version 3.13.4. Supported languages
+Using this module requires at least CMake version 3.20.0. Supported languages
 are C and C++ with Fortran support planned in the future. If your application
 requires building for a specific device architecture you can set the
 ``OpenMPTarget__ARCH=`` variable. Compiler support is best for
Index: openmp/tools/Modules/FindOpenMPTarget.cmake
===
--- openmp/tools/Modules/FindOpenMPTarget.cmake
+++ openmp/tools/Modules/FindOpenMPTarget.cmake
@@ -79,7 +79,7 @@
 # TODO: Test more compilers
 
 cmake_policy(PUSH)
-cmake_policy(VERSION 3.13.4)
+cmake_policy(VERSION 3.20.0)
 
 find_package(OpenMP ${OpenMPTarget_FIND_VERSION} 

[PATCH] D144157: Add 128-bit integer support to enum element

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this needs additional test coverage around `_BitInt` as those can be 
arbitrarily large depending on the target. Also, C2x made changes in this area 
to how we calculate what type to represent the enumerations in; we don't have 
to implement that support as part of this patch, but we should make sure what 
we're doing doesn't conflict with that future work.

(Also, please add a release note for the changes.)




Comment at: clang/lib/Sema/SemaDecl.cpp:19571-19573
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.

h-vetinari wrote:
> That comment is still relevant AFAICT, at least partially (the ISO C 
> restrictions are still there, GCC still extends it)
Not only is it still relevant, there were changes for C2x in this area. See 
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2997.htm for more details.



Comment at: clang/lib/Sema/SemaDecl.cpp:19698-19699
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&

Consider (in C2x):
```
enum E {
  Val = 0x''''''''1wb
};
```



Comment at: clang/lib/Sema/SemaDecl.cpp:19700-19702
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");

Again, `_BitInt`.


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

https://reviews.llvm.org/D144157

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

[PATCH] D144149: [Serialization] Don't warn when a deserialized category is equivalent to an existing one.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144149

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

[PATCH] D144572: [C++20] Stop claiming full support for consteval (for the moment!)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: cor3ntin, erichkeane, royjacobson, 
clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

During Clang 15, 3d2629dd3aab17098813c68b5b76bb864bc5e285 
 claimed 
we achieved full support for `consteval` in C++20. However, further testing 
shows that Clang doesn't correctly handle all of the examples from 
https://wg21.link/P1073R3 and has several other known issues that are 
preventing us from defining the `__cpp_consteval` macro.

I think we should only claim Partial support for the moment. Once we correct 
the major outstanding issues, then I think we should change the status back to 
full support and define `__cpp_consteval` at the same time (even if it's only 
to the `201811L` value instead of the latest value from C++2b). This helps 
users understand the support situation more clearly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144572

Files:
  clang/test/CXX/expr/expr.const/p6-2a.cpp
  clang/test/CXX/expr/expr.const/p8-2a.cpp
  clang/www/cxx_status.html


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1120,7 +1120,14 @@
 
   Immediate functions (consteval)
   https://wg21.link/p1073r3;>P1073R3
-  Clang 15
+  
+Clang 15 (Partial)
+  Clang still incorrectly defers some consteval executions to runtime,
+  resulting in CodeGen crashes. Additionally, Clang does not properly
+  handle default arguments in consteval functions under all
+  circumstances.
+
+  
 

 https://wg21.link/p1937r2;>P1937R2
Index: clang/test/CXX/expr/expr.const/p8-2a.cpp
===
--- /dev/null
+++ clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// expected-no-diagnostics
+
+namespace P1073R3 {
+struct N {
+  constexpr N() {}
+  N(N const&) = delete;
+};
+
+template constexpr void bad_assert_copyable() { T t; T t2 = t; }
+using ineffective = decltype(bad_assert_copyable());
+
+// bad_assert_copyable is not needed for constant evaluation
+// (and thus not instantiated)
+template consteval void assert_copyable() { T t; T t2 = t; }
+using check = decltype(assert_copyable());
+// FIXME: this should give an error because assert_copyable is instantiated
+// (because it is needed for constant evaluation), but the attempt to copy t is
+// ill-formed.
+} // namespace P1073R3
+
Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -41,3 +41,16 @@
   }
 };
 constexpr Temporary t = {3}; // expected-error {{must have constant 
destruction}} expected-note {{created here}} expected-note {{in call}}
+
+namespace P1073R3 {
+consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval auto g() { return f; }
+// FIXME: there should be no diagnostics associated with either h() or r.
+consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to 
consteval function 'P1073R3::g' is not a constant expression}} \
+ expected-note {{declared 
here}} \
+ expected-note {{pointer 
to a consteval declaration is not a constant expression}}
+constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+constexpr auto e = g();  // expected-error {{call to consteval function 
'P1073R3::g' is not a constant expression}} \
+expected-error {{constexpr variable 'e' must be 
initialized by a constant expression}} \
+expected-note 2 {{pointer to a consteval 
declaration is not a constant expression}}
+} // namespace P1073R3


Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1120,7 +1120,14 @@
 
   Immediate functions (consteval)
   https://wg21.link/p1073r3;>P1073R3
-  Clang 15
+  
+Clang 15 (Partial)
+  Clang still incorrectly defers some consteval executions to runtime,
+  resulting in CodeGen crashes. Additionally, Clang does not properly
+  handle default arguments in consteval functions under all
+  circumstances.
+
+  
 

 https://wg21.link/p1937r2;>P1937R2
Index: clang/test/CXX/expr/expr.const/p8-2a.cpp
===
--- /dev/null
+++ clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -0,0 +1,22 @@
+// RUN: 

[PATCH] D143052: [CMake] Unify llvm_check_linker_flag and llvm_check_compiler_linker_flag

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 499561.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143052

Files:
  clang/tools/driver/CMakeLists.txt
  cmake/Modules/LLVMCheckCompilerLinkerFlag.cmake
  compiler-rt/cmake/config-ix.cmake
  libcxx/cmake/config-ix.cmake
  libunwind/cmake/config-ix.cmake
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/cmake/modules/HandleLLVMStdlib.cmake
  llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -78,7 +78,7 @@
 set(LLVM_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../llvm)
 
 include(CheckLibraryExists)
-include(LLVMCheckCompilerLinkerFlag)
+include(CheckLinkerFlag)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 
@@ -110,7 +110,7 @@
   # --unwindlib=none is supported, and use that if possible.
   # Don't add this if not necessary to fix linking, as it can break using
   # e.g. ASAN/TSAN.
-  llvm_check_compiler_linker_flag(C "--unwindlib=none" CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
+  check_linker_flag(C "--unwindlib=none" CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
   if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
 set(ORIG_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}")
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
@@ -140,7 +140,7 @@
 # Check for -nostdlib++ first; if there's no C++ standard library yet,
 # all check_cxx_compiler_flag commands will fail until we add -nostdlib++
 # (or -nodefaultlibs).
-llvm_check_compiler_linker_flag(CXX "-nostdlib++" CXX_SUPPORTS_NOSTDLIBXX_FLAG)
+check_linker_flag(CXX "-nostdlib++" CXX_SUPPORTS_NOSTDLIBXX_FLAG)
 if (CXX_SUPPORTS_NOSTDLIBXX_FLAG)
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
 endif()
Index: llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
===
--- llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
+++ /dev/null
@@ -1,28 +0,0 @@
-include(CheckLinkerFlag OPTIONAL)
-
-if (COMMAND check_linker_flag)
-  macro(llvm_check_linker_flag)
-check_linker_flag(${ARGN})
-  endmacro()
-else()
-  # Until the minimum CMAKE version is 3.18
-
-  include(CheckCXXCompilerFlag)
-
-  # cmake builtin compatible, except we assume lang is C or CXX
-  function(llvm_check_linker_flag lang flag out_var)
-cmake_policy(PUSH)
-cmake_policy(SET CMP0056 NEW)
-set(_CMAKE_EXE_LINKER_FLAGS_SAVE ${CMAKE_EXE_LINKER_FLAGS})
-set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
-if("${lang}" STREQUAL "C")
-  check_c_compiler_flag("" ${out_var})
-elseif("${lang}" STREQUAL "CXX")
-  check_cxx_compiler_flag("" ${out_var})
-else()
-  message(FATAL_ERROR "\"${lang}\" is not C or CXX")
-endif()
-set(CMAKE_EXE_LINKER_FLAGS ${_CMAKE_EXE_LINKER_FLAGS_SAVE})
-cmake_policy(POP)
-  endfunction()
-endif()
Index: llvm/cmake/modules/HandleLLVMStdlib.cmake
===
--- llvm/cmake/modules/HandleLLVMStdlib.cmake
+++ llvm/cmake/modules/HandleLLVMStdlib.cmake
@@ -13,12 +13,12 @@
   endfunction()
 
   include(CheckCXXCompilerFlag)
-  include(LLVMCheckLinkerFlag)
+  include(CheckLinkerFlag)
   set(LLVM_LIBCXX_USED 0)
   if(LLVM_ENABLE_LIBCXX)
 if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
   check_cxx_compiler_flag("-stdlib=libc++" CXX_COMPILER_SUPPORTS_STDLIB)
-  llvm_check_linker_flag(CXX "-stdlib=libc++" CXX_LINKER_SUPPORTS_STDLIB)
+  check_linker_flag(CXX "-stdlib=libc++" CXX_LINKER_SUPPORTS_STDLIB)
   if(CXX_COMPILER_SUPPORTS_STDLIB AND CXX_LINKER_SUPPORTS_STDLIB)
 append("-stdlib=libc++"
   CMAKE_CXX_FLAGS CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS
@@ -36,7 +36,7 @@
 if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
   check_cxx_compiler_flag("-static-libstdc++"
   CXX_COMPILER_SUPPORTS_STATIC_STDLIB)
-  llvm_check_linker_flag(CXX "-static-libstdc++" CXX_LINKER_SUPPORTS_STATIC_STDLIB)
+  check_linker_flag(CXX "-static-libstdc++" CXX_LINKER_SUPPORTS_STATIC_STDLIB)
   if(CXX_COMPILER_SUPPORTS_STATIC_STDLIB AND
 CXX_LINKER_SUPPORTS_STATIC_STDLIB)
 append("-static-libstdc++"
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -968,8 +968,8 @@
   if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR
   CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
 add_compile_options(-gsplit-dwarf)
-include(LLVMCheckLinkerFlag)
-llvm_check_linker_flag(CXX "-Wl,--gdb-index" LINKER_SUPPORTS_GDB_INDEX)
+include(CheckLinkerFlag)
+check_linker_flag(CXX "-Wl,--gdb-index" 

[PATCH] D142569: [OpenMP] Introduce kernel environment

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 499555.
tianshilei1992 marked 2 inline comments as done.
tianshilei1992 added a comment.

rebase and fix comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142569

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/Headers/amdgcn_openmp_device_math_c.c
  clang/test/OpenMP/amdgcn_target_codegen.cpp
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_printf_codegen.c
  clang/test/OpenMP/nvptx_target_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/reduction_implicit_map.cpp
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_debug_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/DeviceRTL/include/Debug.h
  openmp/libomptarget/DeviceRTL/include/Interface.h
  openmp/libomptarget/DeviceRTL/include/State.h
  openmp/libomptarget/DeviceRTL/src/Configuration.cpp
  openmp/libomptarget/DeviceRTL/src/Debug.cpp
  openmp/libomptarget/DeviceRTL/src/Kernel.cpp
  openmp/libomptarget/DeviceRTL/src/State.cpp
  openmp/libomptarget/include/DeviceEnvironment.h
  openmp/libomptarget/include/Environment.h
  openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
  openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins/cuda/src/rtl.cpp

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the comments! I should have addressed everything apart from testing.




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:523
+TransformedInc.Angled = WrittenRef.starts_with("<");
+if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
+  TransformedInc.Resolved = *FE;

kadircet wrote:
> VitaNuo wrote:
> > kadircet wrote:
> > > i don't think we should convert any unresolved includes. it usually means 
> > > header wasn't found, so we can't perform any reliable analysis on them. 
> > > anything i am missing?
> > Ok I will skip unresolved includes. But I am not sure I fully understand. 
> > We do the following:
> > 1. Convert clangd includes to include-cleaner includes.
> > 2. Match include-cleaner includes with symbol providers.
> > 3. If match found, symbol reference is satisfied.
> > 
> > How does it matter in this scenario if the include is resolved? AFAIU as 
> > long as the header is spelled in the main file + it's matched with a symbol 
> > provider, we should say that the symbol reference is satisfied.
> > 
> > Otherwise, it seems like we'll say that the header is missing, although 
> > it's there in the main file and unresolved.
> > 
> > I don't know if this is in any way a realistic scenario. I am just 
> > approaching it with general logic, and in this sense having more 
> > "satisfied" symbols seems better than having less => leads to less false 
> > positives. It can lead to false negatives, too, but AFAIU false negatives 
> > are much less of a risk for missing include management. 
> > How does it matter in this scenario if the include is resolved? AFAIU as 
> > long as the header is spelled in the main file + it's matched with a symbol 
> > provider, we should say that the symbol reference is satisfied.
> 
> It doesn't matter at a high-level view. But since the implementation 
> recognizes headers based on the HeaderID and it's only defined for resolved 
> includes, if we were to have any unresolved include matches somehow (e.g. it 
> has spelling "foo/bar.h", but is unresolved, and we do a match based on 
> spelling because some IWYU pragma pointed at this header), we would hit the 
> assertion around HeaderID always having value.
Ah ok, makes sense.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:535
+  Diag D;
+  D.Message = llvm::formatv("header {0} is missing", Spelling);
+  D.Name = "missing-includes";

kadircet wrote:
> VitaNuo wrote:
> > kadircet wrote:
> > > i think the symbol name should also be part of the diagnostic. as editors 
> > > can show these diagnostics without context (e.g. you've got a big file 
> > > open, there's a diagnostic panel displaying messages/fixes for all of the 
> > > file. you should be able to reason about the diagnostics and fixes 
> > > without jumping all over the file). so maybe something like:
> > > 
> > > `formatv("{0} providing '{1}' is not directly included", Header, Symbol)`
> > Sure. The new design does this, as well as skipping the header name.
> i feel like there's actually value in keeping the header name around, i.e. 
> the user will have some idea about the action, without triggering an extra 
> interaction. this helps especially in cases where the finding is wrong, 
> they'll discover this sooner, hence we'll be more likely to receive bug 
> reports. but don't really have a strong preference, so feel free to keep it 
> that way.
Hm I'd expect we'll actually have a higher probability to receive a bug report 
if the user clicks on the "Quick fix" and gets a wrong header included, because 
that's annoying :)
Having a full message on hover, seeing that it's wrong and just ignoring it 
might not be annoying enough to file a bug ;-)

But this is a pure speculation ofc.
I think the point discussed on the design document makes sense: without 
mentioning the header name it will be a bit easier to extend this to suggesting 
multiple fixes. So if that's the general direction, I'd prefer to keep it the 
current way.




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:549
+  bool Angled = HeaderRef.starts_with("<");
+  std::optional Replacement = HeaderIncludes.insert(
+  HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include);

kadircet wrote:
> VitaNuo wrote:
> > kadircet wrote:
> > > this returns nullopt only if Header is already included in the main file. 
> > > our analysis should never suggest such a header for include, unless the 
> > > include is coming from a PP-disabled region.
> > > 
> > > so i think if Replacement generation fails, we should drop the diagnostic 
> > > completely rather than just dropping the fix. WDYT?
> > Ok, sure. What's a PP-disabled region? Are you talking about #ifdef's and 
> > such?
> > What's a PP-disabled region
> 
> Yes, I was trying to say "preprocessor disabled region". e.g. in a piece of 
> code like:
> ```
> #if 0
> #include 

[PATCH] D143496: [clangd] Add support for missing includes analysis.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 499553.
VitaNuo marked 24 inline comments as done.
VitaNuo added a comment.

Address review comments (apart from testing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header , HeaderSearch ,
-   const FileEntry *Main) {
+std::string spellHeader(const Header , HeaderSearch ,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults , llvm::StringRef Code,
 const format::FormatStyle );
 
+std::string spellHeader(const Header , HeaderSearch ,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -9,11 +9,19 @@
 #include "Annotations.h"
 #include "Config.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -342,7 +350,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),
   ElementsAre(Pointee(writtenInclusion("";
-  EXPECT_THAT(computeUnusedIncludesExperimental(AST),
+  IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+  EXPECT_THAT(Findings.UnusedIncludes,
   ElementsAre(Pointee(writtenInclusion("";
 }
 
@@ -379,12 +388,69 @@
   computeUnusedIncludes(AST),
   UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
Pointee(writtenInclusion("\"dir/unused.h\"";
+
+  IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
   EXPECT_THAT(
-  computeUnusedIncludesExperimental(AST),
+  Findings.UnusedIncludes,
   UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
Pointee(writtenInclusion("\"dir/unused.h\"";
 }
 
+TEST(IncludeCleaner, GetMissingHeaders) {
+  llvm::StringLiteral MainFile = R"cpp(
+#include "a.h"
+#include "dir/c.h"
+#include 
+
+void foo() {
+  b();
+  d();
+  f();
+})cpp";
+  // Build expected ast with symbols coming from headers.
+  TestTU TU;
+  TU.Filename = "foo.cpp";
+  TU.AdditionalFiles["foo.h"] = guard("void foo();");
+  TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
+  TU.AdditionalFiles["b.h"] = guard("void b();");
+
+  TU.AdditionalFiles["dir/c.h"] = guard("#include \"d.h\"");
+  TU.AdditionalFiles["dir/d.h"] = guard("void d();");
+
+  TU.AdditionalFiles["system/e.h"] = guard("#include ");
+  TU.AdditionalFiles["system/f.h"] = guard("void f();");
+  TU.ExtraArgs.push_back("-isystem" + testPath("system"));
+
+  TU.Code = MainFile.str();
+  ParsedAST AST = TU.build();
+
+  IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+  const SourceManager  = AST.getSourceManager();
+  const llvm::ArrayRef  =
+  AST.getTokens().spelledTokens(SM.getMainFileID());
+  for (const auto  : 

[PATCH] D144540: [Clang] [Doc] Explicitly note noreturn bug as a miscompilation

[Sam James via Phabricator via cfe-commits]

thesamesam closed this revision.
thesamesam added a comment.

Thanks! Merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144540

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

[PATCH] D144569: [Clang][OpenMP] Fix accessing of aligned arrays in offloaded target regions

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 created this revision.
doru1004 added reviewers: jdoerfert, jhuber6, ronl, carlo.bertolli, arsenm, 
gregrodgers, ABataev.
doru1004 added a project: OpenMP.
Herald added subscribers: kosarev, kerbowa, guansong, yaxunl, jvesely.
Herald added a project: All.
doru1004 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, wdng.
Herald added a project: clang.

This patch fixes a memory error that occurs when we access an aligned array on 
the device:

  void write_index(int*a, int N) {
  int *aptr __attribute__ ((aligned(64))) = a; // This failed but is fixed 
by this patch.
  #pragma omp target teams distribute parallel for map(tofrom: aptr[0:N])
  for(int i=0;ihttps://reviews.llvm.org/D144569

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c

Index: clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
===
--- /dev/null
+++ clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
@@ -0,0 +1,305 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// REQUIRES: amdgpu-registered-target
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple powerpc64le-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host-amd.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple amdgcn-amd-amdhsa -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host-amd.bc -o - | FileCheck %s --check-prefix=CHECK-AMD
+
+
+void write_to_aligned_array(int *a, int N) {
+  int *aptr __attribute__ ((aligned(64))) = a;
+  #pragma omp target teams distribute parallel for map(tofrom: aptr[0:N])
+  for(int i = 0; i < N; i++) {
+aptr[i] = i;
+  }
+}
+
+#endif
+// CHECK-AMD-LABEL: define {{[^@]+}}@{{__omp_offloading_[0-9a-z]+_[0-9a-z]+}}_write_to_aligned_array_l14
+// CHECK-AMD-SAME: (i64 noundef [[N:%.*]], ptr noundef [[APTR:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-AMD-NEXT:  entry:
+// CHECK-AMD-NEXT:[[N_ADDR:%.*]] = alloca i64, align 8, addrspace(5)
+// CHECK-AMD-NEXT:[[APTR_ADDR:%.*]] = alloca ptr, align 8, addrspace(5)
+// CHECK-AMD-NEXT:[[N_CASTED:%.*]] = alloca i64, align 8, addrspace(5)
+// CHECK-AMD-NEXT:[[DOTZERO_ADDR:%.*]] = alloca i32, align 4, addrspace(5)
+// CHECK-AMD-NEXT:[[DOTTHREADID_TEMP_:%.*]] = alloca i32, align 4, addrspace(5)
+// CHECK-AMD-NEXT:[[N_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[N_ADDR]] to ptr
+// CHECK-AMD-NEXT:[[APTR_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[APTR_ADDR]] to ptr
+// CHECK-AMD-NEXT:[[N_CASTED_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[N_CASTED]] to ptr
+// CHECK-AMD-NEXT:[[DOTZERO_ADDR_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[DOTZERO_ADDR]] to ptr
+// CHECK-AMD-NEXT:[[DOTTHREADID_TEMP__ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[DOTTHREADID_TEMP_]] to ptr
+// CHECK-AMD-NEXT:store i64 [[N]], ptr [[N_ADDR_ASCAST]], align 8
+// CHECK-AMD-NEXT:store ptr [[APTR]], ptr [[APTR_ADDR_ASCAST]], align 8
+// CHECK-AMD-NEXT:[[TMP0:%.*]] = call i32 @__kmpc_target_init(ptr addrspacecast (ptr addrspace(1) @[[GLOB1:[0-9]+]] to ptr), i8 2, i1 false)
+// CHECK-AMD-NEXT:[[EXEC_USER_CODE:%.*]] = icmp eq i32 [[TMP0]], -1
+// CHECK-AMD-NEXT:br i1 [[EXEC_USER_CODE]], label [[USER_CODE_ENTRY:%.*]], label [[WORKER_EXIT:%.*]]
+// CHECK-AMD:   user_code.entry:
+// CHECK-AMD-NEXT:[[TMP1:%.*]] = call i32 @__kmpc_global_thread_num(ptr addrspacecast (ptr addrspace(1) @[[GLOB1]] to ptr))
+// CHECK-AMD-NEXT:[[TMP2:%.*]] = load i32, ptr [[N_ADDR_ASCAST]], align 4
+// CHECK-AMD-NEXT:store i32 [[TMP2]], ptr [[N_CASTED_ASCAST]], align 4
+// CHECK-AMD-NEXT:[[TMP3:%.*]] = load i64, ptr [[N_CASTED_ASCAST]], align 8
+// CHECK-AMD-NEXT:[[TMP4:%.*]] = load ptr, ptr [[APTR_ADDR_ASCAST]], align 8
+// CHECK-AMD-NEXT:store i32 0, ptr [[DOTZERO_ADDR_ASCAST]], align 4
+// CHECK-AMD-NEXT:store i32 [[TMP1]], ptr [[DOTTHREADID_TEMP__ASCAST]], align 4
+// CHECK-AMD-NEXT:call void @__omp_outlined__(ptr [[DOTTHREADID_TEMP__ASCAST]], ptr [[DOTZERO_ADDR_ASCAST]], i64 [[TMP3]], ptr [[TMP4]]) #[[ATTR2:[0-9]+]]
+// CHECK-AMD-NEXT:call void @__kmpc_target_deinit(ptr addrspacecast (ptr addrspace(1) @[[GLOB1]] to ptr), i8 2)
+// CHECK-AMD-NEXT:ret void
+// CHECK-AMD:   worker.exit:
+// CHECK-AMD-NEXT:ret void
+//
+//
+// CHECK-AMD-LABEL: define {{[^@]+}}@__omp_outlined__
+// CHECK-AMD-SAME: (ptr noalias noundef [[DOTGLOBAL_TID_:%.*]], ptr noalias noundef [[DOTBOUND_TID_:%.*]], i64 noundef [[N:%.*]], ptr noundef [[APTR:%.*]]) #[[ATTR1:[0-9]+]] {
+// CHECK-AMD-NEXT:  entry:
+// CHECK-AMD-NEXT:

[PATCH] D142569: [OpenMP] Introduce kernel environment

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:583
+  if (!KernelEnvOrError)
+return KernelEnvOrError.takeError();
+

tianshilei1992 wrote:
> jdoerfert wrote:
> > We used to default to SPMD w/o a _exec_mode. Unsure if this will cause 
> > problems. I am inclined to allow missing _kernel_env as well and use SPMD 
> > then.
> Now we need the global variable to call `target_init` at runtime, so missing 
> it sounds like a bug. I'm not sure if we would optimize the symbol out, but I 
> kinda doubt it.
Missing used to mean, and does, that it's not a "target region" kernel. So it 
might be a con/de-structor or a kernel coming from elsewhere, e.g., CUDA. We 
should continue that behavior I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142569

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

[PATCH] D135495: [clang-tidy] handle exceptions properly in `ExceptionAnalyzer`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I think at this point it is ok to merge. Any other comments can be addressed in 
follow-up commits.


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

https://reviews.llvm.org/D135495

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

[PATCH] D143210: [PowerPC] Include vector bool and pixel when emitting lax warning

[Kamau Bridgeman via Phabricator via cfe-commits]

kamaub added a comment.

We may also need an associated test case for the changed behaviour for using 
`areCompatibleVectorTypes()` instead of `areSameVectorElemTypes()`.
The test coverage should display when warnings are emitted now that we account 
for vector bool, vector pixel and type qualifiers.




Comment at: clang/lib/Sema/SemaExpr.cpp:9861
   isLaxVectorConversion(RHSType, LHSType)) {
-if (VecType->getVectorKind() == VectorType::AltiVecVector)
+if (VecType->getVectorKind() == VectorType::AltiVecVector ||
+VecType->getVectorKind() == VectorType::AltiVecBool ||

Is there test coverage for these addition to the if condition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143210

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

[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:83
+  }
+
   // The universal integral type to use in value range descriptions.

This enum is moved here from other location, and `negateKind` is added.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:124-125
   // requirement would render the initialization of the Summary map infeasible.
+  // A new value constraint is created furthermore by the negate functionality
+  // of the constraint and returned as pointer.
   using ValueConstraintPtr = std::shared_ptr;

Szelethus wrote:
> Is this what you meant?
Probably this is better:
```
  // Mind that a pointer to a new value constraint can be created by negating 
an existing
  // constraint.
```
The important thing is that one reason for the shared pointer is the negate 
function that returns a pointer.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:218
+  /// Check if a single argument falls into a specific range.
+  /// The "range" can be built from sub-ranges that are closed intervals.
   class RangeConstraint : public ValueConstraint {

Szelethus wrote:
> If you want a killer doc, use examples. Not saying this is not good enough, 
> but it wouldn't hurt.
The "range" is not the best term for this object in the documentation. A 
"range" is often used when a single interval is meant, but here it often means 
a set of ranges. I plan to improve this.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:271
+
+  private:
+using RangeApplyFunction =

This new private section is the new added code.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:275
+
+/// @brief Apply a function on all intervals in the range.
+void applyOnWithinRange(

The "@brief" is probably not necessary and not used often, I plan to remove it.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:893
 
-std::string StdLibraryFunctionsChecker::NotNullConstraint::describe(
-DescriptionKind DK, const CallEvent , ProgramStateRef State,
-const Summary ) const {
-  SmallString<48> Result;
-  const auto Violation = ValueConstraint::DescriptionKind::Violation;
-  Result += "the ";
-  Result += getArgDesc(ArgN);
-  Result += " to '";
-  Result += getFunctionName(Call);
-  Result += DK == Violation ? "' should not be NULL" : "' is not NULL";
-  return Result.c_str();
+void StdLibraryFunctionsChecker::RangeConstraint::applyOnWithinRange(
+BasicValueFactory , QualType ArgT, const RangeApplyFunction ) const {

This code is got from `RangeConstraint::applyAsOutOfRange`.
This becomes "within" because in the apply as out of range case all ranges are 
cut out, that is really a loop over ("within") the ranges.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:910
+void StdLibraryFunctionsChecker::RangeConstraint::applyOnOutOfRange(
+BasicValueFactory , QualType ArgT, const RangeApplyFunction ) const {
+  if (Ranges.empty())

This code is got from `RangeConstraint::applyWithinRange`.
That was implemented by removal of all out-of-range intervals. The code here is 
a general version that calls a lambda instead of the actual assume calls, so it 
becomes reusable for other purposes when a loop over all (out-of) intervals is 
used.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1060
+  const Summary ,
+  CheckerContext ) const {
+  SValBuilder  = C.getSValBuilder();

This function is just moved here from the previous (inline) location.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1143
+  if (ExplodedNode *N = C.generateErrorNode(State, NewNode))
+reportBug(Call, N, Constraint.get(), NegatedConstraint.get(), Summary, 
C);
   break;

The new argument in `reportBug` is not used but is used in the next patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143751

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

[PATCH] D144497: [clang][doc] Removes obsolete comment.

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7b6978285c1: [clang][doc] Removes obsolete comment. 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144497

Files:
  clang/docs/StandardCPlusPlusModules.rst


Index: clang/docs/StandardCPlusPlusModules.rst
===
--- clang/docs/StandardCPlusPlusModules.rst
+++ clang/docs/StandardCPlusPlusModules.rst
@@ -603,16 +603,6 @@
 
 For higher level support for proposals, you could visit 
https://clang.llvm.org/cxx_status.html.
 
-Support for clang-scan-deps
-~~~
-
-The support for clang-scan-deps may be the most urgent problem for modules now.
-Without the support for clang-scan-deps, it's hard to involve build systems.
-This means that users could only play with modules through makefiles or by 
writing a parser by hand.
-It blocks more uses for modules, which will block more defect reports or 
requirements.
-
-This is tracked in: https://github.com/llvm/llvm-project/issues/51792.
-
 Ambiguous deduction guide
 ~
 


Index: clang/docs/StandardCPlusPlusModules.rst
===
--- clang/docs/StandardCPlusPlusModules.rst
+++ clang/docs/StandardCPlusPlusModules.rst
@@ -603,16 +603,6 @@
 
 For higher level support for proposals, you could visit https://clang.llvm.org/cxx_status.html.
 
-Support for clang-scan-deps
-~~~
-
-The support for clang-scan-deps may be the most urgent problem for modules now.
-Without the support for clang-scan-deps, it's hard to involve build systems.
-This means that users could only play with modules through makefiles or by writing a parser by hand.
-It blocks more uses for modules, which will block more defect reports or requirements.
-
-This is tracked in: https://github.com/llvm/llvm-project/issues/51792.
-
 Ambiguous deduction guide
 ~
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a7b6978 - [clang][doc] Removes obsolete comment.

[Mark de Wever via cfe-commits]

Author: Mark de Wever
Date: 2023-02-22T17:17:36+01:00
New Revision: a7b6978285c1c07600d1762b206c1dcec35fe6c8

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

LOG: [clang][doc] Removes obsolete comment.

After reading about the documentation improvements on LLVM weekly this
part seems obsolete.

Reviewed By: ChuanqiXu

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

Added: 


Modified: 
clang/docs/StandardCPlusPlusModules.rst

Removed: 




diff  --git a/clang/docs/StandardCPlusPlusModules.rst 
b/clang/docs/StandardCPlusPlusModules.rst
index d4db37ea6e2e2..8447a44e3f8db 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -603,16 +603,6 @@ and add the label ``clang:modules`` (if you have 
permissions for that).
 
 For higher level support for proposals, you could visit 
https://clang.llvm.org/cxx_status.html.
 
-Support for clang-scan-deps
-~~~
-
-The support for clang-scan-deps may be the most urgent problem for modules now.
-Without the support for clang-scan-deps, it's hard to involve build systems.
-This means that users could only play with modules through makefiles or by 
writing a parser by hand.
-It blocks more uses for modules, which will block more defect reports or 
requirements.
-
-This is tracked in: https://github.com/llvm/llvm-project/issues/51792.
-
 Ambiguous deduction guide
 ~
 



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

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune abandoned this revision.
aqjune added a comment.

Closing this as D142388  added a function 
that can be used instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136737

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

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc45d2df05e0e: [Clang] Add options in LTO mode when cross 
compiling for AMDGPU (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144505

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


Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -13,4 +13,4 @@
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
 // LTO: clang{{.*}} "-flto=full"
-// LTO: ld.lld{{.*}}
+// LTO: ld.lld{{.*}}-plugin-opt=mcpu=gfx906
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -540,6 +540,9 @@
   ArgStringList CmdArgs;
   addLinkerCompressDebugSectionsOption(getToolChain(), Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+  if (C.getDriver().isUsingLTO())
+addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
+  C.getDriver().getLTOMode() == LTOK_Thin);
   CmdArgs.push_back("-shared");
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());


Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -13,4 +13,4 @@
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
 // LTO: clang{{.*}} "-flto=full"
-// LTO: ld.lld{{.*}}
+// LTO: ld.lld{{.*}}-plugin-opt=mcpu=gfx906
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -540,6 +540,9 @@
   ArgStringList CmdArgs;
   addLinkerCompressDebugSectionsOption(getToolChain(), Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+  if (C.getDriver().isUsingLTO())
+addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
+  C.getDriver().getLTOMode() == LTOK_Thin);
   CmdArgs.push_back("-shared");
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c45d2df - [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-02-22T10:14:05-06:00
New Revision: c45d2df05e0e7b6f2517aa2933a24928b4942a3f

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

LOG: [Clang] Add options in LTO mode when cross compiling for AMDGPU

The AMDGPU toolchain support directly compiling GPU images using
cross-compilation such as `clang --target=amdgcn-amd-amdhsa foo.c`.
However, when attempting to link bitcode this does not work because the
`-mcpu` options are not forwarded to the linker among others. This patch
simply adds them so that `clang --target=amdgcn-amd-amdhsa foo.c -flto`
works correctly.

Reviewed By: JonChesterfield

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/test/Driver/amdgpu-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index f30c1075eb7a3..08496c1abbc41 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -540,6 +540,9 @@ void amdgpu::Linker::ConstructJob(Compilation , const 
JobAction ,
   ArgStringList CmdArgs;
   addLinkerCompressDebugSectionsOption(getToolChain(), Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
+  if (C.getDriver().isUsingLTO())
+addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
+  C.getDriver().getLTOMode() == LTOK_Thin);
   CmdArgs.push_back("-shared");
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());

diff  --git a/clang/test/Driver/amdgpu-toolchain.c 
b/clang/test/Driver/amdgpu-toolchain.c
index c0a435428de60..a00c75b83f3e0 100644
--- a/clang/test/Driver/amdgpu-toolchain.c
+++ b/clang/test/Driver/amdgpu-toolchain.c
@@ -13,4 +13,4 @@
 // RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
 // LTO: clang{{.*}} "-flto=full"
-// LTO: ld.lld{{.*}}
+// LTO: ld.lld{{.*}}-plugin-opt=mcpu=gfx906



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

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:545
+addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
+  C.getDriver().getLTOMode() == LTOK_Thin);
   CmdArgs.push_back("-shared");

JonChesterfield wrote:
> What's the test against OK_Thin for? ThinLTO is a thing but I don't know if 
> it exists (/works) on amdgpu, is this that?
AMDGPU's linker is `lld` so it works "in theory". But in practice, separate 
linking of object files isn't fully supported by the ABI. So I consider it a 
"your mileage may vary" scenario. FWIW, it functioned properly with my 
experimental libc setup
```
$ cat main.cpp
int foo();

int main() {
  return foo();
}
$ cat foo.cpp
int foo() { return 3; }
$ clang++ crt1.o main.cpp foo.cpp --target=amdgcn-amd-amdhsa -mcpu=gfx1030 
-Wl,-plugin-opt=mcpu=gfx1030 -nogpulib -o image -flto=thin
$ amdhsa_loader image; echo $?
3
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144505

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

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Jon Chesterfield via Phabricator via cfe-commits]

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

NVM the above, all the other call sites to addLTOOptions have that test in them 
so it must be fine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144505

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

[PATCH] D144505: [Clang] Add options in LTO mode when cross compiling for AMDGPU

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:545
+addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
+  C.getDriver().getLTOMode() == LTOK_Thin);
   CmdArgs.push_back("-shared");

What's the test against OK_Thin for? ThinLTO is a thing but I don't know if it 
exists (/works) on amdgpu, is this that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144505

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

[PATCH] D144544: [OpenMP] Improve LIT tests on composite target constructs

[Animesh Kumar via Phabricator via cfe-commits]

animeshk-amd updated this revision to Diff 499513.
animeshk-amd added a comment.

Removed a pragma line which was not required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144544

Files:
  clang/test/OpenMP/target_task_affinity_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_reduction_codegen.cpp

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

[PATCH] D143587: [Docs] Multilib design

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added inline comments.



Comment at: clang/docs/Multilib.rst:237
+
+Stable API
+--

MaskRay wrote:
> What does "API" refer to? A function defined in llvm-project/clang/lib?
Changed "API" to "interface" and reworded to hopefully make it clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143587

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