[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Did the original change make it into the 13 branch? I'm seeing some unexpected 
behavior.

  bool foo(int a, Bar) override;
  bool foo(int a, Bar) override; // comment

becomes

  bool foo(int a, Bar)
  override;
  bool foo(int a, Bar)
  override; // comment


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

https://reviews.llvm.org/D107961

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Logged as https://bugs.llvm.org/show_bug.cgi?id=51470  @krasimir fix resolves 
this


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

https://reviews.llvm.org/D107961

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

FWIW with this additional fix is  also still ok and doesn't revert the 
behaviour, but we'd have to land this in order for @tstellar to move it to the 
branch (and I think he said that needed to be done by Friday 13th)

  bool foo(int a, Bar) override;
  bool foo(int a, Bar) override; // comment
  bool foo(int a, int) override;
  
  bool foo(int a, Bar) final;
  bool foo(int a, Bar) final; // comment


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

https://reviews.llvm.org/D107961

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

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

I was wondering about the connection to OpenACC, so I had a quick look into the 
OpenACC spec to try and understand the background.
OpenACC uses two separate reference counters for structured and unstructured 
map. If one of them is >0, the data is present. If both become 0, data is 
deleted.

I think, the `hold` modifier is not sufficient to replicate OpenACC behavior. 
Consider the following example:

  #pragma acc data copy(a)  // structured ref := 1
  {
  #pragma acc exit data delete(a) // dynamic ref := 0
  #pragma acc enter data copyin(a) // dynamic ref := 1
  } // structured ref := 0 // no copyout because dynamic ref >0

As I understand this will be translated to the following OpenMP:

  #pragma omp target data map(ompx_hold, tofrom:a)  // ref := 1
  {
  #pragma omp exit data map(delete:a) // ref := 0  // no action because of hold
  #pragma omp enter data map(to:a) // ref := 1
  } // ref := 0 // perform map from

I don't think, that trying to map the two openacc reference count to a single 
openmp reference count will work in general.


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

https://reviews.llvm.org/D106509

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

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 366209.
ASDenysPetrov added a comment.

Fixed //smell// code: from `isa'n'cast` to `dyn-cast`.


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

https://reviews.llvm.org/D104285

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/Type.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++14 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -std=c++14 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.uninitialized.Assign,core.builtin,debug.ExprInspection,core.uninitialized.UndefReturn -verify %s
 
 void clang_analyzer_eval(int);
 
@@ -18,3 +18,114 @@
   // FIXME: Should recognize that it is 0.
   clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
 }
+
+void direct_index1() {
+  int const arr[2][2][3] = {};
+  int const *ptr = (int const *)arr;
+  clang_analyzer_eval(ptr[0] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[3] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[4] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[5] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[6] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[7] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[8] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[9] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[10] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[11] == 0); // expected-warning{{TRUE}}
+}
+
+void direct_index2() {
+  int const arr[2][2][3] = {{{1, 2, 3}, {4, 5, 6}}, {{7, 8, 9}, {10, 11, 12}}};
+  int const *ptr = arr[0][0];
+  clang_analyzer_eval(ptr[0] == 1);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 2);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 3);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[3] == 4);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[4] == 5);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[5] == 6);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[6] == 7);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[7] == 8);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[8] == 9);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[9] == 10);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[10] == 11); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[11] == 12); // expected-warning{{TRUE}}
+}
+
+void direct_index3() {
+  int const arr[2][2][3] = {{{}, {}}, {{}, {}}};
+  int const *ptr = &(arr[0][0][0]);
+  clang_analyzer_eval(ptr[0] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[3] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[4] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[5] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[6] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[7] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[8] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[9] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[10] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[11] == 0); // expected-warning{{TRUE}}
+}
+
+void direct_index4() {
+  int const arr[2][2][3] = {{{1, 2}, {}}, {{7, 8}, {10, 11, 12}}};
+  int const *ptr = (int const *)arr[0];
+  clang_analyzer_eval(ptr[0] == 1);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 2);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 0);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[3] == 0);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[4] == 0);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[5] == 0);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[6] == 7);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[7] == 8);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[8] == 0);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[9] == 10);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[10] == 11); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[11] == 12); // expected-warning{{TRUE}}
+}
+
+void direct_index5() {
+  int arr[2][2][3] = {{{1, 2}}, {{7}}};
+  int *ptr = (int *)arr;
+  clang_analyzer_eval(ptr[0] == 1);  // expected-warning{{TR

[clang] 60e07a9 - [AMDGPU][OpenMP] Use llvm-link to link ocml libraries

[Pushpinder Singh via cfe-commits]

Author: Pushpinder Singh
Date: 2021-08-13T13:36:57+05:30
New Revision: 60e07a9568625a196f1ed8ed9e502c8c4d56da7f

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

LOG: [AMDGPU][OpenMP] Use llvm-link to link ocml libraries

This fixes the 'unused linker option: -lm' warning when compiling
program with -c.

Reviewed By: JonChesterfield

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
clang/test/Driver/amdgpu-openmp-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp 
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index 7b335f33aa82..135e3694434d 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -86,14 +86,34 @@ static bool checkSystemForAMDGPU(const ArgList &Args, const 
AMDGPUToolChain &TC,
 } // namespace
 
 const char *AMDGCN::OpenMPLinker::constructLLVMLinkCommand(
-Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
-const ArgList &Args, StringRef SubArchName,
-StringRef OutputFilePrefix) const {
+const toolchains::AMDGPUOpenMPToolChain &AMDGPUOpenMPTC, Compilation &C,
+const JobAction &JA, const InputInfoList &Inputs, const ArgList &Args,
+StringRef SubArchName, StringRef OutputFilePrefix) const {
   ArgStringList CmdArgs;
 
   for (const auto &II : Inputs)
 if (II.isFilename())
   CmdArgs.push_back(II.getFilename());
+
+  if (Args.hasArg(options::OPT_l)) {
+auto Lm = Args.getAllArgValues(options::OPT_l);
+bool HasLibm = false;
+for (auto &Lib : Lm) {
+  if (Lib == "m") {
+HasLibm = true;
+break;
+  }
+}
+
+if (HasLibm) {
+  SmallVector BCLibs =
+  AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
+  llvm::for_each(BCLibs, [&](StringRef BCFile) {
+CmdArgs.push_back(Args.MakeArgString(BCFile));
+  });
+}
+  }
+
   // Add an intermediate output file.
   CmdArgs.push_back("-o");
   const char *OutputFileName =
@@ -182,8 +202,8 @@ void AMDGCN::OpenMPLinker::ConstructJob(Compilation &C, 
const JobAction &JA,
   assert(Prefix.length() && "no linker inputs are files ");
 
   // Each command outputs 
diff erent files.
-  const char *LLVMLinkCommand =
-  constructLLVMLinkCommand(C, JA, Inputs, Args, GPUArch, Prefix);
+  const char *LLVMLinkCommand = constructLLVMLinkCommand(
+  AMDGPUOpenMPTC, C, JA, Inputs, Args, GPUArch, Prefix);
 
   // Produce readable assembly if save-temps is enabled.
   if (C.getDriver().isSaveTempsEnabled())
@@ -234,27 +254,6 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
 
   addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
  getTriple());
-
-  if (!DriverArgs.hasArg(options::OPT_l))
-return;
-
-  auto Lm = DriverArgs.getAllArgValues(options::OPT_l);
-  bool HasLibm = false;
-  for (auto &Lib : Lm) {
-if (Lib == "m") {
-  HasLibm = true;
-  break;
-}
-  }
-
-  if (HasLibm) {
-SmallVector BCLibs =
-getCommonDeviceLibNames(DriverArgs, GPUArch);
-llvm::for_each(BCLibs, [&](StringRef BCFile) {
-  CC1Args.push_back("-mlink-builtin-bitcode");
-  CC1Args.push_back(DriverArgs.MakeArgString(BCFile));
-});
-  }
 }
 
 llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(

diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h 
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
index effca7e212cc..233256bf7378 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
@@ -16,6 +16,10 @@
 namespace clang {
 namespace driver {
 
+namespace toolchains {
+class AMDGPUOpenMPToolChain;
+}
+
 namespace tools {
 
 namespace AMDGCN {
@@ -35,11 +39,11 @@ class LLVM_LIBRARY_VISIBILITY OpenMPLinker : public Tool {
 
 private:
   /// \return llvm-link output file name.
-  const char *constructLLVMLinkCommand(Compilation &C, const JobAction &JA,
-   const InputInfoList &Inputs,
-   const llvm::opt::ArgList &Args,
-   llvm::StringRef SubArchName,
-   llvm::StringRef OutputFilePrefix) const;
+  const char *constructLLVMLinkCommand(
+  const toolchains::AMDGPUOpenMPToolChain &AMDGPUOpenMPTC, Compilation &C,
+  const JobAction &JA, const InputInfoList &Inputs,
+  const llvm::opt::ArgList &Args, llvm::StringRef SubArchName,
+  llvm::StringRef OutputFilePrefix) const;
 
   /// \return llc output file name.
   const char *constructLlcCommand(Compilation &C, const JobAction &JA,

diff  --git a/clang/test/Driver/amdgpu-openmp-toolc

[PATCH] D107952: [AMDGPU][OpenMP] Use llvm-link to link ocml libraries

[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 rG60e07a956862: [AMDGPU][OpenMP] Use llvm-link to link ocml 
libraries (authored by Pushpinder Singh ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107952

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

Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -76,4 +76,4 @@
 // CHECK-EMIT-LLVM-IR: clang{{.*}}"-cc1"{{.*}}"-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
 
 // RUN: env LIBRARY_PATH=%S/Inputs/hip_dev_lib %clang -### -target x86_64-pc-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -lm --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIB-DEVICE
-// CHECK-LIB-DEVICE: clang{{.*}}"-cc1"{{.*}}"-triple" "amdgcn-amd-amdhsa"{{.*}}"-mlink-builtin-bitcode"{{.*}}libomptarget-amdgcn-gfx803.bc"{{.*}}"-mlink-builtin-bitcode"{{.*}}ocml.bc" "-mlink-builtin-bitcode"{{.*}}ockl.bc" "-mlink-builtin-bitcode"{{.*}}oclc_daz_opt_on.bc" "-mlink-builtin-bitcode"{{.*}}oclc_unsafe_math_off.bc" "-mlink-builtin-bitcode"{{.*}}oclc_finite_only_off.bc" "-mlink-builtin-bitcode"{{.*}}oclc_correctly_rounded_sqrt_on.bc" "-mlink-builtin-bitcode"{{.*}}oclc_wavefrontsize64_on.bc" "-mlink-builtin-bitcode"{{.*}}oclc_isa_version_803.bc"
+// CHECK-LIB-DEVICE: {{.*}}llvm-link{{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
@@ -16,6 +16,10 @@
 namespace clang {
 namespace driver {
 
+namespace toolchains {
+class AMDGPUOpenMPToolChain;
+}
+
 namespace tools {
 
 namespace AMDGCN {
@@ -35,11 +39,11 @@
 
 private:
   /// \return llvm-link output file name.
-  const char *constructLLVMLinkCommand(Compilation &C, const JobAction &JA,
-   const InputInfoList &Inputs,
-   const llvm::opt::ArgList &Args,
-   llvm::StringRef SubArchName,
-   llvm::StringRef OutputFilePrefix) const;
+  const char *constructLLVMLinkCommand(
+  const toolchains::AMDGPUOpenMPToolChain &AMDGPUOpenMPTC, Compilation &C,
+  const JobAction &JA, const InputInfoList &Inputs,
+  const llvm::opt::ArgList &Args, llvm::StringRef SubArchName,
+  llvm::StringRef OutputFilePrefix) const;
 
   /// \return llc output file name.
   const char *constructLlcCommand(Compilation &C, const JobAction &JA,
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -86,14 +86,34 @@
 } // namespace
 
 const char *AMDGCN::OpenMPLinker::constructLLVMLinkCommand(
-Compilation &C, const JobAction &JA, const InputInfoList &Inputs,
-const ArgList &Args, StringRef SubArchName,
-StringRef OutputFilePrefix) const {
+const toolchains::AMDGPUOpenMPToolChain &AMDGPUOpenMPTC, Compilation &C,
+const JobAction &JA, const InputInfoList &Inputs, const ArgList &Args,
+StringRef SubArchName, StringRef OutputFilePrefix) const {
   ArgStringList CmdArgs;
 
   for (const auto &II : Inputs)
 if (II.isFilename())
   CmdArgs.push_back(II.getFilename());
+
+  if (Args.hasArg(options::OPT_l)) {
+auto Lm = Args.getAllArgValues(options::OPT_l);
+bool HasLibm = false;
+for (auto &Lib : Lm) {
+  if (Lib == "m") {
+HasLibm = true;
+break;
+  }
+}
+
+if (HasLibm) {
+  SmallVector BCLibs =
+  AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
+  llvm::for_each(BCLibs, [&](StringRef BCFile) {
+CmdArgs.push_back(Args.MakeArgString(BCFile));
+  });
+}
+  }
+
   // Add an intermediate output file.
   CmdArgs.push_back("-o");
   const char *OutputFileName =
@@ -182,8 +202,8 @@
   assert(Prefix.length() && "no linker inputs are files ");
 
   // Each command outputs different files.
-  const char *LLVMLinkCommand =
-  constructLLVMLinkCommand(C, JA, Inputs, Args, GPUArch, Prefix);
+  const char *LLVMLinkCommand = constructLLVMLinkCommand(
+  AMDGPUOpenMPTC, C, JA, Inputs, Args, GPUArch, Prefix);
 

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst:21
+
+   If set to `true`, this check will limit itself to the `builtinType()` 
types. Default is `false`.

cjdb wrote:
> jmarrec wrote:
> > Quuxplusone wrote:
> > > D107873 is related.
> > > 
> > > I'd like to see some tests/discussion around large types, e.g.
> > > ```
> > > struct Widget { int a[1000]; };
> > > void f(const Widget&);
> > > ```
> > > (I think D107873 at least makes a conscious attempt to get the "would it 
> > > be passed in registers?" check right.)
> > > 
> > > I'd like to see some tests/discussion around generic code, e.g.
> > > ```
> > > template
> > > struct Max {
> > > static T max(const T& a, const T& b);
> > > };
> > > int x = Max::max(1, 2);  // passes `int` by const reference, but 
> > > this is fine
> > > ```
> > Should I close this one in favor of https://reviews.llvm.org/D107873?
> Up to you. There's always a chance that D107873 doesn't get approved. What I 
> find most interesting is that we were independently working on this at the 
> same time :-)
@cjdb I guess it's uncanny that we would both decide to do it at the same time. 
A bit unfortunate too probably, I could have saved a few hours :) but I learned 
something about clang-tidy so I don't mind at all!

Yours (D107873) is definitely better documented, you handle to opposite case (I 
only deal with const-ref -> value, not value -> const ret) and I just borrowed 
your implementation for MaxSize / ignoring shared_ptr

As far as I can tell from a cursory look, there are a couple of things I'm 
doing that you aren't:

* I am actually using Options, I don't think you are yet, even for MaxSize. I 
see from your documentation file that you plan to add a few
* The RestrictToBuiltInTypes I find an interesting option too
* I am removing the const as well in addition to the `&`. I guess you are just 
relying on using it in conjunction with 
`readability-avoid-const-params-in-decls` but I think that makes it harder to 
use (you can't just do -checks=-*, you have to remember to add the 
readability-avoid-const-params-in-decls too...).

Perhaps we could discuss adding the above to your PR and consolidate over there 
to make a nice, feature-complete check?


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

https://reviews.llvm.org/D107900

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

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

The following doesn't feel consistent

  bool foo(a, Bar) final; // comment
  bool foo(a, Bar) final;
  Bar foo(a, Bar)
  final; // comment
  Bar foo(a, Bar)
  final;


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

https://reviews.llvm.org/D107961

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

[PATCH] D107349: [Matrix] Overload stride arg in matrix.columnwise.load/store.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D107349#2941385 , @erichkeane 
wrote:

> In D107349#2941257 , @mehdi_amini 
> wrote:
>
>> Revert to unbreak bots (like this one : 
>> https://lab.llvm.org/buildbot/#/builders/13/builds/10930 )
>
> Looks like this should be a pretty trivial test update at least.

Thanks for the heads-up! Recommitted with an updated MLIR test: f999312872b8 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107349

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D107961#2943223 , @MyDeveloperDay 
wrote:

> Did the original change make it into the 13 branch?

What's //the 13 branch//?

> I'm seeing some unexpected behavior.
>
>   bool foo(int a, Bar) override;
>   bool foo(int a, Bar) override; // comment
>
> becomes
>
>   bool foo(int a, Bar)
>   override;
>   bool foo(int a, Bar)
>   override; // comment

How can I reproduce it? The above C++ declarations should be inside a class 
definition, no?

Anyway, here is what I got:

  $ cat .clang-format
  BasedOnStyle: LLVM
  AlwaysBreakAfterReturnType: TopLevelDefinitions
  $ cat foo.cpp
  typedef float Bar;
  class A {
virtual bool foo(int, Bar);
virtual bool bar(int, Bar);
  };
  class B : A {
bool foo(int a, Bar) override;
bool bar(int a, Bar) override; // comment
  };
  $ clang -Wall -std=c++11 -c foo.cpp
  $ clang-format foo.cpp > bar.cpp
  $ diff foo.cpp bar.cpp
  $ 


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

https://reviews.llvm.org/D107961

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

[clang-tools-extra] 7d65cc9 - [clangd] Guard against null Attrs in the AST

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-08-13T10:38:52+02:00
New Revision: 7d65cc98f3508750fcf12240cab625c6e1d5c4ed

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

LOG: [clangd] Guard against null Attrs in the AST

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index a47a5a1aab1d..3b5956037746 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -488,15 +488,22 @@ std::vector getAttributes(const 
DynTypedNode &N) {
   if (const auto *TL = N.get()) {
 for (AttributedTypeLoc ATL = TL->getAs(); !ATL.isNull();
  ATL = ATL.getModifiedLoc().getAs()) {
-  Result.push_back(ATL.getAttr());
+  if (const Attr *A = ATL.getAttr())
+Result.push_back(A);
   assert(!ATL.getModifiedLoc().isNull());
 }
   }
-  if (const auto *S = N.get())
+  if (const auto *S = N.get()) {
 for (; S != nullptr; S = dyn_cast(S->getSubStmt()))
-  llvm::copy(S->getAttrs(), std::back_inserter(Result));
-  if (const auto *D = N.get())
-llvm::copy(D->attrs(), std::back_inserter(Result));
+  for (const Attr *A : S->getAttrs())
+if (A)
+  Result.push_back(A);
+  }
+  if (const auto *D = N.get()) {
+for (const Attr *A : D->attrs())
+  if (A)
+Result.push_back(A);
+  }
   return Result;
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 3898388d1ad2..5a45e5bde8fa 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -465,7 +465,15 @@ TEST(SelectionTest, CommonAncestor) {
 // Digraph syntax for attributes to avoid accidental annotations.
 class <:[gsl::Owner([[in^t]])]:> X{};
   )cpp",
-   "BuiltinTypeLoc"}};
+   "BuiltinTypeLoc"},
+
+  // This case used to crash - AST has a null Attr
+  {R"cpp(
+@interface I
+[[@property(retain, nonnull) <:[My^Object2]:> *x]]; // error-ok
+@end
+  )cpp",
+   "ObjCPropertyDecl"}};
 
   for (const Case &C : Cases) {
 trace::TestTracer Tracer;



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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D107961#2943225 , @MyDeveloperDay 
wrote:

> Logged as https://bugs.llvm.org/show_bug.cgi?id=51470  @krasimir fix resolves 
> this.
>
> I've marked this bug as a 13.0 release blocker ((@tstellar), we can take 
> @krasimir fix or we revert all the recent K&R support out of the 13 branch, 
> @owenpan which would you be happier doing?

@krasimir's fix did not make into the 13 branch. We should take both that fix 
and this one if possible.


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

https://reviews.llvm.org/D107961

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

[PATCH] D107703: [AST][clangd] Expose documentation of Attrs on hover.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D107703#2942853 , @thakis wrote:

> The docs are in rST format. Is that the format LSP wants?

Not really :-) LSP's favorite would be markdown.
But we're used to ingesting comments in unknown formats and trying to make 
sense of them, and our users are used to us failing. (Those heuristics could 
use some love).
So the rst will be thrown into that meatgrinder as if they're comments, and at 
the very least the text will survive, which is the main thing.

Hopefully one day we can do real conversion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107703

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D107961#2943266 , @owenpan wrote:

> In D107961#2943223 , 
> @MyDeveloperDay wrote:
>
>> Did the original change make it into the 13 branch?
>
> ~~What's //the 13 branch//?~~
>
>> I'm seeing some unexpected behavior.
>>
>>   bool foo(int a, Bar) override;
>>   bool foo(int a, Bar) override; // comment
>>
>> becomes
>>
>>   bool foo(int a, Bar)
>>   override;
>>   bool foo(int a, Bar)
>>   override; // comment
>
> How can I reproduce it? The above C++ declarations should be inside a class 
> definition, no?
>
> Anyway, here is what I got:
>
>   $ cat .clang-format
>   BasedOnStyle: LLVM
>   AlwaysBreakAfterReturnType: TopLevelDefinitions
>   $ cat foo.cpp
>   typedef float Bar;
>   class A {
> virtual bool foo(int, Bar);
> virtual bool bar(int, Bar);
>   };
>   class B : A {
> bool foo(int a, Bar) override;
> bool bar(int a, Bar) override; // comment
>   };
>   $ clang -Wall -std=c++11 -c foo.cpp
>   $ clang-format foo.cpp > bar.cpp
>   $ diff foo.cpp bar.cpp
>   $ 



> The above C++ declarations should be inside a class definition, no?

Not really, we don't have to have any context of where we are (in class or 
struct). if you had  for example

  class A
  {
  #include "myfunctions.h"
  }

you still would want the code in myfunctions.h to be formatted just the same 
(less indentation)

So the following code:

  bool foo(int a, Bar)
  override;
  bool foo(int a, Bar)
  override; // comment

Failed on the 13 branch, but is fixed with @krasimir fix, but that isn't 
backported (we need to do that as a minimum I think)

With this revision that is also fixed, the following occurs if I change the 
return type

  Bar foo(int a, Bar)
  override;
  Bar foo(int a, Bar)
  override; // comment

And that doesn't feel correct to me. Well not as by default at least. From my 
perspective there seems to be a propensity to think code is K&R, but that 
should be the exception not the rule.

If that is to be the case then we need some sort of setting that means they 
need to expect it, (like the Language: ) that was proposed before

I just ran this on my 4.5 million line and ended up with way more areas that 
needed clang-formatting than I think should need to (in c++ code not c code)


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

https://reviews.llvm.org/D107961

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

[clang] c064ba3 - [NFC] Add commas in code comments.

[Alexey Bader via cfe-commits]

Author: Alexey Bader
Date: 2021-08-13T08:59:47+03:00
New Revision: c064ba34c7d867c1180279fe3dca8817d835cb28

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

LOG: [NFC] Add commas in code comments.

Added: 


Modified: 
clang/include/clang/Sema/ParsedAttr.h

Removed: 




diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 61a36ddcc1db..408032cec7e8 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -627,7 +627,7 @@ class ParsedAttr final
   /// a Spelling enumeration, the value UINT_MAX is returned.
   unsigned getSemanticSpelling() const;
 
-  /// If this is an OpenCL address space attribute returns its representation
+  /// If this is an OpenCL address space attribute, returns its representation
   /// in LangAS, otherwise returns default address space.
   LangAS asOpenCLLangAS() const {
 switch (getParsedKind()) {
@@ -650,7 +650,7 @@ class ParsedAttr final
 }
   }
 
-  /// If this is an OpenCL address space attribute returns its SYCL
+  /// If this is an OpenCL address space attribute, returns its SYCL
   /// representation in LangAS, otherwise returns default address space.
   LangAS asSYCLLangAS() const {
 switch (getKind()) {



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

[PATCH] D108020: [NFC] Drop idle compiler option from the test.

[Alexey Bader via Phabricator via cfe-commits]

bader created this revision.
bader added a reviewer: erichkeane.
Herald added a subscriber: ebevhan.
bader requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108020

Files:
  clang/test/AST/ast-print-sycl-unique-stable-name.cpp


Index: clang/test/AST/ast-print-sycl-unique-stable-name.cpp
===
--- clang/test/AST/ast-print-sycl-unique-stable-name.cpp
+++ clang/test/AST/ast-print-sycl-unique-stable-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - -triple 
spir64-sycldevice | FileCheck %s
+// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - | FileCheck %s
 
 template 
 void WrappedInTemplate(T t) {


Index: clang/test/AST/ast-print-sycl-unique-stable-name.cpp
===
--- clang/test/AST/ast-print-sycl-unique-stable-name.cpp
+++ clang/test/AST/ast-print-sycl-unique-stable-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - -triple spir64-sycldevice | FileCheck %s
+// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - | FileCheck %s
 
 template 
 void WrappedInTemplate(T t) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108021: [dllexport] Instantiate default ctor default args

[Peter Jiachen via Phabricator via cfe-commits]

peterjc123 created this revision.
peterjc123 added a reviewer: rnk.
peterjc123 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108021

Files:
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp 
===
--- clang/lib/Sema/SemaDeclCXX.cpp 
+++ clang/lib/Sema/SemaDeclCXX.cpp 
@@ -6005,6 +6005,15 @@
   if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
 continue;

+  // If this is an MS ABI dllexport default constructor, instantiate any
+  // default arguments.
+  if (S.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+auto CD = dyn_cast(MD);
+if (CD && CD->isDefaultConstructor()) {
+  S.InstantiateDefaultCtorDefaultArgs(CD);
+}
+  }
+
   S.MarkFunctionReferenced(Class->getLocation(), MD);

   // The function will be passed to the consumer when its definition is


Index: clang/lib/Sema/SemaDeclCXX.cpp 
===
--- clang/lib/Sema/SemaDeclCXX.cpp 
+++ clang/lib/Sema/SemaDeclCXX.cpp 
@@ -6005,6 +6005,15 @@
   if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
 continue;

+  // If this is an MS ABI dllexport default constructor, instantiate any
+  // default arguments.
+  if (S.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+auto CD = dyn_cast(MD);
+if (CD && CD->isDefaultConstructor()) {
+  S.InstantiateDefaultCtorDefaultArgs(CD);
+}
+  }
+
   S.MarkFunctionReferenced(Class->getLocation(), MD);

   // The function will be passed to the consumer when its definition is
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

[George Rokos via Phabricator via cfe-commits]

grokos added a comment.

In D106509#2943239 , @protze.joachim 
wrote:

> I was wondering about the connection to OpenACC, so I had a quick look into 
> the OpenACC spec to try and understand the background.
> OpenACC uses two separate reference counters for structured and unstructured 
> map. If one of them is >0, the data is present. If both become 0, data is 
> deleted.
>
> I think, the `hold` modifier is not sufficient to replicate OpenACC behavior. 
> Consider the following example:
>
>   #pragma acc data copy(a)  // structured ref := 1
>   {
>   #pragma acc exit data delete(a) // dynamic ref := 0
>   #pragma acc enter data copyin(a) // dynamic ref := 1
>   } // structured ref := 0 // no copyout because dynamic ref >0
>
> As I understand this will be translated to the following OpenMP:
>
>   #pragma omp target data map(ompx_hold, tofrom:a)  // ref := 1
>   {
>   #pragma omp exit data map(delete:a) // ref := 0  // no action because of 
> hold
>   #pragma omp enter data map(to:a) // ref := 1
>   } // ref := 0 // perform map from
>
> I don't think, that trying to map the two openacc reference count to a single 
> openmp reference count will work in general.

The next patch in this series (D106510 ) 
modifies libomptarget and introduces a second reference count for ompx_hold. 
There won't be a singe RefCount anymore. I will review that patch once this one 
has been finalized.


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

https://reviews.llvm.org/D106509

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

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 366225.
Sockke retitled this revision from "[clang-tidy] Fix wrong and missing warnings 
in performance-move-const-arg" to "[clang-tidy] Fix wrong FIxIt in 
performance-move-const-arg".
Sockke edited the summary of this revision.
Sockke added a comment.

Fix the wrong AutoFix which blocks the compilation.


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

https://reviews.llvm.org/D107450

Files:
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
  clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
@@ -246,3 +246,27 @@
   };
   f(MoveSemantics());
 }
+
+void showInt(int &&) {}
+void testInt() {
+  int a = 10;
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; consider changing showInt's parameter from 'int'&& to 'int'&
+}
+template 
+void forwardToShowInt(T &&t) {
+  showInt(static_cast(t));
+}
+void testTemplate() {
+  int a = 10;
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; consider changing forwardToShowInt's parameter from 'int'&& to 'int'&
+}
+
+struct Tmp {};
+void showTmp(Tmp &&) {}
+void testTmp() {
+  Tmp t;
+  showTmp(std::move(t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+}
Index: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
===
--- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
+++ clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -36,6 +37,7 @@
 
 private:
   const bool CheckTriviallyCopyableMove;
+  llvm::DenseSet HasCheckedMoveSet;
 };
 
 } // namespace performance
Index: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
===
--- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
@@ -50,7 +50,9 @@
   Finder->addMatcher(
   invocation(forEachArgumentWithParam(
  MoveCallMatcher,
- parmVarDecl(hasType(references(isConstQualified())
+ parmVarDecl(anyOf(hasType(references(isConstQualified())),
+   hasType(rValueReferenceType(
+ .bind("invocation-parm")))
   .bind("receiving-expr"),
   this);
 }
@@ -58,6 +60,15 @@
 void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CallMove = Result.Nodes.getNodeAs("call-move");
   const auto *ReceivingExpr = Result.Nodes.getNodeAs("receiving-expr");
+  const auto *InvocationParm =
+  Result.Nodes.getNodeAs("invocation-parm");
+
+  if (!ReceivingExpr && HasCheckedMoveSet.contains(CallMove))
+return;
+
+  if (ReceivingExpr)
+HasCheckedMoveSet.insert(CallMove);
+
   const Expr *Arg = CallMove->getArg(0);
   SourceManager &SM = Result.Context->getSourceManager();
 
@@ -90,20 +101,45 @@
   return;
 
 bool IsVariable = isa(Arg);
+bool IsRValueReferenceArg = false;
+StringRef FuncName;
 const auto *Var =
 IsVariable ? dyn_cast(Arg)->getDecl() : nullptr;
-auto Diag = diag(FileMoveRange.getBegin(),
- "std::move of the %select{|const }0"
- "%select{expression|variable %4}1 "
- "%select{|of the trivially-copyable type %5 }2"
- "has no effect; remove std::move()"
- "%select{| or make the variable non-const}3")
-<< IsConstArg << IsVariable << IsTriviallyCopyable
-<< (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var
-<< Arg->getType();
+
+if (ReceivingExpr &&
+InvocationParm->getOriginalType()->isRValueReferenceType() &&
+!ReceivingExpr->getType()->isRecordType() && Arg->isLValue()) {
+  IsRValueReferenceArg = true;
+  if (Arg->getType()->isBuiltinType()) {
+const auto *ReceivingCallExpr = dyn_cast(ReceivingExpr);
+if (!ReceivingCallExpr)
+  return;
+FuncName = ReceivingCallExpr->getDirectCallee()->getName();
+  

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

In D106509#2943316 , @grokos wrote:

> The next patch in this series (D106510 ) 
> modifies libomptarget and introduces a second reference count for ompx_hold. 
> There won't be a singe RefCount anymore. I will review that patch once this 
> one has been finalized.

Ok, thanks for the clarification. This change was not obvious from the 
description of the two patches. Makes sense then.


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

https://reviews.llvm.org/D106509

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

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Please remember to create a more representative commit message as the patch no 
longer removes getMaxVScale.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

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

[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens in declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D107292#2943101 , @cjdb wrote:

> In D107292#2939521 , @aaron.ballman 
> wrote:
>
>> One question I have about both declarations and expressions are whether we 
>> have an appetite to diagnose overloaded operators or not. Personally, I 
>> think it'd be reasonable to diagnose something like `foo->operator 
>> bitand();` or `operator not_eq(A, B);` as expressions, but not reasonable to 
>> diagnose the declaration of the overloaded operators using alternative 
>> tokens.
>
> I agree that `bool operator and(T, T);` shouldn't be diagnosed on (and this 
> patch's clang-tidy sibling will one day also diagnose that, but it's way off).
>
> I think that `foo->operator bitand()` and `operator not_eq(expr1, expr2)` 
> should only diagnose if `foo->operator&()` and `operator!=(expr1, expr2)` are 
> diagnosed, //and// I think that should be a separate warning (I'm not saying 
> that's a good or bad thing to do yet: let me sleep on that). I might be 
> misunderstanding your intention though.

The situation I was thinking of was where the the declaration is for an 
`operator&()` function but the expression is calling `operator bitand()` (or 
vice versa), but 1) I don't feel strongly it needs to be diagnosed, mostly just 
exploring the shape of the diagnostic, and 2) I'd be fine if it was handled 
under a separate flag at some later date.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107292

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

[clang] 10c8f78 - [clang][deps] Move `SingleCommandCompilationDatabase` to a header

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-08-13T12:48:14+02:00
New Revision: 10c8f78ab831dca8d7166c86a863d9bcfe626060

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

LOG: [clang][deps] Move `SingleCommandCompilationDatabase` to a header

This makes `SingleCommandCompilationDatabase` reusable.

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 5903ad13c1d84..c6f7d239b8eb5 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -30,6 +30,24 @@ namespace dependencies {
 
 class DependencyScanningWorkerFilesystem;
 
+/// Compilation database that holds and reports a single compile command.
+class SingleCommandCompilationDatabase : public CompilationDatabase {
+  CompileCommand Command;
+
+public:
+  SingleCommandCompilationDatabase(CompileCommand Cmd)
+  : Command(std::move(Cmd)) {}
+
+  std::vector
+  getCompileCommands(StringRef FilePath) const override {
+return {Command};
+  }
+
+  std::vector getAllCompileCommands() const override {
+return {Command};
+  }
+};
+
 class DependencyConsumer {
 public:
   virtual ~DependencyConsumer() {}

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 74784ebd3b9c3..92b9bdd83e396 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -201,24 +201,6 @@ llvm::cl::opt Verbose("v", llvm::cl::Optional,
 
 } // end anonymous namespace
 
-class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
-public:
-  SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
-  : Command(std::move(Cmd)) {}
-
-  std::vector
-  getCompileCommands(StringRef FilePath) const override {
-return {Command};
-  }
-
-  std::vector getAllCompileCommands() const override {
-return {Command};
-  }
-
-private:
-  tooling::CompileCommand Command;
-};
-
 /// Takes the result of a dependency scan and prints error / dependency files
 /// based on the result.
 ///



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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D107961#2943291 , @MyDeveloperDay 
wrote:

>> The above C++ declarations should be inside a class definition, no?
>
> Not really, we don't have to have any context of where we are (in class or 
> struct). if you had  for example
>
>   class A
>   {
>   #include "myfunctions.h"
>   }
>
> you still would want the code in myfunctions.h to be formatted just the same 
> (less indentation)

Got it!

> So the following code:
>
>   bool foo(int a, Bar)
>   override;
>   bool foo(int a, Bar)
>   override; // comment
>
> Failed on the 13 branch, but is fixed with @krasimir fix, but that isn't 
> backported (we need to do that as a minimum I think)
>
> With this revision that is also fixed, the following occurs if I change the 
> return type
>
>   Bar foo(int a, Bar)
>   override;
>   Bar foo(int a, Bar)
>   override; // comment
>
> And that doesn't feel correct to me. Well not as by default at least. From my 
> perspective there seems to be a propensity to think code is K&R, but that 
> should be the exception not the rule.

I agree.


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

https://reviews.llvm.org/D107961

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 366234.
owenpan added a comment.

Add checking for `final` and `override`.


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

https://reviews.llvm.org/D107961

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar foo(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -995,6 +994,12 @@
   Keywords.kw_import, tok::kw_export);
 }
 
+// Checks whether a token is a primitive type in K&R C (aka C78).
+static bool isC78Type(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_char, tok::kw_short, tok::kw_int, tok::kw_long,
+ tok::kw_unsigned, tok::kw_float, tok::kw_double);
+}
+
 // This function checks whether a token starts the first parameter declaration
 // in a K&R C (aka C78) function definition, e.g.:
 //   int f(a, b)
@@ -1002,13 +1007,15 @@
 //   {
 //  return a + b;
 //   }
-static bool isC78ParameterDecl(const FormatToken *Tok) {
+static bool isC78ParameterDecl(const AdditionalKeywords &Keywords,
+   const FormatToken *Tok) {
   if (!Tok)
 return false;
 
-  if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double,
-tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short,
-tok::kw_unsigned, tok::kw_register))
+  if (!isC78Type(*Tok) &&
+  !Tok->isOneOf(tok::kw_register, tok::kw_struct, tok::kw_union) &&
+  (Tok->isNot(tok::identifier) ||
+   Tok->isOneOf(Keywords.kw_final, Keywords.kw_override)))
 return false;
 
   Tok = Tok->Previous;
@@ -1369,7 +1376,7 @@
 case tok::r_brace:
   addUnwrappedLine();
   return;
-case tok::l_paren:
+case tok::l_paren: {
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a 
parameter
   // declaration.
@@ -1377,14 +1384,19 @@
 break;
   if (!Previous || Previous->isNot(tok::identifier))
 break;
-  if (Previous->Previous && Previous->Previous->is(tok::at))
+  const FormatToken *PrevPrev = Previous->Previous;
+  if (!PrevPrev || (!isC78Type(*PrevPrev) &&
+!PrevPrev->isOneOf(tok::identifier, tok::star)))
 break;
-  if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
-  isC78ParameterDecl(FormatTok)) {
+  const FormatToken *Next = AllTokens[Tokens->getPosition() + 1];
+  if (Next && Next->is(tok::l_paren))
+break;
+  if (isC78ParameterDecl(Keywords, FormatTok)) {
 addUnwrappedLine();
 return;
   }
   break;
+}
 case tok::kw_operator:
   nextToken();
   if (FormatTok->isBinaryOperator())


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar foo(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #i

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

This is better as it will catch more cases but there can still be cases we 
could fall foul.

  Bar foo(a, Bar)
  LLVM_OVERRIDE;
  Bar foo(a, Bar)
  LLVM_FINAL;
  Bar foo(a, Bar)
  LLVM_NOEXCEPT;

Is there any way this K&R stuff can be put behind an option, I just feel like 
we'll be chasing shadows.


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

https://reviews.llvm.org/D107961

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D107961#2943445 , @MyDeveloperDay 
wrote:

> This is better as it will catch more cases but there can still be cases we 
> could fall foul.
>
>   Bar foo(a, Bar)
>   LLVM_OVERRIDE;
>   Bar foo(a, Bar)
>   LLVM_FINAL;
>   Bar foo(a, Bar)
>   LLVM_NOEXCEPT;
>
> Is there any way this K&R stuff can be put behind an option, I just feel like 
> we'll be chasing shadows.

We should do it only as the last resort. Let me first give it another shot.

> I'm not making this cases up, these are examples of real cases that we've 
> used in our cross platform environment because no all C++ compilers supported 
> override/final.

Thanks for coming up with all these cases!


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

https://reviews.llvm.org/D107961

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

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

One thing I think is worth asking in this thread is whether what you're 
analyzing is undefined behavior?

Array subscripting is defined in terms of pointer addition per: 
http://eel.is/c++draft/expr.sub#1
Pointer addition has a special behavior for arrays: 
http://eel.is/c++draft/expr.add#4 ("Otherwise, if P points to an array element 
i of an array object x with n elements ... Otherwise, the behavior is 
undefined.")

I am pretty sure that at least in C++, treating a multidimensional array as a 
single dimensional array is UB because of the strength of the type system 
around arrays. And when you turn some of these examples into constant 
expressions, we reject them based on the bounds. e.g., 
https://godbolt.org/z/nYPcY14a8


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

https://reviews.llvm.org/D104285

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 366237.
owenpan added a comment.

Check for `tok::semi` (in addition to `tok::l_paren`) after the first token of 
a potential K&R C parameter declaration.


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

https://reviews.llvm.org/D107961

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar g(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -995,6 +994,13 @@
   Keywords.kw_import, tok::kw_export);
 }
 
+// Checks whether a token is a primitive type in K&R C (aka C78).
+static bool isC78Type(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_char, tok::kw_short, tok::kw_int, tok::kw_long,
+ tok::kw_unsigned, tok::kw_float, tok::kw_double,
+ tok::identifier);
+}
+
 // This function checks whether a token starts the first parameter declaration
 // in a K&R C (aka C78) function definition, e.g.:
 //   int f(a, b)
@@ -1006,9 +1012,8 @@
   if (!Tok)
 return false;
 
-  if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double,
-tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short,
-tok::kw_unsigned, tok::kw_register))
+  if (!isC78Type(*Tok) &&
+  !Tok->isOneOf(tok::kw_register, tok::kw_struct, tok::kw_union))
 return false;
 
   Tok = Tok->Previous;
@@ -1369,7 +1374,7 @@
 case tok::r_brace:
   addUnwrappedLine();
   return;
-case tok::l_paren:
+case tok::l_paren: {
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a 
parameter
   // declaration.
@@ -1377,14 +1382,18 @@
 break;
   if (!Previous || Previous->isNot(tok::identifier))
 break;
-  if (Previous->Previous && Previous->Previous->is(tok::at))
+  const FormatToken *PrevPrev = Previous->Previous;
+  if (!PrevPrev || (!isC78Type(*PrevPrev) && PrevPrev->isNot(tok::star)))
 break;
-  if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
-  isC78ParameterDecl(FormatTok)) {
+  const FormatToken *Next = AllTokens[Tokens->getPosition() + 1];
+  if (Next && Next->isOneOf(tok::l_paren, tok::semi))
+break;
+  if (isC78ParameterDecl(FormatTok)) {
 addUnwrappedLine();
 return;
   }
   break;
+}
 case tok::kw_operator:
   nextToken();
   if (FormatTok->isBinaryOperator())


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar g(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -995,6 +994,13 @@
   Keywords.kw_import, tok::kw_export);
 }
 
+// Checks whether a token is a primitive type in K&R C (aka C78).
+static bool isC78Type(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_ch

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

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

  bool foo(int a, Bar) override;
  bool foo(int a, Bar) override; // comment
  bool foo(int a, Bar) final;
  bool foo(int a, Bar) final; // comment
  bool foo(a, Bar) final; // comment
  bool foo(a, Bar) final;
  Bar foo(a, Bar) final; // comment
  Bar foo(a, Bar) final;
  Bar foo(int a, Bar) final; // comment
  Bar foo(int a, Bar) final;
  Bar foo(a, Bar) noexcept;
  Bar foo(a, Bar) noexcept(false);
  Bar foo(int a, Bar) LLVM_OVERRIDE;
  Bar foo(a, Bar) LLVM_NOEXCEPT;
  Bar foo(a, Bar) LLVM_NOEXCEPT;
  Bar foo(a, Bar) const;
  bool foo(a, Bar) override;
  bool foo(a, Bar) const override;
  bool foo(a, Bar) noexcept override;
  bool foo(a, Bar) /*ABC*/ override;
  auto foo(a, Bar) OVERRIDE;
  [[nodiscard]] bool foo(a, Bar) final;
  [[nodiscard]] Bar foo(a, Bar) final;

This is looking way more robust to me, LGTM..


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

https://reviews.llvm.org/D107961

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

[PATCH] D106030: [Clang] add support for error+warning fn attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783
+: diag::warn_fe_backend_warning_attr)
+<< FD->getDeclName() << EA->getUserDiagnostic();
+  }

The diagnostics engine knows how to properly format a `NamedDecl *` directly 
(and this will take care of properly quoting the name in the diagnostics as 
well).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:964-965
+bool match =
+(EA->isError() && NewAttrSpellingIndex < ErrorAttr::GNU_warning) ||
+(EA->isWarning() && NewAttrSpellingIndex >= ErrorAttr::GNU_warning);
+if (!match) {

nickdesaulniers wrote:
> aaron.ballman wrote:
> > nickdesaulniers wrote:
> > > aaron.ballman wrote:
> > > > It's a bit tricky for me to tell from the patch -- are the enumerators 
> > > > in the correct order that this logic still works for when the [[]] 
> > > > spelling is used?
> > > For reference, the generated `enum Spelling` looks like:
> > > ```
> > >  3611 public: 
> > >   
> > > 
> > >  3612   enum Spelling {   
> > >   
> > > 
> > >  3613 GNU_error = 0,  
> > >   
> > > 
> > >  3614 CXX11_gnu_error = 1,
> > >   
> > > 
> > >  3615 C2x_gnu_error = 2,  
> > >   
> > > 
> > >  3616 GNU_warning = 3,
> > >   
> > > 
> > >  3617 CXX11_gnu_warning = 4,  
> > >   
> > > 
> > >  3618 C2x_gnu_warning = 5,
> > >   
> > > 
> > >  3619   SpellingNotCalculated = 15
> > >   
> > > 
> > >  3620 
> > >   
> > > 
> > >  3621   };
> > > ```
> > > while using `getAttributeSpellingListIndex` rather than 
> > > `getNormalizedFullName` allows us to avoid a pair of string comparisons 
> > > in favor of a pair of `unsigned` comparisons, we now have an issue 
> > > because there are technically six spellings. (I guess it's not clear to 
> > > me what's `CXX11_gnu_*` vs `C2x_gnu_*`?)  We could be explicit against 
> > > checking all six rather than two comparisons.
> > > 
> > > Or I can use local variables with more helpful identifiers like:
> > > 
> > > ```
> > > bool NewAttrIsError = NewAttrSpellingIndex < ErrorAttr::GNU_warning;
> > > bool NewAttrIsWarning = NewAttrSpellingIndex >= ErrorAttr::GNU_warning;
> > > bool Match = (EA->isError() && NewAttrIsError) || (EA->isWarning() && 
> > > NewAttrIsWarning);
> > > ```
> > > 
> > > WDYT?
> > > (I guess it's not clear to me what's CXX11_gnu_* vs C2x_gnu_*?)
> > 
> > A bit of a historical thing. C2x `[[]]` and C++11 `[[]]` are modeled as 
> > different spellings. This allows us to specify attributes with a `[[]]` 
> > spelling in only one of the languages without having to do an awkward dance 
> > involving language options. e.g., it makes it easier to support an 
> > attribute spelled `__attribute__((foo))` and `[[vendor::foo]]` in C2x and 
> > spelled `[[foo]]` in C++. it picks up the `gnu` bit in the spelling by 
> > virtue of being an attribute with a `GCC` spelling -- IIRC, we needed to 
> > distinguish between GCC and Clang there for some reason I no longer recall.
> > 
> > > WDYT?
> > 
> > I think the current approach is reasonable, but I think the previous 
> > approach (doing the string compare) was more readable. If you have a 
> > preference for using the string compare approach as it originally was, I'd 
> > be fine with that now that I see how my suggestion is playing out in 
> > practice. If you'd prefer to keep the current approach, I'd also be fine 
> > with that. Thank you for tryi

[clang] de763c4 - [clang-format] Distinguish K&R C function definition and attribute

[via cfe-commits]

Author: Owen
Date: 2021-08-13T05:28:19-07:00
New Revision: de763c4037157e60551ba227ccd0ed02e109c317

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

LOG: [clang-format] Distinguish K&R C function definition and attribute

This is a follow-up to https://reviews.llvm.org/D107950 which
missed user-defined types in K&R C.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 0c4cacab50506..e234c74ff750b 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -995,6 +994,13 @@ static bool isJSDeclOrStmt(const AdditionalKeywords 
&Keywords,
   Keywords.kw_import, tok::kw_export);
 }
 
+// Checks whether a token is a type in K&R C (aka C78).
+static bool isC78Type(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_char, tok::kw_short, tok::kw_int, tok::kw_long,
+ tok::kw_unsigned, tok::kw_float, tok::kw_double,
+ tok::identifier);
+}
+
 // This function checks whether a token starts the first parameter declaration
 // in a K&R C (aka C78) function definition, e.g.:
 //   int f(a, b)
@@ -1006,9 +1012,8 @@ static bool isC78ParameterDecl(const FormatToken *Tok) {
   if (!Tok)
 return false;
 
-  if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double,
-tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short,
-tok::kw_unsigned, tok::kw_register))
+  if (!isC78Type(*Tok) &&
+  !Tok->isOneOf(tok::kw_register, tok::kw_struct, tok::kw_union))
 return false;
 
   Tok = Tok->Previous;
@@ -1369,7 +1374,7 @@ void UnwrappedLineParser::parseStructuralElement(bool 
IsTopLevel) {
 case tok::r_brace:
   addUnwrappedLine();
   return;
-case tok::l_paren:
+case tok::l_paren: {
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a 
parameter
   // declaration.
@@ -1377,14 +1382,18 @@ void UnwrappedLineParser::parseStructuralElement(bool 
IsTopLevel) {
 break;
   if (!Previous || Previous->isNot(tok::identifier))
 break;
-  if (Previous->Previous && Previous->Previous->is(tok::at))
+  const FormatToken *PrevPrev = Previous->Previous;
+  if (!PrevPrev || (!isC78Type(*PrevPrev) && PrevPrev->isNot(tok::star)))
 break;
-  if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
-  isC78ParameterDecl(FormatTok)) {
+  const FormatToken *Next = AllTokens[Tokens->getPosition() + 1];
+  if (Next && Next->isOneOf(tok::l_paren, tok::semi))
+break;
+  if (isC78ParameterDecl(FormatTok)) {
 addUnwrappedLine();
 return;
   }
   break;
+}
 case tok::kw_operator:
   nextToken();
   if (FormatTok->isBinaryOperator())

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 1283aa67b3370..63e93b775ac6a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) {
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar g(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros



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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Owen Pan 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 rGde763c403715: [clang-format] Distinguish K&R C function 
definition and attribute (authored by Owen 
, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D107961?vs=366237&id=366253#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107961

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar g(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -995,6 +994,13 @@
   Keywords.kw_import, tok::kw_export);
 }
 
+// Checks whether a token is a type in K&R C (aka C78).
+static bool isC78Type(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_char, tok::kw_short, tok::kw_int, tok::kw_long,
+ tok::kw_unsigned, tok::kw_float, tok::kw_double,
+ tok::identifier);
+}
+
 // This function checks whether a token starts the first parameter declaration
 // in a K&R C (aka C78) function definition, e.g.:
 //   int f(a, b)
@@ -1006,9 +1012,8 @@
   if (!Tok)
 return false;
 
-  if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double,
-tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short,
-tok::kw_unsigned, tok::kw_register))
+  if (!isC78Type(*Tok) &&
+  !Tok->isOneOf(tok::kw_register, tok::kw_struct, tok::kw_union))
 return false;
 
   Tok = Tok->Previous;
@@ -1369,7 +1374,7 @@
 case tok::r_brace:
   addUnwrappedLine();
   return;
-case tok::l_paren:
+case tok::l_paren: {
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a 
parameter
   // declaration.
@@ -1377,14 +1382,18 @@
 break;
   if (!Previous || Previous->isNot(tok::identifier))
 break;
-  if (Previous->Previous && Previous->Previous->is(tok::at))
+  const FormatToken *PrevPrev = Previous->Previous;
+  if (!PrevPrev || (!isC78Type(*PrevPrev) && PrevPrev->isNot(tok::star)))
 break;
-  if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
-  isC78ParameterDecl(FormatTok)) {
+  const FormatToken *Next = AllTokens[Tokens->getPosition() + 1];
+  if (Next && Next->isOneOf(tok::l_paren, tok::semi))
+break;
+  if (isC78ParameterDecl(FormatTok)) {
 addUnwrappedLine();
 return;
   }
   break;
+}
 case tok::kw_operator:
   nextToken();
   if (FormatTok->isBinaryOperator())


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8247,6 +8247,16 @@
"  return a + b < c;\n"
"};",
Style);
+  verifyFormat("byte *\n" // Break here.
+   "f(a)\n"   // Break here.
+   "byte a[];\n"
+   "{\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar g(int a, Bar) final; // comment",
+   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,7 +14,6 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_o

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: eugenis, melver, browneee, dvyukov.
Herald added subscribers: ormris, dexonsmith, jdoerfert, steven_wu, hiraditya.
Herald added a reviewer: aaron.ballman.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

The purpose of __attribute__((no_sanitizer_instrumentation)) is to
prevent all kinds of sanitizer instrumentation applied to a certain
function.

The no_sanitize(...) attribute drops instrumentation checks, but may
still insert code preventing false positive reports. In some cases
though (e.g. when building Linux kernel with -fsanitize=kernel-memory
or -fsanitize=thread) the users may want to avoid any kind of
instrumentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/Attributes.td

Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -175,6 +175,9 @@
 /// No SanitizeCoverage instrumentation.
 def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", [FnAttr]>;
 
+/// Do not instrument function with sanitizers.
+def NoSanitizerInstrumentation: EnumAttr<"no_sanitizer_instrumentation", [FnAttr]>;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", [FnAttr]>;
 
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -219,6 +219,7 @@
   kw_nocf_check,
   kw_nounwind,
   kw_nosanitize_coverage,
+  kw_no_sanitizer_instrumentation,
   kw_null_pointer_is_valid,
   kw_optforfuzzing,
   kw_optnone,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,6 +102,7 @@
 // CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
+// CHECK-NEXT: NoSanitizerInstrumentation (SubjectMatchRule_function)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
 // CHECK-NEXT: NoStackProtector (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((no_sanitizer_instrumentation)) {
+}
+// CHECK: no_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: no_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2285,6 +2285,7 @@
   /// ShouldInstrumentFunction - Return true if the current function should be
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
+  bool ShouldSkipSanitizerInstrumentation();
 
   /// ShouldXRayInstrument - Return true if the current function should be
   /// instrumented with XRay nop sleds.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -381,6 +381,9 @@
"__cyg_profile_func_exit");
   }
 
+  if (ShouldSkipSanitizerInstrumentation())
+CurFn->addFnAttr(llvm::Attribute::NoSanitizerInstrumentation);
+
   // Emit debug descriptor for function end.
   if (CGDebugInfo *DI = getDebugInfo())
 DI->EmitFunctionEnd(Builder, CurFn);
@@ -517,6 +520,14 @@
   return true;
 }
 
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;
+  if (CurFuncDecl->hasAttr())
+return true;
+  return false;
+}
+
 /// ShouldXRayInstrument - Return true if the current function should be
 /// instrumented with XRay nop sleds.
 bool CodeGenFunction::ShouldXRayInstrumentFunction() const {
Index: clang/include

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366256.
glider added a comment.

Added a comment to clang/lib/CodeGen/CodeGenFunction.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/Attributes.td

Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -175,6 +175,9 @@
 /// No SanitizeCoverage instrumentation.
 def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", [FnAttr]>;
 
+/// Do not instrument function with sanitizers.
+def NoSanitizerInstrumentation: EnumAttr<"no_sanitizer_instrumentation", [FnAttr]>;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", [FnAttr]>;
 
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -219,6 +219,7 @@
   kw_nocf_check,
   kw_nounwind,
   kw_nosanitize_coverage,
+  kw_no_sanitizer_instrumentation,
   kw_null_pointer_is_valid,
   kw_optforfuzzing,
   kw_optnone,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,6 +102,7 @@
 // CHECK-NEXT: NoProfileFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
+// CHECK-NEXT: NoSanitizerInstrumentation (SubjectMatchRule_function)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
 // CHECK-NEXT: NoStackProtector (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((no_sanitizer_instrumentation)) {
+}
+// CHECK: no_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: no_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// ShouldSkipSanitizerInstrumentation - Return true if the current function
+  /// should not be instrumented with sanitizers.
+  bool ShouldSkipSanitizerInstrumentation();
+
   /// ShouldXRayInstrument - Return true if the current function should be
   /// instrumented with XRay nop sleds.
   bool ShouldXRayInstrumentFunction() const;
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -381,6 +381,9 @@
"__cyg_profile_func_exit");
   }
 
+  if (ShouldSkipSanitizerInstrumentation())
+CurFn->addFnAttr(llvm::Attribute::NoSanitizerInstrumentation);
+
   // Emit debug descriptor for function end.
   if (CGDebugInfo *DI = getDebugInfo())
 DI->EmitFunctionEnd(Builder, CurFn);
@@ -517,6 +520,14 @@
   return true;
 }
 
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;
+  if (CurFuncDecl->hasAttr())
+return true;
+  return false;
+}
+
 /// ShouldXRayInstrument - Return true if the current function should be
 /// instrumented with XRay nop sleds.
 bool CodeGenFunction::ShouldXRayInstrumentFunction() const {
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -2592,6 +2592,17 @@
   }];
 }
 
+def NoSanitizerInstrumentationDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Use the ``no_sanitizer_instrumentation`` attribute on a function to specify
+that no sanitizer instrumentation should be applied.
+
+This is not the same as ``

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Marco Elver via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/AsmParser/LLToken.h:222
   kw_nosanitize_coverage,
+  kw_no_sanitizer_instrumentation,
   kw_null_pointer_is_valid,

LLVM seems to just call them nofoo i.e. here it'd be 
nosanitizer_instrumentation.
I think Clang and LLVM are a bit inconsistent in their naming (no_foo vs nofoo) 
. See e.g. nosanitize_coverage which is similarly inconsistent yet consistent 
with other LLVM attrs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests: http://45.33.8.238/linux/53600/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107961

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

[clang] cfdfb75 - [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

[Justas Janickas via cfe-commits]

Author: Justas Janickas
Date: 2021-08-13T13:55:22+01:00
New Revision: cfdfb75c1ff354471bcea5fad872e40e345016ae

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

LOG: [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

Some Clang diagnostics could only report OpenCL C version. Because
C++ for OpenCL can be used as an alternative to OpenCL C, the text
for diagnostics should reflect that.

Desrciptions modified for these diagnostics:
`err_opencl_unknown_type_specifier`
`warn_option_invalid_ocl_version`
`err_attribute_requires_opencl_version`
`warn_opencl_attr_deprecated_ignored`
`ext_opencl_ext_vector_type_rgba_selector`

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.h
clang/lib/Basic/LangOptions.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Frontend/opencl.cl
clang/test/SemaOpenCL/ext_vectors.cl
clang/test/SemaOpenCL/nosvm.cl

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 4dff3379ed35b..4e21e276c69c5 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -149,8 +149,8 @@ def err_nullability_conflicting : Error<
 
 // OpenCL Section 6.8.g
 def err_opencl_unknown_type_specifier : Error<
-  "%select{OpenCL C|C++ for OpenCL}0 version %1 does not support the "
-  "'%2' %select{type qualifier|storage class specifier}3">;
+  "%0 does not support the '%1' "
+  "%select{type qualifier|storage class specifier}2">;
 
 def warn_unknown_attribute_ignored : Warning<
   "unknown attribute %0 ignored">, InGroup;

diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 4bc5097762186..2bbc93d5682ce 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -247,7 +247,7 @@ def err_invalid_vfs_overlay : Error<
   "invalid virtual filesystem overlay file '%0'">, DefaultFatal;
 
 def warn_option_invalid_ocl_version : Warning<
-  "OpenCL version %0 does not support the option '%1'">, InGroup;
+  "%0 does not support the option '%1'">, InGroup;
 
 def err_builtin_needs_feature : Error<"%0 needs target feature %1">;
 def err_function_needs_feature : Error<

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 73217b418e81f..cbcc0e1f11c48 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2959,7 +2959,7 @@ def err_attribute_requires_positive_integer : Error<
   "%0 attribute requires a %select{positive|non-negative}1 "
   "integral compile time constant expression">;
 def err_attribute_requires_opencl_version : Error<
-  "%0 attribute requires OpenCL version %1%select{| or above}2">;
+  "attribute %0 is supported in the OpenCL version %1%select{| onwards}2">;
 def err_invalid_branch_protection_spec : Error<
   "invalid or misplaced branch protection specification '%0'">;
 def warn_unsupported_target_attribute
@@ -10098,8 +10098,7 @@ def 
err_opencl_type_can_only_be_used_as_function_parameter : Error <
 def err_opencl_type_not_found : Error<
   "%0 type %1 not found; include the base header with 
-finclude-default-header">;
 def warn_opencl_attr_deprecated_ignored : Warning <
-  "%0 attribute is deprecated and ignored in OpenCL version %1">,
-  InGroup;
+  "%0 attribute is deprecated and ignored in %1">, InGroup;
 def err_opencl_variadic_function : Error<
   "invalid prototype, variadic arguments are not allowed in OpenCL">;
 def err_opencl_requires_extension : Error<
@@ -10164,7 +10163,7 @@ def err_opencl_builtin_expected_type : Error<
 
 // OpenCL v3.0 s6.3.7 - Vector Components
 def ext_opencl_ext_vector_type_rgba_selector: ExtWarn<
-  "vector component name '%0' is an OpenCL C version 3.0 feature">,
+  "vector component name '%0' is a feature from OpenCL version 3.0 onwards">,
   InGroup;
 
 def err_openclcxx_placement_new : Error<

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 72793114dbb38..ea6a792f0598b 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -444,6 +444,10 @@ class LangOptions : public LangOptionsBase {
   /// Return the OpenCL C or C++ version as a VersionTuple.
   Vers

[PATCH] D107648: [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

[Justas Janickas 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 rGcfdfb75c1ff3: [OpenCL] Clang diagnostics allow reporting C++ 
for OpenCL version. (authored by Topotuna).

Changed prior to commit:
  https://reviews.llvm.org/D107648?vs=365179&id=366259#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107648

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Frontend/opencl.cl
  clang/test/SemaOpenCL/ext_vectors.cl
  clang/test/SemaOpenCL/nosvm.cl

Index: clang/test/SemaOpenCL/nosvm.cl
===
--- clang/test/SemaOpenCL/nosvm.cl
+++ clang/test/SemaOpenCL/nosvm.cl
@@ -1,13 +1,16 @@
 // RUN: %clang_cc1 -verify %s
-// RUN: %clang_cc1 -verify -cl-std=CL2.0 -D CL20 %s
+// RUN: %clang_cc1 -verify -cl-std=CL2.0 %s
+// RUN: %clang_cc1 -verify -cl-std=clc++1.0 %s
 // RUN: %clang_cc1 -verify -x c -D NOCL %s
 
 #ifndef NOCL
 kernel void f(__attribute__((nosvm)) global int* a);
-#ifndef CL20
-// expected-error@-2 {{'nosvm' attribute requires OpenCL version 2.0}}
+#if (__OPENCL_C_VERSION__ == 200)
+// expected-warning@-2 {{'nosvm' attribute is deprecated and ignored in OpenCL C version 2.0}}
+#elif (__OPENCL_CPP_VERSION__ == 100)
+// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in C++ for OpenCL version 1.0}}
 #else
-// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in OpenCL version 2.0}}
+// expected-error@-6 {{attribute 'nosvm' is supported in the OpenCL version 2.0 onwards}}
 #endif
 
 __attribute__((nosvm)) void g(); // expected-warning {{'nosvm' attribute only applies to variables}}
Index: clang/test/SemaOpenCL/ext_vectors.cl
===
--- clang/test/SemaOpenCL/ext_vectors.cl
+++ clang/test/SemaOpenCL/ext_vectors.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.1
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 
@@ -9,12 +10,12 @@
 
   V = V.abgr;
 #if (__OPENCL_C_VERSION__ < 300)
-  // expected-warning@-2 {{vector component name 'a' is an OpenCL C version 3.0 feature}}
+  // expected-warning@-2 {{vector component name 'a' is a feature from OpenCL version 3.0 onwards}}
 #endif
 
   V = V.xyzr;
   // expected-error@-1 {{illegal vector component name 'r'}}
 #if (__OPENCL_C_VERSION__ < 300)
-  // expected-warning@-3 {{vector component name 'r' is an OpenCL C version 3.0 feature}}
+  // expected-warning@-3 {{vector component name 'r' is a feature from OpenCL version 3.0 onwards}}
 #endif
 }
Index: clang/test/Frontend/opencl.cl
===
--- clang/test/Frontend/opencl.cl
+++ clang/test/Frontend/opencl.cl
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.1 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.2 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL2.0 -DSYNTAX
-// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=clc++ -DSYNTAX
+// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=clc++1.0 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -fblocks -DBLOCKS -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.1 -fblocks -DBLOCKS -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.2 -fblocks -DBLOCKS -DSYNTAX
@@ -10,6 +10,7 @@
 // RUN: %clang_cc1 -cl-std=CL1.1 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION11 %s
 // RUN: %clang_cc1 -cl-std=CL1.2 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION12 %s
 // RUN: %clang_cc1 -cl-std=CL2.0 -cl-strict-aliasing %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION20 %s
+// RUN: %clang_cc1 -cl-std=clc++1.0 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCLCPP-VERSION10 %s
 
 #ifdef SYNTAX
 class test{
@@ -30,6 +31,7 @@
   // expected-error@-6{{blocks support disabled - compile with -fblocks or pick a deployment target that supports them}}
 #endif
 
-// CHECK-INVALID-OPENCL-VERSION11: warning: OpenCL version 1.1 does not support the option '-cl-strict-aliasing'
-// CHECK-INVALID-OPENCL-VERSION12: warning: OpenCL version 1.2 does not support the option '-cl-strict-aliasing'
-// CHECK-INVALID-O

[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

Probably want to address the other cleanups in another patch; the parens fixes 
and test LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107843

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

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think that C and C++ should behave the same here; at least, I don't see any 
reason why they should have different capabilities.

The paper said that there is no expected code breakage from this change, but 
have you tried building a diverse corpus of code (like a distro's worth of 
packages) under this patch to see if anything actually breaks in practice? (I 
don't expect breakage that isn't identifying an actual issue in the code, but 
having some verification would be appreciated.) This would also help to 
identify whether the change is appropriate for C as well.




Comment at: clang/test/CodeGen/string-literal-short-wstring.c:1-2
-// RUN: %clang_cc1 -x c++ -triple %itanium_abi_triple -emit-llvm 
-fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s 
--check-prefix=CHECK --check-prefix=ITANIUM
-// RUN: %clang_cc1 -x c++ -triple %ms_abi_triple -emit-llvm -fwchar-type=short 
-fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=MSABI
-// Runs in c++ mode so that wchar_t is available.
+// RUN: %clang_cc1 -x c -triple %itanium_abi_triple -emit-llvm 
-fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s 
--check-prefix=CHECK --check-prefix=ITANIUM
+// RUN: %clang_cc1 -x c -triple %ms_abi_triple -emit-llvm -fwchar-type=short 
-fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=MSABI
+

No need to tell the file to be in C mode; it is by virtue of the extension.



Comment at: clang/test/CodeGen/string-literal-short-wstring.c:17
   // MSABI: linkonce_odr dso_local unnamed_addr constant [3 x i16] [i16 65, 
i16 66, i16 0]
-  const wchar_t *foo = L"AB";
+  const unsigned short *foo = L"AB";
 

I'd feel most comfortable if the top of the file did:
```
typedef __WCHAR_TYPE__ wchar_t;
```
as that's the definition of `wchar_t` from the `stddef.h` provided by Clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

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

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D106215#2943611 , @aaron.ballman 
wrote:

> I think that C and C++ should behave the same here; at least, I don't see any 
> reason why they should have different capabilities.

I agree but as WG14 hasn't weighted in I didn't want to make that call.
What do you think?

> The paper said that there is no expected code breakage from this change, but 
> have you tried building a diverse corpus of code (like a distro's worth of 
> packages) under this patch to see if anything actually breaks in practice? (I 
> don't expect breakage that isn't identifying an actual issue in the code, but 
> having some verification would be appreciated.) This would also help to 
> identify whether the change is appropriate for C as well.

We have done regexes over various repositories (every vcpkg package) with no 
match. Not running a complete compiler


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

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

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D106577#2906542 , @rsmith wrote:

> If Aaron's checked with WG14 and the intent is for this to only constrain how 
> literals are represented, and not the complete implementation, then I'm 
> entirely fine with us defining the macro ourselves. But that's not the 
> interpretation that several other vendors have taken. If we're confident that 
> the intent is just that this macro lists (effectively) the latest version of 
> the Unicode standard that we've heard of, we should let the various libc 
> vendors that currently define the macro know that they're doing it wrong and 
> the definition belongs in the compiler.

The discussion on the WG14 reflectors has wound down. Summarizing the key point 
of the discussion, Richard asked:

> One specific example I'd like to be considered:
> Suppose the C standard library implementation's mbstowcs converts a certain 
> multi-byte character C to somewhere in the Unicode private use area, because 
> Unicode version N doesn't have a corresponding character. Suppose further 
> that the compiler is aware of Unicode version N+1, in which a character 
> corresponding to C was added. Is an implementation formed by that combination 
> of compiler and standard library, that defines `__STDC_ISO_10646__` to N+1, 
> conforming? Or is it non-conforming because it represents character C as 
> something other than the corresponding short name from Unicode version N+1?

And David Keaton (long-time WG14 member and current convener) replied:

> Yikes!  It does indeed sound like the library would affect the value of 
> `__STDC_ISO_10646__` in that case.  Thanks for clarifying the details.

There was no further discussion after that point, so I think the unofficial 
WG14 stance is that the compiler and the library need to collude on setting the 
value of that macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577

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

[PATCH] D108032: [analyzer] Retrieve a character from CompoundLiteralExpr as an initializer for constant arrays.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: NoQ, vsavchenko, steakhal, martong.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Add support of handling `CompoundLiteralExpr` in 
`RegionStoreManager::getConstantValFromConstArrayInitializer`. Retrieve a value 
from `CompoundLiteralExpr` which is an initializer of constant arrays in global 
and local scopes. This patch also disables direct binding a compound literal 
for constant arrays of local storage duration.

Example:

  const int arr[8] = (const int[8]){1, 2, 3, 4}; // compound literal


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108032

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/compound-literals.c

Index: clang/test/Analysis/compound-literals.c
===
--- clang/test/Analysis/compound-literals.c
+++ clang/test/Analysis/compound-literals.c
@@ -21,3 +21,139 @@
   clang_analyzer_eval(pointers[0] == NULL); // expected-warning{{FALSE}}
   clang_analyzer_eval(pointers[1] == NULL); // expected-warning{{TRUE}}
 }
+
+void local_arr_index2() {
+  const int local_arr[8] = (const int[8]){[2] = 3, [0] = 1, [1] = 2, [3] = 4};
+  clang_analyzer_eval(local_arr[0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1] == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[2] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[3] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[4] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[5] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[6] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[7] == 0); // expected-warning{{TRUE}}
+}
+
+void local_arr_out_of_bound_index3() {
+  const int local_arr[8] = (const int[8]){[2] = 3, [0] = 1, [1] = 2, [3] = 4};
+  int x = -42;
+  int res = local_arr[x]; // expected-warning{{garbage or undefined}}
+}
+
+void local_arr_out_of_bound_index4() {
+  const int local_arr[8] = (const int[8]){[2] = 3, [0] = 1, [1] = 2, [3] = 4};
+  int x = 42;
+  int res = local_arr[x]; // expected-warning{{garbage or undefined}}
+}
+
+void local_arr_index3() {
+  const int local_arr[8] = (const int[8]){1, 2, 3, 4};
+  clang_analyzer_eval(local_arr[0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1] == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[2] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[3] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[4] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[5] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[6] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[7] == 0); // expected-warning{{TRUE}}
+}
+
+void local_arr_index4() {
+  int const local_arr[][2][3] = (int const[2][2][3]){{{1, 2}, {}}, {{7, 8}, {10, 11, 12}}};
+  clang_analyzer_eval(local_arr[0][0][0] == 1);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[0][0][1] == 2);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[0][0][2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[0][1][0] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[0][1][1] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[0][1][2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1][0][0] == 7);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1][0][1] == 8);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1][0][2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1][1][0] == 10); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1][1][1] == 11); // expected-warning{{TRUE}}
+  clang_analyzer_eval(local_arr[1][1][2] == 12); // expected-warning{{TRUE}}
+}
+
+void local_arr_index5() {
+  int const(*ptr_arr)[2][3] = (int const[2][2][3]){{{1, 2}, {}}, {{7, 8}, {10, 11, 12}}};
+  clang_analyzer_eval(ptr_arr[0][0][0] == 1);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[0][0][1] == 2);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[0][0][2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[0][1][0] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[0][1][1] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[0][1][2] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[1][0][0] == 7);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[1][0][1] == 8);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr_arr[1][0][2] == 0);  // expected-warning{{TRUE}}
+  clang_anal

[clang] d754b97 - [NFC] Drop idle compiler option from the test.

[Alexey Bader via cfe-commits]

Author: Alexey Bader
Date: 2021-08-13T13:20:11+03:00
New Revision: d754b970eddb83b1c68346a36b067391d93166b0

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

LOG: [NFC] Drop idle compiler option from the test.

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

Added: 


Modified: 
clang/test/AST/ast-print-sycl-unique-stable-name.cpp

Removed: 




diff  --git a/clang/test/AST/ast-print-sycl-unique-stable-name.cpp 
b/clang/test/AST/ast-print-sycl-unique-stable-name.cpp
index 3f49ea9ee733c..b55d01aa1b6b6 100644
--- a/clang/test/AST/ast-print-sycl-unique-stable-name.cpp
+++ b/clang/test/AST/ast-print-sycl-unique-stable-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - -triple 
spir64-sycldevice | FileCheck %s
+// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - | FileCheck %s
 
 template 
 void WrappedInTemplate(T t) {



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

[PATCH] D108020: [NFC] Drop idle compiler option from the test.

[Alexey Bader 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 rGd754b970eddb: [NFC] Drop idle compiler option from the test. 
(authored by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108020

Files:
  clang/test/AST/ast-print-sycl-unique-stable-name.cpp


Index: clang/test/AST/ast-print-sycl-unique-stable-name.cpp
===
--- clang/test/AST/ast-print-sycl-unique-stable-name.cpp
+++ clang/test/AST/ast-print-sycl-unique-stable-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - -triple 
spir64-sycldevice | FileCheck %s
+// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - | FileCheck %s
 
 template 
 void WrappedInTemplate(T t) {


Index: clang/test/AST/ast-print-sycl-unique-stable-name.cpp
===
--- clang/test/AST/ast-print-sycl-unique-stable-name.cpp
+++ clang/test/AST/ast-print-sycl-unique-stable-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - -triple spir64-sycldevice | FileCheck %s
+// RUN: %clang_cc1 -ast-print -fsycl-is-device %s -o - | FileCheck %s
 
 template 
 void WrappedInTemplate(T t) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D106215#2943631 , @cor3ntin wrote:

> In D106215#2943611 , @aaron.ballman 
> wrote:
>
>> I think that C and C++ should behave the same here; at least, I don't see 
>> any reason why they should have different capabilities.
>
> I agree but as WG14 hasn't weighted in I didn't want to make that call.
> What do you think?

My reading of C2x is that this is implementation-defined there as well.

6.4.4.4p13:

A wide character constant prefixed by the letter L has type wchar_t, an integer 
type defined in the
 header; a wide character constant prefixed by the letter u or U has 
type char16_t or
char32_t, respectively, unsigned integer types defined in the  header. 
The value of a
wide character constant containing a single multibyte character that maps to a 
single member of the
extended execution character set is the wide character corresponding to that 
multibyte character,
as defined by the mbtowc, mbrtoc16, or mbrtoc32 function as appropriate for its 
type, with an
implementation-defined current locale. The value of a wide character constant 
containing more
than one multibyte character or a single multibyte character that maps to 
multiple members of
the extended execution character set, or containing a multibyte character or 
escape sequence not
represented in the extended execution character set, is implementation-defined.

Do you agree?

>> The paper said that there is no expected code breakage from this change, but 
>> have you tried building a diverse corpus of code (like a distro's worth of 
>> packages) under this patch to see if anything actually breaks in practice? 
>> (I don't expect breakage that isn't identifying an actual issue in the code, 
>> but having some verification would be appreciated.) This would also help to 
>> identify whether the change is appropriate for C as well.
>
> We have done regexes over various repositories (every vcpkg package) with no 
> match. Not running a complete compiler

Regexes are a good start but they miss the goofy (and sometimes awful) stuff 
that people do with token pasting, line continuations, and other random tricks. 
Would you be willing to try this as an experiment, or am I asking too much? :-) 
My thinking is that if we don't see any breakage from compiling a diverse 
corpus of code, we've done enough due diligence to suggest this is safe for 
both C and C++, but if we see some breakage, we can either identify that 
there's some valid use for this that we've not considered (less likely) and 
would be informative for both WG21 and WG14, or we can identify that we helped 
find bugs in real world code (more likely) which is also good feedback for the 
committees.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

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

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 366265.
cor3ntin added a comment.

Improve tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/LiteralSupport.cpp
  clang/test/CodeGen/char-literal.c
  clang/test/CodeGen/string-literal-short-wstring.c
  clang/test/Lexer/char-literal.cpp
  clang/test/Preprocessor/Weverything_pragma.c

Index: clang/test/Preprocessor/Weverything_pragma.c
===
--- clang/test/Preprocessor/Weverything_pragma.c
+++ clang/test/Preprocessor/Weverything_pragma.c
@@ -10,21 +10,21 @@
 // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 {
  // A diagnostic without DefaultIgnore, and not part of a group.
- (void) L'ab'; // expected-warning {{extraneous characters in character constant ignored}}
+ (void) 'ab'; // expected-warning {{multi-character character constant}}
 
 #pragma clang diagnostic warning "-Weverything" // Should not change anyhting.
 #define UNUSED_MACRO2 1 // expected-warning{{macro is not used}}
- (void) L'cd'; // expected-warning {{extraneous characters in character constant ignored}}
+ (void) 'cd'; // expected-warning {{multi-character character constant}}
 
 #pragma clang diagnostic ignored "-Weverything" // Ignore warnings now.
 #define UNUSED_MACRO2 1 // no warning
- (void) L'ef'; // no warning here
+ (void) 'ef'; // no warning here
 
 #pragma clang diagnostic warning "-Weverything" // Revert back to warnings.
 #define UNUSED_MACRO3 1 // expected-warning{{macro is not used}}
- (void) L'gh'; // expected-warning {{extraneous characters in character constant ignored}}
+ (void) 'gh'; // expected-warning {{multi-character character constant}}
 
 #pragma clang diagnostic error "-Weverything"  // Give errors now.
 #define UNUSED_MACRO4 1 // expected-error{{macro is not used}}
- (void) L'ij'; // expected-error {{extraneous characters in character constant ignored}}
+ (void) 'ij'; // expected-error {{multi-character character constant}}
 }
Index: clang/test/Lexer/char-literal.cpp
===
--- clang/test/Lexer/char-literal.cpp
+++ clang/test/Lexer/char-literal.cpp
@@ -21,7 +21,12 @@
 char16_t g = u'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
 char16_t h = u'\U0010FFFD'; // expected-error {{character too large for enclosing character literal type}}
 
+#ifdef __cplusplus
+wchar_t i = L'ab'; // expected-error {{wide character literals may not contain multiple characters}}
+
+#else
 wchar_t i = L'ab'; // expected-warning {{extraneous characters in character constant ignored}}
+#endif
 wchar_t j = L'\U0010FFFD';
 
 char32_t k = U'\U0010FFFD';
Index: clang/test/CodeGen/string-literal-short-wstring.c
===
--- clang/test/CodeGen/string-literal-short-wstring.c
+++ clang/test/CodeGen/string-literal-short-wstring.c
@@ -1,11 +1,14 @@
-// RUN: %clang_cc1 -x c++ -triple %itanium_abi_triple -emit-llvm -fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=ITANIUM
-// RUN: %clang_cc1 -x c++ -triple %ms_abi_triple -emit-llvm -fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=MSABI
-// Runs in c++ mode so that wchar_t is available.
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=ITANIUM
+// RUN: %clang_cc1 -triple %ms_abi_triple -emit-llvm -fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=MSABI
+
+// Run in C mode as wide multichar literals are not valid in C++
 
 // XFAIL: hexagon
 // Hexagon aligns arrays of size 8+ bytes to a 64-bit boundary, which fails
 // the first check line with "align 1".
 
+typedef __WCHAR_TYPE__ wchar_t;
+
 int main() {
   // This should convert to utf8.
   // CHECK: private unnamed_addr constant [10 x i8] c"\E1\84\A0\C8\A0\F4\82\80\B0\00", align 1
@@ -20,8 +23,6 @@
   // MSABI: linkonce_odr dso_local unnamed_addr constant [5 x i16] [i16 4384, i16 544, i16 -9272, i16 -9168, i16 0]
   const wchar_t *bar = L"\u1120\u0220\U00102030";
 
-
-
   // Should pick second character.
   // CHECK: store i8 98
   char c = 'ab';
Index: clang/test/CodeGen/char-literal.c
===
--- clang/test/CodeGen/char-literal.c
+++ clang/test/CodeGen/char-literal.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -check-prefix=CHECK-C %s
-// RUN: %clang_cc1 -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -check-prefix=CHECK-C %s
 // RUN: %clang_cc1 -x c++ -std=c++11 -triple i386-unknown-unk

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Corentin Jabot via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/string-literal-short-wstring.c:17
   // MSABI: linkonce_odr dso_local unnamed_addr constant [3 x i16] [i16 65, 
i16 66, i16 0]
-  const wchar_t *foo = L"AB";
+  const unsigned short *foo = L"AB";
 

aaron.ballman wrote:
> I'd feel most comfortable if the top of the file did:
> ```
> typedef __WCHAR_TYPE__ wchar_t;
> ```
> as that's the definition of `wchar_t` from the `stddef.h` provided by Clang.
Thanks, I was looking for exactly that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

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

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Corentin Jabot via Phabricator via cfe-commits]

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

In D106215#2943653 , @aaron.ballman 
wrote:

> In D106215#2943631 , @cor3ntin 
> wrote:
>
>> In D106215#2943611 , 
>> @aaron.ballman wrote:
>>
>>> I think that C and C++ should behave the same here; at least, I don't see 
>>> any reason why they should have different capabilities.
>>
>> I agree but as WG14 hasn't weighted in I didn't want to make that call.
>> What do you think?
>
> My reading of C2x is that this is implementation-defined there as well.
>
> 6.4.4.4p13:
>
> A wide character constant prefixed by the letter L has type wchar_t, an 
> integer type defined in the
>  header; a wide character constant prefixed by the letter u or U 
> has type char16_t or
> char32_t, respectively, unsigned integer types defined in the  
> header. The value of a
> wide character constant containing a single multibyte character that maps to 
> a single member of the
> extended execution character set is the wide character corresponding to that 
> multibyte character,
> as defined by the mbtowc, mbrtoc16, or mbrtoc32 function as appropriate for 
> its type, with an
> implementation-defined current locale. The value of a wide character constant 
> containing more
> than one multibyte character or a single multibyte character that maps to 
> multiple members of
> the extended execution character set, or containing a multibyte character or 
> escape sequence not
> represented in the extended execution character set, is 
> implementation-defined.
>
> Do you agree?

Yes, I agree.
I think clang could make it ill-formed if it wanted to!
If we want to do that we could probably remove some more code :)

>>> The paper said that there is no expected code breakage from this change, 
>>> but have you tried building a diverse corpus of code (like a distro's worth 
>>> of packages) under this patch to see if anything actually breaks in 
>>> practice? (I don't expect breakage that isn't identifying an actual issue 
>>> in the code, but having some verification would be appreciated.) This would 
>>> also help to identify whether the change is appropriate for C as well.
>>
>> We have done regexes over various repositories (every vcpkg package) with no 
>> match. Not running a complete compiler
>
> Regexes are a good start but they miss the goofy (and sometimes awful) stuff 
> that people do with token pasting, line continuations, and other random 
> tricks. Would you be willing to try this as an experiment, or am I asking too 
> much? :-) My thinking is that if we don't see any breakage from compiling a 
> diverse corpus of code, we've done enough due diligence to suggest this is 
> safe for both C and C++, but if we see some breakage, we can either identify 
> that there's some valid use for this that we've not considered (less likely) 
> and would be informative for both WG21 and WG14, or we can identify that we 
> helped find bugs in real world code (more likely) which is also good feedback 
> for the committees.

Unless there is a script to do that easily, I'm not sure I'll be able to get to 
it any time soon.
But really, there is 0 use for these things! And you can't do much goofiness  
`L  ## 'ab'` certainly - but that wouldn't be very useful either


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

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

[PATCH] D107893: [clang] [MinGW] Consider the per-target libc++ include directory too

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

Any chance this patch could be backported onto the LLVM 13 branch? I have been 
using a per target runtime builds for a few versions now but the functionality 
has regressed in LLVM 13, which would force me back into a normal single target 
layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107893

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

[PATCH] D105819: [analyzer] MallocChecker: Add a visitor to leave a note on functions that could have, but did not change ownership on leaked memory

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

Szelethus updated this revision to Diff 366269.
Szelethus set the repository for this revision to rG LLVM Github Monorepo.
Szelethus added a comment.

- Add the new feature behind a new, off-by-default alpha checker option.
- Restore `test/Analysis/self-assign.cpp` (to be formatted in a standalone 
commit).

If you don't mind, I'll commit this as is. We seem to agree on the general 
direction, and doesn't bother any users from now on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105819

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/NewDeleteLeaks.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -116,6 +116,7 @@
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = true
 // CHECK-NEXT: track-conditions-debug = false
+// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- /dev/null
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -0,0 +1,142 @@
+// RUN: %clang_analyze_cc1 -verify -analyzer-output=text %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus \
+// RUN:   -analyzer-checker=unix \
+// RUN:   -analyzer-config \
+// RUN: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes=false
+
+// RUN: %clang_analyze_cc1 -verify=expected,ownership -analyzer-output=text %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus \
+// RUN:   -analyzer-checker=unix \
+// RUN:   -analyzer-config \
+// RUN: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes=true
+
+#include "Inputs/system-header-simulator-for-malloc.h"
+
+//===--===//
+// Report for which we expect NoOwnershipChangeVisitor to add a new note.
+//===--===//
+
+bool coin();
+
+namespace memory_allocated_in_fn_call {
+
+void sink(int *P) {
+} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}}
+
+void foo() {
+  sink(new int(5)); // expected-note {{Memory is allocated}}
+// ownership-note@-1 {{Calling 'sink'}}
+// ownership-note@-2 {{Returning from 'sink'}}
+} // expected-warning {{Potential memory leak [cplusplus.NewDeleteLeaks]}}
+// expected-note@-1 {{Potential memory leak}}
+
+} // namespace memory_allocated_in_fn_call
+
+namespace memory_passed_to_fn_call {
+
+void sink(int *P) {
+  if (coin()) // ownership-note {{Assuming the condition is false}}
+  // ownership-note@-1 {{Taking false branch}}
+delete P;
+} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}}
+
+void foo() {
+  int *ptr = new int(5); // expected-note {{Memory is allocated}}
+  sink(ptr); // ownership-note {{Calling 'sink'}}
+ // ownership-note@-1 {{Returning from 'sink'}}
+} // expected-warning {{Potential leak of memory pointed to by 'ptr' [cplusplus.NewDeleteLeaks]}}
+// expected-note@-1 {{Potential leak}}
+
+} // namespace memory_passed_to_fn_call
+
+namespace memory_shared_with_ptr_of_shorter_lifetime {
+
+void sink(int *P) {
+  int *Q = P;
+  if (coin()) // ownership-note {{Assuming the condition is false}}
+  // ownership-note@-1 {{Taking false branch}}
+delete P;
+  (void)Q;
+} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}}
+
+void foo() {
+  int *ptr = new int(5); // expected-note {{Memory is allocated}}
+  sink(ptr); // ownership-note {{Calling 'sink'}}
+ // ownership-note@-1 {{Returning from 'sink'}}
+} // expected-warning {{Potential leak of memory pointed to by 'ptr' [cplusplus.NewDeleteLeaks]}}
+// expected-note@-1 {{Potential leak}}
+
+} // namespace memory_shared_with_ptr_of_shorter_lifetime
+
+//===--===//
+// Report for which we *do not* expect NoOwnershipChangeVisitor add a new note,
+// nor do we want it to.
+//===--===//
+
+namespace memory_not_passed_to_fn_call {
+
+void sink(int *P) {
+  if (coin())
+delete P;
+}
+
+void foo() {
+ 

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov accepted this revision.
dvyukov added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:524
+bool CodeGenFunction::ShouldSkipSanitizerInstrumentation() {
+  if (!CurFuncDecl)
+return false;

When is CurFuncDecl nullptr? Maybe we should look at definition in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D104975: Implement P1949

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:1444
+static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges);
+return C == '_' || XIDStartChars.contains(C) ||
+   XIDContinueChars.contains(C);

Is looking at start chars correct? I was thinking this should only look at the 
continuation characters because `isAllowedInitiallyIDChar` handles the start of 
an identifier.



Comment at: clang/test/CXX/drs/dr2xx.cpp:600
 
-namespace dr248 { // dr248: yes c++11
-  // FIXME: Should this also apply to c++98 mode? This was a DR against C++98.

This means we're going to drop our support of this DR on 
https://clang.llvm.org/cxx_dr_status.html when that page gets regenerated. What 
should our status of that DR be with these changes?



Comment at: clang/test/CXX/drs/dr6xx.cpp:721
 
-namespace dr663 { // dr663: yes c++11
-  int ЍЎ = 123;

Same concern here as above.



Comment at: clang/test/Lexer/unicode.c:40-43
+extern int 👷‍♀; // expected-warning {{declaration does not declare anything}}
+//expected-error@-1 {{character  not allowed in identifier}}
+//expected-error@-2 {{character  not allowed in identifier}}
+//expected-error@-3 {{character  not allowed in identifier}}

Then we don't have to do line number math to see what line the diagnostics are 
attached to. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D106215#2943675 , @cor3ntin wrote:

> In D106215#2943653 , @aaron.ballman 
> wrote:
>
>> In D106215#2943631 , @cor3ntin 
>> wrote:
>>
>>> In D106215#2943611 , 
>>> @aaron.ballman wrote:
>>>
 I think that C and C++ should behave the same here; at least, I don't see 
 any reason why they should have different capabilities.
>>>
>>> I agree but as WG14 hasn't weighted in I didn't want to make that call.
>>> What do you think?
>>
>> My reading of C2x is that this is implementation-defined there as well.
>>
>> 6.4.4.4p13:
>>
>> A wide character constant prefixed by the letter L has type wchar_t, an 
>> integer type defined in the
>>  header; a wide character constant prefixed by the letter u or U 
>> has type char16_t or
>> char32_t, respectively, unsigned integer types defined in the  
>> header. The value of a
>> wide character constant containing a single multibyte character that maps to 
>> a single member of the
>> extended execution character set is the wide character corresponding to that 
>> multibyte character,
>> as defined by the mbtowc, mbrtoc16, or mbrtoc32 function as appropriate for 
>> its type, with an
>> implementation-defined current locale. The value of a wide character 
>> constant containing more
>> than one multibyte character or a single multibyte character that maps to 
>> multiple members of
>> the extended execution character set, or containing a multibyte character or 
>> escape sequence not
>> represented in the extended execution character set, is 
>> implementation-defined.
>>
>> Do you agree?
>
> Yes, I agree.
> I think clang could make it ill-formed if it wanted to!
> If we want to do that we could probably remove some more code :)

I don't see a reason why we'd want C and C++ to diverge here for our 
implementation, so I'd say let's do C as well. @rsmith -- do you have any 
concerns with that?

 The paper said that there is no expected code breakage from this change, 
 but have you tried building a diverse corpus of code (like a distro's 
 worth of packages) under this patch to see if anything actually breaks in 
 practice? (I don't expect breakage that isn't identifying an actual issue 
 in the code, but having some verification would be appreciated.) This 
 would also help to identify whether the change is appropriate for C as 
 well.
>>>
>>> We have done regexes over various repositories (every vcpkg package) with 
>>> no match. Not running a complete compiler
>>
>> Regexes are a good start but they miss the goofy (and sometimes awful) stuff 
>> that people do with token pasting, line continuations, and other random 
>> tricks. Would you be willing to try this as an experiment, or am I asking 
>> too much? :-) My thinking is that if we don't see any breakage from 
>> compiling a diverse corpus of code, we've done enough due diligence to 
>> suggest this is safe for both C and C++, but if we see some breakage, we can 
>> either identify that there's some valid use for this that we've not 
>> considered (less likely) and would be informative for both WG21 and WG14, or 
>> we can identify that we helped find bugs in real world code (more likely) 
>> which is also good feedback for the committees.
>
> Unless there is a script to do that easily, I'm not sure I'll be able to get 
> to it any time soon.
> But really, there is 0 use for these things! And you can't do much goofiness  
> `L  ## 'ab'` certainly - but that wouldn't be very useful either

Okay, I'm probably making too big of an ask here. I can't think of reasonable 
code that would be broken by this change. If that turns out to be incorrect, we 
can address it when we get a real world use case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

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

[PATCH] D107960: [clang][analyzer] Make ReturnPtrRange checker warn at negative index.

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

balazske added a comment.

`assumeInBound` should check if `0<=Idxhttps://reviews.llvm.org/D107960/new/

https://reviews.llvm.org/D107960

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

[clang] 027c5a6 - [analyzer][NFC] Make test/Analysis/self-assign.cpp readable

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-08-13T16:14:54+02:00
New Revision: 027c5a6adcb3a41c29533680a650fae7262f2b31

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

LOG: [analyzer][NFC] Make test/Analysis/self-assign.cpp readable

Added: 


Modified: 
clang/test/Analysis/self-assign.cpp

Removed: 




diff  --git a/clang/test/Analysis/self-assign.cpp 
b/clang/test/Analysis/self-assign.cpp
index ca28c534f1e25..7d3ea99b005de 100644
--- a/clang/test/Analysis/self-assign.cpp
+++ b/clang/test/Analysis/self-assign.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection 
-analyzer-config eagerly-assume=false %s -verify -analyzer-output=text
+// RUN: %clang_analyze_cc1 -std=c++11 %s -verify -analyzer-output=text \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false
 
 extern "C" char *strdup(const char* s);
 extern "C" void free(void* ptr);
@@ -28,18 +33,31 @@ StringUsed::~StringUsed() {
   free(str);
 }
 
-StringUsed& StringUsed::operator=(const StringUsed &rhs) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} 
expected-note{{Assuming rhs != *this}}
-  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} 
expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+StringUsed &StringUsed::operator=(const StringUsed &rhs) {
+  // expected-note@-1{{Assuming rhs == *this}}
+  // expected-note@-2{{Assuming rhs == *this}}
+  // expected-note@-3{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}}
+ // expected-warning@-1{{UNKNOWN}}
+ // expected-note@-2{{TRUE}}
+ // expected-note@-3{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
-  str = strdup(rhs.str); // expected-warning{{Use of memory after it is 
freed}}  expected-note{{Use of memory after it is freed}}
-// expected-note@-1{{Memory is allocated}}
+  str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}
+ // expected-note@-1{{Use of memory after it is freed}}
+ // expected-note@-2{{Memory is allocated}}
   return *this;
 }
 
-StringUsed& StringUsed::operator=(StringUsed &&rhs) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
-  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} 
expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+StringUsed &StringUsed::operator=(StringUsed &&rhs) {
+  // expected-note@-1{{Assuming rhs == *this}}
+  // expected-note@-2{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}}
+ // expected-warning@-1{{UNKNOWN}}
+ // expected-note@-2{{TRUE}}
+ // expected-note@-3{{UNKNOWN}}
   str = rhs.str;
-  rhs.str = nullptr; // expected-warning{{Potential memory leak}} 
expected-note{{Potential memory leak}}
+  rhs.str = nullptr; // expected-warning{{Potential memory leak}}
+ // expected-note@-1{{Potential memory leak}}
   return *this;
 }
 
@@ -63,15 +81,27 @@ StringUnused::~StringUnused() {
   free(str);
 }
 
-StringUnused& StringUnused::operator=(const StringUnused &rhs) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} 
expected-note{{Assuming rhs != *this}}
-  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} 
expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+StringUnused &StringUnused::operator=(const StringUnused &rhs) {
+  // expected-note@-1{{Assuming rhs == *this}}
+  // expected-note@-2{{Assuming rhs == *this}}
+  // expected-note@-3{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}}
+ // expected-warning@-1{{UNKNOWN}}
+ // expected-note@-2{{TRUE}}
+ // expected-note@-3{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
-  str = strdup(rhs.str); // expected-warning{{Use of memory after it is 
freed}}  expected-note{{Use of memory after it is freed}}
+  str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}
+ // expected-note@-1{{Use of memory after it is freed}}
   return *this;
 }
 
-StringUnused& StringUnused::operator=(StringUnused &&rhs) { // 
expected-note{{Assuming rhs == *this

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Basic/AttrDocs.td:2601-2602
+
+This is not the same as ``__attribute__((no_sanitize(...)))``, which depending
+on the tool may still insert instrumentation to prevent false positive reports.
+  }];

Has there been agreement that this isn't actually a bug? My understanding of 
`no_sanitize` is that it disables sanitizer support for a function or global. 
The documentation for that attribute says:
```
Use the ``no_sanitize`` attribute on a function or a global variable
declaration to specify that a particular instrumentation or set of
instrumentations should not be applied.
```
That sure reads like "do not instrument this at all" to me, and makes me think 
we don't need a second attribute that says "no, really, I mean it this time."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:1444
+static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges);
+return C == '_' || XIDStartChars.contains(C) ||
+   XIDContinueChars.contains(C);

aaron.ballman wrote:
> Is looking at start chars correct? I was thinking this should only look at 
> the continuation characters because `isAllowedInitiallyIDChar` handles the 
> start of an identifier.
Yes,  But Maybe it needs a comment.
Per Unicode spec, XIDContinueChars is a superset of XIDStartChars, and in the 
code, we don't deduplicate the table to not make them too big.
So we look at both tables, starting with start as it's the most likely to 
contain the character 



Comment at: clang/test/CXX/drs/dr2xx.cpp:600
 
-namespace dr248 { // dr248: yes c++11
-  // FIXME: Should this also apply to c++98 mode? This was a DR against C++98.

aaron.ballman wrote:
> This means we're going to drop our support of this DR on 
> https://clang.llvm.org/cxx_dr_status.html when that page gets regenerated. 
> What should our status of that DR be with these changes?
Good question!
I will change it to `Superseded by P1949`. What do you think?



Comment at: clang/test/Lexer/unicode.c:40-43
+extern int 👷‍♀; // expected-warning {{declaration does not declare anything}}
+//expected-error@-1 {{character  not allowed in identifier}}
+//expected-error@-2 {{character  not allowed in identifier}}
+//expected-error@-3 {{character  not allowed in identifier}}

aaron.ballman wrote:
> Then we don't have to do line number math to see what line the diagnostics 
> are attached to. :-)
It makes me feel dirty but I'll change it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D77670: [CUDA] Add partial support for recent CUDA versions.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.
Herald added a subscriber: dexonsmith.
Herald added a project: LLVM.

@tra The split between `LATEST` and `LATEST_SUPPORTED` leads to very weird 
warning and error messages:

  clang-14: warning: unknown CUDA version: cuda.h: CUDA_VERSION=11040.; 
assuming the latest supported version 10.1 [-Wunknown-cuda-version]
  clang-14: error: cannot find libdevice for sm_20; provide path to different 
CUDA installation via '--cuda-path', or pass '-nocudalib' to build without 
linking with libdevice  
  
  clang-14: error: GPU arch sm_20 is supported by CUDA versions between 7.0 and 
8.0 (inclusive), but installation at /usr/local/cuda-11.4 is 11.2; use 
'--cuda-path' to specify a different CUDA install, pass a different GPU arch 
with '--cuda-gpu-arch', or pass '--no-cuda-version-check'

Clang is mentioning three different CUDA versions here: 11.4 is what I really 
have installed, 11.2 is `LATEST` and therefore the one returned by 
`getCudaVersion` or as the "last resort" in `CudaInstallationDetector`, and the 
first warning says that Clang assumes the latest supported version 10.1. As a 
developer looking into the code, I get that the first warning is about saying 
that 10.1 is the latest fully supported version in terms of features, but I 
think this is really confusing to users. Do you see a chance to improve this? 
(other than adding just 11.3 and 11.4 to the enumerations where we'll always 
run behind)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77670

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

[PATCH] D104975: Implement P1949

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: rsmith.
aaron.ballman added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:1444
+static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges);
+return C == '_' || XIDStartChars.contains(C) ||
+   XIDContinueChars.contains(C);

cor3ntin wrote:
> aaron.ballman wrote:
> > Is looking at start chars correct? I was thinking this should only look at 
> > the continuation characters because `isAllowedInitiallyIDChar` handles the 
> > start of an identifier.
> Yes,  But Maybe it needs a comment.
> Per Unicode spec, XIDContinueChars is a superset of XIDStartChars, and in the 
> code, we don't deduplicate the table to not make them too big.
> So we look at both tables, starting with start as it's the most likely to 
> contain the character 
Ah, that explanation makes sense, thank you! I agree that a comment would be 
helpful there.



Comment at: clang/test/CXX/drs/dr2xx.cpp:600
 
-namespace dr248 { // dr248: yes c++11
-  // FIXME: Should this also apply to c++98 mode? This was a DR against C++98.

cor3ntin wrote:
> aaron.ballman wrote:
> > This means we're going to drop our support of this DR on 
> > https://clang.llvm.org/cxx_dr_status.html when that page gets regenerated. 
> > What should our status of that DR be with these changes?
> Good question!
> I will change it to `Superseded by P1949`. What do you think?
Because we're treating this as a DR, hopefully there's a DR number we could use 
instead of the paper number. @rsmith -- how should we handle this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D104975: Implement P1949

[Aaron Ballman via Phabricator via cfe-commits]

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

Adding @rsmith to hopefully answer the question about how to handle the dr 
status page for these changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D106215: Make wide multi-character character literals ill-formed in C++ mode

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 366279.
cor3ntin added a comment.

Make wide multi char ill-formed regardless of language mode


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/LiteralSupport.cpp
  clang/test/CodeGen/char-literal.c
  clang/test/CodeGen/string-literal-short-wstring.c
  clang/test/Lexer/char-literal.cpp
  clang/test/Lexer/wchar.c
  clang/test/Misc/warning-flags.c
  clang/test/Preprocessor/Weverything_pragma.c

Index: clang/test/Preprocessor/Weverything_pragma.c
===
--- clang/test/Preprocessor/Weverything_pragma.c
+++ clang/test/Preprocessor/Weverything_pragma.c
@@ -10,21 +10,21 @@
 // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 {
  // A diagnostic without DefaultIgnore, and not part of a group.
- (void) L'ab'; // expected-warning {{extraneous characters in character constant ignored}}
+ (void) 'ab'; // expected-warning {{multi-character character constant}}
 
 #pragma clang diagnostic warning "-Weverything" // Should not change anyhting.
 #define UNUSED_MACRO2 1 // expected-warning{{macro is not used}}
- (void) L'cd'; // expected-warning {{extraneous characters in character constant ignored}}
+ (void) 'cd'; // expected-warning {{multi-character character constant}}
 
 #pragma clang diagnostic ignored "-Weverything" // Ignore warnings now.
 #define UNUSED_MACRO2 1 // no warning
- (void) L'ef'; // no warning here
+ (void) 'ef'; // no warning here
 
 #pragma clang diagnostic warning "-Weverything" // Revert back to warnings.
 #define UNUSED_MACRO3 1 // expected-warning{{macro is not used}}
- (void) L'gh'; // expected-warning {{extraneous characters in character constant ignored}}
+ (void) 'gh'; // expected-warning {{multi-character character constant}}
 
 #pragma clang diagnostic error "-Weverything"  // Give errors now.
 #define UNUSED_MACRO4 1 // expected-error{{macro is not used}}
- (void) L'ij'; // expected-error {{extraneous characters in character constant ignored}}
+ (void) 'ij'; // expected-error {{multi-character character constant}}
 }
Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (68):
+CHECK: Warnings without flags (67):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -50,7 +50,6 @@
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
-CHECK-NEXT:   warn_extraneous_char_constant
 CHECK-NEXT:   warn_fe_backend_unsupported
 CHECK-NEXT:   warn_fe_cc_log_diagnostics_failure
 CHECK-NEXT:   warn_fe_cc_print_header_failure
Index: clang/test/Lexer/wchar.c
===
--- clang/test/Lexer/wchar.c
+++ clang/test/Lexer/wchar.c
@@ -3,10 +3,8 @@
 void f() {
   (void)L"\U0001"; // unicode escape produces UTF-16 sequence, so no warning
 
-  (void)L'\U0001'; // expected-error {{character too large for enclosing character literal type}}
+  (void)L'ab';  // expected-error {{wide character literals may not contain multiple characters}}
 
-  (void)L'ab';  // expected-warning {{extraneous characters in character constant ignored}}
-
-  (void)L'a\u1000';  // expected-warning {{extraneous characters in character constant ignored}}
+  (void)L'a\u1000';  // expected-error {{wide character literals may not contain multiple characters}}
 }
 
Index: clang/test/Lexer/char-literal.cpp
===
--- clang/test/Lexer/char-literal.cpp
+++ clang/test/Lexer/char-literal.cpp
@@ -21,7 +21,8 @@
 char16_t g = u'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
 char16_t h = u'\U0010FFFD'; // expected-error {{character too large for enclosing character literal type}}
 
-wchar_t i = L'ab'; // expected-warning {{extraneous characters in character constant ignored}}
+wchar_t i = L'ab'; // expected-error {{wide character literals may not contain multiple characters}}
+
 wchar_t j = L'\U0010FFFD';
 
 char32_t k = U'\U0010FFFD';
Index: clang/test/CodeGen/string-literal-short-wstring.c
===
--- clang/test/CodeGen/string-literal-short-wstring.c
+++ clang/test/CodeGen/string-literal-short-wstring.c
@@ -1,11 +1,14 @@
-// RUN: %clang_cc1 -x c++ -triple %itanium_abi_triple -emit-llvm -fwchar-type=short -fno-signed-wchar %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=ITANIUM
-// RUN: %clang_cc1 -x c++ -tripl

[PATCH] D104854: Introduce intrinsic llvm.isnan

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Patch D108037  must make lowering of 
`llvm.isnan` more close to the code produced by `__builtin_isnan` earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104854

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

[PATCH] D108038: [C++4OpenCL] C++ for OpenCL version 2021 introduced to command line

[Justas Janickas via Phabricator via cfe-commits]

Topotuna created this revision.
Topotuna added a reviewer: Anastasia.
Herald added subscribers: ldrumm, dexonsmith, dang, yaxunl.
Topotuna requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Introduces language standard `lang_openclcpp2021` and allows
`clc++2021` as a version flag for `-cl-std` in command line.
Defines macros related to C++ for OpenCL version 2021.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108038

Files:
  clang/include/clang/Basic/LangStandards.def
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/autocomplete.c
  clang/test/Driver/unknown-std.cl
  clang/test/Frontend/stdlang.c

Index: clang/test/Frontend/stdlang.c
===
--- clang/test/Frontend/stdlang.c
+++ clang/test/Frontend/stdlang.c
@@ -7,12 +7,16 @@
 // RUN: %clang_cc1 -x cl -cl-std=cl2.0 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=cl3.0 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=clc++ -DOPENCL %s
+// RUN: %clang_cc1 -x cl -cl-std=clc++1.0 -DOPENCL %s
+// RUN: %clang_cc1 -x cl -cl-std=clc++2021 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=CL -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=CL3.0 -DOPENCL %s
 // RUN: %clang_cc1 -x cl -cl-std=CLC++ -DOPENCL %s
+// RUN: %clang_cc1 -x cl -cl-std=CLC++1.0 -DOPENCL %s
+// RUN: %clang_cc1 -x cl -cl-std=CLC++2021 -DOPENCL %s
 // RUN: not %clang_cc1 -x cl -std=c99 -DOPENCL %s 2>&1 | FileCheck --check-prefix=CHECK-C99 %s
 // RUN: not %clang_cc1 -x cl -cl-std=invalid -DOPENCL %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID %s
 // CHECK-C99: error: invalid argument '-std=c99' not allowed with 'OpenCL'
Index: clang/test/Driver/unknown-std.cl
===
--- clang/test/Driver/unknown-std.cl
+++ clang/test/Driver/unknown-std.cl
@@ -12,6 +12,7 @@
 // CHECK-NEXT: note: use 'cl2.0' for 'OpenCL 2.0' standard
 // CHECK-NEXT: note: use 'cl3.0' for 'OpenCL 3.0' standard
 // CHECK-NEXT: note: use 'clc++1.0' or 'clc++' for 'C++ for OpenCL 1.0' standard
+// CHECK-NEXT: note: use 'clc++2021' for 'C++ for OpenCL 2021' standard
 
 // Make sure that no other output is present.
 // CHECK-NOT: {{^.+$}}
Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -51,6 +51,8 @@
 // CLSTDALL-NEXT: CLC++
 // CLSTDALL-NEXT: clc++1.0
 // CLSTDALL-NEXT: CLC++1.0
+// CLSTDALL-NEXT: clc++2021
+// CLSTDALL-NEXT: CLC++2021
 // RUN: %clang --autocomplete=-fno-sanitize-coverage=,f | FileCheck %s -check-prefix=FNOSANICOVER
 // FNOSANICOVER: func
 // RUN: %clang --autocomplete=-fno-sanitize-coverage= | FileCheck %s -check-prefix=FNOSANICOVERALL
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -433,11 +433,18 @@
   // OpenCL v1.0/1.1 s6.9, v1.2/2.0 s6.10: Preprocessor Directives and Macros.
   if (LangOpts.OpenCL) {
 if (LangOpts.CPlusPlus) {
-  if (LangOpts.OpenCLCPlusPlusVersion == 100)
+  switch (LangOpts.OpenCLCPlusPlusVersion) {
+  case 100:
 Builder.defineMacro("__OPENCL_CPP_VERSION__", "100");
-  else
+break;
+  case 202100:
+Builder.defineMacro("__OPENCL_CPP_VERSION__", "202100");
+break;
+  default:
 llvm_unreachable("Unsupported C++ version for OpenCL");
+  }
   Builder.defineMacro("__CL_CPP_VERSION_1_0__", "100");
+  Builder.defineMacro("__CL_CPP_VERSION_2021__", "202100");
 } else {
   // OpenCL v1.0 and v1.1 do not have a predefined macro to indicate the
   // language standard with which the program is compiled. __OPENCL_VERSION__
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3165,6 +3165,8 @@
 Opts.OpenCLVersion = 300;
   else if (LangStd == LangStandard::lang_openclcpp10)
 Opts.OpenCLCPlusPlusVersion = 100;
+  else if (LangStd == LangStandard::lang_openclcpp2021)
+Opts.OpenCLCPlusPlusVersion = 202100;
 
   // OpenCL has some additional defaults.
   if (Opts.OpenCL) {
@@ -3314,6 +3316,7 @@
   case LangStandard::lang_opencl20:
   case LangStandard::lang_opencl30:
   case LangStandard::lang_openclcpp10:
+  case LangStandard::lang_openclcpp2021:
 StdOpt = OPT_cl_std_EQ;
 break;
   default:
@@ -3612,6 +3615,7 @@
 .Cases("cl3.0", "CL3.0", LangStandard::lang_opencl30)
 .Cases("clc++", "CLC++", LangStandard::l

[PATCH] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 366286.
cor3ntin added a comment.

Update dr_cxx_status and add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/UnicodeCharSets.h
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/FixIt/fixit-unicode.c
  clang/test/Lexer/unicode.c
  clang/test/Parser/cxx11-user-defined-literals.cpp
  clang/test/Preprocessor/ucn-allowed-chars.c
  clang/test/Preprocessor/utf8-allowed-chars.c
  clang/www/cxx_dr_status.html
  clang/www/cxx_status.html
  llvm/include/llvm/Support/UnicodeCharRanges.h

Index: llvm/include/llvm/Support/UnicodeCharRanges.h
===
--- llvm/include/llvm/Support/UnicodeCharRanges.h
+++ llvm/include/llvm/Support/UnicodeCharRanges.h
@@ -62,6 +62,14 @@
   /// Returns true if the character set contains the Unicode code point
   /// \p C.
   bool contains(uint32_t C) const {
+if (C < 127) {
+  for (const auto &R : Ranges) {
+if (R.Lower <= C && R.Upper >= C)
+  return true;
+if (R.Lower > C)
+  return false;
+  }
+}
 return std::binary_search(Ranges.begin(), Ranges.end(), C);
   }
 
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1311,7 +1311,7 @@
 
   C++ identifier syntax using UAX 31
   https://wg21.link/P1949R7";>P1949R7
-  No
+  Clang 14
 
 
   Mixed string literal concatenation
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -1527,7 +1527,7 @@
 https://wg21.link/cwg248";>248
 C++11
 Identifier characters
-Yes (C++11 onwards)
+Superseded by https://wg21.link/P1949R7";>P1949R7
   
   
 https://wg21.link/cwg249";>249
@@ -4020,7 +4020,7 @@
 https://wg21.link/cwg663";>663
 CD1
 Valid Cyrillic identifier characters
-Yes (C++11 onwards)
+Superseded by https://wg21.link/P1949R7";>P1949R7
   
   
 https://wg21.link/cwg664";>664
Index: clang/test/Preprocessor/utf8-allowed-chars.c
===
--- clang/test/Preprocessor/utf8-allowed-chars.c
+++ clang/test/Preprocessor/utf8-allowed-chars.c
@@ -23,46 +23,30 @@
 
 
 
-
-
-
-
 #if __cplusplus
-# if __cplusplus >= 201103L
-// C++11
-// expected-warning@9 {{using this character in an identifier is incompatible with C++98}}
-// expected-warning@10 {{using this character in an identifier is incompatible with C++98}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-warning@14 {{using this character in an identifier is incompatible with C++98}}
+// expected-error@11 {{not allowed in identifiers}}
+// expected-error@13 {{not allowed in identifiers}}
+// expected-error@20 {{expected unqualified-id}}
 // expected-error@21 {{expected unqualified-id}}
 
-# else
-// C++03
-// expected-error@9 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@10 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@14 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@21 {{non-ASCII characters are not allowed outside of literals and identifiers}} expected-warning@21 {{declaration does not declare anything}}
-
-# endif
 #else
 # if __STDC_VERSION__ >= 201112L
 // C11
 // expected-warning@9 {{using this character in an identifier is incompatible with C99}}
 // expected-warning@11 {{using this character in an identifier is incompatible with C99}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
+// expected-error@13 {{not allowed in identifiers}}
 // expected-warning@14 {{using this character in an identifier is incompatible with C99}}
 // expected-warning@20 {{starting an identifier with this character is incompatible with C99}}
 // expected-error@21 {{expected identifier}}
 
 # else
 // C99
-// expected-error@9 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@11 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-error@14 {{non-ASCII characters are not allowed outside of literals and identifiers}}
+// expected-error@9 {{not allowed in identifiers}}
+// expected-error@11 {{not allowed in identifiers}}
+//

[PATCH] D106889: [examples] Fix the clang-interpreter example for changes in 2487db1f2862

[Ryan Mansfield via Phabricator via cfe-commits]

rmansfield abandoned this revision.
rmansfield added a comment.

This got fixed by b4c0307d598004cfd96c770d2a4a84a37c838ba9 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106889

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

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D106577#2904960 , @rsmith wrote:

>> One specific example I'd like to be considered:
>> Suppose the C standard library implementation's mbstowcs converts a certain 
>> multi-byte character C to somewhere in the Unicode private use area, because 
>> Unicode version N doesn't have a corresponding character. Suppose further 
>> that the compiler is aware of Unicode version N+1, in which a character 
>> corresponding to C was added. Is an implementation formed by that 
>> combination of compiler and standard library, that defines 
>> `__STDC_ISO_10646__` to N+1, conforming? Or is it non-conforming because it 
>> represents character C as something other than the corresponding short name 
>> from Unicode version N+1?
>
> And David Keaton (long-time WG14 member and current convener) replied:
>
>> Yikes!  It does indeed sound like the library would affect the value of 
>> `__STDC_ISO_10646__` in that case.  Thanks for clarifying the details.
>
> There was no further discussion after that point, so I think the unofficial 
> WG14 stance is that the compiler and the library need to collude on setting 
> the value of that macro.

I don't think that scenario is valid. MBCS-to-unicode mappings are a part of 
the definition of the MBCS (sometimes officially, sometimes de-facto defined by 
major vendors), not in the definition of Unicode.

And in fact, we have a real-life example of this: the GB18030 encoding. That 
standard specifies 24 characters mappings to private-use-area unicode 
codepoints in the most recent version, GB18030-2005. (Which is down from 80 PUA 
mappings in its predecessor encoding GBK, and 25 in GB18030-2000.) Yet, a new 
version of Unicode coming out will not affect that. Rather, I should say, DID 
NOT affect that -- all of those 24 characters mapped to PUAs in GB18030-2005 
were actually assigned official unicode codepoints by 2005 (some in Unicode 
3.1, some in Unicode 4.1). But no matter -- GB18030 still maps those to PUA 
code-points. The only way that can change is if GB18030 gets updated.

I do note that some implementations (e.g. glibc) have taken it upon themselves 
to modify the official GB18030 character mapping table, and to decode those 24 
codepoints to the newly-defined unicode characters, instead of the specified 
PUA codepoints. But there's no way that can be described as a requirement -- 
it's not even technically correct!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106577

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

[PATCH] D108021: [dllexport] Instantiate default ctor default args

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I think it looks good, but it needs a test.

I worry that we might have the same bug for copy closures, but the fix will be 
different, since those are used in exception handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108021

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

[clang] 98eb348 - Revert "[clang-format] Distinguish K&R C function definition and attribute"

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2021-08-13T16:25:32+01:00
New Revision: 98eb348eb38aaba63aa149000c97d27a7e5190c3

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

LOG: Revert "[clang-format] Distinguish K&R C function definition and attribute"

This reverts commit de763c4037157e60551ba227ccd0ed02e109c317.

Causing test failures on the Arm/AArch64 quick bots:
https://lab.llvm.org/buildbot/#/builders/188/builds/2202

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index e234c74ff750b..0c4cacab50506 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,6 +14,7 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -994,13 +995,6 @@ static bool isJSDeclOrStmt(const AdditionalKeywords 
&Keywords,
   Keywords.kw_import, tok::kw_export);
 }
 
-// Checks whether a token is a type in K&R C (aka C78).
-static bool isC78Type(const FormatToken &Tok) {
-  return Tok.isOneOf(tok::kw_char, tok::kw_short, tok::kw_int, tok::kw_long,
- tok::kw_unsigned, tok::kw_float, tok::kw_double,
- tok::identifier);
-}
-
 // This function checks whether a token starts the first parameter declaration
 // in a K&R C (aka C78) function definition, e.g.:
 //   int f(a, b)
@@ -1012,8 +1006,9 @@ static bool isC78ParameterDecl(const FormatToken *Tok) {
   if (!Tok)
 return false;
 
-  if (!isC78Type(*Tok) &&
-  !Tok->isOneOf(tok::kw_register, tok::kw_struct, tok::kw_union))
+  if (!Tok->isOneOf(tok::kw_int, tok::kw_char, tok::kw_float, tok::kw_double,
+tok::kw_struct, tok::kw_union, tok::kw_long, tok::kw_short,
+tok::kw_unsigned, tok::kw_register))
 return false;
 
   Tok = Tok->Previous;
@@ -1374,7 +1369,7 @@ void UnwrappedLineParser::parseStructuralElement(bool 
IsTopLevel) {
 case tok::r_brace:
   addUnwrappedLine();
   return;
-case tok::l_paren: {
+case tok::l_paren:
   parseParens();
   // Break the unwrapped line if a K&R C function definition has a 
parameter
   // declaration.
@@ -1382,18 +1377,14 @@ void UnwrappedLineParser::parseStructuralElement(bool 
IsTopLevel) {
 break;
   if (!Previous || Previous->isNot(tok::identifier))
 break;
-  const FormatToken *PrevPrev = Previous->Previous;
-  if (!PrevPrev || (!isC78Type(*PrevPrev) && PrevPrev->isNot(tok::star)))
+  if (Previous->Previous && Previous->Previous->is(tok::at))
 break;
-  const FormatToken *Next = AllTokens[Tokens->getPosition() + 1];
-  if (Next && Next->isOneOf(tok::l_paren, tok::semi))
-break;
-  if (isC78ParameterDecl(FormatTok)) {
+  if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
+  isC78ParameterDecl(FormatTok)) {
 addUnwrappedLine();
 return;
   }
   break;
-}
 case tok::kw_operator:
   nextToken();
   if (FormatTok->isBinaryOperator())

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 63e93b775ac6a..1283aa67b3370 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8247,16 +8247,6 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) {
"  return a + b < c;\n"
"};",
Style);
-  verifyFormat("byte *\n" // Break here.
-   "f(a)\n"   // Break here.
-   "byte a[];\n"
-   "{\n"
-   "  return a;\n"
-   "}",
-   Style);
-  verifyFormat("bool f(int a, int) override;\n"
-   "Bar g(int a, Bar) final; // comment",
-   Style);
 
   // The return breaking style doesn't affect:
   // * function and object definitions with attribute-like macros



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

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst:21
+
+   If set to `true`, this check will limit itself to the `builtinType()` 
types. Default is `false`.

jmarrec wrote:
> cjdb wrote:
> > jmarrec wrote:
> > > Quuxplusone wrote:
> > > > D107873 is related.
> > > > 
> > > > I'd like to see some tests/discussion around large types, e.g.
> > > > ```
> > > > struct Widget { int a[1000]; };
> > > > void f(const Widget&);
> > > > ```
> > > > (I think D107873 at least makes a conscious attempt to get the "would 
> > > > it be passed in registers?" check right.)
> > > > 
> > > > I'd like to see some tests/discussion around generic code, e.g.
> > > > ```
> > > > template
> > > > struct Max {
> > > > static T max(const T& a, const T& b);
> > > > };
> > > > int x = Max::max(1, 2);  // passes `int` by const reference, but 
> > > > this is fine
> > > > ```
> > > Should I close this one in favor of https://reviews.llvm.org/D107873?
> > Up to you. There's always a chance that D107873 doesn't get approved. What 
> > I find most interesting is that we were independently working on this at 
> > the same time :-)
> @cjdb I guess it's uncanny that we would both decide to do it at the same 
> time. A bit unfortunate too probably, I could have saved a few hours :) but I 
> learned something about clang-tidy so I don't mind at all!
> 
> Yours (D107873) is definitely better documented, you handle to opposite case 
> (I only deal with const-ref -> value, not value -> const ret) and I just 
> borrowed your implementation for MaxSize / ignoring shared_ptr
> 
> As far as I can tell from a cursory look, there are a couple of things I'm 
> doing that you aren't:
> 
> * I am actually using Options, I don't think you are yet, even for MaxSize. I 
> see from your documentation file that you plan to add a few
> * The RestrictToBuiltInTypes I find an interesting option too
> * I am removing the const as well in addition to the `&`. I guess you are 
> just relying on using it in conjunction with 
> `readability-avoid-const-params-in-decls` but I think that makes it harder to 
> use (you can't just do -checks=-*, you have to remember to add the 
> readability-avoid-const-params-in-decls too...).
> 
> Perhaps we could discuss adding the above to your PR and consolidate over 
> there to make a nice, feature-complete check?
Sounds good.


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

https://reviews.llvm.org/D107900

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

[PATCH] D108021: [dllexport] Instantiate default ctor default args

[Peter Jiachen via Phabricator via cfe-commits]

peterjc123 added a comment.

In D108021#2943842 , @rnk wrote:

> I think it looks good, but it needs a test.
>
> I worry that we might have the same bug for copy closures, but the fix will 
> be different, since those are used in exception handling.

Thanks for your review. Since I'm pretty new to this repo, could you please 
tell me how to write a test here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108021

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

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:2601-2602
+
+This is not the same as ``__attribute__((no_sanitize(...)))``, which depending
+on the tool may still insert instrumentation to prevent false positive reports.
+  }];

aaron.ballman wrote:
> Has there been agreement that this isn't actually a bug? My understanding of 
> `no_sanitize` is that it disables sanitizer support for a function or global. 
> The documentation for that attribute says:
> ```
> Use the ``no_sanitize`` attribute on a function or a global variable
> declaration to specify that a particular instrumentation or set of
> instrumentations should not be applied.
> ```
> That sure reads like "do not instrument this at all" to me, and makes me 
> think we don't need a second attribute that says "no, really, I mean it this 
> time."
No, this isn't a bug, but might need to be better clarified in the docs.
ThreadSanitizer and MemorySanitizer do insert some instrumentation into ignore 
functions to prevent false positives:

> ThreadSanitizer still instruments such functions to avoid false positives and 
> provide meaningful stack traces.

(https://clang.llvm.org/docs/ThreadSanitizer.html#attribute-no-sanitize-thread)

and 

> MemorySanitizer may still instrument such functions to avoid false positives.

(https://clang.llvm.org/docs/MemorySanitizer.html#attribute-no-sanitize-memory)

This is the behavior people rely onto, although at this point I don't think 
`no_sanitize(...)` is the best name for attribute not disabling instrumentation 
completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108013: [NFC] Rename AttributeList::has/getAttribute() -> has/getAttributeImpl()

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

There's a related change here: https://reviews.llvm.org/D105485

Honestly, I like this approach better. Adding a strongly-typed integer wrapper 
is a pretty heavyweight solution for a problem that seems solvable with better 
named methods.

As a practical matter, consider splitting this up to reduce churn in case 
anything gets reverted. Most of the `getAttribute` -> `getParamAttr` calls can 
be migrated ahead of time as NFC commits to improve readability. The most 
risk-averse version of this change is to leave behind the `getAttribute` entry 
points as aliases to `getAttributeImpl`, and then remove them in a final commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108013

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

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

With several Linux kernel `defconfig` builds, I see the following instances of 
the warning:

  drivers/net/wireless/intel/iwlwifi/mvm/scan.c:835:3: warning: bitwise and of 
boolean expressions; did you mean logical and? [-Wbool-operation-and]
  drivers/staging/rtl8192u/r8192U_core.c:4268:20: warning: bitwise and of 
boolean expressions; did you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1043:7: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1075:30: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1294:7: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1353:30: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1932:7: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1956:22: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1977:23: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:2024:29: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]

Which correspond to:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/intel/iwlwifi/mvm/scan.c#n830

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/rtl8192u/r8192U_core.c#n4268

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/zstd/compress.c#n1294

I can do `allmodconfig` builds later today.


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

https://reviews.llvm.org/D108003

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

[clang] 4190d99 - [X86] Add parentheses around casts in some of the X86 intrinsic headers.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2021-08-13T09:36:16-07:00
New Revision: 4190d99dfcab45cd67c32030d363f391c35570f2

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

LOG: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

This covers the SSE and AVX/AVX2 headers. AVX512 has a lot more macros
due to rounding mode.

Fixes part of PR51324.

Reviewed By: pengfei

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

Added: 


Modified: 
clang/lib/Headers/__wmmintrin_aes.h
clang/lib/Headers/avx2intrin.h
clang/lib/Headers/avxintrin.h
clang/lib/Headers/emmintrin.h
clang/lib/Headers/smmintrin.h
clang/lib/Headers/tmmintrin.h
clang/lib/Headers/xmmintrin.h
clang/test/CodeGen/X86/sse41-builtins.c

Removed: 




diff  --git a/clang/lib/Headers/__wmmintrin_aes.h 
b/clang/lib/Headers/__wmmintrin_aes.h
index f540319c7fd2b..3010b38711e67 100644
--- a/clang/lib/Headers/__wmmintrin_aes.h
+++ b/clang/lib/Headers/__wmmintrin_aes.h
@@ -133,7 +133,7 @@ _mm_aesimc_si128(__m128i __V)
 ///An 8-bit round constant used to generate the AES encryption key.
 /// \returns A 128-bit round key for AES encryption.
 #define _mm_aeskeygenassist_si128(C, R) \
-  (__m128i)__builtin_ia32_aeskeygenassist128((__v2di)(__m128i)(C), (int)(R))
+  ((__m128i)__builtin_ia32_aeskeygenassist128((__v2di)(__m128i)(C), (int)(R)))
 
 #undef __DEFAULT_FN_ATTRS
 

diff  --git a/clang/lib/Headers/avx2intrin.h b/clang/lib/Headers/avx2intrin.h
index cc16720949ea3..5064c87c2bb19 100644
--- a/clang/lib/Headers/avx2intrin.h
+++ b/clang/lib/Headers/avx2intrin.h
@@ -20,8 +20,8 @@
 
 /* SSE4 Multiple Packed Sums of Absolute Difference.  */
 #define _mm256_mpsadbw_epu8(X, Y, M) \
-  (__m256i)__builtin_ia32_mpsadbw256((__v32qi)(__m256i)(X), \
- (__v32qi)(__m256i)(Y), (int)(M))
+  ((__m256i)__builtin_ia32_mpsadbw256((__v32qi)(__m256i)(X), \
+  (__v32qi)(__m256i)(Y), (int)(M)))
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_abs_epi8(__m256i __a)
@@ -114,8 +114,8 @@ _mm256_adds_epu16(__m256i __a, __m256i __b)
 }
 
 #define _mm256_alignr_epi8(a, b, n) \
-  (__m256i)__builtin_ia32_palignr256((__v32qi)(__m256i)(a), \
- (__v32qi)(__m256i)(b), (n))
+  ((__m256i)__builtin_ia32_palignr256((__v32qi)(__m256i)(a), \
+  (__v32qi)(__m256i)(b), (n)))
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_and_si256(__m256i __a, __m256i __b)
@@ -149,8 +149,8 @@ _mm256_blendv_epi8(__m256i __V1, __m256i __V2, __m256i __M)
 }
 
 #define _mm256_blend_epi16(V1, V2, M) \
-  (__m256i)__builtin_ia32_pblendw256((__v16hi)(__m256i)(V1), \
- (__v16hi)(__m256i)(V2), (int)(M))
+  ((__m256i)__builtin_ia32_pblendw256((__v16hi)(__m256i)(V1), \
+  (__v16hi)(__m256i)(V2), (int)(M)))
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_cmpeq_epi8(__m256i __a, __m256i __b)
@@ -467,13 +467,13 @@ _mm256_shuffle_epi8(__m256i __a, __m256i __b)
 }
 
 #define _mm256_shuffle_epi32(a, imm) \
-  (__m256i)__builtin_ia32_pshufd256((__v8si)(__m256i)(a), (int)(imm))
+  ((__m256i)__builtin_ia32_pshufd256((__v8si)(__m256i)(a), (int)(imm)))
 
 #define _mm256_shufflehi_epi16(a, imm) \
-  (__m256i)__builtin_ia32_pshufhw256((__v16hi)(__m256i)(a), (int)(imm))
+  ((__m256i)__builtin_ia32_pshufhw256((__v16hi)(__m256i)(a), (int)(imm)))
 
 #define _mm256_shufflelo_epi16(a, imm) \
-  (__m256i)__builtin_ia32_pshuflw256((__v16hi)(__m256i)(a), (int)(imm))
+  ((__m256i)__builtin_ia32_pshuflw256((__v16hi)(__m256i)(a), (int)(imm)))
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_sign_epi8(__m256i __a, __m256i __b)
@@ -494,10 +494,10 @@ _mm256_sign_epi32(__m256i __a, __m256i __b)
 }
 
 #define _mm256_slli_si256(a, imm) \
-  (__m256i)__builtin_ia32_pslldqi256_byteshift((__v4di)(__m256i)(a), 
(int)(imm))
+  ((__m256i)__builtin_ia32_pslldqi256_byteshift((__v4di)(__m256i)(a), 
(int)(imm)))
 
 #define _mm256_bslli_epi128(a, imm) \
-  (__m256i)__builtin_ia32_pslldqi256_byteshift((__v4di)(__m256i)(a), 
(int)(imm))
+  ((__m256i)__builtin_ia32_pslldqi256_byteshift((__v4di)(__m256i)(a), 
(int)(imm)))
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_slli_epi16(__m256i __a, int __count)
@@ -560,10 +560,10 @@ _mm256_sra_epi32(__m256i __a, __m128i __count)
 }
 
 #define _mm256_srli_si256(a, imm) \
-  (__m256i)__builtin_ia32_psrldqi256_byteshift((__m256i)(a), (int)(imm))
+  ((__m256i)__builtin_ia32_psrldqi256_byteshift((__m256i)(a), (int)(imm)))
 
 #define _mm256_bsrli_epi128(a, imm) \
-  (__m256i)__builtin_ia32_psrldqi256_byteshift((__m256i)(a), (int)(imm))
+  ((__m256i)__builtin_ia32_psrldqi256_byteshift((__m256i)(a), (int)

[clang] 606735c - [Clang] Add an explicit makeArrayRef to appease gcc 5.4.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2021-08-13T09:42:28-07:00
New Revision: 606735c045b93cafe17bacbad10352b5a1c35cc0

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

LOG: [Clang] Add an explicit makeArrayRef to appease gcc 5.4.

Added: 


Modified: 
clang/lib/Sema/ParsedAttr.cpp

Removed: 




diff  --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index 4538ad25075b..045847d0ce0f 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -146,7 +146,7 @@ const ParsedAttrInfo &ParsedAttrInfo::get(const 
AttributeCommonInfo &A) {
 }
 
 ArrayRef ParsedAttrInfo::getAllBuiltin() {
-  return AttrInfoMap;
+  return llvm::makeArrayRef(AttrInfoMap);
 }
 
 unsigned ParsedAttr::getMinArgs() const { return getInfo().NumArgs; }



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

[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

[Craig Topper 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 rG4190d99dfcab: [X86] Add parentheses around casts in some of 
the X86 intrinsic headers. (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107843

Files:
  clang/lib/Headers/__wmmintrin_aes.h
  clang/lib/Headers/avx2intrin.h
  clang/lib/Headers/avxintrin.h
  clang/lib/Headers/emmintrin.h
  clang/lib/Headers/smmintrin.h
  clang/lib/Headers/tmmintrin.h
  clang/lib/Headers/xmmintrin.h
  clang/test/CodeGen/X86/sse41-builtins.c

Index: clang/test/CodeGen/X86/sse41-builtins.c
===
--- clang/test/CodeGen/X86/sse41-builtins.c
+++ clang/test/CodeGen/X86/sse41-builtins.c
@@ -393,3 +393,11 @@
   // CHECK: call i32 @llvm.x86.sse41.ptestz(<2 x i64> %{{.*}}, <2 x i64> %{{.*}})
   return _mm_testz_si128(x, y);
 }
+
+// Make sure brackets work after macro intrinsics.
+float pr51324(__m128 a) {
+  // CHECK-LABEL: pr51324
+  // CHECK: call <4 x float> @llvm.x86.sse41.round.ps(<4 x float> %{{.*}}, i32 0)
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  return _mm_round_ps(a, 0)[0];
+}
Index: clang/lib/Headers/xmmintrin.h
===
--- clang/lib/Headers/xmmintrin.h
+++ clang/lib/Headers/xmmintrin.h
@@ -2181,7 +2181,7 @@
 ///3: Bits [63:48] are copied to the destination.
 /// \returns A 16-bit integer containing the extracted 16 bits of packed data.
 #define _mm_extract_pi16(a, n) \
-  (int)__builtin_ia32_vec_ext_v4hi((__v4hi)a, (int)n)
+  ((int)__builtin_ia32_vec_ext_v4hi((__v4hi)a, (int)n))
 
 /// Copies data from the 64-bit vector of [4 x i16] to the destination,
 ///and inserts the lower 16-bits of an integer operand at the 16-bit offset
@@ -2212,7 +2212,7 @@
 /// \returns A 64-bit integer vector containing the copied packed data from the
 ///operands.
 #define _mm_insert_pi16(a, d, n) \
-  (__m64)__builtin_ia32_vec_set_v4hi((__v4hi)a, (int)d, (int)n)
+  ((__m64)__builtin_ia32_vec_set_v4hi((__v4hi)a, (int)d, (int)n))
 
 /// Compares each of the corresponding packed 16-bit integer values of
 ///the 64-bit integer vectors, and writes the greater value to the
@@ -2359,7 +2359,7 @@
 ///11: assigned from bits [63:48] of \a a.
 /// \returns A 64-bit integer vector containing the shuffled values.
 #define _mm_shuffle_pi16(a, n) \
-  (__m64)__builtin_ia32_pshufw((__v4hi)(__m64)(a), (n))
+  ((__m64)__builtin_ia32_pshufw((__v4hi)(__m64)(a), (n)))
 
 /// Conditionally copies the values from each 8-bit element in the first
 ///64-bit integer vector operand to the specified memory location, as
@@ -2601,8 +2601,8 @@
 ///11: Bits [127:96] copied from the specified operand.
 /// \returns A 128-bit vector of [4 x float] containing the shuffled values.
 #define _mm_shuffle_ps(a, b, mask) \
-  (__m128)__builtin_ia32_shufps((__v4sf)(__m128)(a), (__v4sf)(__m128)(b), \
-(int)(mask))
+  ((__m128)__builtin_ia32_shufps((__v4sf)(__m128)(a), (__v4sf)(__m128)(b), \
+ (int)(mask)))
 
 /// Unpacks the high-order (index 2,3) values from two 128-bit vectors of
 ///[4 x float] and interleaves them into a 128-bit vector of [4 x float].
Index: clang/lib/Headers/tmmintrin.h
===
--- clang/lib/Headers/tmmintrin.h
+++ clang/lib/Headers/tmmintrin.h
@@ -145,8 +145,8 @@
 /// \returns A 128-bit integer vector containing the concatenated right-shifted
 ///value.
 #define _mm_alignr_epi8(a, b, n) \
-  (__m128i)__builtin_ia32_palignr128((__v16qi)(__m128i)(a), \
- (__v16qi)(__m128i)(b), (n))
+  ((__m128i)__builtin_ia32_palignr128((__v16qi)(__m128i)(a), \
+  (__v16qi)(__m128i)(b), (n)))
 
 /// Concatenates the two 64-bit integer vector operands, and right-shifts
 ///the result by the number of bytes specified in the immediate operand.
@@ -168,7 +168,7 @@
 /// \returns A 64-bit integer vector containing the concatenated right-shifted
 ///value.
 #define _mm_alignr_pi8(a, b, n) \
-  (__m64)__builtin_ia32_palignr((__v8qi)(__m64)(a), (__v8qi)(__m64)(b), (n))
+  ((__m64)__builtin_ia32_palignr((__v8qi)(__m64)(a), (__v8qi)(__m64)(b), (n)))
 
 /// Horizontally adds the adjacent pairs of values contained in 2 packed
 ///128-bit vectors of [8 x i16].
Index: clang/lib/Headers/smmintrin.h
===
--- clang/lib/Headers/smmintrin.h
+++ clang/lib/Headers/smmintrin.h
@@ -231,7 +231,7 @@
 ///  11: Truncated
 /// \returns A 128-bit vector of [4 x float] containing the rounded values.
 #define _mm_round_ps(X, M) \
-  (__m128)__builtin_ia32_roundps((__v4sf)(__m128)(X), (M))
+  ((__m128)__builtin_ia32_r

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1388
 break;
-  if (!Line->Tokens.begin()->Tok->is(tok::kw_typedef) &&
-  isC78ParameterDecl(FormatTok)) {
+  const FormatToken *Next = AllTokens[Tokens->getPosition() + 1];
+  if (Next && Next->isOneOf(tok::l_paren, tok::semi))

Maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107961

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:8258
+  verifyFormat("bool f(int a, int) override;\n"
+   "Bar g(int a, Bar) final; // comment",
+   Style);

can you check with out the comment and without a having a type (I know it 
shocking code) but just want to be sure.

```
Bar g(int a, Bar) final;
Bar g(a, Bar) final;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107961

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

[PATCH] D107961: [clang-format] Distinguish K&R C function definition and attribute

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@owenpan If its been reverted is it OK to just reopen the review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107961

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

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:263
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' 
of the trivially-copyable type 'int' has no effect; consider changing 
forwardToShowInt's parameter from 'int'&& to 'int'&
+}

`forwardToShowInt` takes `T&&`, not `int&&`, and so it can't be changed in the 
way the diagnostic is suggesting. I think the right answer here is not to emit 
the diagnostic at all, when the offending function is a template.

(Relatively minor nits, all moot because this diagnostic should not be emitted 
at all: `'int'&&` should be `'int&&'`, `trivially copyable` should not be 
hyphenated, and `int&` is a strictly worse suggestion than either `const int&` 
or plain `int`.  IMVHO it would be reasonable to land a very trivial patch to 
remove the hyphen from `trivially copyable` and update the tests, and then 
rebase this PR on top of that.)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:271
+  showTmp(std::move(t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 't' 
of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+}

This is precisely the bogus message you're trying to eliminate, isn't it? 
Removing `std::move` would make the code fail to compile because `t` is an 
lvalue but `showTmp` requires an rvalue.


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

https://reviews.llvm.org/D107450

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366112.
gandhi21299 added a comment.

requested changes from reviewer

- added memory scope tests and updated remarks and tests accordingly
- still working on clang/test/CodeGenCUDA/fp-atomics-optremarks.cu and 
clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:618
   expandAtomicRMWToCmpXchg(AI, createCmpXchgInstFun);
+  Ctx.getSyncScopeNames(SSNs);
+  auto MemScope = SSNs[AI->getSyncScopeID()].empty()

Only if SSNs.empty().



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:624
+Remark << "A compare and swap loop was generated for an "
+   << AI->getOpcodeName() << "operation at " << MemScope
+   << " memory scope";

I believe getOpcodeName() will return "atomicrmw" instead of the operation. 
Also missing space after it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

rampitec wrote:
> Need tests for all scopes.
`__atomic_fetch_add` does not take scope as an argument, how could I add tests 
with different scopes?



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:25
+
+// GFX90A-CAS-LABEL: @atomic_cas_system
+// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("workgroup-one-as") monotonic

For some reason, remarks are not emitted here. The command to run looks right 
above...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

gandhi21299 wrote:
> rampitec wrote:
> > Need tests for all scopes.
> `__atomic_fetch_add` does not take scope as an argument, how could I add 
> tests with different scopes?
At least in the IR test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:618
   expandAtomicRMWToCmpXchg(AI, createCmpXchgInstFun);
+  Ctx.getSyncScopeNames(SSNs);
+  auto MemScope = SSNs[AI->getSyncScopeID()].empty()

rampitec wrote:
> Only if SSNs.empty().
Sorry, what do you mean? SSN will be empty at that point.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:624
+Remark << "A compare and swap loop was generated for an "
+   << AI->getOpcodeName() << "operation at " << MemScope
+   << " memory scope";

rampitec wrote:
> I believe getOpcodeName() will return "atomicrmw" instead of the operation. 
> Also missing space after it.
getOpcodeName() returns `atomicrmwoperation`, as per the tests the spacing 
looks correct to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added inline comments.



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

rampitec wrote:
> gandhi21299 wrote:
> > rampitec wrote:
> > > Need tests for all scopes.
> > `__atomic_fetch_add` does not take scope as an argument, how could I add 
> > tests with different scopes?
> At least in the IR test.
What do you mean by that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

gandhi21299 wrote:
> rampitec wrote:
> > gandhi21299 wrote:
> > > rampitec wrote:
> > > > Need tests for all scopes.
> > > `__atomic_fetch_add` does not take scope as an argument, how could I add 
> > > tests with different scopes?
> > At least in the IR test.
> What do you mean by that?
You need to test all of that. If you cannot write a proper .cu test, then write 
an IR test and run llc.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:618
   expandAtomicRMWToCmpXchg(AI, createCmpXchgInstFun);
+  Ctx.getSyncScopeNames(SSNs);
+  auto MemScope = SSNs[AI->getSyncScopeID()].empty()

gandhi21299 wrote:
> rampitec wrote:
> > Only if SSNs.empty().
> Sorry, what do you mean? SSN will be empty at that point.
I thought want to cache it. But really just declare it here.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:624
+Remark << "A compare and swap loop was generated for an "
+   << AI->getOpcodeName() << "operation at " << MemScope
+   << " memory scope";

gandhi21299 wrote:
> rampitec wrote:
> > I believe getOpcodeName() will return "atomicrmw" instead of the operation. 
> > Also missing space after it.
> getOpcodeName() returns `atomicrmwoperation`, as per the tests the spacing 
> looks correct to me.
The operation to report is AI->getOperation(). Spacing is wrong, "operation" is 
your text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

rampitec wrote:
> gandhi21299 wrote:
> > rampitec wrote:
> > > gandhi21299 wrote:
> > > > rampitec wrote:
> > > > > Need tests for all scopes.
> > > > `__atomic_fetch_add` does not take scope as an argument, how could I 
> > > > add tests with different scopes?
> > > At least in the IR test.
> > What do you mean by that?
> You need to test all of that. If you cannot write a proper .cu test, then 
> write an IR test and run llc.
Should I discard this test then since the test fp-atomics-remarks-gfx90a.ll 
already satisfies?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366131.
gandhi21299 added a comment.

- corrected atomics-remarks-gfx90a.cl test to emit remark as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Optimization Remark Emitter
 ; GCN-O1-OPTS-NEXT:

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

gandhi21299 wrote:
> rampitec wrote:
> > gandhi21299 wrote:
> > > rampitec wrote:
> > > > gandhi21299 wrote:
> > > > > rampitec wrote:
> > > > > > Need tests for all scopes.
> > > > > `__atomic_fetch_add` does not take scope as an argument, how could I 
> > > > > add tests with different scopes?
> > > > At least in the IR test.
> > > What do you mean by that?
> > You need to test all of that. If you cannot write a proper .cu test, then 
> > write an IR test and run llc.
> Should I discard this test then since the test fp-atomics-remarks-gfx90a.ll 
> already satisfies?
CU test is still needed. You also need it in the .cl test below.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:585
+  TLI->shouldExpandAtomicRMWInIR(AI, ORE);
+  OptimizationRemark Remark(DEBUG_TYPE, "Passed", AI->getFunction());
+  switch (Kind) {

What should this "Passed" do and why wouldn't just declare it where you use it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366127.
gandhi21299 added a comment.

- corrected remarks by replacing the operation name and updated tests 
accordingly
- code format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Optimization Rema

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366132.
gandhi21299 marked 3 inline comments as done.
gandhi21299 added a comment.

no way to pass memory_scope in `__atomic_fetch_add(...)`, discarded the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Optimization Remark Emitter
 ; GCN-O1-OP

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCUDA/fp-atomics-optremarks.cu:10
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic operation 
at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf

rampitec wrote:
> gandhi21299 wrote:
> > rampitec wrote:
> > > gandhi21299 wrote:
> > > > rampitec wrote:
> > > > > gandhi21299 wrote:
> > > > > > rampitec wrote:
> > > > > > > Need tests for all scopes.
> > > > > > `__atomic_fetch_add` does not take scope as an argument, how could 
> > > > > > I add tests with different scopes?
> > > > > At least in the IR test.
> > > > What do you mean by that?
> > > You need to test all of that. If you cannot write a proper .cu test, then 
> > > write an IR test and run llc.
> > Should I discard this test then since the test fp-atomics-remarks-gfx90a.ll 
> > already satisfies?
> CU test is still needed. You also need it in the .cl test below.
Alright, I am not sure how I can test for the other scopes though.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:585
+  TLI->shouldExpandAtomicRMWInIR(AI, ORE);
+  OptimizationRemark Remark(DEBUG_TYPE, "Passed", AI->getFunction());
+  switch (Kind) {

rampitec wrote:
> What should this "Passed" do and why wouldn't just declare it where you use 
> it?
https://llvm.org/docs/Remarks.html

Since this is an informative pass and not that pass failed to optimize, the 
"Passed" argument is used. I will move it downwards, I thought it might be 
useful in the future for other operations. Its better below for now anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

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



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:585
+  TLI->shouldExpandAtomicRMWInIR(AI, ORE);
+  OptimizationRemark Remark(DEBUG_TYPE, "Passed", AI->getFunction());
+  switch (Kind) {

gandhi21299 wrote:
> rampitec wrote:
> > What should this "Passed" do and why wouldn't just declare it where you use 
> > it?
> https://llvm.org/docs/Remarks.html
> 
> Since this is an informative pass and not that pass failed to optimize, the 
> "Passed" argument is used. I will move it downwards, I thought it might be 
> useful in the future for other operations. Its better below for now anyways.
Actually I am getting a runtime error at the line where I declare Remark when I 
bring it down.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366294.
gandhi21299 added a comment.

- added clang/test/CodeGenCUDA/fp-atomics-optremarks.cu back
- moved `Remark` declaration into the `else` block


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Op

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366301.
gandhi21299 added a comment.

- rebased against main branch
- cleaned up code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Optimization Remark Emitter
 ; GCN-O1-OPTS-NEXT:  Expand Atomi

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for atomics generating CAS loop

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

Please retitle it without AMDGPU and remove the changes to pass ORE to targets. 
It is not a part of this change, it is a part of the folloup target specific 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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