[clang] 29e11a1 - Revert "[Remarks] Emit optimization remarks for atomics generating CAS loop"

[Anshil Gandhi via cfe-commits]

Author: Anshil Gandhi
Date: 2021-08-13T23:58:04-06:00
New Revision: 29e11a1aa303cf81b81fdbab74fad4f31e5018d3

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

LOG: Revert "[Remarks] Emit optimization remarks for atomics generating CAS 
loop"

This reverts commit c4e5425aa579d21530ef1766d7144b38a347f247.

Added: 


Modified: 
llvm/lib/CodeGen/AtomicExpandPass.cpp
llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
llvm/test/CodeGen/X86/O0-pipeline.ll
llvm/test/CodeGen/X86/opt-pipeline.ll

Removed: 
clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll



diff  --git a/clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu 
b/clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
deleted file mode 100644
index 96892286fd75e..0
--- a/clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
+++ /dev/null
@@ -1,16 +0,0 @@
-// RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -fcuda-is-device \
-// RUN:   -target-cpu gfx90a -Rpass=atomic-expand -S -o - 2>&1 | \
-// RUN:   FileCheck %s --check-prefix=GFX90A-CAS
-
-// REQUIRES: amdgpu-registered-target
-
-#include "Inputs/cuda.h"
-#include 
-
-// GFX90A-CAS: A compare and swap loop was generated for an atomic fadd 
operation at system memory scope
-// GFX90A-CAS-LABEL: _Z14atomic_add_casPf
-// GFX90A-CAS:  flat_atomic_cmpswap v0, v[2:3], v[4:5] glc
-// GFX90A-CAS:  s_cbranch_execnz
-__device__ float atomic_add_cas(float *p) {
-  return __atomic_fetch_add(p, 1.0f, memory_order_relaxed);
-}

diff  --git a/clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl 
b/clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
deleted file mode 100644
index 2d8b68f83b9d6..0
--- a/clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
+++ /dev/null
@@ -1,46 +0,0 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
-// RUN: -Rpass=atomic-expand -S -o - 2>&1 | \
-// RUN: FileCheck %s --check-prefix=REMARK
-
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
-// RUN: -Rpass=atomic-expand -S -emit-llvm -o - 2>&1 | \
-// RUN: FileCheck %s --check-prefix=GFX90A-CAS
-
-// REQUIRES: amdgpu-registered-target
-
-typedef enum memory_order {
-  memory_order_relaxed = __ATOMIC_RELAXED,
-  memory_order_acquire = __ATOMIC_ACQUIRE,
-  memory_order_release = __ATOMIC_RELEASE,
-  memory_order_acq_rel = __ATOMIC_ACQ_REL,
-  memory_order_seq_cst = __ATOMIC_SEQ_CST
-} memory_order;
-
-typedef enum memory_scope {
-  memory_scope_work_item = __OPENCL_MEMORY_SCOPE_WORK_ITEM,
-  memory_scope_work_group = __OPENCL_MEMORY_SCOPE_WORK_GROUP,
-  memory_scope_device = __OPENCL_MEMORY_SCOPE_DEVICE,
-  memory_scope_all_svm_devices = __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES,
-#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups)
-  memory_scope_sub_group = __OPENCL_MEMORY_SCOPE_SUB_GROUP
-#endif
-} memory_scope;
-
-// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at workgroup-one-as memory scope [-Rpass=atomic-expand]
-// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at agent-one-as memory scope [-Rpass=atomic-expand]
-// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at one-as memory scope [-Rpass=atomic-expand]
-// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at wavefront-one-as memory scope [-Rpass=atomic-expand]
-// GFX90A-CAS-LABEL: @atomic_cas
-// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("workgroup-one-as") monotonic
-// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("agent-one-as") monotonic
-// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} syncscope("one-as") 
monotonic
-// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("wavefront-one-as") monotonic
-float atomic_cas(__global atomic_float *d, float a) {
-  float ret1 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_work_group);
-  float ret2 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_device);
-  float ret3 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_all_svm_devices);
-  float ret4 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_sub_group);
-}
-
-
-

diff  --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp 
b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index 5b5458e1058e8..125a3be585cb5 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -17,7 +17,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
-#include 

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

[Peter Jiachen via Phabricator via cfe-commits]

peterjc123 added a comment.

@rnk @hans Test added. Please take some time to review the changes.


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] D108021: [dllexport] Instantiate default ctor default args

[Peter Jiachen via Phabricator via cfe-commits]

peterjc123 updated this revision to Diff 366399.
peterjc123 added a comment.

Add test


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

https://reviews.llvm.org/D108021

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp


Index: clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
===
--- clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
+++ clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++14 \
+// RUN:-fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases \
+// RUN:-disable-llvm-passes -o - %s -w -fms-compatibility-version=19.00 | \
+// RUN:FileCheck %s
+
+struct HasDtor {
+  ~HasDtor();
+  int o;
+};
+struct HasImplicitDtor1 { HasDtor o; };
+struct __declspec(dllexport) CtorClosureOuter {
+  struct __declspec(dllexport) CtorClosureInner {
+CtorClosureInner(const HasImplicitDtor1  = {}) {}
+  };
+};
+
+// CHECK-LABEL: define weak_odr dso_local dllexport x86_thiscallcc void 
@"??_FCtorClosureInner@CtorClosureOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
+// CHECK-LABEL: $"??1HasImplicitDtor1@@QAE@XZ" = comdat any
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/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
===
--- clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
+++ clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++14 \
+// RUN:-fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases \
+// RUN:-disable-llvm-passes -o - %s -w -fms-compatibility-version=19.00 | \
+// RUN:FileCheck %s
+
+struct HasDtor {
+  ~HasDtor();
+  int o;
+};
+struct HasImplicitDtor1 { HasDtor o; };
+struct __declspec(dllexport) CtorClosureOuter {
+  struct __declspec(dllexport) CtorClosureInner {
+CtorClosureInner(const HasImplicitDtor1  = {}) {}
+  };
+};
+
+// CHECK-LABEL: define weak_odr dso_local dllexport x86_thiscallcc void @"??_FCtorClosureInner@CtorClosureOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
+// CHECK-LABEL: $"??1HasImplicitDtor1@@QAE@XZ" = comdat any
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] D108021: [dllexport] Instantiate default ctor default args

[Peter Jiachen via Phabricator via cfe-commits]

peterjc123 updated this revision to Diff 366394.
peterjc123 added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108021

Files:
  clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp


Index: clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
===
--- clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
+++ clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++14 \
+// RUN:-fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases \
+// RUN:-disable-llvm-passes -o - %s -w -fms-compatibility-version=19.00 | \
+// RUN:FileCheck %s
+
+struct HasDtor {
+  ~HasDtor();
+  int o;
+};
+struct HasImplicitDtor1 { HasDtor o; };
+struct __declspec(dllexport) CtorClosureOuter {
+  struct __declspec(dllexport) CtorClosureInner {
+CtorClosureInner(const HasImplicitDtor1  = {}) {}
+  };
+};
+
+// CHECK-LABEL: define weak_odr dso_local dllexport x86_thiscallcc void 
@"??_FCtorClosureInner@CtorClosureOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
+// CHECK-LABEL: $"??1HasImplicitDtor1@@QAE@XZ" = comdat any


Index: clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
===
--- clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
+++ clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++14 \
+// RUN:-fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases \
+// RUN:-disable-llvm-passes -o - %s -w -fms-compatibility-version=19.00 | \
+// RUN:FileCheck %s
+
+struct HasDtor {
+  ~HasDtor();
+  int o;
+};
+struct HasImplicitDtor1 { HasDtor o; };
+struct __declspec(dllexport) CtorClosureOuter {
+  struct __declspec(dllexport) CtorClosureInner {
+CtorClosureInner(const HasImplicitDtor1  = {}) {}
+  };
+};
+
+// CHECK-LABEL: define weak_odr dso_local dllexport x86_thiscallcc void @"??_FCtorClosureInner@CtorClosureOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
+// CHECK-LABEL: $"??1HasImplicitDtor1@@QAE@XZ" = comdat any
___
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 updated this revision to Diff 366393.
peterjc123 added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108021

Files:
  clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp


Index: clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
===
--- clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
+++ clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++14 \
+// RUN:-fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases \
+// RUN:-disable-llvm-passes -o - %s -w -fms-compatibility-version=19.00 | \
+// RUN:FileCheck %s
+
+struct HasDtor {
+  ~HasDtor();
+  int o;
+};
+struct HasImplicitDtor1 { HasDtor o; };
+struct __declspec(dllexport) CtorClosureOuter {
+  struct __declspec(dllexport) CtorClosureInner {
+CtorClosureInner(const HasImplicitDtor1  = {}) {}
+  };
+};
+
+// CHECK-LABEL: define weak_odr dso_local dllexport x86_thiscallcc void 
@"??_FCtorClosureInner@CtorClosureOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
+// CHECK-LABEL: $"??1HasImplicitDtor1@@QAE@XZ" = comdat any


Index: clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
===
--- clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
+++ clang/test/CodeGenCXX/dllexport-ctor-closure-nested.cpp   
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -std=c++14 \
+// RUN:-fno-threadsafe-statics -fms-extensions -O1 -mconstructor-aliases \
+// RUN:-disable-llvm-passes -o - %s -w -fms-compatibility-version=19.00 | \
+// RUN:FileCheck %s
+
+struct HasDtor {
+  ~HasDtor();
+  int o;
+};
+struct HasImplicitDtor1 { HasDtor o; };
+struct __declspec(dllexport) CtorClosureOuter {
+  struct __declspec(dllexport) CtorClosureInner {
+CtorClosureInner(const HasImplicitDtor1  = {}) {}
+  };
+};
+
+// CHECK-LABEL: define weak_odr dso_local dllexport x86_thiscallcc void @"??_FCtorClosureInner@CtorClosureOuter@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat
+// CHECK-LABEL: $"??1HasImplicitDtor1@@QAE@XZ" = comdat any
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c4e5425 - [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via cfe-commits]

Author: Anshil Gandhi
Date: 2021-08-13T22:44:08-06:00
New Revision: c4e5425aa579d21530ef1766d7144b38a347f247

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

LOG: [Remarks] Emit optimization remarks for atomics generating CAS loop

Implements ORE in AtomicExpandPass to report atomics generating a compare
and swap loop.

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

Added: 
clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Modified: 
llvm/lib/CodeGen/AtomicExpandPass.cpp
llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
llvm/test/CodeGen/X86/O0-pipeline.ll
llvm/test/CodeGen/X86/opt-pipeline.ll

Removed: 




diff  --git a/clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu 
b/clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
new file mode 100644
index 0..96892286fd75e
--- /dev/null
+++ b/clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple=amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   -target-cpu gfx90a -Rpass=atomic-expand -S -o - 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=GFX90A-CAS
+
+// REQUIRES: amdgpu-registered-target
+
+#include "Inputs/cuda.h"
+#include 
+
+// GFX90A-CAS: A compare and swap loop was generated for an atomic fadd 
operation at system memory scope
+// GFX90A-CAS-LABEL: _Z14atomic_add_casPf
+// GFX90A-CAS:  flat_atomic_cmpswap v0, v[2:3], v[4:5] glc
+// GFX90A-CAS:  s_cbranch_execnz
+__device__ float atomic_add_cas(float *p) {
+  return __atomic_fetch_add(p, 1.0f, memory_order_relaxed);
+}

diff  --git a/clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl 
b/clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
new file mode 100644
index 0..2d8b68f83b9d6
--- /dev/null
+++ b/clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=atomic-expand -S -o - 2>&1 | \
+// RUN: FileCheck %s --check-prefix=REMARK
+
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=atomic-expand -S -emit-llvm -o - 2>&1 | \
+// RUN: FileCheck %s --check-prefix=GFX90A-CAS
+
+// REQUIRES: amdgpu-registered-target
+
+typedef enum memory_order {
+  memory_order_relaxed = __ATOMIC_RELAXED,
+  memory_order_acquire = __ATOMIC_ACQUIRE,
+  memory_order_release = __ATOMIC_RELEASE,
+  memory_order_acq_rel = __ATOMIC_ACQ_REL,
+  memory_order_seq_cst = __ATOMIC_SEQ_CST
+} memory_order;
+
+typedef enum memory_scope {
+  memory_scope_work_item = __OPENCL_MEMORY_SCOPE_WORK_ITEM,
+  memory_scope_work_group = __OPENCL_MEMORY_SCOPE_WORK_GROUP,
+  memory_scope_device = __OPENCL_MEMORY_SCOPE_DEVICE,
+  memory_scope_all_svm_devices = __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES,
+#if defined(cl_intel_subgroups) || defined(cl_khr_subgroups)
+  memory_scope_sub_group = __OPENCL_MEMORY_SCOPE_SUB_GROUP
+#endif
+} memory_scope;
+
+// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at workgroup-one-as memory scope [-Rpass=atomic-expand]
+// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at agent-one-as memory scope [-Rpass=atomic-expand]
+// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at one-as memory scope [-Rpass=atomic-expand]
+// REMARK: remark: A compare and swap loop was generated for an atomic fadd 
operation at wavefront-one-as memory scope [-Rpass=atomic-expand]
+// GFX90A-CAS-LABEL: @atomic_cas
+// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("workgroup-one-as") monotonic
+// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("agent-one-as") monotonic
+// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} syncscope("one-as") 
monotonic
+// GFX90A-CAS: atomicrmw fadd float addrspace(1)* {{.*}} 
syncscope("wavefront-one-as") monotonic
+float atomic_cas(__global atomic_float *d, float a) {
+  float ret1 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_work_group);
+  float ret2 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_device);
+  float ret3 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_all_svm_devices);
+  float ret4 = __opencl_atomic_fetch_add(d, a, memory_order_relaxed, 
memory_scope_sub_group);
+}
+
+
+

diff  --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp 
b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index 125a3be585cb5..5b5458e1058e8 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include 

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

[liushuai wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f2d40c47f5f: [clang-tidy] fix duplicate {} in 
cppcoreguidelines-pro-type-member-init (authored by Sockke, committed by MTC).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107641

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -208,9 +208,8 @@
   PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these fields: A, B
 
-  // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;
-  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+  // CHECK-FIXES: int A{}, B{};
 };
 
 typedef struct {
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -10,6 +10,7 @@
 #define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -72,6 +73,10 @@
   // instead of brace initialization. Only effective in C++11 mode. Default is
   // false.
   bool UseAssignment;
+
+  // Record the member variables that have been initialized to prevent repeated
+  // initialization.
+  llvm::DenseSet HasRecordClassMemberSet;
 };
 
 } // namespace cppcoreguidelines
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -433,17 +433,25 @@
[&](const FieldDecl *F) { OrderedFields.push_back(F); });
 
   // Collect all the fields we need to initialize, including indirect fields.
+  // It only includes fields that have not been fixed
   SmallPtrSet AllFieldsToInit;
-  forEachField(ClassDecl, FieldsToInit,
-   [&](const FieldDecl *F) { AllFieldsToInit.insert(F); });
-  if (AllFieldsToInit.empty())
+  forEachField(ClassDecl, FieldsToInit, [&](const FieldDecl *F) {
+if (!HasRecordClassMemberSet.contains(F)) {
+  AllFieldsToInit.insert(F);
+  HasRecordClassMemberSet.insert(F);
+}
+  });
+  if (FieldsToInit.empty())
 return;
 
   DiagnosticBuilder Diag =
   diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(),
"%select{|union }0constructor %select{does not|should}0 initialize "
"%select{|one of }0these fields: %1")
-  << IsUnion << toCommaSeparatedString(OrderedFields, AllFieldsToInit);
+  << IsUnion << toCommaSeparatedString(OrderedFields, FieldsToInit);
+
+  if (AllFieldsToInit.empty())
+return;
 
   // Do not propose fixes for constructors in macros since we cannot place them
   // correctly.


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -208,9 +208,8 @@
   PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
 
-  // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;
-  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+  // CHECK-FIXES: int A{}, B{};
 };
 
 typedef struct {
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -72,6 +73,10 @@
   // instead of brace initialization. Only effective in C++11 mode. Default is
   // false.
   bool UseAssignment;
+
+  

[clang-tools-extra] 1f2d40c - [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

[via cfe-commits]

Author: liuke
Date: 2021-08-14T10:48:04+08:00
New Revision: 1f2d40c47f5f8fd01d91d73a1f52044fe1c83225

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

LOG: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

The overload of the constructor will repeatedly fix the member variables that 
need to be initialized.
Removed the duplicate '{}'.

```
struct A {
  A() {}
  A(int) {}
  int _var;  // int _var{}{};  <--  wrong fix
};
```

Reviewed By: aaron.ballman

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

Added: 


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

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

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
index 43812fe17a1c7..a191598415217 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -433,17 +433,25 @@ void 
ProTypeMemberInitCheck::checkMissingMemberInitializer(
[&](const FieldDecl *F) { OrderedFields.push_back(F); });
 
   // Collect all the fields we need to initialize, including indirect fields.
+  // It only includes fields that have not been fixed
   SmallPtrSet AllFieldsToInit;
-  forEachField(ClassDecl, FieldsToInit,
-   [&](const FieldDecl *F) { AllFieldsToInit.insert(F); });
-  if (AllFieldsToInit.empty())
+  forEachField(ClassDecl, FieldsToInit, [&](const FieldDecl *F) {
+if (!HasRecordClassMemberSet.contains(F)) {
+  AllFieldsToInit.insert(F);
+  HasRecordClassMemberSet.insert(F);
+}
+  });
+  if (FieldsToInit.empty())
 return;
 
   DiagnosticBuilder Diag =
   diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(),
"%select{|union }0constructor %select{does not|should}0 initialize "
"%select{|one of }0these fields: %1")
-  << IsUnion << toCommaSeparatedString(OrderedFields, AllFieldsToInit);
+  << IsUnion << toCommaSeparatedString(OrderedFields, FieldsToInit);
+
+  if (AllFieldsToInit.empty())
+return;
 
   // Do not propose fixes for constructors in macros since we cannot place them
   // correctly.

diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
index 5b4144396eab4..af7b14ec68ad9 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -10,6 +10,7 @@
 #define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -72,6 +73,10 @@ class ProTypeMemberInitCheck : public ClangTidyCheck {
   // instead of brace initialization. Only effective in C++11 mode. Default is
   // false.
   bool UseAssignment;
+
+  // Record the member variables that have been initialized to prevent repeated
+  // initialization.
+  llvm::DenseSet HasRecordClassMemberSet;
 };
 
 } // namespace cppcoreguidelines

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
index 403f28baf99d4..8cab4fd755752 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -208,9 +208,8 @@ struct PositiveMultipleConstructors {
   PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these fields: A, B
 
-  // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;
-  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+  // CHECK-FIXES: int A{}, B{};
 };
 
 typedef struct {



___
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()

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 366381.
aeubanks added a comment.

one more place


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108013

Files:
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
  llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/Linker/IRMover.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Utils/ValueMapper.cpp
  llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

Index: llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
===
--- llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
+++ llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
@@ -88,7 +88,7 @@
  SetIdx != SetEndIdx; ++SetIdx) {
   AttrPtrIdxVecVecTy AttributesToPreserve;
   AttributesToPreserve.first = SetIdx;
-  visitAttributeSet(AL.getAttributes(AttributesToPreserve.first),
+  visitAttributeSet(AL.getAttributesImpl(AttributesToPreserve.first),
 AttributesToPreserve.second);
   if (!AttributesToPreserve.second.empty())
 AttributeSetsToPreserve.emplace_back(std::move(AttributesToPreserve));
Index: llvm/lib/Transforms/Utils/ValueMapper.cpp
===
--- llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -947,7 +947,7 @@
   for (Attribute::AttrKind TypedAttr :
  {Attribute::ByVal, Attribute::StructRet, Attribute::ByRef,
   Attribute::InAlloca}) {
-if (Type *Ty = Attrs.getAttribute(i, TypedAttr).getValueAsType()) {
+if (Type *Ty = Attrs.getAttributeImpl(i, TypedAttr).getValueAsType()) {
   Attrs = Attrs.replaceAttributeType(C, i, TypedAttr,
  TypeMapper->remapType(Ty));
   break;
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -111,8 +111,8 @@
 return Res;
 
   for (unsigned i = L.index_begin(), e = L.index_end(); i != e; ++i) {
-AttributeSet LAS = L.getAttributes(i);
-AttributeSet RAS = R.getAttributes(i);
+AttributeSet LAS = L.getAttributesImpl(i);
+AttributeSet RAS = R.getAttributesImpl(i);
 AttributeSet::iterator LI = LAS.begin(), LE = LAS.end();
 AttributeSet::iterator RI = RAS.begin(), RE = RAS.end();
 for (; LI != LE && RI != RE; ++LI, ++RI) {
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -2662,7 +2662,7 @@
 R.addAttribute(Attribute::get(Ctx, Attribute::DereferenceableOrNull,
   AH.getDereferenceableOrNullBytes(Index)));
   for (auto Attr : ParamAttrsToStrip)
-if (AH.getAttributes().hasAttribute(Index, Attr))
+if (AH.getAttributes().hasAttributeImpl(Index, Attr))
   R.addAttribute(Attr);
 
   if (!R.empty())
Index: llvm/lib/Transforms/IPO/Attributor.cpp
===
--- llvm/lib/Transforms/IPO/Attributor.cpp
+++ llvm/lib/Transforms/IPO/Attributor.cpp
@@ -382,27 +382,27 @@
 
   if (Attr.isEnumAttribute()) {
 Attribute::AttrKind Kind = Attr.getKindAsEnum();
-if (Attrs.hasAttribute(AttrIdx, Kind))
+if (Attrs.hasAttributeImpl(AttrIdx, Kind))
   if (!ForceReplace &&
-  isEqualOrWorse(Attr, Attrs.getAttribute(AttrIdx, Kind)))
+  isEqualOrWorse(Attr, Attrs.getAttributeImpl(AttrIdx, Kind)))
 return false;
 Attrs = Attrs.addAttribute(Ctx, AttrIdx, Attr);
 return true;
   }
   if (Attr.isStringAttribute()) {
 StringRef Kind = Attr.getKindAsString();
-if (Attrs.hasAttribute(AttrIdx, Kind))
+if (Attrs.hasAttributeImpl(AttrIdx, Kind))
   if (!ForceReplace &&
-  isEqualOrWorse(Attr, Attrs.getAttribute(AttrIdx, Kind)))
+  isEqualOrWorse(Attr, Attrs.getAttributeImpl(AttrIdx, Kind)))
 return false;
 Attrs = Attrs.addAttribute(Ctx, AttrIdx, Attr);
 return true;
   }
   if (Attr.isIntAttribute()) {
 Attribute::AttrKind Kind = Attr.getKindAsEnum();
-if (Attrs.hasAttribute(AttrIdx, Kind))
+if (Attrs.hasAttributeImpl(AttrIdx, Kind))
   if (!ForceReplace &&
-  isEqualOrWorse(Attr, Attrs.getAttribute(AttrIdx, Kind)))
+  isEqualOrWorse(Attr, Attrs.getAttributeImpl(AttrIdx, 

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

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 366380.
aeubanks added a comment.

rebase past cleanups


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108013

Files:
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
  llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/Linker/IRMover.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Utils/ValueMapper.cpp

Index: llvm/lib/Transforms/Utils/ValueMapper.cpp
===
--- llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -947,7 +947,7 @@
   for (Attribute::AttrKind TypedAttr :
  {Attribute::ByVal, Attribute::StructRet, Attribute::ByRef,
   Attribute::InAlloca}) {
-if (Type *Ty = Attrs.getAttribute(i, TypedAttr).getValueAsType()) {
+if (Type *Ty = Attrs.getAttributeImpl(i, TypedAttr).getValueAsType()) {
   Attrs = Attrs.replaceAttributeType(C, i, TypedAttr,
  TypeMapper->remapType(Ty));
   break;
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -111,8 +111,8 @@
 return Res;
 
   for (unsigned i = L.index_begin(), e = L.index_end(); i != e; ++i) {
-AttributeSet LAS = L.getAttributes(i);
-AttributeSet RAS = R.getAttributes(i);
+AttributeSet LAS = L.getAttributesImpl(i);
+AttributeSet RAS = R.getAttributesImpl(i);
 AttributeSet::iterator LI = LAS.begin(), LE = LAS.end();
 AttributeSet::iterator RI = RAS.begin(), RE = RAS.end();
 for (; LI != LE && RI != RE; ++LI, ++RI) {
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -2662,7 +2662,7 @@
 R.addAttribute(Attribute::get(Ctx, Attribute::DereferenceableOrNull,
   AH.getDereferenceableOrNullBytes(Index)));
   for (auto Attr : ParamAttrsToStrip)
-if (AH.getAttributes().hasAttribute(Index, Attr))
+if (AH.getAttributes().hasAttributeImpl(Index, Attr))
   R.addAttribute(Attr);
 
   if (!R.empty())
Index: llvm/lib/Transforms/IPO/Attributor.cpp
===
--- llvm/lib/Transforms/IPO/Attributor.cpp
+++ llvm/lib/Transforms/IPO/Attributor.cpp
@@ -382,27 +382,27 @@
 
   if (Attr.isEnumAttribute()) {
 Attribute::AttrKind Kind = Attr.getKindAsEnum();
-if (Attrs.hasAttribute(AttrIdx, Kind))
+if (Attrs.hasAttributeImpl(AttrIdx, Kind))
   if (!ForceReplace &&
-  isEqualOrWorse(Attr, Attrs.getAttribute(AttrIdx, Kind)))
+  isEqualOrWorse(Attr, Attrs.getAttributeImpl(AttrIdx, Kind)))
 return false;
 Attrs = Attrs.addAttribute(Ctx, AttrIdx, Attr);
 return true;
   }
   if (Attr.isStringAttribute()) {
 StringRef Kind = Attr.getKindAsString();
-if (Attrs.hasAttribute(AttrIdx, Kind))
+if (Attrs.hasAttributeImpl(AttrIdx, Kind))
   if (!ForceReplace &&
-  isEqualOrWorse(Attr, Attrs.getAttribute(AttrIdx, Kind)))
+  isEqualOrWorse(Attr, Attrs.getAttributeImpl(AttrIdx, Kind)))
 return false;
 Attrs = Attrs.addAttribute(Ctx, AttrIdx, Attr);
 return true;
   }
   if (Attr.isIntAttribute()) {
 Attribute::AttrKind Kind = Attr.getKindAsEnum();
-if (Attrs.hasAttribute(AttrIdx, Kind))
+if (Attrs.hasAttributeImpl(AttrIdx, Kind))
   if (!ForceReplace &&
-  isEqualOrWorse(Attr, Attrs.getAttribute(AttrIdx, Kind)))
+  isEqualOrWorse(Attr, Attrs.getAttributeImpl(AttrIdx, Kind)))
 return false;
 Attrs = Attrs.removeAttribute(Ctx, AttrIdx, Kind);
 Attrs = Attrs.addAttribute(Ctx, AttrIdx, Attr);
@@ -658,9 +658,9 @@
   else
 AttrList = getAssociatedFunction()->getAttributes();
 
-  bool HasAttr = AttrList.hasAttribute(getAttrIdx(), AK);
+  bool HasAttr = AttrList.hasAttributeImpl(getAttrIdx(), AK);
   if (HasAttr)
-Attrs.push_back(AttrList.getAttribute(getAttrIdx(), AK));
+Attrs.push_back(AttrList.getAttributeImpl(getAttrIdx(), AK));
   return HasAttr;
 }
 
Index: llvm/lib/Linker/IRMover.cpp
===
--- llvm/lib/Linker/IRMover.cpp
+++ llvm/lib/Linker/IRMover.cpp
@@ -651,8 +651,8 @@
 for (Attribute::AttrKind TypedAttr :
  

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

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

xbolva00 added a comment.

In D108003#2944178 , @aeubanks wrote:

> I ran this over Chrome and ran into a use case that looks legitimate. It 
> seems like the pattern in LLVM where we want to run a bunch of 
> transformations and get if any of them changed anything.
>
> https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/api/audio/echo_canceller3_config.cc;drc=cbdbb8c1666fd08a094422905e73391706a05b03;l=111

Maybe “res &= foo ();” would be better? “Changed |= foo();” is very typical for 
LLVM.


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

[PATCH] D104975: Implement P1949

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:117
+def err_character_not_allowed : Error<
+  "character  not allowed in identifiers">;
 def ext_unicode_whitespace : ExtWarn<

The old diagnostic text here was bad in the case where the character was 
supposed to be part of an identifier. The new diagnostic text is bad in the 
case where the character is not supposed to be part of an identifier (eg, if 
you copy-paste a smart-quote into the source file). Is there some way we can 
phrase this diagnostic so it's reasonable regardless of the programmer's intent?

Perhaps the best we can do is to say that if an identifier is immediately 
followed by one or more "bad" Unicode characters that were probably intended to 
be identifier characters (things we don't recognize as whitespace or homoglyphs 
or anything else), then produce the nice "not allowed in identifiers" 
diagnostic (and maybe even treat the characters as part of the identifier for 
error recovery purposes), and otherwise diagnose as simply "unexpected 
character "? I also wonder if perhaps we should treat all unexpected 
characters as identifier characters for error recovery purposes.



Comment at: clang/lib/Lex/Lexer.cpp:3182
+}
+Diag(BufferPtr, diag::err_character_not_allowed)
+<< CharBuf

It's a bit strange that we produce this warning only on characters that aren't 
allowed in identifiers, and don't warn on identifier continuation characters 
that aren't valid identifier start characters (such as a standalone combining 
character). Is there a good reason for that (specifically for the 
`!isAllowedIDChar` check above)?


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] D107690: [Modules] Do not remove failed modules after the control block phase

[Ben Barham via Phabricator via cfe-commits]

bnbarham updated this revision to Diff 366378.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107690

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/test/VFS/Inputs/UsesFoo.framework/Headers/UsesFoo.h
  clang/test/VFS/Inputs/UsesFoo.framework/Modules/module.modulemap
  clang/test/VFS/module-header-mismatches.m
  clang/test/VFS/umbrella-mismatch.m

Index: clang/test/VFS/umbrella-mismatch.m
===
--- clang/test/VFS/umbrella-mismatch.m
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: rm -rf %t
-// RUN: sed -e "s;INPUT_DIR;%/S/Inputs;g" -e "s;OUT_DIR;%/S/Inputs;g" %S/Inputs/vfsoverlay.yaml > %t.yaml
-
-// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -F %S/Inputs -fsyntax-only %s -Wno-atimport-in-framework-header -verify
-// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs -fsyntax-only %s -Wno-atimport-in-framework-header -verify
-// expected-no-diagnostics
-@import UsesFoo;
Index: clang/test/VFS/module-header-mismatches.m
===
--- /dev/null
+++ clang/test/VFS/module-header-mismatches.m
@@ -0,0 +1,86 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s;TEST_DIR;%/t;g" %t/sed-overlay.yaml > %t/overlay.yaml
+
+// These tests first build with an overlay such that the header is resolved
+// to %t/other/Mismatch.h. They then build again with the header resolved
+// to the one in their directory.
+//
+// This should cause a rebuild if the contents is different (and thus multiple
+// PCMs), but this currently isn't the case. We should at least not error,
+// since this does happen in real projects (with a different copy of the same
+// file).
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/hf-mcp -ivfsoverlay %t/overlay.yaml -F %t/header-frameworks -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/hf-mcp -F %t/header-frameworks -fsyntax-only -verify %t/use.m
+// RUN: find %t/hf-mcp -name "Mismatch-*.pcm" | count 1
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/df-mcp -ivfsoverlay %t/overlay.yaml -F %t/dir-frameworks -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/hf-mcp -F %t/dir-frameworks -fsyntax-only -verify %t/use.m
+// RUN: find %t/df-mcp -name "Mismatch-*.pcm" | count 1
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/nf-mcp -ivfsoverlay %t/overlay.yaml -F %t/norm-frameworks -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/nf-mcp -F %t/norm-frameworks -fsyntax-only -verify %t/use.m
+// RUN: find %t/nf-mcp -name "Mismatch-*.pcm" | count 1
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/m-mcp -ivfsoverlay %t/overlay.yaml -I %t/mod -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/m-mcp -I %t/mod -fsyntax-only -verify %t/use.m
+// RUN: find %t/m-mcp -name "Mismatch-*.pcm" | count 1
+
+//--- use.m
+// expected-no-diagnostics
+@import Mismatch;
+
+//--- header-frameworks/Mismatch.framework/Modules/module.modulemap
+framework module Mismatch {
+  umbrella header "Mismatch.h"
+}
+//--- header-frameworks/Mismatch.framework/Headers/Mismatch.h
+
+//--- dir-frameworks/Mismatch.framework/Modules/module.modulemap
+framework module Mismatch {
+  umbrella "someheaders"
+}
+//--- dir-frameworks/Mismatch.framework/someheaders/Mismatch.h
+
+//--- norm-frameworks/Mismatch.framework/Modules/module.modulemap
+framework module Mismatch {
+  header "Mismatch.h"
+}
+//--- norm-frameworks/Mismatch.framework/Headers/Mismatch.h
+
+//--- mod/module.modulemap
+module Mismatch {
+  umbrella header "Mismatch.h"
+}
+//--- mod/Mismatch.h
+
+//--- other/Mismatch.h
+
+//--- sed-overlay.yaml
+{
+  'version': 0,
+  'roots': [
+{ 'name': 'TEST_DIR', 'type': 'directory',
+  'contents': [
+{ 'name': 'header-frameworks/Mismatch.framework/Headers/Mismatch.h',
+  'type': 'file',
+  'external-contents': 'TEST_DIR/other/Mismatch.h'
+},
+{ 'name': 'dir-frameworks/Mismatch.framework/someheaders',
+  'type': 'directory',
+  'external-contents': 'TEST_DIR/others'
+},
+{ 'name': 'norm-frameworks/Mismatch.framework/Headers/Mismatch.h',
+  'type': 'file',
+  'external-contents': 'TEST_DIR/other/Mismatch.h'
+},
+{ 'name': 'mod/Mismatch.h',
+  'type': 'file',
+  'external-contents': 'TEST_DIR/other/Mismatch.h'
+}
+  ]
+}
+  ]
+}
+
Index: 

[PATCH] D104975: Implement P1949

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D104975#2944313 , @cor3ntin wrote:

> @rsmith: I modified the script locally to support dxx: dup P - let me 
> know if you think that's a good solution

Not sure if that's a typo: did you mean "sup" rather than "dup"? In this case, 
it seems like "sup" is right if the old DR resolution no longer applies. (If 
the old resolution does still apply, but was generalized, then I think we 
should just leave the DR status table alone for that issue, because that 
issue's resolution is still implemented.)


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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



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:
> 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?
I don't think there is nor will ever be a DR number for this; using the P 
number here seems like the best option available to us.

Ultimately the "sup" marking here serves two purposes:
- tracking what is done, when it was done, and what's still left to do
- providing a way to determine why we're not doing what a DR resolution says to 
do

I think it would be fine to write this as `dr248: sup P1949R7`, and in the 
`make_cxx_status` script, generate text that says "Superseded by P1949R7" like 
in your edits to `cxx_dr_status.html` and color DRs superseded by papers as 
`class="na"` (because the DR no longer applies due to the paper). (Not saying 
that's the only option, but I think it would be a reasonable choice.)

Separately, please don't remove the tests here; instead update them to show 
what the new behavior is, and add tests if necessary so that it's clear in what 
way the DR's behavior has been superseded.


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] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

Using the checker now in our production BuildBot - no crashes and no false 
positives. Can't say if there are false negatives, but at any rate the checker 
is better than most colleagues at finding what should be declared const.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D96033#298 , @leonardchan wrote:

> We're still hitting the OOMs when building clang-repl with LTO even with 
> `-DLLVM_PARALLEL_LINK_JOBS=32`. While we don't build this target explicitly 
> in our toolchain, it is built when running tests via `stage2-check-clang`. Is 
> there perhaps a simple cmake flag that allows us to not run clang-repl tests 
> so we don't build it?

To clarify, this is on a machine with 512GB RAM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:211-214
+case 224:
+  n += 400;
+case 225: // expected-warning{{unannotated fall-through between switch 
labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this 
warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;

nickdesaulniers wrote:
> These 4 lines don't add anything to the test coverage.  Remove them?
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 366363.
nathanchance added a comment.

- -Wimplicit-fallthrough-unreachable -> -Wunreachable-code-fallthrough, enabled 
under -Wunreachable-code (Dávid, Aaron).

- Drop "case 225" from new test, as it does not add to test coverage (Nick).

- Add -Wunreachable-code-fallthrough to PR30636 test command, as it is 
explicitly testing that there is no unreachable warning with template function 
instantiation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/P30636.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 
 
 int fallthrough(int n) {
@@ -193,6 +193,26 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-fallthrough"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+  }
+#pragma clang diagnostic pop
+
   long p = static_cast(n) * n;
   switch (sizeof(p)) {
 case 9:
Index: clang/test/SemaCXX/P30636.cpp
===
--- clang/test/SemaCXX/P30636.cpp
+++ clang/test/SemaCXX/P30636.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 // expected-no-diagnostics
 
 template
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1125,7 +1125,7 @@
 // unreachable in all instantiations of the template.
 if (!IsTemplateInstantiation)
   S.Diag(AS->getBeginLoc(),
- diag::warn_fallthrough_attr_unreachable);
+ diag::warn_unreachable_fallthrough_attr);
 markFallthroughVisited(AS);
 ++AnnotatedCnt;
 break;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -682,6 +682,9 @@
 def warn_unreachable_loop_increment : Warning<
   "loop will run at most once (loop increment never executed)">,
   InGroup, DefaultIgnore;
+def warn_unreachable_fallthrough_attr : Warning<
+  "fallthrough annotation in unreachable code">,
+  InGroup, DefaultIgnore;
 def note_unreachable_silence : Note<
   "silence by adding parentheses to mark code as explicitly dead">;
 
@@ -9578,9 +9581,6 @@
   "fallthrough annotation is outside switch statement">;
 def err_fallthrough_attr_invalid_placement : Error<
   "fallthrough annotation does not directly precede switch label">;
-def warn_fallthrough_attr_unreachable : Warning<
-  "fallthrough annotation in unreachable code">,
-  InGroup, DefaultIgnore;
 
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -819,8 +819,10 @@
 //  under separate flags.
 //
 def UnreachableCodeLoopIncrement : 
DiagGroup<"unreachable-code-loop-increment">;
+def UnreachableCodeFallthrough : DiagGroup<"unreachable-code-fallthrough">;
 def UnreachableCode : DiagGroup<"unreachable-code",
-[UnreachableCodeLoopIncrement]>;
+[UnreachableCodeLoopIncrement,
+ UnreachableCodeFallthrough]>;
 def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
 def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
 def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp

[PATCH] D107961: [clang-format] Distinguish K 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.

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] D108045: [clangd] Fix clangd crash when including a header

[Queen Dela Cruz via Phabricator via cfe-commits]

qdelacru updated this revision to Diff 366358.

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

https://reviews.llvm.org/D108045

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -546,6 +546,14 @@
   // Ensure they are dropeed when a patched preamble is used.
   EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
 }
+
+TEST(PreamblePatch, MacroLoc) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral Modified =
+  "#include \"header.h\"\n#define MACRO 12\nint num = MACRO;";
+  auto AST = createPatchedAST(Baseline, Modified);
+  ASSERT_TRUE(AST);
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3260,6 +3260,23 @@
 EXPECT_THAT(Results.Completions, UnorderedElementsAre(Labeled("BarExt")));
   }
 }
+
+TEST(CompletionTest, PreambleCodeComplete) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral ModifiedCC =
+  "#include \"header.h\"\n#define MACRO 12\nint num = MACRO; int num2 = 
M^";
+
+  Annotations Test(ModifiedCC);
+  auto BaselineTU = TestTU::withCode(Baseline);
+  auto ModifiedTU = TestTU::withCode(Test.code());
+
+  MockFS FS;
+  auto Inputs = ModifiedTU.inputs(FS);
+  auto Result = codeComplete(testPath(ModifiedTU.Filename), Test.point(),
+ BaselineTU.preamble().get(), Inputs, {});
+  EXPECT_THAT(Result.Completions, Not(testing::IsEmpty()));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -142,10 +142,11 @@
   unsigned DirectiveLine;
   // Full text that's representing the directive, including the `#`.
   std::string Text;
+  SourceLocation Loc;
 
   bool operator==(const TextualPPDirective ) const {
-return std::tie(DirectiveLine, Text) ==
-   std::tie(RHS.DirectiveLine, RHS.Text);
+return std::tie(DirectiveLine, Text, Loc) ==
+   std::tie(RHS.DirectiveLine, RHS.Text, RHS.Loc);
   }
 };
 
@@ -213,6 +214,7 @@
 TextualPPDirective  = TextualDirectives.back();
 
 const auto *MI = MD->getMacroInfo();
+TD.Loc = MI->getDefinitionLoc();
 TD.Text =
 spellDirective("#define ",
CharSourceRange::getTokenRange(
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1865,10 +1865,11 @@
   return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
  ? std::move(Flow).runWithoutSema(ParseInput.Contents, *Offset,
   *ParseInput.TFS)
- : std::move(Flow).run({FileName, *Offset, *Preamble,
-// We want to serve code completions with
-// low latency, so don't bother patching.
-/*PreamblePatch=*/llvm::None, ParseInput});
+ : std::move(Flow).run(
+   {FileName, *Offset, *Preamble,
+/*PreamblePatch=*/
+PreamblePatch::create(FileName, ParseInput, *Preamble),
+ParseInput});
 }
 
 SignatureHelp signatureHelp(PathRef FileName, Position Pos,


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -546,6 +546,14 @@
   // Ensure they are dropeed when a patched preamble is used.
   EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
 }
+
+TEST(PreamblePatch, MacroLoc) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral Modified =
+  "#include \"header.h\"\n#define MACRO 12\nint num = MACRO;";
+  auto AST = createPatchedAST(Baseline, Modified);
+  ASSERT_TRUE(AST);
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D107933#2944204 , @aaron.ballman 
wrote:

> In D107933#2944135 , @nathanchance 
> wrote:
>
>> In D107933#2942430 , @xbolva00 
>> wrote:
>>
>>> Yes, something like that, plus I think you want put 
>>> UnreachableCodeFallthrough into group UnreachableCode as well.
>>
>> So you would recommend adding it to `UnreachableCode` rather than 
>> `UnreachableCodeAggressive`?
>
> I would recommend adding it to `UnreachableCode` as I don't see this being a 
> particularly aggressive unreachable warning. FWIW, I would be opposed to 
> dropping the diagnostic entirely as the standard recommends diagnosing an 
> unreachable fallthrough statement 
> (https://eel.is/c++draft/dcl.attr.fallthrough#2.sentence-2 and the similar 
> wording in C2x 6.7.11.5p3.

(totally not to derail this, but... - I'm not sure that wording in the spec is 
especially informative/worth worrying about too much. If we have a warning we 
know isn't especially informative and is off-by-default, I'm not sure it makes 
too much difference whether we have it all. The spec doesn't have a lot of 
practice spec'ing warnings, which have a bunch of nuance that the spec doesn't 
usually have to deal with)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

___
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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 366356.
nickdesaulniers added a comment.

- remove unnecessary parens


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106030

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-error.c
  clang/test/CodeGen/attr-warning.c
  clang/test/Frontend/backend-attribute-error-warning-optimize.c
  clang/test/Frontend/backend-attribute-error-warning.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Misc/serialized-diags-driver.c
  clang/test/Sema/attr-error.c
  clang/test/Sema/attr-warning.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/test/CodeGen/X86/attr-dontcall.ll
  llvm/test/ThinLTO/X86/dontcall.ll

Index: llvm/test/ThinLTO/X86/dontcall.ll
===
--- /dev/null
+++ llvm/test/ThinLTO/X86/dontcall.ll
@@ -0,0 +1,33 @@
+; RUN: split-file %s %t
+; RUN: opt -module-summary %t/a.s -o %t/a.bc
+; RUN: opt -module-summary %t/b.s -o %t/b.bc
+; RUN: not llvm-lto2 run %t/a.bc %t/b.bc -o %t/out -save-temps 2>&1 \
+; RUN:   -r=%t/a.bc,callee,px \
+; RUN:   -r=%t/b.bc,callee,x  \
+; RUN:   -r=%t/b.bc,caller,px
+
+; TODO: As part of LTO, we check that types match, but *we don't yet check that
+; attributes match!!! What should happen if we remove "dontcall" from the
+; definition or declaration of @callee?
+
+; CHECK: call to callee marked "dontcall"
+
+;--- a.s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @callee() "dontcall" noinline {
+  ret i32 42
+}
+
+;--- b.s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i32 @callee() "dontcall"
+
+define i32 @caller() {
+entry:
+  %0 = call i32 @callee()
+  ret i32 %0
+}
Index: llvm/test/CodeGen/X86/attr-dontcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/attr-dontcall.ll
@@ -0,0 +1,11 @@
+; RUN: not llc -global-isel=0 -fast-isel=0 -stop-after=finalize-isel %s 2>&1 | FileCheck %s
+; RUN: not llc -global-isel=0 -fast-isel=1 -stop-after=finalize-isel %s 2>&1 | FileCheck %s
+; RUN: not llc -global-isel=1 -fast-isel=0 -stop-after=irtranslator %s 2>&1 | FileCheck %s
+
+declare void @foo() "dontcall"
+define void @bar() {
+  call void @foo()
+  ret void
+}
+
+; CHECK: error: call to foo marked "dontcall"
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -401,3 +401,7 @@
 
 void OptimizationRemarkAnalysisFPCommute::anchor() {}
 void OptimizationRemarkAnalysisAliasing::anchor() {}
+
+void DiagnosticInfoDontCall::print(DiagnosticPrinter ) const {
+  DP << "call to " << getFunctionName() << " marked \"dontcall\"";
+}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -69,6 +69,7 @@
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/InlineAsm.h"
@@ -7950,6 +7951,15 @@
   }
 
   if (Function *F = I.getCalledFunction()) {
+if (F->hasFnAttribute("dontcall")) {
+  unsigned LocCookie = 0;
+  if (MDNode *MD = I.getMetadata("srcloc"))
+LocCookie =
+mdconst::extract(MD->getOperand(0))->getZExtValue();
+  DiagnosticInfoDontCall D(F->getName(), LocCookie);
+  DAG.getContext()->diagnose(D);
+}
+
 if (F->isDeclaration()) {
   // Is this an LLVM intrinsic or a target-specific intrinsic?
   unsigned IID = F->getIntrinsicID();
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -75,6 +75,7 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
+#include 

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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:3368
+  Diag(Old->getLocation(), diag::note_previous_declaration);
+  New->dropAttr();
+}

Should be dropping `ErrorAttr`.



Comment at: clang/test/Sema/attr-error.c:26
+
+__attribute__((error("foo"), warning("foo"))) // expected-error {{'warning' 
and 'error' attributes are not compatible}}
+int

aaron.ballman wrote:
> Can you also add a test for:
> ```
> __attribute__((error("foo"))) int bad5(void); // expected-note {{conflicting 
> attribute is here}}
> __attribute__((warning("foo"))) int bad5(void); // expected-error {{'warning' 
> and 'error' attributes are not compatible}}
> ```
Ah, that exposes the case you discussed above:
> I think you need to follow the attribute merge pattern used in SemaDecl.cpp.

So I did a relatively larger refactor to support that. Doing so required me to 
make this an InheritableAttr. PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106030

___
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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 366355.
nickdesaulniers marked 4 inline comments as done.
nickdesaulniers added a comment.

- fix tests on windows, let diag eng format decl, drop correct attr, refactor 
to handle merging as InheritableAttr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106030

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-error.c
  clang/test/CodeGen/attr-warning.c
  clang/test/Frontend/backend-attribute-error-warning-optimize.c
  clang/test/Frontend/backend-attribute-error-warning.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Misc/serialized-diags-driver.c
  clang/test/Sema/attr-error.c
  clang/test/Sema/attr-warning.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/test/CodeGen/X86/attr-dontcall.ll
  llvm/test/ThinLTO/X86/dontcall.ll

Index: llvm/test/ThinLTO/X86/dontcall.ll
===
--- /dev/null
+++ llvm/test/ThinLTO/X86/dontcall.ll
@@ -0,0 +1,33 @@
+; RUN: split-file %s %t
+; RUN: opt -module-summary %t/a.s -o %t/a.bc
+; RUN: opt -module-summary %t/b.s -o %t/b.bc
+; RUN: not llvm-lto2 run %t/a.bc %t/b.bc -o %t/out -save-temps 2>&1 \
+; RUN:   -r=%t/a.bc,callee,px \
+; RUN:   -r=%t/b.bc,callee,x  \
+; RUN:   -r=%t/b.bc,caller,px
+
+; TODO: As part of LTO, we check that types match, but *we don't yet check that
+; attributes match!!! What should happen if we remove "dontcall" from the
+; definition or declaration of @callee?
+
+; CHECK: call to callee marked "dontcall"
+
+;--- a.s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @callee() "dontcall" noinline {
+  ret i32 42
+}
+
+;--- b.s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i32 @callee() "dontcall"
+
+define i32 @caller() {
+entry:
+  %0 = call i32 @callee()
+  ret i32 %0
+}
Index: llvm/test/CodeGen/X86/attr-dontcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/attr-dontcall.ll
@@ -0,0 +1,11 @@
+; RUN: not llc -global-isel=0 -fast-isel=0 -stop-after=finalize-isel %s 2>&1 | FileCheck %s
+; RUN: not llc -global-isel=0 -fast-isel=1 -stop-after=finalize-isel %s 2>&1 | FileCheck %s
+; RUN: not llc -global-isel=1 -fast-isel=0 -stop-after=irtranslator %s 2>&1 | FileCheck %s
+
+declare void @foo() "dontcall"
+define void @bar() {
+  call void @foo()
+  ret void
+}
+
+; CHECK: error: call to foo marked "dontcall"
Index: llvm/lib/IR/DiagnosticInfo.cpp
===
--- llvm/lib/IR/DiagnosticInfo.cpp
+++ llvm/lib/IR/DiagnosticInfo.cpp
@@ -401,3 +401,7 @@
 
 void OptimizationRemarkAnalysisFPCommute::anchor() {}
 void OptimizationRemarkAnalysisAliasing::anchor() {}
+
+void DiagnosticInfoDontCall::print(DiagnosticPrinter ) const {
+  DP << "call to " << getFunctionName() << " marked \"dontcall\"";
+}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -69,6 +69,7 @@
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/InlineAsm.h"
@@ -7950,6 +7951,15 @@
   }
 
   if (Function *F = I.getCalledFunction()) {
+if (F->hasFnAttribute("dontcall")) {
+  unsigned LocCookie = 0;
+  if (MDNode *MD = I.getMetadata("srcloc"))
+LocCookie =
+mdconst::extract(MD->getOperand(0))->getZExtValue();
+  DiagnosticInfoDontCall D(F->getName(), LocCookie);
+  DAG.getContext()->diagnose(D);
+}
+
 if (F->isDeclaration()) {
   // Is this an LLVM intrinsic or a target-specific intrinsic?
   unsigned IID = F->getIntrinsicID();
Index: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ 

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

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D77670#292 , @Hahnfeld wrote:

>> It's also time to bump the default GPU target to something that's supported 
>> by the CUDA versions we reasonably expect to see. That should probably be 
>> sm_35 as that's probably the oldest GPU platform that's still widely 
>> available (e.g. there are tons of them on Google cloud and AWS) and is still 
>> supported by all CUDA versions clang accepts.
>
> +1 for at least `sm_35` - that would match recent `nvcc`s, right?

NVCC in 11.4.1 defaults to sm_52 as the oldest non-deprecated GPU. I don't 
think it's time for clang to go that far as, unlike NVCC, we have to deal with 
older CUDA versions, too. For us the lowest common denominator for supported 
CUDA versions and GPUs hardware availability is sm_35.

I could also argue the other way around -- it may make sense to set default GPU 
to the most recent one supported by all CUDA versions. That will allow clang to 
compile larger subset of existing CUDA code (new GPUs support more 
builtins/features the code may rely on.

We could set target GPU to the most recent GPU variant supported by the CUDA 
version we've found. This, however will mean that the target will change from 
one system to another, depending on which CUDA version happens to be installed. 
I think that would be pushing it too far.

In any case, there's no universally good choice as we don't know which GPU the 
user needs the code for. If our choice is wrong, the app will not run. In 
practice users do need to specify both CUDA SDK path and the list of GPUs they 
want to compile for. The defaults, especially the default GPU target, is likely 
to be wrong more often than not.

> Yeah, the problem was that I didn't have better suggestions either when I 
> wrote the first comment. But maybe now: How about having a "past-the-latest" 
> value in the enum that Clang remembers if it detects a version more recent 
> than it knows about? Then we could have two warnings:
>
> - If we have a "past-the-latest" version, tell the user that Clang has no 
> clue about this version and we assume the `LATEST` version; things might 
> work, but no guarantees.
> - If we have a version that is greater than the latest supported version, 
> emit the current warning and say that support is "best-effort" (or something 
> along that line). In that case, both the detected version and the "assumed" 
> supported version should make sense to the user.

SGTM. I'll send the patch next week and we can discuss the details there.


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] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

We're still hitting the OOMs when building clang-repl with LTO even with 
`-DLLVM_PARALLEL_LINK_JOBS=32`. While we don't build this target explicitly in 
our toolchain, it is built when running tests via `stage2-check-clang`. Is 
there perhaps a simple cmake flag that allows us to not run clang-repl tests so 
we don't build it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

___
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.

In D77670#2944192 , @tra wrote:

> In D77670#2943753 , @Hahnfeld wrote:
>
>> @tra The split between `LATEST` and `LATEST_SUPPORTED` leads to very weird 
>> warning and error messages:
>
> Agreed, it's far from ideal. There's also more than one issue involved.

Unfortunately, yes...

>> clang-14: warning: unknown CUDA version: cuda.h: CUDA_VERSION=11040.; 
>> assuming the latest supported version 10.1 [-Wunknown-cuda-version]
>
> The good news is that we've grown support for enough clang builtins and PTX 
> instructions to bump the "latest supported" to ~CUDA-11.3 or, maybe, even 
> 11.4.  At least, clang  should be able to compile all CUDA headers in those 
> versions.
> This should reduce the noise.

Great!

>> 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
>
> It's also time to bump the default GPU target to something that's supported 
> by the CUDA versions we reasonably expect to see. That should probably be 
> sm_35 as that's probably the oldest GPU platform that's still widely 
> available (e.g. there are tons of them on Google cloud and AWS) and is still 
> supported by all CUDA versions clang accepts.

+1 for at least `sm_35` - that would match recent `nvcc`s, right?

>> 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'
>
> Perhaps it's time to start considering decommisioning sm_20 support in clang 
> and NVPTX. nvcc has done that long ago and is already on the way to dropping 
> sm_3x, too. sm_30 is no longer supported and sm_35 has been deprecated and is 
> expected be gone in the next CUDA release.

+1 - given that Clang 13.x just branched, now may be an ideal moment to make 
this cut.

>> 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)
>
> I'm open to suggestions. This was the least bad compromise I managed to come 
> up with.
>
> We could report the actually detected version, instead of the 'latest' 
> version clang knows about. Or not report it at all as it's not particularly 
> helpful for the end user. That would mitigate one source of confusion.
>
> As for the `latest supported`, I think we may still want to have it in some 
> form. Clang has to deal with version-specific CUDA quirks, so a CUDA version 
> outside of the range that clang is known to work with puts the user in 
> uncharted waters. E.g. until recently clang worked well enough with 
> CUDA-11.3, but only if you were compiling for the older GPUs. Attempts to 
> compile some headers for sm_80 would fail and that *was* confusing to users 
> who ran into that when the warning was disabled.

Yeah, the problem was that I didn't have better suggestions either when I wrote 
the first comment. But maybe now: How about having a "past-the-latest" value in 
the enum that Clang remembers if it detects a version more recent than it knows 
about? Then we could have two warnings:

- If we have a "past-the-latest" version, tell the user that Clang has no clue 
about this version and we assume the `LATEST` version; things might work, but 
no guarantees.
- If we have a version that is greater than the latest supported version, emit 
the current warning and say that support is "best-effort" (or something along 
that line). In that case, both the detected version and the "assumed" supported 
version should make sense to the user.


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] 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.

In D108003#2944413 , @Quuxplusone 
wrote:

> FWIW, all three of @nathanchance's detected lines look like good true 
> positives to me: even if they're not //bugs//, they all look like places the 
> programmer //meant// to write `&&` and only wrote `&` by accident. The third 
> one might even be a bug bug, since it's doing essentially `(bounds-check 
> offset_1) & (pointer-math-on offset_1)`.

Agreed, I do plan to send patches to fix these up. I will test the warning 
against a wider range of code later to help evaluate how noisy it is and look 
for false positives.


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

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

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

Quuxplusone added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7428-7430
+def warn_bitwise_and_bool : Warning<
+  "bitwise and of boolean expressions; did you mean logical and?">,
+  InGroup;

xbolva00 wrote:
> Quuxplusone wrote:
> > I suggest that the name and wording of this diagnostic should match 
> > `warn_logical_instead_of_bitwise`, currently `"use of logical '%0' with 
> > constant operand"`. So:
> > ```
> > def warn_bitwise_instead_of_logical : Warning<
> >   "use of bitwise '%0' with boolean operand">,
> > ```
> > This neatly sidesteps the problem of what to call the `&` operator: I was 
> > not thrilled with the phrase `bitwise and of`, but have no problem with 
> > `use of bitwise '&'`.
> I see the point but then I will not be able to provide -Wbool-operation-and 
> flag to enable/disable this warning.
> 
> For example I know that Google prefers a new flag for every new warning so 
> they dont have to disable eg. -Wbool-operation, but just this new warning 
> while there are working on fixes for new warnings.
> 
> @hans what do you think?
> 
> 
I don't understand your comment. My guess is that you didn't notice that 
`-Wbitwise-instead-of-logical` would still be a different flag from 
`-Wlogical-instead-of-bitwise`. I'm not suggesting putting them under the 
//same// flag; just making the newly added flag a little more consistent and 
(heh) logical, based on what's already there.


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

[PATCH] D107921: [Modules] Fix bug where header resolution in modules doesn't work when compiling with relative paths.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 366344.
akhuang added a comment.

undo previous change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107921

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -819,9 +819,19 @@
   bool IncluderIsSystemHeader =
   Includer ? getFileInfo(Includer).DirInfo != SrcMgr::C_User :
   BuildSystemModule;
-  if (Optional FE = getFileAndSuggestModule(
-  TmpDir, IncludeLoc, IncluderAndDir.second, 
IncluderIsSystemHeader,
-  RequestingModule, SuggestedModule)) {
+
+  Optional FE = getFileAndSuggestModule(
+  TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+  RequestingModule, SuggestedModule);
+
+  // If this is a system header, we should also search from the current
+  // working directory and not the directory of the module map.
+  if (!FE && IncluderIsSystemHeader)
+FE = getFileAndSuggestModule(
+Filename, IncludeLoc, IncluderAndDir.second, 
IncluderIsSystemHeader,
+RequestingModule, SuggestedModule);
+
+  if (FE) {
 if (!Includer) {
   assert(First && "only first includer can have no file");
   return FE;


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -819,9 +819,19 @@
   bool IncluderIsSystemHeader =
   Includer ? getFileInfo(Includer).DirInfo != SrcMgr::C_User :
   BuildSystemModule;
-  if (Optional FE = getFileAndSuggestModule(
-  TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
-  RequestingModule, SuggestedModule)) {
+
+  Optional FE = getFileAndSuggestModule(
+  TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+  RequestingModule, SuggestedModule);
+
+  // If this is a system header, we should also search from the current
+  // working directory and not the directory of the module map.
+  if (!FE && IncluderIsSystemHeader)
+FE = getFileAndSuggestModule(
+Filename, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+RequestingModule, SuggestedModule);
+
+  if (FE) {
 if (!Includer) {
   assert(First && "only first includer can have no file");
   return FE;
___
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

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

Quuxplusone added a comment.

FWIW, all three of @nathanchance's detected lines look like good true positives 
to me: even if they're not //bugs//, they all look like places the programmer 
//meant// to write `&&` and only wrote `&` by accident. The third one might 
even be a bug bug, since it's doing essentially `(bounds-check offset_1) & 
(pointer-math-on offset_1)`.
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

Data point: I've run this patch over my employer's medium-sized mostly-modern 
codebase and found no (true or false) positives at all.


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

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

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:17
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"

HazardyKnusperkeks wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > Why is this not needed anymore?
> > It was added in [[ https://reviews.llvm.org/D107950 | D107950 ]] but not 
> > needed even there.
> I would put that in a different commit, it has nothing to do with what is 
> changed here. And the revert is the example why I would put that in different 
> commits. :)
> 
> But I will not stand in the way to land this.
I included the test case here though. It's related to the bug I had in previous 
diffs.


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 C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 366342.
owenpan added a comment.
This revision is now accepted and ready to land.

Fix the assertion failure and add more test cases.


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,20 @@
"  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;\n"
+   "Bar h(a, Bar) final;",
+   Style);
+  verifyFormat("int\n"
+   "f(a)",
+   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 C (aka C78).
+static bool isC78Type(const FormatToken ) {
+  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 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 C function definition has a 
parameter
   // declaration.
@@ -1377,14 +1382,22 @@
 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 unsigned Position = Tokens->getPosition() + 1;
+  if (Position == AllTokens.size())
+break;
+  assert(Position < AllTokens.size());
+  const FormatToken *Next = AllTokens[Position];
+  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,20 @@
"  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;\n"
+   "Bar h(a, Bar) final;",
+   Style);
+  verifyFormat("int\n"
+   "f(a)",
+   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 

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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:17
 #include "FormatToken.h"
-#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/STLExtras.h"

owenpan wrote:
> HazardyKnusperkeks wrote:
> > Why is this not needed anymore?
> It was added in [[ https://reviews.llvm.org/D107950 | D107950 ]] but not 
> needed even there.
I would put that in a different commit, it has nothing to do with what is 
changed here. And the revert is the example why I would put that in different 
commits. :)

But I will not stand in the way to land this.


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 C function definition and attribute

[Owen Pan via Phabricator via cfe-commits]

owenpan 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))

MyDeveloperDay wrote:
> Maybe?
Yep!



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

MyDeveloperDay wrote:
> 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;
> ```
Sure. Will also add a test case for the assertion failure.


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] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

Please restore opencl test.




Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:622
+return OptimizationRemark(DEBUG_TYPE, "Passed", AI->getFunction())
+   << "A compare and swap loop was generated for an "
+   << AI->getOperationName(AI->getOperation()) << " operation at "

gandhi21299 wrote:
> arsenm wrote:
> > Missing word atomic?
> Its already part of the OperationName
Matt is right, missing "atomic" word.


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] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@rsmith: I modified the script to support dxx: dup P - let me know if you 
think that's a good solution


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

[clang] 8e9ffa1 - [NFC] Cleanup callers of AttributeList::hasAttributes()

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2021-08-13T12:16:52-07:00
New Revision: 8e9ffa1dc6988367c8e3d688044859c5aa7cf485

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

LOG: [NFC] Cleanup callers of AttributeList::hasAttributes()

AttributeList::hasAttributes() is confusing, use clearer methods like
hasFnAttrs().

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/IR/Attributes.h
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/Verifier.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index fa8312d30ad2..ebd3cc7d7dcc 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3613,7 +3613,7 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
   assert(F->getName() == MangledName && "name was uniqued!");
   if (D)
 SetFunctionAttributes(GD, F, IsIncompleteFunction, IsThunk);
-  if (ExtraAttrs.hasAttributes(llvm::AttributeList::FunctionIndex)) {
+  if (ExtraAttrs.hasFnAttrs()) {
 llvm::AttrBuilder B(ExtraAttrs, llvm::AttributeList::FunctionIndex);
 F->addAttributes(llvm::AttributeList::FunctionIndex, B);
   }

diff  --git a/llvm/include/llvm/IR/Attributes.h 
b/llvm/include/llvm/IR/Attributes.h
index dacab3938f55..08220b1d6413 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -657,17 +657,18 @@ class AttributeList {
 return hasAttribute(ReturnIndex, Kind);
   }
 
-  /// Return true if attributes exists for the return value.
+  /// Return true if attributes exist for the return value.
   bool hasRetAttrs() const { return hasAttributes(ReturnIndex); }
 
-  /// Equivalent to hasAttribute(AttributeList::FunctionIndex, Kind) but
-  /// may be faster.
+  /// Return true if the attribute exists for the function.
   bool hasFnAttr(Attribute::AttrKind Kind) const;
 
-  /// Equivalent to hasAttribute(AttributeList::FunctionIndex, Kind) but
-  /// may be faster.
+  /// Return true if the attribute exists for the function.
   bool hasFnAttr(StringRef Kind) const;
 
+  /// Return true the attributes exist for the function.
+  bool hasFnAttrs() const { return hasAttributes(FunctionIndex); }
+
   /// Return true if the specified attribute is set for at least one
   /// parameter or for the return value. If Index is not nullptr, the index
   /// of a parameter with the specified attribute is provided.

diff  --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index b3c11133e830..6ad28e9099ea 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -3682,7 +3682,7 @@ void AssemblyWriter::printFunction(const Function *F) {
 Out << "; Materializable\n";
 
   const AttributeList  = F->getAttributes();
-  if (Attrs.hasAttributes(AttributeList::FunctionIndex)) {
+  if (Attrs.hasFnAttrs()) {
 AttributeSet AS = Attrs.getFnAttrs();
 std::string AttrStr;
 
@@ -3720,7 +3720,7 @@ void AssemblyWriter::printFunction(const Function *F) {
   }
 
   FunctionType *FT = F->getFunctionType();
-  if (Attrs.hasAttributes(AttributeList::ReturnIndex))
+  if (Attrs.hasRetAttrs())
 Out << Attrs.getAsString(AttributeList::ReturnIndex) << ' ';
   TypePrinter.print(F->getReturnType(), Out);
   Out << ' ';
@@ -3769,7 +3769,7 @@ void AssemblyWriter::printFunction(const Function *F) {
   if (F->getAddressSpace() != 0 || !Mod ||
   Mod->getDataLayout().getProgramAddressSpace() != 0)
 Out << " addrspace(" << F->getAddressSpace() << ")";
-  if (Attrs.hasAttributes(AttributeList::FunctionIndex))
+  if (Attrs.hasFnAttrs())
 Out << " #" << Machine.getAttributeGroupSlot(Attrs.getFnAttrs());
   if (F->hasSection()) {
 Out << " section \"";
@@ -4126,7 +4126,7 @@ void AssemblyWriter::printInstruction(const Instruction 
) {
 Type *RetTy = FTy->getReturnType();
 const AttributeList  = CI->getAttributes();
 
-if (PAL.hasAttributes(AttributeList::ReturnIndex))
+if (PAL.hasRetAttrs())
   Out << ' ' << PAL.getAsString(AttributeList::ReturnIndex);
 
 // Only print addrspace(N) if necessary:
@@ -4155,7 +4155,7 @@ void AssemblyWriter::printInstruction(const Instruction 
) {
   Out << ", ...";
 
 Out << ')';
-if (PAL.hasAttributes(AttributeList::FunctionIndex))
+if (PAL.hasFnAttrs())
   Out << " #" << Machine.getAttributeGroupSlot(PAL.getFnAttrs());
 
 writeOperandBundles(CI);
@@ -4171,7 +4171,7 @@ void AssemblyWriter::printInstruction(const Instruction 
) {
   PrintCallingConv(II->getCallingConv(), Out);
 }
 
-if (PAL.hasAttributes(AttributeList::ReturnIndex))
+if (PAL.hasRetAttrs())
   Out << ' ' << PAL.getAsString(AttributeList::ReturnIndex);
 
 // Only print addrspace(N) if necessary:
@@ -4193,7 +4193,7 @@ void 

[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

While manually editing `ordered_codegen.cpp` gives nicer results, re-running 
`update_cc_test_checks.py` would be less work. Your manual changes would be 
also be dropped the next time somebody runs update_cc_test_checks.py and 
commits.

The code seems derived from @fghanim single/master/etc implementation, would be 
nice if they could have a look.

The non-OMPBuilder code emits an outlined `__captured_stmt`, but not with 
OMPBuilder enabled. I assume this is due to the missing `finatlize` call.




Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:5335
+  llvm::BasicBlock ) {
+// TODO: The ordered directive with simd clause.
+

Also, out and `assert`/`llvm_unreachable` here?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5323-5325
+Range = AssertSuccess(Actions.BuildBinOp(
+nullptr, {}, BO_Add, Range,
+Actions.ActOnIntegerConstant(SourceLocation(), 1).get()));

Haven't really understood why this is necessary, but this new version LGTM.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2154
+  Builder.restoreIP(
+  OMPBuilder.createOrdered(Builder, BodyGenCB, FiniCB, false));
+

Did you intend to pass IsThreads=true. Currently it is failing because no 
`__kmpc_ordered` is emitted.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2156-2157
+
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_EQ(EntryBBTI, nullptr);
+

Consider emitting a terminator, call `finalize()` and `verifyModule`.


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

https://reviews.llvm.org/D107430

___
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

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

The new attribute should be very rare, and not something that a typical end 
user should even know about.
That's why I'd prefer if the name reflected how the attribute affects code 
generation rather than user-visible behavior: disable_sanitizer_instrumentation 
sounds perfect to me.


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] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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.
+  }];

melver wrote:
> aaron.ballman wrote:
> > glider wrote:
> > > 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.
> > Thank you for the information!
> > 
> > Having two attributes with basically the same name to perform this 
> > functionality is confusing because users (understandably) will reach for 
> > the succinctly named one and make assumptions about what it does from the 
> > name.
> > 
> > One possibility would be to change `no_sanitize` to take an additional 
> > argument, as in: `__attribute__((no_sanitize(fully_disable, "thread")))`. 
> > Perhaps another solution would be to give the proposed attribute a more 
> > distinct name, like `disable_sanitizer_instrumentation`, 
> > `sanitizer_instrumentation_disabled`, or something else.
> Last I looked at `no_sanitize`, it's quite awkward that it is an attribute 
> that accepts arguments, because it makes it very hard to query for existence 
> of attribute additions/changes with `__has_attribute()`. Given this new 
> attribute is meant to be semantically quite different, the cleaner and less 
> intrusive way with that in mind is to create a new attribute. Unless of 
> course there's a nice way to make `__has_attribute()` work.
> 
> Here's another suggestion for name, which actually makes the difference 
> between `no_sanitize` and the new one obvious: 
> `no_sanitize_any_permit_false_positives`
> 
> At least it would semantically tell a user what might happen, which in turn 
> would hopefully make them avoid this attribute (also because it's hard enough 
> to type) unless they are absolutely sure.
> Given this new attribute is meant to be semantically quite different, the 
> cleaner and less intrusive way with that in mind is to create a new 
> attribute. Unless of course there's a nice way to make __has_attribute() work.

That sounds like good rationale for a separate attribute.

> Here's another suggestion for name, which actually makes the difference 
> between no_sanitize and the new one obvious: 
> no_sanitize_any_permit_false_positives
>
> At least it would semantically tell a user what might happen, which in turn 
> would hopefully make them avoid this attribute (also because it's hard enough 
> to type) unless they are absolutely sure.

That would certainly solve my concerns! Do you envision this being used far 
less often than `no_sanitize`? (That's my intuition, so I'm just 
double-checking that this isn't expected to be a popular replacement or 
something where the long name may be really undesirable.)


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] D107893: [clang] [MinGW] Consider the per-target libc++ include directory too

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

zero9178 added a comment.

In D107893#2944193 , @mstorsjo wrote:

> In D107893#2943679 , @zero9178 
> wrote:
>
>> 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.
>
> I presume this should be pretty trivial to backport yes - can you file a bug 
> about it in bugzilla, CC @tstellar and mark it as blocking the 13.0.0 release 
> bug?
>
> In which way was this a regression - I guess clang didn't change here - did 
> libcxx start installing the `__config_site` file differently during this 
> release?

Since this release libc++ splits what was previously the `__config` header into 
`__config_site` and `__config`, where `__config_site` contains all the defines 
set at configure time by CMake and gets included by `__config`. Without this 
patch the include path to `__config_site`s directory is missing in per target 
builds and therefore including any file of libc++ causes a compilation error.


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] D107893: [clang] [MinGW] Consider the per-target libc++ include directory too

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D107893#2944193 , @mstorsjo wrote:

> In D107893#2943679 , @zero9178 
> wrote:
>
>> 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.
>
> I presume this should be pretty trivial to backport yes - can you file a bug 
> about it in bugzilla, CC @tstellar and mark it as blocking the 13.0.0 release 
> bug?
>
> In which way was this a regression - I guess clang didn't change here - did 
> libcxx start installing the `__config_site` file differently during this 
> release?

Oh, also, do note that this only fixes the issue for the MinGW driver - this 
needs to be fixed separately for all drivers it seems (or better - refactor 
them so they'd use one shared codepath for libc++ includes, fixing it all in 
one place).


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] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/www/cxx_dr_status.html:3
   "http://www.w3.org/TR/html4/strict.dtd;>
 
 

aaron.ballman wrote:
> You might have missed this at the top of the file -- this one is a generated 
> file, and that's why I was trying to figure out how to best handle the 
> markings in the test files. `make_cxx_dr_status` basically looks for those 
> `dr` comments to generate this content.
I did, completely.
As we talked, I was considering hacking the script but I think the simplest 
solution might be to mark them `na`.
I'll wait for @rsmith 's opinion!


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] D107933: [clang] Expose unreachable fallthrough annotation warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D107933#2944135 , @nathanchance 
wrote:

> In D107933#2942430 , @xbolva00 
> wrote:
>
>> Yes, something like that, plus I think you want put 
>> UnreachableCodeFallthrough into group UnreachableCode as well.
>
> So you would recommend adding it to `UnreachableCode` rather than 
> `UnreachableCodeAggressive`?

I would recommend adding it to `UnreachableCode` as I don't see this being a 
particularly aggressive unreachable warning. FWIW, I would be opposed to 
dropping the diagnostic entirely as the standard recommends diagnosing an 
unreachable fallthrough statement 
(https://eel.is/c++draft/dcl.attr.fallthrough#2.sentence-2 and the similar 
wording in C2x 6.7.11.5p3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

___
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

[Marco Elver via Phabricator via cfe-commits]

melver 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:
> glider wrote:
> > 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.
> Thank you for the information!
> 
> Having two attributes with basically the same name to perform this 
> functionality is confusing because users (understandably) will reach for the 
> succinctly named one and make assumptions about what it does from the name.
> 
> One possibility would be to change `no_sanitize` to take an additional 
> argument, as in: `__attribute__((no_sanitize(fully_disable, "thread")))`. 
> Perhaps another solution would be to give the proposed attribute a more 
> distinct name, like `disable_sanitizer_instrumentation`, 
> `sanitizer_instrumentation_disabled`, or something else.
Last I looked at `no_sanitize`, it's quite awkward that it is an attribute that 
accepts arguments, because it makes it very hard to query for existence of 
attribute additions/changes with `__has_attribute()`. Given this new attribute 
is meant to be semantically quite different, the cleaner and less intrusive way 
with that in mind is to create a new attribute. Unless of course there's a nice 
way to make `__has_attribute()` work.

Here's another suggestion for name, which actually makes the difference between 
`no_sanitize` and the new one obvious: `no_sanitize_any_permit_false_positives`

At least it would semantically tell a user what might happen, which in turn 
would hopefully make them avoid this attribute (also because it's hard enough 
to type) unless they are absolutely sure.


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] D107893: [clang] [MinGW] Consider the per-target libc++ include directory too

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a subscriber: tstellar.
mstorsjo added a comment.

In D107893#2943679 , @zero9178 wrote:

> 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.

I presume this should be pretty trivial to backport yes - can you file a bug 
about it in bugzilla, CC @tstellar and mark it as blocking the 13.0.0 release 
bug?

In which way was this a regression - I guess clang didn't change here - did 
libcxx start installing the `__config_site` file differently during this 
release?


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] D77670: [CUDA] Add partial support for recent CUDA versions.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D77670#2943753 , @Hahnfeld wrote:

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

Agreed, it's far from ideal. There's also more than one issue involved.

> clang-14: warning: unknown CUDA version: cuda.h: CUDA_VERSION=11040.; 
> assuming the latest supported version 10.1 [-Wunknown-cuda-version]

Thye good news is that we've grown support for enough clang builtins and PTX 
instructions to bump the "latest supported" to ~CUDA-11.3 or, maybe, even 11.4. 
 At least, clang  should be able to compile all CUDA headers in those versions.
This should reduce the noise.

> 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

It's also time to bump the default GPU target to something that's supported by 
the CUDA versions we reasonably expect to see. That should probably be sm_35 as 
that's probably the oldest GPU platform that's still widely available (e.g. 
there are tons of them on Google cloud and AWS) and is still supported by all 
CUDA versions clang accepts.

> 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'

Perhaps it's time to start considering decommisioning sm_20 support in clang 
and NVPTX. nvcc has done that long ago and is already on the way to dropping 
sm_3x, too. sm_30 is no longer supported and sm_35 has been deprecated and is 
expected be gone in the next CUDA release.

> 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)

I'm open to suggestions. This was the least bad compromise I managed to come up 
with.

We could report the actually detected version, instead of the 'latest' version 
clang knows about. Or not report it at all as it's not particularly helpful for 
the end user. That would mitigate one source of confusion.

As for the `latest supported`, I think we may still want to have it in some 
form. Clang has to deal with version-specific CUDA quirks, so a CUDA version 
outside of the range that clang is known to work with puts the user in 
uncharted waters. E.g. until recently clang worked well enough with CUDA-11.3, 
but only if you were compiling for the older GPUs. Attempts to compile some 
headers for sm_80 would fail and that *was* confusing to users who ran into 
that when the warning was disabled.


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] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

I ran this over Chrome and ran into a use case that looks legitimate. It seems 
like the pattern in LLVM where we want to run a bunch of transformations and 
get if any of them changed anything.

https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/api/audio/echo_canceller3_config.cc;drc=cbdbb8c1666fd08a094422905e73391706a05b03;l=111


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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, but going to wait a bit to land it in case @rsmith has opinions.


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] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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.
+  }];

glider wrote:
> 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.
Thank you for the information!

Having two attributes with basically the same name to perform this 
functionality is confusing because users (understandably) will reach for the 
succinctly named one and make assumptions about what it does from the name.

One possibility would be to change `no_sanitize` to take an additional 
argument, as in: `__attribute__((no_sanitize(fully_disable, "thread")))`. 
Perhaps another solution would be to give the proposed attribute a more 
distinct name, like `disable_sanitizer_instrumentation`, 
`sanitizer_instrumentation_disabled`, or something else.


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

[clang] 80ea2bb - [NFC] Rename AttributeList::getParam/Ret/FnAttributes() -> get*Attributes()

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2021-08-13T11:16:52-07:00
New Revision: 80ea2bb57450a65cc724565ecfc9971ad93a3f15

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

LOG: [NFC] Rename AttributeList::getParam/Ret/FnAttributes() -> get*Attributes()

This is more consistent with similar methods.

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/Analysis/TargetLibraryInfo.h
llvm/include/llvm/IR/Attributes.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Function.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
llvm/lib/Transforms/IPO/IROutliner.cpp
llvm/lib/Transforms/IPO/OpenMPOpt.cpp
llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
llvm/lib/Transforms/Utils/AssumeBundleBuilder.cpp
llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
llvm/lib/Transforms/Utils/CloneFunction.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
llvm/tools/bugpoint/CrashDebugger.cpp
llvm/unittests/IR/IRBuilderTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 13520861fe9b6..fa8312d30ad2d 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4749,7 +4749,7 @@ static void replaceUsesOfNonProtoConstant(llvm::Constant 
*old,
   }
 
   // Add any parameter attributes.
-  newArgAttrs.push_back(oldAttrs.getParamAttributes(argNo));
+  newArgAttrs.push_back(oldAttrs.getParamAttrs(argNo));
   argNo++;
 }
 if (dontTransform)
@@ -4777,9 +4777,9 @@ static void replaceUsesOfNonProtoConstant(llvm::Constant 
*old,
 
 if (!newCall->getType()->isVoidTy())
   newCall->takeName(callSite);
-newCall->setAttributes(llvm::AttributeList::get(
-newFn->getContext(), oldAttrs.getFnAttributes(),
-oldAttrs.getRetAttributes(), newArgAttrs));
+newCall->setAttributes(
+llvm::AttributeList::get(newFn->getContext(), oldAttrs.getFnAttrs(),
+ oldAttrs.getRetAttrs(), newArgAttrs));
 newCall->setCallingConv(callSite->getCallingConv());
 
 // Finally, remove the old call, replacing any uses with the new one.

diff  --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.h 
b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
index 22bfeda0efd0d..c27e109e8687b 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -238,7 +238,7 @@ class TargetLibraryInfo {
 else {
   // Disable individual libc/libm calls in TargetLibraryInfo.
   LibFunc LF;
-  AttributeSet FnAttrs = (*F)->getAttributes().getFnAttributes();
+  AttributeSet FnAttrs = (*F)->getAttributes().getFnAttrs();
   for (const Attribute  : FnAttrs) {
 if (!Attr.isStringAttribute())
   continue;

diff  --git a/llvm/include/llvm/IR/Attributes.h 
b/llvm/include/llvm/IR/Attributes.h
index 019fe45094c98..3b1cead212c85 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -620,13 +620,13 @@ class AttributeList {
 
   /// The attributes for the argument or parameter at the given index are
   /// returned.
-  AttributeSet getParamAttributes(unsigned ArgNo) const;
+  AttributeSet getParamAttrs(unsigned ArgNo) const;
 
   /// The attributes for the ret value are returned.
-  AttributeSet getRetAttributes() const;
+  AttributeSet getRetAttrs() const;
 
   /// The function attributes are returned.
-  AttributeSet getFnAttributes() const;
+  AttributeSet getFnAttrs() const;
 
   /// Return true if the attribute exists at the given index.
   bool hasAttribute(unsigned Index, Attribute::AttrKind Kind) const;

diff  --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 799cb03c8c8c5..156b46cc94534 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -140,7 +140,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) 

[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 366317.
cor3ntin added a comment.

Describe this change in the changelog.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106215

Files:
  clang/docs/ReleaseNotes.rst
  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 

[PATCH] D104975: Implement P1949

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/www/cxx_dr_status.html:3
   "http://www.w3.org/TR/html4/strict.dtd;>
 
 

You might have missed this at the top of the file -- this one is a generated 
file, and that's why I was trying to figure out how to best handle the markings 
in the test files. `make_cxx_dr_status` basically looks for those `dr` comments 
to generate this content.


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

Re: [PATCH] D106577: [clang] Define __STDC_ISO_10646__

[Corentin via cfe-commits]

On Fri, Aug 13, 2021 at 7:42 PM Aaron Ballman via Phabricator <
revi...@reviews.llvm.org> wrote:

> aaron.ballman added a comment.
>
> In D106577#2943837 , @jyknight
> wrote:
>
> > 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.
>
> Isn't that scenario basically the one we're in today where the compiler is
> unaware of what mappings the library provides?
>
> > 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!
>
> Does that imply that an implementation supporting that encoding can't
> define __STDC_ISO_10646__ because it doesn't meet the "has the same value
> as the short identifier" requirement?
>

FYI, there should be a revision of GB18030 this year that will not use the
PUA anymore.
In general the PUA is considered "not for interchange" so if you have a
system that interprets PUA codepoints differently at different points in
time you are outside of any guarantees provided by Unicode.
GB18030-2005 is a weird exception as in general the standard library should
never transcode to the PUA as this is not portable.

GB18030, despite having a 1-1 mapping to unicode has to be considered a
distinct character set from Unicode, as such, a system where wide literals
are GB18030 encoded should not define
__STDC_ISO_10646__


>
> @jyknight, are you on the WG14 reflectors btw? Would you like to carry on
> with this discussion over there (or would you like me to convey your
> viewpoints on your behalf)?
>
>
> 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] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

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

- eliminated irrelevant changes to this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  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
@@ -45,6 +45,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
@@ -182,6 +187,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
@@ -434,6 +444,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 Atomic instructions
 ; GCN-O1-OPTS-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
@@ -719,6 +734,11 @@
 ; GCN-O2-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O2-NEXT:FunctionPass Manager
 ; GCN-O2-NEXT:  Infer address spaces
+; GCN-O2-NEXT:  Dominator Tree Construction
+; GCN-O2-NEXT:  Natural Loop Information
+; GCN-O2-NEXT:  

[PATCH] D106891: [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:622
+return OptimizationRemark(DEBUG_TYPE, "Passed", AI->getFunction())
+   << "A compare and swap loop was generated for an "
+   << AI->getOperationName(AI->getOperation()) << " operation at "

arsenm wrote:
> Missing word atomic?
Its already part of the OperationName


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] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D107933#2942430 , @xbolva00 wrote:

> Yes, something like that, plus I think you want put 
> UnreachableCodeFallthrough into group UnreachableCode as well.

So you would recommend adding it to `UnreachableCode` rather than 
`UnreachableCodeAggressive`?

In D107933#2942432 , @dblaikie wrote:

> Probably still worth fixing the bug too? (though maybe not a priority once 
> it's moved out into -Wunreachable-code) - though the general fix, as 
> discussed on the bug (comment 18 and 19 about why this doesn't already 
> produce an unreachable-code warning), may require a significant amount of 
> work.

I guess not warning on fallthrough attributes that are preceded by an if 
statement with an integer constant would remove all problematic instances in 
the kernel I believe. I am just not sure how to put that into code :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

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

[clang] 92ce6db - [NFC] Rename AttributeList::hasFnAttribute() -> hasFnAttr()

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2021-08-13T11:09:18-07:00
New Revision: 92ce6db9ee7666a347fccf0f72ba3225b199d6d1

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

LOG: [NFC] Rename AttributeList::hasFnAttribute() -> hasFnAttr()

This is more consistent with similar methods.

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
llvm/include/llvm/IR/Attributes.h
llvm/include/llvm/IR/Function.h
llvm/include/llvm/IR/InstrTypes.h
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/lib/CodeGen/MachineVerifier.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
llvm/unittests/IR/AttributesTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 43be6755a0745..04ecfacf02c02 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5232,7 +5232,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
 CannotThrow = true;
   } else {
 // Otherwise, nounwind call sites will never throw.
-CannotThrow = Attrs.hasFnAttribute(llvm::Attribute::NoUnwind);
+CannotThrow = Attrs.hasFnAttr(llvm::Attribute::NoUnwind);
 
 if (auto *FPtr = dyn_cast(CalleePtr))
   if (FPtr->hasFnAttribute(llvm::Attribute::NoUnwind))

diff  --git a/llvm/include/llvm/IR/Attributes.h 
b/llvm/include/llvm/IR/Attributes.h
index d28a4b4de1bf7..019fe45094c98 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -654,11 +654,11 @@ class AttributeList {
 
   /// Equivalent to hasAttribute(AttributeList::FunctionIndex, Kind) but
   /// may be faster.
-  bool hasFnAttribute(Attribute::AttrKind Kind) const;
+  bool hasFnAttr(Attribute::AttrKind Kind) const;
 
   /// Equivalent to hasAttribute(AttributeList::FunctionIndex, Kind) but
   /// may be faster.
-  bool hasFnAttribute(StringRef Kind) const;
+  bool hasFnAttr(StringRef Kind) const;
 
   /// Return true if the specified attribute is set for at least one
   /// parameter or for the return value. If Index is not nullptr, the index

diff  --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index 6d98f53157d27..d1c8a231d45aa 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -353,12 +353,12 @@ class Function : public GlobalObject, public 
ilist_node {
 
   /// Return true if the function has the attribute.
   bool hasFnAttribute(Attribute::AttrKind Kind) const {
-return AttributeSets.hasFnAttribute(Kind);
+return AttributeSets.hasFnAttr(Kind);
   }
 
   /// Return true if the function has the attribute.
   bool hasFnAttribute(StringRef Kind) const {
-return AttributeSets.hasFnAttribute(Kind);
+return AttributeSets.hasFnAttr(Kind);
   }
 
   /// Return the attribute for the given attribute kind.

diff  --git a/llvm/include/llvm/IR/InstrTypes.h 
b/llvm/include/llvm/IR/InstrTypes.h
index ef2c279ed4552..aab51f113fe38 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -2258,7 +2258,7 @@ class CallBase : public Instruction {
   bool hasFnAttrOnCalledFunction(StringRef Kind) const;
 
   template  bool hasFnAttrImpl(AttrKind Kind) const {
-if (Attrs.hasFnAttribute(Kind))
+if (Attrs.hasFnAttr(Kind))
   return true;
 
 // Operand bundles override attributes on the called function, but don't

diff  --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp 
b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 0214f8901a46d..a85dd7553b086 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -479,7 +479,7 @@ static void computeFunctionSummary(
   F.hasFnAttribute(Attribute::NoRecurse), F.returnDoesNotAlias(),
   // FIXME: refactor this to use the same code that inliner is using.
   // Don't try to import functions with noinline attribute.
-  F.getAttributes().hasFnAttribute(Attribute::NoInline),
+  F.getAttributes().hasFnAttr(Attribute::NoInline),
   F.hasFnAttribute(Attribute::AlwaysInline)};
   std::vector ParamAccesses;
   if (auto *SSI = GetSSICallback(F))

diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp 
b/llvm/lib/CodeGen/MachineVerifier.cpp
index 7e3198af02cd6..2b980ecb0236a 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1392,7 +1392,7 @@ void 

[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 366316.
cor3ntin added a comment.

Rebasing


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++ -triple %ms_abi_triple -emit-llvm -fwchar-type=short 

[PATCH] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

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

Rebasing


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  : 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
@@ -1314,7 +1314,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}}
+// expected-error@13 {{allowed in 

[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'm okay with the changes, but I think we should call this out in the release 
notes as there's technically the potential for this to be a breaking change (in 
both C and C++).


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] D108045: [clangd] Fix clangd crash when including a header

[Queen Dela Cruz via Phabricator via cfe-commits]

qdelacru created this revision.
qdelacru added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
qdelacru requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/819

SourceLocation of macros change when a header file is included above it. This 
is not checked when creating a PreamblePatch, resulting in reusing previously 
built preamble with an incorrect source location for the macro in the example 
test case. 
This patch stores the SourceLocation in the struct TextualPPDirective so that 
it gets checked when comparing old vs new preambles.

Also creates a preamble patch for code completion parsing so that clangd does 
not crash when following the example test case with a large file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108045

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -546,6 +546,14 @@
   // Ensure they are dropeed when a patched preamble is used.
   EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
 }
+
+TEST(PreamblePatch, MacroLoc) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral Modified =
+  "#include \"header.h\"\n#define MACRO 12\nint num = MACRO;";
+  auto AST = createPatchedAST(Baseline, Modified);
+  ASSERT_TRUE(AST);
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3260,6 +3260,23 @@
 EXPECT_THAT(Results.Completions, UnorderedElementsAre(Labeled("BarExt")));
   }
 }
+
+TEST(CompletionTest, PreambleCodeComplete) {
+  llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+  llvm::StringLiteral ModifiedCC =
+  "#include \"header.h\"\n#define MACRO 12\nint num = MACRO; int num2 = 
M^";
+
+  Annotations Test(ModifiedCC);
+  auto BaselineTU = TestTU::withCode(Baseline);
+  auto ModifiedTU = TestTU::withCode(Test.code());
+
+  MockFS FS;
+  auto Inputs = ModifiedTU.inputs(FS);
+  auto result = codeComplete(testPath(ModifiedTU.Filename), Test.point(),
+ BaselineTU.preamble().get(), Inputs, {});
+  EXPECT_THAT(result.Completions, Not(testing::IsEmpty()));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -142,10 +142,11 @@
   unsigned DirectiveLine;
   // Full text that's representing the directive, including the `#`.
   std::string Text;
+  SourceLocation Loc;
 
   bool operator==(const TextualPPDirective ) const {
-return std::tie(DirectiveLine, Text) ==
-   std::tie(RHS.DirectiveLine, RHS.Text);
+return std::tie(DirectiveLine, Text, Loc) ==
+   std::tie(RHS.DirectiveLine, RHS.Text, RHS.Loc);
   }
 };
 
@@ -213,6 +214,7 @@
 TextualPPDirective  = TextualDirectives.back();
 
 const auto *MI = MD->getMacroInfo();
+TD.Loc = MI->getDefinitionLoc();
 TD.Text =
 spellDirective("#define ",
CharSourceRange::getTokenRange(
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1865,10 +1865,11 @@
   return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
  ? std::move(Flow).runWithoutSema(ParseInput.Contents, *Offset,
   *ParseInput.TFS)
- : std::move(Flow).run({FileName, *Offset, *Preamble,
-// We want to serve code completions with
-// low latency, so don't bother patching.
-/*PreamblePatch=*/llvm::None, ParseInput});
+ : std::move(Flow).run(
+   {FileName, *Offset, *Preamble,
+/*PreamblePatch=*/
+PreamblePatch::create(FileName, ParseInput, *Preamble),
+ParseInput});
 }
 
 SignatureHelp signatureHelp(PathRef FileName, Position Pos,


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D106577#2943837 , @jyknight wrote:

> 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.

Isn't that scenario basically the one we're in today where the compiler is 
unaware of what mappings the library provides?

> 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!

Does that imply that an implementation supporting that encoding can't define 
__STDC_ISO_10646__ because it doesn't meet the "has the same value as the short 
identifier" requirement?

@jyknight, are you on the WG14 reflectors btw? Would you like to carry on with 
this discussion over there (or would you like me to convey your viewpoints on 
your behalf)?


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] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Thanks!  I'll wait for the D106510  review 
before trying to land.


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] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

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

xbolva00 added a comment.

Thanks!

Well, for those cases, && instead of & looks like a better choice. WDYT?

Or is it a very noisy? Restrict more so both sides must have side effects?

Maybe we could suggest to use cast - (int)boolRHS - as a way how to silence 
this warning.. Or any better suggestion?


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

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

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:622
+return OptimizationRemark(DEBUG_TYPE, "Passed", AI->getFunction())
+   << "A compare and swap loop was generated for an "
+   << AI->getOperationName(AI->getOperation()) << " operation at "

Missing word atomic?


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 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

[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 

[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:  

[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 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 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
 ; 

[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 

[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
 ; 

[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

[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 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

[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

[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

[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

[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:  

[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] D107961: [clang-format] Distinguish K 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] D107961: [clang-format] Distinguish K 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 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] 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))
+  

[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 ::get(const 
AttributeCommonInfo ) {
 }
 
 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

[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), 

[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

[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] 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] 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] 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

[clang] 98eb348 - Revert "[clang-format] Distinguish K 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 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.kw_import, tok::kw_export);
 }
 
-// Checks whether a token is a type in K C (aka C78).
-static bool isC78Type(const FormatToken ) {
-  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 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 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] 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