[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173986.
void added a comment.

ImpCast.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastEx

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Zixuan Wu via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/vector.cpp:26
   float &fr1 = f1(ll16);
-  f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
-  f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
+  f1(c16e);
+  f1(ll16e);

hubert.reinterpretcast wrote:
> Check the return types here like with the two lines above.
I use pointer or reference return value to distinguish the candidate.


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Zixuan Wu via Phabricator via cfe-commits]

wuzish updated this revision to Diff 173981.
wuzish added a comment.

Use return type to distinguish which overload candidate is chosen because 
different candidate has different pointer return type which can not be 
converted implicitly without reporting error.


https://reviews.llvm.org/D53417

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Sema/altivec-generic-overload.c
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -17,14 +17,14 @@
   f0(ll16e);
 }
 
-int &f1(char16); // expected-note 2{{candidate function}}
-float &f1(longlong16); // expected-note 2{{candidate function}}
+int &f1(char16);
+float &f1(longlong16);
 
 void f1_test(char16 c16, longlong16 ll16, char16_e c16e, longlong16_e ll16e) {
   int &ir1 = f1(c16);
   float &fr1 = f1(ll16);
-  f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
-  f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
+  int &ir2 = f1(c16e);
+  float &fr2 = f1(ll16e);
 }
 
 void f2(char16_e); // expected-note{{no known conversion from 'longlong16_e' (vector of 2 'long long' values) to 'char16_e' (vector of 16 'char' values) for 1st argument}} \
Index: clang/test/Sema/altivec-generic-overload.c
===
--- /dev/null
+++ clang/test/Sema/altivec-generic-overload.c
@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only
+
+typedef signed char __v16sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v16uc __attribute__((__vector_size__(16)));
+typedef signed short __v8ss __attribute__((__vector_size__(16)));
+typedef unsigned short __v8us __attribute__((__vector_size__(16)));
+typedef signed int __v4si __attribute__((__vector_size__(16)));
+typedef unsigned int __v4ui __attribute__((__vector_size__(16)));
+typedef signed long long __v2sll __attribute__((__vector_size__(16)));
+typedef unsigned long long __v2ull __attribute__((__vector_size__(16)));
+typedef signed __int128 __v1slll __attribute__((__vector_size__(16)));
+typedef unsigned __int128 __v1ulll __attribute__((__vector_size__(16)));
+typedef float __v4f __attribute__((__vector_size__(16)));
+typedef double __v2d __attribute__((__vector_size__(16)));
+
+__v16sc *__attribute__((__overloadable__)) convert1(vector signed char);
+__v16uc *__attribute__((__overloadable__)) convert1(vector unsigned char);
+__v8ss *__attribute__((__overloadable__)) convert1(vector signed short);
+__v8us *__attribute__((__overloadable__)) convert1(vector unsigned short);
+__v4si *__attribute__((__overloadable__)) convert1(vector signed int);
+__v4ui *__attribute__((__overloadable__)) convert1(vector unsigned int);
+__v2sll *__attribute__((__overloadable__)) convert1(vector signed long long);
+__v2ull *__attribute__((__overloadable__)) convert1(vector unsigned long long);
+__v1slll *__attribute__((__overloadable__)) convert1(vector signed __int128);
+__v1ulll *__attribute__((__overloadable__)) convert1(vector unsigned __int128);
+__v4f *__attribute__((__overloadable__)) convert1(vector float);
+__v2d *__attribute__((__overloadable__)) convert1(vector double);
+void __attribute__((__overloadable__)) convert1(vector bool int);
+void __attribute__((__overloadable__)) convert1(vector pixel short);
+
+vector signed char *__attribute__((__overloadable__)) convert2(__v16sc);
+vector unsigned char *__attribute__((__overloadable__)) convert2(__v16uc);
+vector signed short *__attribute__((__overloadable__)) convert2(__v8ss);
+vector unsigned short *__attribute__((__overloadable__)) convert2(__v8us);
+vector signed int *__attribute__((__overloadable__)) convert2(__v4si);
+vector unsigned int *__attribute__((__overloadable__)) convert2(__v4ui);
+vector signed long long *__attribute__((__overloadable__)) convert2(__v2sll);
+vector unsigned long long *__attribute__((__overloadable__)) convert2(__v2ull);
+vector signed __int128 *__attribute__((__overloadable__)) convert2(__v1slll);
+vector unsigned __int128 *__attribute__((__overloadable__)) convert2(__v1ulll);
+vector float *__attribute__((__overloadable__)) convert2(__v4f);
+vector double *__attribute__((__overloadable__)) convert2(__v2d);
+
+void test() {
+  __v16sc gv1;
+  __v16uc gv2;
+  __v8ss gv3;
+  __v8us gv4;
+  __v4si gv5;
+  __v4ui gv6;
+  __v2sll gv7;
+  __v2ull gv8;
+  __v1slll gv9;
+  __v1ulll gv10;
+  __v4f gv11;
+  __v2d gv12;
+
+  vector signed char av1;
+  vector unsigned char av2;
+  vector signed short av3;
+  vector unsigned short av4;
+  vector signed int av5;
+  vector unsigned int av6;
+  vector signed long long av7;
+  vector unsigned long long av8;
+  vector signed __int128 av9;
+  vector unsigned __int128 av10;
+  vector float av11;
+  vector double av12;
+  vector bool int av13;
+  vector pixel short av1

[PATCH] D54496: [HIP] Fix device only compilation

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346828: [HIP] Fix device only compilation (authored by 
yaxunl, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D54496

Files:
  lib/Driver/Driver.cpp
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -157,6 +157,7 @@
 // HBIN-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
 // HBIN-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (host-[[T]])
 // HBIN-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (host-[[T]])
+// HBIN-NOT: device
 //
 // Test single gpu architecture up to the assemble phase in host-only
 // compilation mode.
@@ -172,6 +173,7 @@
 // HASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-[[T]])
 // HASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
 // HASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// HASM-NOT: device
 
 //
 // Test two gpu architectures with complete compilation in host-only
@@ -190,6 +192,7 @@
 // HBIN2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
 // HBIN2-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (host-[[T]])
 // HBIN2-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (host-[[T]])
+// HBIN2-NOT: device
 
 //
 // Test two gpu architectures up to the assemble phase in host-only
@@ -206,6 +209,7 @@
 // HASM2-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-[[T]])
 // HASM2-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
 // HASM2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// HASM2-NOT: device
 
 //
 // Test single gpu architecture with complete compilation in device-only
@@ -224,7 +228,7 @@
 // DBIN_NV-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
 // DBIN_NV-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (device-[[T]], [[ARCH]])
 // DBIN_NV-DAG: [[P5:[0-9]+]]: offload, "device-[[T]] (nvptx64-nvidia-cuda:[[ARCH]])" {[[P4]]}, object
-
+// DBIN-NOT: host
 //
 // Test single gpu architecture up to the assemble phase in device-only
 // compilation mode.
@@ -241,6 +245,7 @@
 // DASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
 // DASM_NV-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
 // DASM_NV-DAG: [[P4:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE:nvptx64-nvidia-cuda|amdgcn-amd-amdhsa]]:[[ARCH]])" {[[P3]]}, assembler
+// DASM-NOT: host
 
 //
 // Test two gpu architectures with complete compilation in device-only
@@ -265,7 +270,7 @@
 // DBIN2_NV-DAG: [[P9:[0-9]+]]: backend, {[[P8]]}, assembler, (device-[[T]], [[ARCH2]])
 // DBIN2_NV-DAG: [[P10:[0-9]+]]: assembler, {[[P9]]}, object, (device-[[T]], [[ARCH2]])
 // DBIN2_NV-DAG: [[P11:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH2]])" {[[P10]]}, object
-
+// DBIN2-NOT: host
 //
 // Test two gpu architectures up to the assemble phase in device-only
 // compilation mode.
@@ -288,3 +293,4 @@
 // DASM2-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (device-[[T]], [[ARCH2]])
 // DASM2_NV-DAG: [[P8:[0-9]+]]: backend, {[[P7]]}, assembler, (device-[[T]], [[ARCH2]])
 // DASM2_NV-DAG: [[P9:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH2]])" {[[P8]]}, assembler
+// DASM2-NOT: host
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2616,17 +2616,19 @@
 C.MakeAction(CudaDeviceActions,
 types::TY_HIP_FATBIN);
 
-DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
-   AssociatedOffloadKind);
-// Clear the fat binary, it is already a dependence to an host
-// action.
-CudaFatBinary = nullptr;
+if (!CompileDeviceOnly) {
+  DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
+ AssociatedOffloadKind);
+  // Clear the fat binary, it is already a dependence to an host
+  // action.
+  CudaFatBinary = nullptr;
+}
 
 // Remove the CUDA actions as they are already connected to an host
 // action or fat binary.
 CudaDeviceActions.clear();
 
-return ABRT_Success;
+return CompileDeviceOnly ? ABRT_Ignore_Host : ABRT_Success;
   } else if (CurPhase == phases::Link) {
 // Save CudaDeviceActions to DeviceLinkerInputs for each GPU subarch.
 // This happens to each device action originated from each input file.
@@ -3014,8 +3016,10 @@
 }
 
 // If we can use the bundler, replace the host action by the bundling one in
-// the resulting list. Otherwise, just append the device actions.
-if (CanUseBundler && !OffloadAL.empty()) {
+// the resulting list. Otherwise, just append the device actions. For
+// device only compilation, HostAction is a null po

r346828 - [HIP] Fix device only compilation

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Nov 13 20:47:31 2018
New Revision: 346828

URL: http://llvm.org/viewvc/llvm-project?rev=346828&view=rev
Log:
[HIP] Fix device only compilation

Fix a bug causing host code being compiled when --cude-device-only is set.

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/cuda-phases.cu

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=346828&r1=346827&r2=346828&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Nov 13 20:47:31 2018
@@ -2616,17 +2616,19 @@ class OffloadingActionBuilder final {
 C.MakeAction(CudaDeviceActions,
 types::TY_HIP_FATBIN);
 
-DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
-   AssociatedOffloadKind);
-// Clear the fat binary, it is already a dependence to an host
-// action.
-CudaFatBinary = nullptr;
+if (!CompileDeviceOnly) {
+  DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
+ AssociatedOffloadKind);
+  // Clear the fat binary, it is already a dependence to an host
+  // action.
+  CudaFatBinary = nullptr;
+}
 
 // Remove the CUDA actions as they are already connected to an host
 // action or fat binary.
 CudaDeviceActions.clear();
 
-return ABRT_Success;
+return CompileDeviceOnly ? ABRT_Ignore_Host : ABRT_Success;
   } else if (CurPhase == phases::Link) {
 // Save CudaDeviceActions to DeviceLinkerInputs for each GPU subarch.
 // This happens to each device action originated from each input file.
@@ -3014,8 +3016,10 @@ public:
 }
 
 // If we can use the bundler, replace the host action by the bundling one 
in
-// the resulting list. Otherwise, just append the device actions.
-if (CanUseBundler && !OffloadAL.empty()) {
+// the resulting list. Otherwise, just append the device actions. For
+// device only compilation, HostAction is a null pointer, therefore only do
+// this when HostAction is not a null pointer.
+if (CanUseBundler && HostAction && !OffloadAL.empty()) {
   // Add the host action to the list in order to create the bundling 
action.
   OffloadAL.push_back(HostAction);
 

Modified: cfe/trunk/test/Driver/cuda-phases.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cuda-phases.cu?rev=346828&r1=346827&r2=346828&view=diff
==
--- cfe/trunk/test/Driver/cuda-phases.cu (original)
+++ cfe/trunk/test/Driver/cuda-phases.cu Tue Nov 13 20:47:31 2018
@@ -157,6 +157,7 @@
 // HBIN-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
 // HBIN-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (host-[[T]])
 // HBIN-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (host-[[T]])
+// HBIN-NOT: device
 //
 // Test single gpu architecture up to the assemble phase in host-only
 // compilation mode.
@@ -172,6 +173,7 @@
 // HASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, 
(host-[[T]])
 // HASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
 // HASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// HASM-NOT: device
 
 //
 // Test two gpu architectures with complete compilation in host-only
@@ -190,6 +192,7 @@
 // HBIN2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
 // HBIN2-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (host-[[T]])
 // HBIN2-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (host-[[T]])
+// HBIN2-NOT: device
 
 //
 // Test two gpu architectures up to the assemble phase in host-only
@@ -206,6 +209,7 @@
 // HASM2-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, 
(host-[[T]])
 // HASM2-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
 // HASM2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// HASM2-NOT: device
 
 //
 // Test single gpu architecture with complete compilation in device-only
@@ -224,7 +228,7 @@
 // DBIN_NV-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], 
[[ARCH]])
 // DBIN_NV-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (device-[[T]], 
[[ARCH]])
 // DBIN_NV-DAG: [[P5:[0-9]+]]: offload, "device-[[T]] 
(nvptx64-nvidia-cuda:[[ARCH]])" {[[P4]]}, object
-
+// DBIN-NOT: host
 //
 // Test single gpu architecture up to the assemble phase in device-only
 // compilation mode.
@@ -241,6 +245,7 @@
 // DASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
 // DASM_NV-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], 
[[ARCH]])
 // DASM_NV-DAG: [[P4:[0-9]+]]: offload, "device-[[T]] 
([[TRIPLE:nvptx64-nvidia-cuda|amdgcn-amd-amdhsa]]:[[ARCH]])" {[[P3]]}, assembler
+// DASM-NO

[PATCH] D54505: [CMake] Include clang-apply-replacements in Fuchsia toolchain

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346827: [CMake] Include clang-apply-replacements in Fuchsia 
toolchain (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54505?vs=173969&id=173980#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54505

Files:
  cmake/caches/Fuchsia-stage2.cmake


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -156,6 +156,7 @@
   libclang
   lld
   LTO
+  clang-apply-replacements
   clang-format
   clang-headers
   clang-include-fixer


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -156,6 +156,7 @@
   libclang
   lld
   LTO
+  clang-apply-replacements
   clang-format
   clang-headers
   clang-include-fixer
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346827 - [CMake] Include clang-apply-replacements in Fuchsia toolchain

[Petr Hosek via cfe-commits]

Author: phosek
Date: Tue Nov 13 20:06:47 2018
New Revision: 346827

URL: http://llvm.org/viewvc/llvm-project?rev=346827&view=rev
Log:
[CMake] Include clang-apply-replacements in Fuchsia toolchain

This is needed for run-clang-tidy.py.

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

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=346827&r1=346826&r2=346827&view=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Tue Nov 13 20:06:47 2018
@@ -156,6 +156,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   libclang
   lld
   LTO
+  clang-apply-replacements
   clang-format
   clang-headers
   clang-include-fixer


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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173974.
void added a comment.

Fix overflow flag.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const Impl

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

I think this is ready now. PTAL.




Comment at: lib/CodeGen/CGBuiltin.cpp:1930-1931
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+  // At -O0, we don't perform inlining, so we don't need to delay the
+  // processing.
+  return RValue::get(ConstantInt::get(Int32Ty, 0));

rsmith wrote:
> Are there cases where a call to an `always_inline` function could change the 
> outcome?
Yes, but the result of bcp isn't guaranteed to be the same for optimized and 
unoptimized code. GCC's documentation says that it won't return 1 unless `-O` 
is used. Their code appears to back this up somewhat:

```
  switch (fcode)
{
case BUILT_IN_CONSTANT_P:
  {
tree val = fold_builtin_constant_p (arg0);

/* Gimplification will pull the CALL_EXPR for the builtin out of
   an if condition.  When not optimizing, we'll not CSE it back.
   To avoid link error types of regressions, return false now.  */
if (!val && !optimize)
  val = integer_zero_node;

return val;
  }
...
}
```

Our inlining is different of course, and I'm not sure when GCC performs their 
`always_inline` inlining. It may be before the above code is called. But I 
think we're within reasonable bounds with respect to the documentation.

(That said, GCC has a tendency to violate its own documentation. So caveat 
emptor, etc.)



Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+  Arg = ConstantExpr::Create(Context, Arg);
+

rsmith wrote:
> void wrote:
> > rsmith wrote:
> > > We should create this node as part of checking that the argument is an 
> > > ICE (in `SemaBuiltinConstantArg`).
> > It turns out that `SemaBuiltinConstantArg` isn't called for 
> > `__builtin_constant_p()`. Its argument type is `.`, which...I'm not sure 
> > what that means. It looks like a vararg, but that doesn't make sense for 
> > the builtin.
> > 
> > Should I change its signature in `Builtins.def`?
> It's also marked `t`, which means "signature is meaningless, uses custom type 
> checking".
> 
> The argument to `__builtin_constant_p` isn't required to be an ICE, though, 
> so we shouldn't be calling `SemaBuiltinConstantArg` for it / creating a 
> `ConstantExpr` wrapping it, as far as I can tell.
Right. I think I was looking at something else. I changed it.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173970.
void marked an inline comment as not done.
void added a comment.

Updated to address comments.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -52

[PATCH] D54505: [CMake] Include clang-apply-replacements in Fuchsia toolchain

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: mcgrathr, juliehockett.
Herald added subscribers: cfe-commits, mgorny.

This is needed for run-clang-tidy.py.


Repository:
  rC Clang

https://reviews.llvm.org/D54505

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -156,6 +156,7 @@
   libclang
   lld
   LTO
+  clang-apply-replacements
   clang-format
   clang-headers
   clang-include-fixer


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -156,6 +156,7 @@
   libclang
   lld
   LTO
+  clang-apply-replacements
   clang-format
   clang-headers
   clang-include-fixer
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

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

Quuxplusone added inline comments.



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

rjmccall wrote:
> Quuxplusone wrote:
> > rjmccall wrote:
> > > Quuxplusone wrote:
> > > > rjmccall wrote:
> > > > > Quuxplusone wrote:
> > > > > > @rjmccall wrote:
> > > > > > > trivial_abi permits annotated types to be passed and returned in 
> > > > > > > registers, which is ABI-breaking. Skimming the blog post, it 
> > > > > > > looks like trivially_relocatable does not permit this — it merely 
> > > > > > > signifies that destruction is a no-op after a move construction 
> > > > > > > or assignment.
> > > > > > 
> > > > > > Not necessarily a "no-op"; my canonical example is a 
> > > > > > CopyOnlyCXX03SharedPtr which increments a refcount on construction 
> > > > > > and decrements on destruction. But move-construction plus 
> > > > > > destruction should "balance out" and result in no observable side 
> > > > > > effects.
> > > > > > 
> > > > > > > This is usefully different in the design space, since it means 
> > > > > > > you can safely add the attribute retroactively to e.g. 
> > > > > > > std::unique_ptr, and other templates can then detect that 
> > > > > > > std::unique_ptr is trivially-relocatable and optimize themselves 
> > > > > > > to use memcpy or realloc or whatever it is that they want to do. 
> > > > > > > So in that sense trivial_abi is a *stronger* attribute, not a 
> > > > > > > *weaker* one: the property it determines ought to imply 
> > > > > > > trivially_relocatable.
> > > > > > 
> > > > > > `trivial_abi` is an "orthogonal" attribute: you can have 
> > > > > > `trivial_abi` types with non-trivial constructors and destructors, 
> > > > > > which can have observable side effects. For example,
> > > > > > ```
> > > > > > struct [[clang::trivial_abi]] DestructionAnnouncer {
> > > > > > ~DestructionAnnouncer() { puts("hello!"); }
> > > > > > };
> > > > > > ```
> > > > > > is `trivial_abi` (because of the annotation) yet not trivially 
> > > > > > relocatable, because its "move plus destroy" operation has 
> > > > > > observable side effects.
> > > > > > 
> > > > > > > The only interesting question in the language design that I know 
> > > > > > > of is what happens if you put the attribute on a template that's 
> > > > > > > instantiated to contain a sub-object that is definitely not 
> > > > > > > trivially relocatable / trivial-ABI. For trivial_abi, we decided 
> > > > > > > that the attribute is simply ignored — it implicitly only applies 
> > > > > > > to specializations where the attribute would be legal. I haven't 
> > > > > > > dug into the design enough to know what trivially_relocatable 
> > > > > > > decides in this situation, but the three basic options are:
> > > > > > >
> > > > > > > - the attribute always has effect and allows trivial relocation 
> > > > > > > regardless of the subobject types; this is obviously unsafe, so 
> > > > > > > it limits the safe applicability of the attribute to templates
> > > > > > > - the attribute is ignored, like trivial_abi is
> > > > > > > - the attribute is ill-formed, and you'll need to add a 
> > > > > > > [[trivially_relocatable(bool)]] version to support templates
> > > > > > 
> > > > > > What happens is basically the first thing you said, except that I 
> > > > > > disagree that it's "obviously unsafe." Right now, conditionally 
> > > > > > trivial relocation is possible via template metaprogramming; see 
> > > > > > the libcxx patch at e.g.
> > > > > > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > > > > > Since the attribute is an opt-in mechanism, it makes perfect sense 
> > > > > > to me that if you put it on a class (or class template), then it 
> > > > > > applies to the class, without any further sanity-checking by the 
> > > > > > compiler. The compiler has no reason to second-guess the programmer 
> > > > > > here.
> > > > > > 
> > > > > > However, there's one more interesting case. Suppose the programmer 
> > > > > > puts the attribute on a class that isn't relocatable at all! (For 
> > > > > > example, the union case @erichkeane mentioned, or a class type with 
> > > > > > a deleted destructor.) In that case, this patch *does* give an 
> > > > > > error... *unless* the class was produced by instantiating a 
> > > > > > template, in which case we *don't* give an error, because it's not 
> > > > > > the template-writer's fault.
> > > > > > https://p1144.godbolt.org/z/wSZPba
> > > > > > trivial_abi is an "orthogonal" attribute: you can have trivial_abi 
> > > > > > types with non-trivial constructors and destructors, which can have 
> > > > > > observable side effects. 
> > > > > 
> > > > > Let me cut this conversation short.  `trivial_abi` is not such an old 
> > > > > and widely-established attribute that we are una

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote:

> I'm not worried about the mangler being re-used for multiple declarations, 
> I'm worried about a global flag changing how we mangle all components of a 
> type when we only mean to change it at the top level.


Hmm, but don't we want it to affect all components? For example, consider 
something like:

  @interface I
  @end
  
  template  class C {};
  
  void f();
  void g() {
try {
  f();
} catch (C *) {
}
  }

I would say that we want the RTTI for `C *` to have the discriminator for 
`I`. It turns out my current patch doesn't actually do that; I guess there's a 
sub-mangler being constructed somewhere that's not inheriting the RTTI-ness. Or 
did you mean something else?


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D54503: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346822: [HeaderSearch] loadSubdirectoryModuleMaps should 
respect -working-directory (authored by arphaman, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54503?vs=173958&id=173968#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54503

Files:
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
  
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
  cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m


Index: 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
===
--- 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
+++ 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
@@ -0,0 +1,5 @@
+module ModuleInSubdir {
+header "h1.h"
+  export *
+}
+
Index: 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
===
--- 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
+++ 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
@@ -0,0 +1 @@
+int bar();
Index: cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
===
--- cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
+++ cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t \
+// RUN:-working-directory %S/Inputs \
+// RUN:-I subdirectory-module-maps-working-dir \
+// RUN:%s -Werror=implicit-function-declaration -Xclang -verify
+
+@import ModuleInSubdir;
+
+void foo() {
+  int x = bar();
+}
+
+// expected-no-diagnostics
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1638,8 +1638,10 @@
 return;
 
   std::error_code EC;
+  SmallString<128> Dir = SearchDir.getDir()->getName();
+  FileMgr.makeAbsolutePath(Dir);
   SmallString<128> DirNative;
-  llvm::sys::path::native(SearchDir.getDir()->getName(), DirNative);
+  llvm::sys::path::native(Dir, DirNative);
   llvm::vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {


Index: cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
===
--- cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
+++ cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
@@ -0,0 +1,5 @@
+module ModuleInSubdir {
+header "h1.h"
+  export *
+}
+
Index: cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
===
--- cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
+++ cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
@@ -0,0 +1 @@
+int bar();
Index: cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
===
--- cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
+++ cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t \
+// RUN:-working-directory %S/Inputs \
+// RUN:-I subdirectory-module-maps-working-dir \
+// RUN:%s -Werror=implicit-function-declaration -Xclang -verify
+
+@import ModuleInSubdir;
+
+void foo() {
+  int x = bar();
+}
+
+// expected-no-diagnostics
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -1638,8 +1638,10 @@
 return;
 
   std::error_code EC;
+  SmallString<128> Dir = SearchDir.getDir()->getName();
+  FileMgr.makeAbsolutePath(Dir);
   SmallString<128> DirNative;
-  llvm::sys::path::native(SearchDir.getDir()->getName(), DirNative);
+  llvm::sys::path::native(Dir, DirNative);
   llvm::vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {
___
cfe-commits mailing list
cfe

r346822 - [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Nov 13 17:08:03 2018
New Revision: 346822

URL: http://llvm.org/viewvc/llvm-project?rev=346822&view=rev
Log:
[HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

Include search paths can be relative paths. The loadSubdirectoryModuleMaps 
function
should account for that and respect the -working-directory parameter given to 
Clang.

rdar://46045849

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

Added:
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/

cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/

cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h

cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=346822&r1=346821&r2=346822&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Tue Nov 13 17:08:03 2018
@@ -1638,8 +1638,10 @@ void HeaderSearch::loadSubdirectoryModul
 return;
 
   std::error_code EC;
+  SmallString<128> Dir = SearchDir.getDir()->getName();
+  FileMgr.makeAbsolutePath(Dir);
   SmallString<128> DirNative;
-  llvm::sys::path::native(SearchDir.getDir()->getName(), DirNative);
+  llvm::sys::path::native(Dir, DirNative);
   llvm::vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {

Added: 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h?rev=346822&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
 Tue Nov 13 17:08:03 2018
@@ -0,0 +1 @@
+int bar();

Added: 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map?rev=346822&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
 Tue Nov 13 17:08:03 2018
@@ -0,0 +1,5 @@
+module ModuleInSubdir {
+header "h1.h"
+  export *
+}
+

Added: cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m?rev=346822&view=auto
==
--- cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m (added)
+++ cfe/trunk/test/Modules/subdirectory-module-maps-working-dir.m Tue Nov 13 
17:08:03 2018
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t \
+// RUN:-working-directory %S/Inputs \
+// RUN:-I subdirectory-module-maps-working-dir \
+// RUN:%s -Werror=implicit-function-declaration -Xclang -verify
+
+@import ModuleInSubdir;
+
+void foo() {
+  int x = bar();
+}
+
+// expected-no-diagnostics


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

[PATCH] D54466: [Analyzer] [WIP] Iterator Checkers - Use the base region of C++ Base Object Regions (recursively) for iterators stored in a region

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> I marked this patch as WIP because I could not create a test-case for it. 
> However in real projects this patch seems to reduce false positives 
> significantly.

False positives are hard to reduce via delta-debugging because they can be 
accidentally reduced into true positives, because the distinction between true 
positives and false positives cannot be tested automatically. But once you fix 
the false positive, `creduce` can be used to reduce them by writing down the 
condition as "the positive is there on the original clang but not on the 
modified clang". Strongly recommended :)

> Hmmm, shouldn't we add this to `MemRegion`'s interface instead?

I wouldn't insist, but this does indeed sound useful. I suggest 
`MemRegion::getMostDerivedObjectRegion()` or something like that.

> The member function `getBaseRegion()` of `MemRegion` does not work here 
> because it recursively retrieves the base region of multiple kinds of 
> `SubRegion`.

Also, ugh, that nomenclature: the base region of `CXXBaseObjectRegion` in fact 
represents the //derived// object.


Repository:
  rC Clang

https://reviews.llvm.org/D54466



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

[PATCH] D54503: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D54503



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm not worried about the mangler being re-used for multiple declarations, I'm 
worried about a global flag changing how we mangle all components of a type 
when we only mean to change it at the top level.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

@rnk pointed out on IRC that the MicrosoftCXXNameMangler is actually 
specifically designed to manage the mangling of only a single name, in which 
case adding state to it for handling RTTI seems like a natural approach. 
@rjmccall, what do you think? I think this is much cleaner than having to 
thread through the RTTI state to every individual method. The ForRTTI_t enum is 
modeled after the ForDefinition_t enum used in CGM, but I'm happy to switch to 
a more general struct (as you'd mentioned before) if you prefer.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai updated this revision to Diff 173965.
smeenai added a comment.

Stateful approach


Repository:
  rC Clang

https://reviews.llvm.org/D52674

Files:
  lib/AST/MicrosoftMangle.cpp
  test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm

Index: test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/msabi-objc-exceptions-gnustep.mm
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i686-windows-msvc -fobjc-runtime=gnustep-2.0 -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X86 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fobjc-runtime=gnustep-2.0 -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefix=X64 %s
+
+// Ensure we have the __ObjC::class discriminator in the RTTI and the RTTI name.
+// X86-DAG: @"??_R0PAU?$Class@Uobjc_object@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [36 x i8] c".PAU?$Class@Uobjc_object@@@__ObjC@@\00" }, comdat
+// X64-DAG: @"??_R0PEAU?$Class@Uobjc_object@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [37 x i8] c".PEAU?$Class@Uobjc_object@@@__ObjC@@\00" }, comdat
+
+@class I;
+// X86-DAG: @"??_R0PAU?$Class@UI@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [26 x i8] c".PAU?$Class@UI@@@__ObjC@@\00" }, comdat
+// X64-DAG: @"??_R0PEAU?$Class@UI@@@__ObjC@@@8" = linkonce_odr global %{{[^ ]+}} { i8** @"??_7type_info@@6B@", i8* null, [27 x i8] c".PEAU?$Class@UI@@@__ObjC@@\00" }, comdat
+
+void f();
+void g() {
+  @try {
+f();
+  } @catch (I *) {
+  } @catch (id) {
+  }
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -252,6 +252,11 @@
 /// MicrosoftCXXNameMangler - Manage the mangling of a single name for the
 /// Microsoft Visual C++ ABI.
 class MicrosoftCXXNameMangler {
+public:
+  enum QualifierMangleMode { QMM_Drop, QMM_Mangle, QMM_Escape, QMM_Result };
+  enum ForRTTI_t : bool { NotForRTTI = false, ForRTTI = true };
+
+private:
   MicrosoftMangleContextImpl &Context;
   raw_ostream &Out;
 
@@ -276,25 +281,35 @@
   // this check into mangleQualifiers().
   const bool PointersAre64Bit;
 
-public:
-  enum QualifierMangleMode { QMM_Drop, QMM_Mangle, QMM_Escape, QMM_Result };
+  ForRTTI_t IsForRTTI;
 
+public:
   MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_)
   : Context(C), Out(Out_), Structor(nullptr), StructorType(-1),
 PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
- 64) {}
+ 64),
+IsForRTTI(NotForRTTI) {}
 
   MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_,
   const CXXConstructorDecl *D, CXXCtorType Type)
   : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
 PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
- 64) {}
+ 64),
+IsForRTTI(NotForRTTI) {}
 
   MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_,
   const CXXDestructorDecl *D, CXXDtorType Type)
   : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
 PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
- 64) {}
+ 64),
+IsForRTTI(NotForRTTI) {}
+
+  MicrosoftCXXNameMangler(MicrosoftMangleContextImpl &C, raw_ostream &Out_,
+  ForRTTI_t IsForRTTI)
+  : Context(C), Out(Out_), Structor(nullptr), StructorType(-1),
+PointersAre64Bit(C.getASTContext().getTargetInfo().getPointerWidth(0) ==
+ 64),
+IsForRTTI(IsForRTTI) {}
 
   raw_ostream &getStream() const { return Out; }
 
@@ -339,6 +354,7 @@
   void
   mangleTemplateInstantiationName(const TemplateDecl *TD,
   const TemplateArgumentList &TemplateArgs);
+  void mangleObjCClassName(StringRef Name);
   void mangleObjCMethodName(const ObjCMethodDecl *MD);
 
   void mangleArgumentType(QualType T, SourceRange Range);
@@ -1277,6 +1293,27 @@
   }
 }
 
+void MicrosoftCXXNameMangler::mangleObjCClassName(StringRef Name) {
+  // Obj-C classes are normally mangled as C++ structs with the same name, but
+  // we want to be able to distinguish a C++ struct X from an Obj-C class X for
+  // the purposes of exception handling, so we add a discriminator when mangling
+  // for RTTI.
+  if (!IsForRTTI) {
+mangleArtificalTagType(TTK_Struct, Name);
+return;
+  }
+
+  llvm::SmallString<64> TemplateMangling;
+  llvm::raw_svector_ostream Stream(TemplateMangling);
+  MicrosoftCXXNameMangler Extra(Context, Stream);
+
+  Stream << "?$";
+  Extra.mangleSourceName("Class");
+  Ex

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

For the integer conversion though, I was going to add that in a separate 
fixed-point-to-int and int-to-fixed-point patch.


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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

[PATCH] D54489: Implement -frecord-gcc-switches

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D54489#1297509, @scott.linder wrote:

> In https://reviews.llvm.org/D54489#1297504, @troyj wrote:
>
> > I realize that you're probably striving for option compatibility with gcc, 
> > but continuing to name it -frecord-gcc-switches when it actually records 
> > Clang switches seems weird to me.  It almost sounds like something that 
> > would dump gcc equivalents of all Clang options, or maybe let you know 
> > which Clang options you've used that match gcc options.  Either way, by the 
> > name -- if you aren't familiar with the gcc option -- it doesn't read like 
> > it records Clang options.
> >
> > Would it be that bad to name it -frecord-clang-switches?  Or just 
> > -frecord-switches?
>
>
> I agree, and this was my original plan, but then I noticed that Clang already 
> implements -grecord-gcc-switches and so I decided to mirror the naming for 
> the -f variant as well.
>
> If anything I think dropping the -gcc- altogether would make the most sense. 
> I don't understand why GCC includes it in the first place.


Well, having something in the name that make it clear that this is about 
command line switches and not switch statements seems sensible.  It's a little 
unfortunate to give a useful feature a vendor-specific name, but I think the 
right approach is probably to accept that for compatibility and then also 
accept a more neutral and descriptive name.

The code seems fine.


Repository:
  rC Clang

https://reviews.llvm.org/D54489



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Sorry for the delay in responding to this. Been working on a few other patches.

A couple of the bigger changes so far:

- Added `getCommonSemantics` to `FixedPointSemantics` for finding a common full 
precision semantic.
- `getFixedPointSemantics` accepts an int type.
- `FixedPointConversions` was renamed to `handleFixedPointConversions` and is a 
part of `UsualArithmeticConversions`
- `EmitFixedPointAdd` now works by converting both operands to a common 
`FixedPointSemantics` that can hold the full precision of the result, 
performing a regular `add` or call to the addition intrinsics, and converting 
back to the result type if necessary.
  - This also fixed the case with calling the sat add intrinsics.
- Change `EmitFixedPointConversion` to accept `FixedPointSemantics`. I kept the 
original one still which takes `QualTypes` that are fixed point or integer 
types and gets the `FixedPointSemantics` from them.

The only thing I didn't include in this patch were the suggested changes to 
`FixedPointSemantics` which would indicate if the semantics were original from 
an integer type.  I'm not sure how necessary this is because AFAIK the only 
time we would need to care if the semantics came from an int is during 
conversion from a fixed point type to an int, which should only occur during 
casting and not binary operations. In that sense, I don't think this info needs 
to be something bound to the semantics, but more to the conversion operation. I 
also don't think that would be relevant for this patch since all integers get 
converted to fixed point types anyway and not the other way around.




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3208
+  if (LHSWidth < CommonWidth)
+LHS = LHSSign ? Builder.CreateSExt(LHS, CommonTy)
+  : Builder.CreateZExt(LHS, CommonTy);

bjope wrote:
> `LHS = Builder.CreateIntCast(LHS, CommonTy, LHSSign);`
> 
> (I think that it even is possible to remove the condition and always do this 
> (the cast will be a no-op if LHSWidth==CommonWidth))
Replaced this part with converting the operands to common semantics, then 
regular addition.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3211
+  if (RHSWidth < CommonWidth)
+RHS = RHSSign ? Builder.CreateSExt(RHS, CommonTy)
+  : Builder.CreateZExt(RHS, CommonTy);

bjope wrote:
> Same as above, this can be simplified as:
>   RHS = Builder.CreateIntCast(RHS, CommonTy, RHSSign);
> 
Same as above.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3214
+
+  // Align scales
+  unsigned CommonScale = std::max({LHSScale, RHSScale, ResultScale});

bjope wrote:
> Arithmetically I think that it would be enough to align the scale to
> ```
>  std::max(std::min(LHSScale, RHSScale), ResultScale)
> ```
> As adding low fractional bits outside the result precision, when we know that 
> those bits are zero in either of the inputs, won't impact any more 
> significant bits in the result.
> 
> Maybe your solution is easier and good enough for a first implementation. And 
> maybe `opt` is smart enough to optimize this anyway. Or maybe codegen could 
> be worse due to not using legal types. Or maybe codegen could be improved due 
> to using sadd.sat in more cases?
> Lots of "maybes", so I don't think you need to do anything about this right 
> now. It is just an idea from my side.
Same as above.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3223-3247
+if (ResultWidth < CommonWidth) {
+  // In the event we extended the sign of both operands, the intrinsic will
+  // not saturate to the initial bit width of the result type. In this 
case,
+  // we can default to use min/max clamping. This can arise from adding an
+  // operand of an int type whose width is larger than the width of the
+  // other fixed point operand.
+  // If we end up implementing the intrinsics for saturating ints to

leonardchan wrote:
> There is a case when we cannot use the sadd/uadd intrinsics because those 
> intrinsics saturate to the width of the operands when instead we want to 
> saturate to the width of the resulting fixed point type. The problem is that 
> if we are given operands with different scales that require extending then 
> shifting before adding, the bit width no longer matches that of the resulting 
> saturated type, so the sat intrinsics instead saturate to the extended width 
> instead of the result type width. This could be a problem with my logic 
> though and there could be a better way to implement addition.
> 
> Otherwise, as far as I can tell, this could be addressed by replacing the 
> clamping with an intrinsic, which would create a use case for the 
> signed/unsigned saturation intrinsics. Alternatively, the addition intrinsics 
> could be expanded to also provide another argument that contains a desired 
> saturation bit width, whi

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 173961.
leonardchan marked 11 inline comments as done.

Repository:
  rC Clang

https://reviews.llvm.org/D53738

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Frontend/fixed_point_add.c
  clang/test/Frontend/fixed_point_add_ast.c
  clang/test/Frontend/fixed_point_conversions.c

Index: clang/test/Frontend/fixed_point_conversions.c
===
--- clang/test/Frontend/fixed_point_conversions.c
+++ clang/test/Frontend/fixed_point_conversions.c
@@ -214,19 +214,17 @@
   // Only get overflow checking if signed fract to unsigned accum
   sat_ua = sat_sf;
   // DEFAULT:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i17
-  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i17 [[FRACT_EXT]], 9
-  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i17 [[ACCUM]], 0
-  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i17 0, i17 [[ACCUM]]
-  // DEFAULT-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i17 [[RESULT]] to i32
-  // DEFAULT-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 9
+  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // DEFAULT-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
   // SAME:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i16
-  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i16 [[FRACT_EXT]], 8
-  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i16 [[ACCUM]], 0
-  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i16 0, i16 [[ACCUM]]
-  // SAME-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i16 [[RESULT]] to i32
-  // SAME-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 8
+  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // SAME-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
 }
 
 void TestFixedPointCastBetFractAccum() {
Index: clang/test/Frontend/fixed_point_add_ast.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_add_ast.c
@@ -0,0 +1,337 @@
+// RUN: %clang_cc1 -x c -ffixed-point -ast-dump %s | FileCheck %s --strict-whitespace
+
+void SignedAdditions() {
+  // CHECK-LABEL: SignedAdditions
+  short _Accum sa;
+  _Accum a;
+  long _Accum la;
+  unsigned short _Accum usa;
+  unsigned _Accum ua;
+  unsigned long _Accum ula;
+
+  short _Fract sf;
+  _Fract f;
+  long _Fract lf;
+  unsigned short _Fract usf;
+  unsigned _Fract uf;
+  unsigned long _Fract ulf;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  sa = sa + sa;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  a = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'long _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'long _Accum' {{.*}} 'la' 'long _Accum'
+  la = sa + la;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT:`-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:  |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:  `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT:`-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  la = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} '

r346818 - Complete reverting r346191

[Tatyana Krasnukha via cfe-commits]

Author: tkrasnukha
Date: Tue Nov 13 15:59:25 2018
New Revision: 346818

URL: http://llvm.org/viewvc/llvm-project?rev=346818&view=rev
Log:
Complete reverting r346191

Removed:
cfe/trunk/clang/

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

r346819 - Complete reverting r346191

[Tatyana Krasnukha via cfe-commits]

Author: tkrasnukha
Date: Tue Nov 13 16:08:34 2018
New Revision: 346819

URL: http://llvm.org/viewvc/llvm-project?rev=346819&view=rev
Log:
Complete reverting r346191

Removed:
cfe/trunk/CodeGen/
cfe/trunk/Headers/

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

[PATCH] D54503: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added a reviewer: bruno.
Herald added a subscriber: dexonsmith.

Include search paths can be relative paths. The `loadSubdirectoryModuleMaps` 
function should account for that and respect the `-working-directory` parameter 
given to Clang.


Repository:
  rC Clang

https://reviews.llvm.org/D54503

Files:
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
  
test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
  test/Modules/subdirectory-module-maps-working-dir.m


Index: test/Modules/subdirectory-module-maps-working-dir.m
===
--- /dev/null
+++ test/Modules/subdirectory-module-maps-working-dir.m
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t \
+// RUN:-working-directory %S/Inputs \
+// RUN:-I subdirectory-module-maps-working-dir \
+// RUN:%s -Werror=implicit-function-declaration -Xclang -verify
+
+@import ModuleInSubdir;
+
+void foo() {
+  int x = bar();
+}
+
+// expected-no-diagnostics
Index: 
test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
===
--- /dev/null
+++ 
test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
@@ -0,0 +1,5 @@
+module ModuleInSubdir {
+header "h1.h"
+  export *
+}
+
Index: 
test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
===
--- /dev/null
+++ test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
@@ -0,0 +1 @@
+int bar();
Index: lib/Lex/HeaderSearch.cpp
===
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -1638,8 +1638,10 @@
 return;
 
   std::error_code EC;
+  SmallString<128> Dir = SearchDir.getDir()->getName();
+  FileMgr.makeAbsolutePath(Dir);
   SmallString<128> DirNative;
-  llvm::sys::path::native(SearchDir.getDir()->getName(), DirNative);
+  llvm::sys::path::native(Dir, DirNative);
   llvm::vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {


Index: test/Modules/subdirectory-module-maps-working-dir.m
===
--- /dev/null
+++ test/Modules/subdirectory-module-maps-working-dir.m
@@ -0,0 +1,13 @@
+// RUN: rm -rf %t
+// RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t \
+// RUN:-working-directory %S/Inputs \
+// RUN:-I subdirectory-module-maps-working-dir \
+// RUN:%s -Werror=implicit-function-declaration -Xclang -verify
+
+@import ModuleInSubdir;
+
+void foo() {
+  int x = bar();
+}
+
+// expected-no-diagnostics
Index: test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
===
--- /dev/null
+++ test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/module.map
@@ -0,0 +1,5 @@
+module ModuleInSubdir {
+header "h1.h"
+  export *
+}
+
Index: test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
===
--- /dev/null
+++ test/Modules/Inputs/subdirectory-module-maps-working-dir/subdir_module/h1.h
@@ -0,0 +1 @@
+int bar();
Index: lib/Lex/HeaderSearch.cpp
===
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -1638,8 +1638,10 @@
 return;
 
   std::error_code EC;
+  SmallString<128> Dir = SearchDir.getDir()->getName();
+  FileMgr.makeAbsolutePath(Dir);
   SmallString<128> DirNative;
-  llvm::sys::path::native(SearchDir.getDir()->getName(), DirNative);
+  llvm::sys::path::native(Dir, DirNative);
   llvm::vfs::FileSystem &FS = *FileMgr.getVirtualFileSystem();
   for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r346820 - [CMake] Passthrough CFLAGS when checking the compiler-rt path

[Petr Hosek via cfe-commits]

Author: phosek
Date: Tue Nov 13 16:09:26 2018
New Revision: 346820

URL: http://llvm.org/viewvc/llvm-project?rev=346820&view=rev
Log:
[CMake] Passthrough CFLAGS when checking the compiler-rt path

This is needed when cross-compiling for a different target since
CFLAGS may contain additional flags like -resource-dir which
change the location in which compiler-rt builtins are found.

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

Modified:
libunwind/trunk/cmake/Modules/HandleCompilerRT.cmake

Modified: libunwind/trunk/cmake/Modules/HandleCompilerRT.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/cmake/Modules/HandleCompilerRT.cmake?rev=346820&r1=346819&r2=346820&view=diff
==
--- libunwind/trunk/cmake/Modules/HandleCompilerRT.cmake (original)
+++ libunwind/trunk/cmake/Modules/HandleCompilerRT.cmake Tue Nov 13 16:09:26 
2018
@@ -8,6 +8,9 @@ function(find_compiler_rt_library name d
   if (CMAKE_CXX_COMPILER_ID MATCHES Clang AND CMAKE_CXX_COMPILER_TARGET)
 list(APPEND CLANG_COMMAND "--target=${CMAKE_CXX_COMPILER_TARGET}")
   endif()
+  get_property(LIBUNWIND_CXX_FLAGS CACHE CMAKE_CXX_FLAGS PROPERTY VALUE)
+  string(REPLACE " " ";" LIBUNWIND_CXX_FLAGS "${LIBUNWIND_CXX_FLAGS}")
+  list(APPEND CLANG_COMMAND ${LIBUNWIND_CXX_FLAGS})
   execute_process(
   COMMAND ${CLANG_COMMAND}
   RESULT_VARIABLE HAD_ERROR


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

[PATCH] D54055: CGDecl::emitStoresForConstant fix synthesized constant's name

[John McCall via Phabricator via cfe-commits]

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

Seems reasonable to me.


Repository:
  rC Clang

https://reviews.llvm.org/D54055



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

[PATCH] D54496: [HIP] Fix device only compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D54496#1297710, @tra wrote:

> Do I understand it correctly that the bug appears to affect HIP compilation 
> only?


Yes. Only HIP.


https://reviews.llvm.org/D54496



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

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

rsmith added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:1930-1931
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+  // At -O0, we don't perform inlining, so we don't need to delay the
+  // processing.
+  return RValue::get(ConstantInt::get(Int32Ty, 0));

Are there cases where a call to an `always_inline` function could change the 
outcome?



Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+  Arg = ConstantExpr::Create(Context, Arg);
+

void wrote:
> rsmith wrote:
> > We should create this node as part of checking that the argument is an ICE 
> > (in `SemaBuiltinConstantArg`).
> It turns out that `SemaBuiltinConstantArg` isn't called for 
> `__builtin_constant_p()`. Its argument type is `.`, which...I'm not sure what 
> that means. It looks like a vararg, but that doesn't make sense for the 
> builtin.
> 
> Should I change its signature in `Builtins.def`?
It's also marked `t`, which means "signature is meaningless, uses custom type 
checking".

The argument to `__builtin_constant_p` isn't required to be an ICE, though, so 
we shouldn't be calling `SemaBuiltinConstantArg` for it / creating a 
`ConstantExpr` wrapping it, as far as I can tell.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

Quuxplusone wrote:
> rjmccall wrote:
> > Quuxplusone wrote:
> > > rjmccall wrote:
> > > > Quuxplusone wrote:
> > > > > @rjmccall wrote:
> > > > > > trivial_abi permits annotated types to be passed and returned in 
> > > > > > registers, which is ABI-breaking. Skimming the blog post, it looks 
> > > > > > like trivially_relocatable does not permit this — it merely 
> > > > > > signifies that destruction is a no-op after a move construction or 
> > > > > > assignment.
> > > > > 
> > > > > Not necessarily a "no-op"; my canonical example is a 
> > > > > CopyOnlyCXX03SharedPtr which increments a refcount on construction 
> > > > > and decrements on destruction. But move-construction plus destruction 
> > > > > should "balance out" and result in no observable side effects.
> > > > > 
> > > > > > This is usefully different in the design space, since it means you 
> > > > > > can safely add the attribute retroactively to e.g. std::unique_ptr, 
> > > > > > and other templates can then detect that std::unique_ptr is 
> > > > > > trivially-relocatable and optimize themselves to use memcpy or 
> > > > > > realloc or whatever it is that they want to do. So in that sense 
> > > > > > trivial_abi is a *stronger* attribute, not a *weaker* one: the 
> > > > > > property it determines ought to imply trivially_relocatable.
> > > > > 
> > > > > `trivial_abi` is an "orthogonal" attribute: you can have 
> > > > > `trivial_abi` types with non-trivial constructors and destructors, 
> > > > > which can have observable side effects. For example,
> > > > > ```
> > > > > struct [[clang::trivial_abi]] DestructionAnnouncer {
> > > > > ~DestructionAnnouncer() { puts("hello!"); }
> > > > > };
> > > > > ```
> > > > > is `trivial_abi` (because of the annotation) yet not trivially 
> > > > > relocatable, because its "move plus destroy" operation has observable 
> > > > > side effects.
> > > > > 
> > > > > > The only interesting question in the language design that I know of 
> > > > > > is what happens if you put the attribute on a template that's 
> > > > > > instantiated to contain a sub-object that is definitely not 
> > > > > > trivially relocatable / trivial-ABI. For trivial_abi, we decided 
> > > > > > that the attribute is simply ignored — it implicitly only applies 
> > > > > > to specializations where the attribute would be legal. I haven't 
> > > > > > dug into the design enough to know what trivially_relocatable 
> > > > > > decides in this situation, but the three basic options are:
> > > > > >
> > > > > > - the attribute always has effect and allows trivial relocation 
> > > > > > regardless of the subobject types; this is obviously unsafe, so it 
> > > > > > limits the safe applicability of the attribute to templates
> > > > > > - the attribute is ignored, like trivial_abi is
> > > > > > - the attribute is ill-formed, and you'll need to add a 
> > > > > > [[trivially_relocatable(bool)]] version to support templates
> > > > > 
> > > > > What happens is basically the first thing you said, except that I 
> > > > > disagree that it's "obviously unsafe." Right now, conditionally 
> > > > > trivial relocation is possible via template metaprogramming; see the 
> > > > > libcxx patch at e.g.
> > > > > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > > > > Since the attribute is an opt-in mechanism, it makes perfect sense to 
> > > > > me that if you put it on a class (or class template), then it applies 
> > > > > to the class, without any further sanity-checking by the compiler. 
> > > > > The compiler has no reason to second-guess the programmer here.
> > > > > 
> > > > > However, there's one more interesting case. Suppose the programmer 
> > > > > puts the attribute on a class that isn't relocatable at all! (For 
> > > > > example, the union case @erichkeane mentioned, or a class type with a 
> > > > > deleted destructor.) In that case, this patch *does* give an error... 
> > > > > *unless* the class was produced by instantiating a template, in which 
> > > > > case we *don't* give an error, because it's not the template-writer's 
> > > > > fault.
> > > > > https://p1144.godbolt.org/z/wSZPba
> > > > > trivial_abi is an "orthogonal" attribute: you can have trivial_abi 
> > > > > types with non-trivial constructors and destructors, which can have 
> > > > > observable side effects. 
> > > > 
> > > > Let me cut this conversation short.  `trivial_abi` is not such an old 
> > > > and widely-established attribute that we are unable to revise its 
> > > > definition.  I am comfortable making the same semantic guarantees for 
> > > > `trivial_abi` that you're making for `trivially_relocatable`, because I 
> > > > think it is in the language

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 173946.
jfb added a comment.

- clang-format


Repository:
  rC Clang

https://reviews.llvm.org/D54055

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/decl.c
  test/CodeGen/dump-struct-builtin.c
  test/CodeGenCXX/amdgcn-string-literal.cpp
  test/CodeGenCXX/const-init-cxx11.cpp
  test/CodeGenCXX/cxx2a-init-statement.cpp
  test/CodeGenCXX/float128-declarations.cpp
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenOpenCL/amdgpu-nullptr.cl
  test/CodeGenOpenCL/constant-addr-space-globals.cl
  test/CodeGenOpenCL/partial_initializer.cl
  test/CodeGenOpenCL/private-array-initialization.cl
  test/Modules/templates.mm

Index: test/Modules/templates.mm
===
--- test/Modules/templates.mm
+++ test/Modules/templates.mm
@@ -14,8 +14,8 @@
 
 // CHECK-DAG: @list_left = global %[[LIST:.*]] { %[[LISTNODE:.*]]* null, i32 8 }, align 8
 // CHECK-DAG: @list_right = global %[[LIST]] { %[[LISTNODE]]* null, i32 12 }, align 8
-// CHECK-DAG: @_ZZ15testMixedStructvE1l = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 1 }, align 8
-// CHECK-DAG: @_ZZ15testMixedStructvE1r = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 2 }, align 8
+// CHECK-DAG: @__const._Z15testMixedStructv.l = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 1 }, align 8
+// CHECK-DAG: @__const._Z15testMixedStructv.r = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 2 }, align 8
 // CHECK-DAG: @_ZN29WithUndefinedStaticDataMemberIA_iE9undefinedE = external global
 
 void testTemplateClasses() {
@@ -77,10 +77,10 @@
   // CHECK: %[[l:.*]] = alloca %[[ListInt:[^ ]*]], align 8
   // CHECK: %[[r:.*]] = alloca %[[ListInt]], align 8
 
-  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @_ZZ15testMixedStructvE1l to i8*), i64 16,
+  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @__const._Z15testMixedStructv.l to i8*), i64 16,
   ListInt_left l{0, 1};
 
-  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @_ZZ15testMixedStructvE1r to i8*), i64 16,
+  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @__const._Z15testMixedStructv.r to i8*), i64 16,
   ListInt_right r{0, 2};
 
   // CHECK: call void @_Z10useListIntR4ListIiE(%[[ListInt]]* dereferenceable({{[0-9]+}}) %[[l]])
Index: test/CodeGenOpenCL/private-array-initialization.cl
===
--- test/CodeGenOpenCL/private-array-initialization.cl
+++ test/CodeGenOpenCL/private-array-initialization.cl
@@ -6,11 +6,11 @@
 void test() {
   __private int arr[] = {1, 2, 3};
 // PRIVATE0:  %[[arr_i8_ptr:[0-9]+]] = bitcast [3 x i32]* %arr to i8*
-// PRIVATE0:  call void @llvm.memcpy.p0i8.p2i8.i32(i8* align 4 %[[arr_i8_ptr]], i8 addrspace(2)* align 4 bitcast ([3 x i32] addrspace(2)* @test.arr to i8 addrspace(2)*), i32 12, i1 false)
+// PRIVATE0:  call void @llvm.memcpy.p0i8.p2i8.i32(i8* align 4 %[[arr_i8_ptr]], i8 addrspace(2)* align 4 bitcast ([3 x i32] addrspace(2)* @__const.test.arr to i8 addrspace(2)*), i32 12, i1 false)
 
 // PRIVATE5: %arr = alloca [3 x i32], align 4, addrspace(5)
 // PRIVATE5: %0 = bitcast [3 x i32] addrspace(5)* %arr to i8 addrspace(5)*
-// PRIVATE5: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(4)* align 4 bitcast ([3 x i32] addrspace(4)* @test.arr to i8 addrspace(4)*), i64 12, i1 false)
+// PRIVATE5: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(4)* align 4 bitcast ([3 x i32] addrspace(4)* @__const.test.arr to i8 addrspace(4)*), i64 12, i1 false)
 }
 
 __kernel void initializer_cast_is_valid_crash() {
Index: test/CodeGenOpenCL/partial_initializer.cl
===
--- test/CodeGenOpenCL/partial_initializer.cl
+++ test/CodeGenOpenCL/partial_initializer.cl
@@ -24,7 +24,7 @@
 // CHECK: @GV2 = addrspace(1) global <4 x i32> , align 16
 int4 GV2 = (int4)(1);
 
-// CHECK: @f.S = private unnamed_addr addrspace(2) constant %struct.StrucTy { i32 1, i32 2, i32 0 }, align 4
+// CHECK: @__const.f.S = private unnamed_addr addrspace(2) constant %struct.StrucTy { i32 1, i32 2, i32 0 }, align 4
 
 // CHECK-LABEL: define spir_func void @f()
 void f(void) {
@@ -46,7 +46,7 @@
   float A[6][6]  = {1.0f, 2.0f};
 
   // CHECK: %[[v5:.*]] = bitcast %struct.StrucTy* %S to i8*
-  // CHECK: call void @llvm.memcpy.p0i8.p2i8.i32(i8* align 4 %[[v5]], i8 addrspace(2)* align 4 bitcast (%struct.StrucTy addrspace(2)* @f.S to i8 addrspace(2)*), i32 12, i1 false)
+  // CHECK: call void @llvm.memcpy.p0i8.p2i8.i32(i8* align 4 %[[v5]], i8 addrspace(2)* align 4 bitcast (%struct.StrucTy addrspace(2)* @__const.f.S to i8 addrspace(2)*), i32 12, i1 false)
   StrucTy S = {1, 2};
 
   // CHECK: store <2 x i32> , <2 x i32>* %[[compoundliteral1]], align 8
Index: test/CodeGenOpenC

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Nope, unit tests aren't quite useful here. I can't unit-test the behavior of 
API that i'm about to //remove//... right?


https://reviews.llvm.org/D18860



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

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In https://reviews.llvm.org/D54055#1286523, @rjmccall wrote:

> Pfft, if I'm going to be held responsible for my own work, I'm in a lot of 
> trouble. :)
>
> Function name + local variable name WFM.


Your wish is my command (in this specific instance anyways).


Repository:
  rC Clang

https://reviews.llvm.org/D54055



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

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 173945.
jfb added a comment.
Herald added subscribers: nhaehnle, jvesely.

- As discussed on the review, change the constant generation to name the 
synthesized constant using '__const.' + function_name + '.' variable_name. The 
previous code wasn't generally correct because it was trying to mangle the 
variable as if it were a static, and in some cases this could pull in e.g. 
parameter names (which is nonsensical for a static). This only occurs with a 
patch which I haven't submitted yet, but my change is pretty well tested 
already as the modified tests show.


Repository:
  rC Clang

https://reviews.llvm.org/D54055

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/decl.c
  test/CodeGen/dump-struct-builtin.c
  test/CodeGenCXX/amdgcn-string-literal.cpp
  test/CodeGenCXX/const-init-cxx11.cpp
  test/CodeGenCXX/cxx2a-init-statement.cpp
  test/CodeGenCXX/float128-declarations.cpp
  test/CodeGenCXX/float16-declarations.cpp
  test/CodeGenOpenCL/amdgpu-nullptr.cl
  test/CodeGenOpenCL/constant-addr-space-globals.cl
  test/CodeGenOpenCL/partial_initializer.cl
  test/CodeGenOpenCL/private-array-initialization.cl
  test/Modules/templates.mm

Index: test/Modules/templates.mm
===
--- test/Modules/templates.mm
+++ test/Modules/templates.mm
@@ -14,8 +14,8 @@
 
 // CHECK-DAG: @list_left = global %[[LIST:.*]] { %[[LISTNODE:.*]]* null, i32 8 }, align 8
 // CHECK-DAG: @list_right = global %[[LIST]] { %[[LISTNODE]]* null, i32 12 }, align 8
-// CHECK-DAG: @_ZZ15testMixedStructvE1l = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 1 }, align 8
-// CHECK-DAG: @_ZZ15testMixedStructvE1r = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 2 }, align 8
+// CHECK-DAG: @__const._Z15testMixedStructv.l = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 1 }, align 8
+// CHECK-DAG: @__const._Z15testMixedStructv.r = {{.*}} constant %[[LIST]] { %{{.*}}* null, i32 2 }, align 8
 // CHECK-DAG: @_ZN29WithUndefinedStaticDataMemberIA_iE9undefinedE = external global
 
 void testTemplateClasses() {
@@ -77,10 +77,10 @@
   // CHECK: %[[l:.*]] = alloca %[[ListInt:[^ ]*]], align 8
   // CHECK: %[[r:.*]] = alloca %[[ListInt]], align 8
 
-  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @_ZZ15testMixedStructvE1l to i8*), i64 16,
+  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @__const._Z15testMixedStructv.l to i8*), i64 16,
   ListInt_left l{0, 1};
 
-  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @_ZZ15testMixedStructvE1r to i8*), i64 16,
+  // CHECK: call {{.*}}memcpy{{.*}}(i8* align {{[0-9]+}} %{{.*}}, i8* align {{[0-9]+}} bitcast ({{.*}}* @__const._Z15testMixedStructv.r to i8*), i64 16,
   ListInt_right r{0, 2};
 
   // CHECK: call void @_Z10useListIntR4ListIiE(%[[ListInt]]* dereferenceable({{[0-9]+}}) %[[l]])
Index: test/CodeGenOpenCL/private-array-initialization.cl
===
--- test/CodeGenOpenCL/private-array-initialization.cl
+++ test/CodeGenOpenCL/private-array-initialization.cl
@@ -6,11 +6,11 @@
 void test() {
   __private int arr[] = {1, 2, 3};
 // PRIVATE0:  %[[arr_i8_ptr:[0-9]+]] = bitcast [3 x i32]* %arr to i8*
-// PRIVATE0:  call void @llvm.memcpy.p0i8.p2i8.i32(i8* align 4 %[[arr_i8_ptr]], i8 addrspace(2)* align 4 bitcast ([3 x i32] addrspace(2)* @test.arr to i8 addrspace(2)*), i32 12, i1 false)
+// PRIVATE0:  call void @llvm.memcpy.p0i8.p2i8.i32(i8* align 4 %[[arr_i8_ptr]], i8 addrspace(2)* align 4 bitcast ([3 x i32] addrspace(2)* @__const.test.arr to i8 addrspace(2)*), i32 12, i1 false)
 
 // PRIVATE5: %arr = alloca [3 x i32], align 4, addrspace(5)
 // PRIVATE5: %0 = bitcast [3 x i32] addrspace(5)* %arr to i8 addrspace(5)*
-// PRIVATE5: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(4)* align 4 bitcast ([3 x i32] addrspace(4)* @test.arr to i8 addrspace(4)*), i64 12, i1 false)
+// PRIVATE5: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(4)* align 4 bitcast ([3 x i32] addrspace(4)* @__const.test.arr to i8 addrspace(4)*), i64 12, i1 false)
 }
 
 __kernel void initializer_cast_is_valid_crash() {
Index: test/CodeGenOpenCL/partial_initializer.cl
===
--- test/CodeGenOpenCL/partial_initializer.cl
+++ test/CodeGenOpenCL/partial_initializer.cl
@@ -24,7 +24,7 @@
 // CHECK: @GV2 = addrspace(1) global <4 x i32> , align 16
 int4 GV2 = (int4)(1);
 
-// CHECK: @f.S = private unnamed_addr addrspace(2) constant %struct.StrucTy { i32 1, i32 2, i32 0 }, align 4
+// CHECK: @__const.f.S = private unnamed_addr addrspace(2) constant %struct.StrucTy { i32 1, i32 2, i32 0 }, align 4
 
 // CHECK-LABEL: define spir_func void @f()
 void f(void) {
@@ -46,7 +46,7 @@
   float A[6][6]  = {1.0f, 2.0f};
 
   // CHECK: %[[v5:.*]] = b

[PATCH] D54496: [HIP] Fix device only compilation

[Artem Belevich via Phabricator via cfe-commits]

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

Do I understand it correctly that the bug appears to affect HIP compilation 
only?


https://reviews.llvm.org/D54496



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void marked an inline comment as not done.
void added inline comments.



Comment at: lib/AST/Expr.cpp:2915-2916
+  case ConstantExprClass: {
+const Expr *Exp = cast(this)->getSubExpr();
+return Exp->isConstantInitializer(Ctx, false, Culprit);
+  }

void wrote:
> rsmith wrote:
> > Can we just return `true` here? If not, please add a FIXME saying that we 
> > should be able to.
> To be honest, I don't know. Theoretically we should be able to. Let me give 
> it a try and see how it goes.
I tested this and it looks like it could lead to an extra warning like this:

```
File /sandbox/morbo/llvm/llvm-mirror/tools/clang/test/Sema/array-init.c Line 
285: cannot initialize array of type 'int [5]' with non-constant array of type 
'int [5]'
```

(Similar in `Sema/compound-literal.c`.) I'll add a comment.



Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+  Arg = ConstantExpr::Create(Context, Arg);
+

rsmith wrote:
> We should create this node as part of checking that the argument is an ICE 
> (in `SemaBuiltinConstantArg`).
It turns out that `SemaBuiltinConstantArg` isn't called for 
`__builtin_constant_p()`. Its argument type is `.`, which...I'm not sure what 
that means. It looks like a vararg, but that doesn't make sense for the builtin.

Should I change its signature in `Builtins.def`?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

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

Quuxplusone added inline comments.



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

rjmccall wrote:
> Quuxplusone wrote:
> > rjmccall wrote:
> > > Quuxplusone wrote:
> > > > @rjmccall wrote:
> > > > > trivial_abi permits annotated types to be passed and returned in 
> > > > > registers, which is ABI-breaking. Skimming the blog post, it looks 
> > > > > like trivially_relocatable does not permit this — it merely signifies 
> > > > > that destruction is a no-op after a move construction or assignment.
> > > > 
> > > > Not necessarily a "no-op"; my canonical example is a 
> > > > CopyOnlyCXX03SharedPtr which increments a refcount on construction and 
> > > > decrements on destruction. But move-construction plus destruction 
> > > > should "balance out" and result in no observable side effects.
> > > > 
> > > > > This is usefully different in the design space, since it means you 
> > > > > can safely add the attribute retroactively to e.g. std::unique_ptr, 
> > > > > and other templates can then detect that std::unique_ptr is 
> > > > > trivially-relocatable and optimize themselves to use memcpy or 
> > > > > realloc or whatever it is that they want to do. So in that sense 
> > > > > trivial_abi is a *stronger* attribute, not a *weaker* one: the 
> > > > > property it determines ought to imply trivially_relocatable.
> > > > 
> > > > `trivial_abi` is an "orthogonal" attribute: you can have `trivial_abi` 
> > > > types with non-trivial constructors and destructors, which can have 
> > > > observable side effects. For example,
> > > > ```
> > > > struct [[clang::trivial_abi]] DestructionAnnouncer {
> > > > ~DestructionAnnouncer() { puts("hello!"); }
> > > > };
> > > > ```
> > > > is `trivial_abi` (because of the annotation) yet not trivially 
> > > > relocatable, because its "move plus destroy" operation has observable 
> > > > side effects.
> > > > 
> > > > > The only interesting question in the language design that I know of 
> > > > > is what happens if you put the attribute on a template that's 
> > > > > instantiated to contain a sub-object that is definitely not trivially 
> > > > > relocatable / trivial-ABI. For trivial_abi, we decided that the 
> > > > > attribute is simply ignored — it implicitly only applies to 
> > > > > specializations where the attribute would be legal. I haven't dug 
> > > > > into the design enough to know what trivially_relocatable decides in 
> > > > > this situation, but the three basic options are:
> > > > >
> > > > > - the attribute always has effect and allows trivial relocation 
> > > > > regardless of the subobject types; this is obviously unsafe, so it 
> > > > > limits the safe applicability of the attribute to templates
> > > > > - the attribute is ignored, like trivial_abi is
> > > > > - the attribute is ill-formed, and you'll need to add a 
> > > > > [[trivially_relocatable(bool)]] version to support templates
> > > > 
> > > > What happens is basically the first thing you said, except that I 
> > > > disagree that it's "obviously unsafe." Right now, conditionally trivial 
> > > > relocation is possible via template metaprogramming; see the libcxx 
> > > > patch at e.g.
> > > > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > > > Since the attribute is an opt-in mechanism, it makes perfect sense to 
> > > > me that if you put it on a class (or class template), then it applies 
> > > > to the class, without any further sanity-checking by the compiler. The 
> > > > compiler has no reason to second-guess the programmer here.
> > > > 
> > > > However, there's one more interesting case. Suppose the programmer puts 
> > > > the attribute on a class that isn't relocatable at all! (For example, 
> > > > the union case @erichkeane mentioned, or a class type with a deleted 
> > > > destructor.) In that case, this patch *does* give an error... *unless* 
> > > > the class was produced by instantiating a template, in which case we 
> > > > *don't* give an error, because it's not the template-writer's fault.
> > > > https://p1144.godbolt.org/z/wSZPba
> > > > trivial_abi is an "orthogonal" attribute: you can have trivial_abi 
> > > > types with non-trivial constructors and destructors, which can have 
> > > > observable side effects. 
> > > 
> > > Let me cut this conversation short.  `trivial_abi` is not such an old and 
> > > widely-established attribute that we are unable to revise its definition. 
> > >  I am comfortable making the same semantic guarantees for `trivial_abi` 
> > > that you're making for `trivially_relocatable`, because I think it is in 
> > > the language's interest for `trivial_abi` to be strictly stronger than 
> > > `trivially_relocatable`.
> > > 
> > > > What happens is basically the first thing you said, except that I 
> > > > disagree that

[PATCH] D54162: OpenCL: Don't warn on v printf modifier

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r346806


https://reviews.llvm.org/D54162



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

r346806 - OpenCL: Don't warn on v printf modifier

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Tue Nov 13 14:30:35 2018
New Revision: 346806

URL: http://llvm.org/viewvc/llvm-project?rev=346806&view=rev
Log:
OpenCL: Don't warn on v printf modifier

This avoids spurious warnings, but could use
a lot of work. For example the number of vector
elements is not verified, and the passed
value type is not checked.

Fixes bug 39486

Added:
cfe/trunk/test/SemaOpenCL/printf-format-strings.cl
Modified:
cfe/trunk/include/clang/AST/FormatString.h
cfe/trunk/lib/AST/FormatString.cpp
cfe/trunk/lib/AST/PrintfFormatString.cpp
cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/include/clang/AST/FormatString.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/FormatString.h?rev=346806&r1=346805&r2=346806&view=diff
==
--- cfe/trunk/include/clang/AST/FormatString.h (original)
+++ cfe/trunk/include/clang/AST/FormatString.h Tue Nov 13 14:30:35 2018
@@ -166,6 +166,8 @@ public:
 
 ZArg, // MS extension
 
+VArg, // OpenCL vectors
+
 // Objective-C specific specifiers.
 ObjCObjArg, // '@'
 ObjCBeg = ObjCObjArg,

Modified: cfe/trunk/lib/AST/FormatString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/FormatString.cpp?rev=346806&r1=346805&r2=346806&view=diff
==
--- cfe/trunk/lib/AST/FormatString.cpp (original)
+++ cfe/trunk/lib/AST/FormatString.cpp Tue Nov 13 14:30:35 2018
@@ -618,6 +618,9 @@ const char *ConversionSpecifier::toStrin
 
   // MS specific specifiers.
   case ZArg: return "Z";
+
+ // OpenCL specific specifiers.
+  case VArg: return "v";
   }
   return nullptr;
 }
@@ -875,6 +878,8 @@ bool FormatSpecifier::hasStandardConvers
 case ConversionSpecifier::CArg:
 case ConversionSpecifier::SArg:
   return LangOpt.ObjC;
+case ConversionSpecifier::VArg:
+  return LangOpt.OpenCL;
 case ConversionSpecifier::InvalidSpecifier:
 case ConversionSpecifier::FreeBSDbArg:
 case ConversionSpecifier::FreeBSDDArg:

Modified: cfe/trunk/lib/AST/PrintfFormatString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/PrintfFormatString.cpp?rev=346806&r1=346805&r2=346806&view=diff
==
--- cfe/trunk/lib/AST/PrintfFormatString.cpp (original)
+++ cfe/trunk/lib/AST/PrintfFormatString.cpp Tue Nov 13 14:30:35 2018
@@ -362,6 +362,12 @@ static PrintfSpecifierResult ParsePrintf
 case 'Z':
   if (Target.getTriple().isOSMSVCRT())
 k = ConversionSpecifier::ZArg;
+  break;
+// OpenCL specific.
+case 'v':
+  if (LO.OpenCL)
+k = ConversionSpecifier::VArg;
+  break;
   }
 
   // Check to see if we used the Objective-C modifier flags with
@@ -1026,6 +1032,7 @@ bool PrintfSpecifier::hasValidPrecision(
   case ConversionSpecifier::FreeBSDrArg:
   case ConversionSpecifier::FreeBSDyArg:
   case ConversionSpecifier::PArg:
+  case ConversionSpecifier::VArg:
 return true;
 
   default:

Modified: cfe/trunk/test/Sema/format-strings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=346806&r1=346805&r2=346806&view=diff
==
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Tue Nov 13 14:30:35 2018
@@ -613,6 +613,11 @@ void pr12761(char c) {
   printf("%hhx", c);
 }
 
+void test_opencl_vector_format(int x) {
+  printf("%v4d", x); // expected-warning{{invalid conversion specifier 'v'}}
+  printf("%vd", x); // expected-warning{{invalid conversion specifier 'v'}}
+  printf("%0vd", x); // expected-warning{{invalid conversion specifier 'v'}}
+}
 
 // Test that we correctly merge the format in both orders.
 extern void test14_foo(const char *, const char *, ...)

Added: cfe/trunk/test/SemaOpenCL/printf-format-strings.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/printf-format-strings.cl?rev=346806&view=auto
==
--- cfe/trunk/test/SemaOpenCL/printf-format-strings.cl (added)
+++ cfe/trunk/test/SemaOpenCL/printf-format-strings.cl Tue Nov 13 14:30:35 2018
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -cl-std=CL1.2 -fsyntax-only -verify %s
+
+typedef __attribute__((ext_vector_type(2))) float float2;
+typedef __attribute__((ext_vector_type(4))) float float4;
+typedef __attribute__((ext_vector_type(4))) int int4;
+
+int printf(__constant const char* st, ...) __attribute__((format(printf, 1, 
2)));
+
+kernel void format_v4f32(float4 arg)
+{
+printf("%v4f\n", arg); // expected-no-diagnostics
+}
+
+kernel void format_v4f32_wrong_num_elts(float2 arg)
+{
+printf("%v4f\n", arg); // expected-no-diagnostics
+}
+
+kernel void vector_precision_modifier_v4f32(float4 arg)
+{
+printf("%.2v4f\n", arg); // expected-no-diagnostics
+}
+

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Richard Membarth via Phabricator via cfe-commits]

richardmembarth added a comment.

> In https://reviews.llvm.org/D54258#1297191, @Anastasia wrote:
> 
> I agree with the change itself... but it's quite annoying that such things 
> can't be tested. :(

Yes, that's a pity :(

Is there anything missing so that this can be merged?


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54496: [HIP] Fix device only compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.

Fix a bug causing host code being compiled when `--cude-device-only` is set.


https://reviews.llvm.org/D54496

Files:
  lib/Driver/Driver.cpp
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -157,6 +157,7 @@
 // HBIN-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
 // HBIN-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (host-[[T]])
 // HBIN-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (host-[[T]])
+// HBIN-NOT: device
 //
 // Test single gpu architecture up to the assemble phase in host-only
 // compilation mode.
@@ -172,6 +173,7 @@
 // HASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-[[T]])
 // HASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
 // HASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// HASM-NOT: device
 
 //
 // Test two gpu architectures with complete compilation in host-only
@@ -190,6 +192,7 @@
 // HBIN2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
 // HBIN2-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (host-[[T]])
 // HBIN2-DAG: [[P5:[0-9]+]]: linker, {[[P4]]}, image, (host-[[T]])
+// HBIN2-NOT: device
 
 //
 // Test two gpu architectures up to the assemble phase in host-only
@@ -206,6 +209,7 @@
 // HASM2-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-[[T]])
 // HASM2-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
 // HASM2-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// HASM2-NOT: device
 
 //
 // Test single gpu architecture with complete compilation in device-only
@@ -224,7 +228,7 @@
 // DBIN_NV-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
 // DBIN_NV-DAG: [[P4:[0-9]+]]: assembler, {[[P3]]}, object, (device-[[T]], [[ARCH]])
 // DBIN_NV-DAG: [[P5:[0-9]+]]: offload, "device-[[T]] (nvptx64-nvidia-cuda:[[ARCH]])" {[[P4]]}, object
-
+// DBIN-NOT: host
 //
 // Test single gpu architecture up to the assemble phase in device-only
 // compilation mode.
@@ -241,6 +245,7 @@
 // DASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-[[T]], [[ARCH]])
 // DASM_NV-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-[[T]], [[ARCH]])
 // DASM_NV-DAG: [[P4:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE:nvptx64-nvidia-cuda|amdgcn-amd-amdhsa]]:[[ARCH]])" {[[P3]]}, assembler
+// DASM-NOT: host
 
 //
 // Test two gpu architectures with complete compilation in device-only
@@ -265,7 +270,7 @@
 // DBIN2_NV-DAG: [[P9:[0-9]+]]: backend, {[[P8]]}, assembler, (device-[[T]], [[ARCH2]])
 // DBIN2_NV-DAG: [[P10:[0-9]+]]: assembler, {[[P9]]}, object, (device-[[T]], [[ARCH2]])
 // DBIN2_NV-DAG: [[P11:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH2]])" {[[P10]]}, object
-
+// DBIN2-NOT: host
 //
 // Test two gpu architectures up to the assemble phase in device-only
 // compilation mode.
@@ -288,3 +293,4 @@
 // DASM2-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (device-[[T]], [[ARCH2]])
 // DASM2_NV-DAG: [[P8:[0-9]+]]: backend, {[[P7]]}, assembler, (device-[[T]], [[ARCH2]])
 // DASM2_NV-DAG: [[P9:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH2]])" {[[P8]]}, assembler
+// DASM2-NOT: host
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2616,17 +2616,19 @@
 C.MakeAction(CudaDeviceActions,
 types::TY_HIP_FATBIN);
 
-DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
-   AssociatedOffloadKind);
-// Clear the fat binary, it is already a dependence to an host
-// action.
-CudaFatBinary = nullptr;
+if (!CompileDeviceOnly) {
+  DA.add(*CudaFatBinary, *ToolChains.front(), /*BoundArch=*/nullptr,
+ AssociatedOffloadKind);
+  // Clear the fat binary, it is already a dependence to an host
+  // action.
+  CudaFatBinary = nullptr;
+}
 
 // Remove the CUDA actions as they are already connected to an host
 // action or fat binary.
 CudaDeviceActions.clear();
 
-return ABRT_Success;
+return CompileDeviceOnly ? ABRT_Ignore_Host : ABRT_Success;
   } else if (CurPhase == phases::Link) {
 // Save CudaDeviceActions to DeviceLinkerInputs for each GPU subarch.
 // This happens to each device action originated from each input file.
@@ -3014,8 +3016,10 @@
 }
 
 // If we can use the bundler, replace the host action by the bundling one in
-// the resulting list. Otherwise, just append the device actions.
-if (CanUseBundler && !OffloadAL.empty()) {
+// the resulting list. Otherwise, just append the device actions. For
+// device only compilation, HostAction is a null pointer, therefore only do
+// this when HostAction is n

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Thanks for the review everyone!

Let me know if there are any further changes that you want me to make or any 
further action required on my part to land this 😁


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 173936.
stephanemoore added a comment.

Reworded "upper camel case" to "Pascal case".


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+bool IsPrime(int a);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'IsPrime' not using function naming conventions described by Google Objective-C style guide
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -122,6 +122,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in Pascal case. Functions whose storage class is
+not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.

[PATCH] D53770: Support g++ headers in include/g++

[David Greene via Phabricator via cfe-commits]

greened added a comment.

Oops, I didn't realize this hadn't been formally accepted yet.  Still learning 
the Phab process.  Let me know if you want it reverted for a formal accept.


Repository:
  rL LLVM

https://reviews.llvm.org/D53770



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

[PATCH] D54493: [OPENMP] Check target architecture supports unified shared memory for requires directive

[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


Repository:
  rC Clang

https://reviews.llvm.org/D54493



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

r346802 - [Driver] Support g++ headers in include/g++

[David Greene via cfe-commits]

Author: greened
Date: Tue Nov 13 13:38:45 2018
New Revision: 346802

URL: http://llvm.org/viewvc/llvm-project?rev=346802&view=rev
Log:
[Driver] Support g++ headers in include/g++

ray's gcc installation puts C++ headers in PREFIX/include/g++ without
indicating a gcc version at all. Typically this is because the version
is encoded somewhere in PREFIX.

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


Added:
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/g++/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/g++/backward/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/g++/backward/.keep
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/c++/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/c++/4.8/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/c++/4.8/.keep
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/x86_64-suse-linux/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/x86_64-suse-linux/8.2.0/

cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o
Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/test/Driver/linux-header-search.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=346802&r1=346801&r2=346802&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Nov 13 13:38:45 2018
@@ -930,6 +930,9 @@ void Linux::addLibStdCxxIncludePaths(con
   // Freescale SDK C++ headers are directly in /usr/include/c++,
   // without a subdirectory corresponding to the gcc version.
   LibDir.str() + "/../include/c++",
+  // Cray's gcc installation puts headers under "g++" without a
+  // version suffix.
+  LibDir.str() + "/../include/g++",
   };
 
   for (const auto &IncludePath : LibStdCXXIncludePathCandidates) {

Added: 
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/g++/backward/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/g%2B%2B/backward/.keep?rev=346802&view=auto
==
(empty)

Added: 
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o?rev=346802&view=auto
==
(empty)

Added: cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/c++/4.8/.keep
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/c%2B%2B/4.8/.keep?rev=346802&view=auto
==
(empty)

Added: 
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o?rev=346802&view=auto
==
(empty)

Modified: cfe/trunk/test/Driver/linux-header-search.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-header-search.cpp?rev=346802&r1=346801&r2=346802&view=diff
==
--- cfe/trunk/test/Driver/linux-header-search.cpp (original)
+++ cfe/trunk/test/Driv

[PATCH] D53770: Support g++ headers in include/g++

[David Greene via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346802: [Driver] Support g++ headers in include/g++ 
(authored by greened, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53770?vs=171801&id=173934#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53770

Files:
  cfe/trunk/lib/Driver/ToolChains/Linux.cpp
  
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/include/g++/backward/.keep
  
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o
  cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/include/c++/4.8/.keep
  
cfe/trunk/test/Driver/Inputs/cray_suse_gcc_tree/usr/lib/gcc/x86_64-suse-linux/8.2.0/crtbegin.o
  cfe/trunk/test/Driver/linux-header-search.cpp


Index: cfe/trunk/test/Driver/linux-header-search.cpp
===
--- cfe/trunk/test/Driver/linux-header-search.cpp
+++ cfe/trunk/test/Driver/linux-header-search.cpp
@@ -517,3 +517,15 @@
 // CHECK-OE-AARCH64: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-OE-AARCH64: "-internal-isystem" 
"[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../include/c++/6.3.0"
 // CHECK-OE-AARCH64: "-internal-isystem" 
"[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../include/c++/6.3.0/backward"
+
+// Check header search with Cray's gcc package.
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-unknown-linux-gnu -stdlib=libstdc++ \
+// RUN: --sysroot=%S/Inputs/cray_suse_gcc_tree \
+// RUN: --gcc-toolchain="%S/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos" \
+// RUN:   | FileCheck --check-prefix=CHECK-CRAY-X86 %s
+
+// CHECK-CRAY-X86: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-CRAY-X86: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-CRAY-X86: "-internal-isystem" 
"[[SYSROOT]]/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/../../../../include/g++"
+// CHECK-CRAY-X86: "-internal-isystem" 
"[[SYSROOT]]/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/../../../../include/g++/backward"
Index: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp
@@ -930,6 +930,9 @@
   // Freescale SDK C++ headers are directly in /usr/include/c++,
   // without a subdirectory corresponding to the gcc version.
   LibDir.str() + "/../include/c++",
+  // Cray's gcc installation puts headers under "g++" without a
+  // version suffix.
+  LibDir.str() + "/../include/g++",
   };
 
   for (const auto &IncludePath : LibStdCXXIncludePathCandidates) {


Index: cfe/trunk/test/Driver/linux-header-search.cpp
===
--- cfe/trunk/test/Driver/linux-header-search.cpp
+++ cfe/trunk/test/Driver/linux-header-search.cpp
@@ -517,3 +517,15 @@
 // CHECK-OE-AARCH64: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-OE-AARCH64: "-internal-isystem" "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../include/c++/6.3.0"
 // CHECK-OE-AARCH64: "-internal-isystem" "[[SYSROOT]]/usr/lib64/aarch64-oe-linux/6.3.0/../../../include/c++/6.3.0/backward"
+
+// Check header search with Cray's gcc package.
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN: -target x86_64-unknown-linux-gnu -stdlib=libstdc++ \
+// RUN: --sysroot=%S/Inputs/cray_suse_gcc_tree \
+// RUN: --gcc-toolchain="%S/Inputs/cray_suse_gcc_tree/opt/gcc/8.2.0/snos" \
+// RUN:   | FileCheck --check-prefix=CHECK-CRAY-X86 %s
+
+// CHECK-CRAY-X86: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-CRAY-X86: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-CRAY-X86: "-internal-isystem" "[[SYSROOT]]/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/../../../../include/g++"
+// CHECK-CRAY-X86: "-internal-isystem" "[[SYSROOT]]/opt/gcc/8.2.0/snos/lib/gcc/x86_64-suse-linux/8.2.0/../../../../include/g++/backward"
Index: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp
@@ -930,6 +930,9 @@
   // Freescale SDK C++ headers are directly in /usr/include/c++,
   // without a subdirectory corresponding to the gcc version.
   LibDir.str() + "/../include/c++",
+  // Cray's gcc installation puts headers under "g++" without a
+  // version suffix.
+  LibDir.str() + "/../include/g++",
   };
 
   for (const auto &IncludePath : LibStdCXXIncludePathCandidates) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346801 - [AST] Revert r346793 and r346781

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Nov 13 13:33:22 2018
New Revision: 346801

URL: http://llvm.org/viewvc/llvm-project?rev=346801&view=rev
Log:
[AST] Revert r346793 and r346781

This somehow breaks the msan bots. Revert while I figure it out.


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/Expr.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346801&r1=346800&r2=346801&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Nov 13 13:33:22 2018
@@ -1876,27 +1876,27 @@ private:
   unsigned Opc : 5;
   unsigned CanOverflow : 1;
   SourceLocation Loc;
-  Stmt *Operand;
-
+  Stmt *Val;
 public:
-  UnaryOperator(Expr *Operand, Opcode Opc, QualType Ty, ExprValueKind VK,
-ExprObjectKind OK, SourceLocation Loc, bool CanOverflow)
-  : Expr(UnaryOperatorClass, Ty, VK, OK,
- Operand->isTypeDependent() || Ty->isDependentType(),
- Operand->isValueDependent(),
- (Operand->isInstantiationDependent() ||
-  Ty->isInstantiationDependentType()),
- Operand->containsUnexpandedParameterPack()),
-Opc(Opc), CanOverflow(CanOverflow), Loc(Loc), Operand(Operand) {}
+  UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK,
+ExprObjectKind OK, SourceLocation l, bool CanOverflow)
+  : Expr(UnaryOperatorClass, type, VK, OK,
+ input->isTypeDependent() || type->isDependentType(),
+ input->isValueDependent(),
+ (input->isInstantiationDependent() ||
+  type->isInstantiationDependentType()),
+ input->containsUnexpandedParameterPack()),
+Opc(opc), CanOverflow(CanOverflow), Loc(l), Val(input) {}
 
   /// Build an empty unary operator.
-  explicit UnaryOperator(EmptyShell Empty) : Expr(UnaryOperatorClass, Empty) {}
+  explicit UnaryOperator(EmptyShell Empty)
+: Expr(UnaryOperatorClass, Empty), Opc(UO_AddrOf) { }
 
   Opcode getOpcode() const { return static_cast(Opc); }
   void setOpcode(Opcode O) { Opc = O; }
 
-  Expr *getSubExpr() const { return cast(Operand); }
-  void setSubExpr(Expr *E) { Operand = E; }
+  Expr *getSubExpr() const { return cast(Val); }
+  void setSubExpr(Expr *E) { Val = E; }
 
   /// getOperatorLoc - Return the location of the operator.
   SourceLocation getOperatorLoc() const { return Loc; }
@@ -1912,41 +1912,45 @@ public:
   void setCanOverflow(bool C) { CanOverflow = C; }
 
   /// isPostfix - Return true if this is a postfix operation, like x++.
-  static bool isPostfix(Opcode Opc) {
-return Opc == UO_PostInc || Opc == UO_PostDec;
+  static bool isPostfix(Opcode Op) {
+return Op == UO_PostInc || Op == UO_PostDec;
   }
 
   /// isPrefix - Return true if this is a prefix operation, like --x.
-  static bool isPrefix(Opcode Opc) {
-return Opc == UO_PreInc || Opc == UO_PreDec;
+  static bool isPrefix(Opcode Op) {
+return Op == UO_PreInc || Op == UO_PreDec;
   }
 
   bool isPrefix() const { return isPrefix(getOpcode()); }
   bool isPostfix() const { return isPostfix(getOpcode()); }
 
-  static bool isIncrementOp(Opcode Opc) {
-return Opc == UO_PreInc || Opc == UO_PostInc;
+  static bool isIncrementOp(Opcode Op) {
+return Op == UO_PreInc || Op == UO_PostInc;
+  }
+  bool isIncrementOp() const {
+return isIncrementOp(getOpcode());
   }
-  bool isIncrementOp() const { return isIncrementOp(getOpcode()); }
 
-  static bool isDecrementOp(Opcode Opc) {
-return Opc == UO_PreDec || Opc == UO_PostDec;
+  static bool isDecrementOp(Opcode Op) {
+return Op == UO_PreDec || Op == UO_PostDec;
+  }
+  bool isDecrementOp() const {
+return isDecrementOp(getOpcode());
   }
-  bool isDecrementOp() const { return isDecrementOp(getOpcode()); }
 
-  static bool isIncrementDecrementOp(Opcode Opc) { return Opc <= UO_PreDec; }
+  static bool isIncrementDecrementOp(Opcode Op) { return Op <= UO_PreDec; }
   bool isIncrementDecrementOp() const {
 return isIncrementDecrementOp(getOpcode());
   }
 
-  static bool isArithmeticOp(Opcode Opc) {
-return Opc >= UO_Plus && Opc <= UO_LNot;
+  static bool isArithmeticOp(Opcode Op) {
+return Op >= UO_Plus && Op <= UO_LNot;
   }
   bool isArithmeticOp() const { return isArithmeticOp(getOpcode()); }
 
   /// getOpcodeStr - Turn an Opcode enum value into the punctuation char it
   /// corresponds to, e.g. "sizeof" or "[pre]++"
-  static StringRef getOpcodeStr(Opcode Opc);
+  static StringRef getOpcodeStr(Opcode Op);
 
   /// Retrieve the unary opcode that corresponds to the given
   /// overloaded operator.
@@ -1957,21 +1961,21 @@ public:
   static OverloadedOperatorKind getOverloadedOperator(Opcode Opc);
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
-return isPostfix() ? Operand->getBeginLoc() : g

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 173932.
NoQ added a comment.

In https://reviews.llvm.org/D18860#1284805, @NoQ wrote:

> `RetainCountChecker` is affected, as demonstrated by the newly added test.


`RetainCountChecker` is affected due to temporary insanity in the checker that 
was induced by trusting summaries of inlined calls, which was reverted in 
https://reviews.llvm.org/D53902. I'll keep the FP/TN test, but i'll need to 
come up with a new test in order to test whatever i wanted to test, which will 
most likely be a unit test.


https://reviews.llvm.org/D18860

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  test/Analysis/MisusedMovedObject.cpp
  test/Analysis/keychainAPI.m
  test/Analysis/loop-block-counts.c
  test/Analysis/pr22954.c
  test/Analysis/retain-release-cpp-classes.cpp
  test/Analysis/self-assign.cpp
  test/Analysis/simple-stream-checks.c
  test/Analysis/unions.cpp

Index: test/Analysis/unions.cpp
===
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -90,9 +90,8 @@
 char str[] = "abc";
 vv.s = str;
 
-// FIXME: This is a leak of uu.s.
 uu = vv;
-  }
+  } // expected-warning{{leak}}
 
   void testIndirectInvalidation() {
 IntOrString uu;
Index: test/Analysis/simple-stream-checks.c
===
--- test/Analysis/simple-stream-checks.c
+++ test/Analysis/simple-stream-checks.c
@@ -89,3 +89,8 @@
   fs.p = fopen("myfile.txt", "w");
   fakeSystemHeaderCall(&fs); // invalidates fs, making fs.p unreachable
 }  // no-warning
+
+void testOverwrite() {
+  FILE *fp = fopen("myfile.txt", "w");
+  fp = 0;
+} // expected-warning {{Opened file is never closed; potential resource leak}}
Index: test/Analysis/self-assign.cpp
===
--- test/Analysis/self-assign.cpp
+++ test/Analysis/self-assign.cpp
@@ -32,13 +32,14 @@
   clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
   str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}  expected-note{{Use of memory after it is freed}}
+// expected-note@-1{{Memory is allocated}}
   return *this;
 }
 
 StringUsed& StringUsed::operator=(StringUsed &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
   clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
   str = rhs.str;
-  rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+  rhs.str = nullptr; // expected-warning{{Potential memory leak}} expected-note{{Potential memory leak}}
   return *this;
 }
 
@@ -83,7 +84,7 @@
 
 int main() {
   StringUsed s1 ("test"), s2;
-  s2 = s1;
-  s2 = std::move(s1);
+  s2 = s1; // expected-note{{Calling copy assignment operator for 'StringUsed'}} // expected-note{{Returned allocated memory}}
+  s2 = std::move(s1); // expected-note{{Calling move assignment operator for 'StringUsed'}}
   return 0;
 }
Index: test/Analysis/retain-release-cpp-classes.cpp
===
--- /dev/null
+++ test/Analysis/retain-release-cpp-classes.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-output=text -verify %s
+
+// expected-no-diagnostics
+
+typedef void *CFTypeRef;
+typedef struct _CFURLCacheRef *CFURLCacheRef;
+
+CFTypeRef CustomCFRetain(CFTypeRef);
+void invalidate(void *);
+struct S1 {
+  CFTypeRef s;
+  CFTypeRef returnFieldAtPlus0() {
+return s;
+  }
+};
+struct S2 {
+  S1 *s1;
+};
+void foo(S1 *s1) {
+  invalidate(s1);
+  S2 s2;
+  s2.s1 = s1;
+  CustomCFRetain(s1->returnFieldAtPlus0());
+
+  // Definitely no leak end-of-path note here. The retained pointer
+  // is still accessible through s1 and s2.
+  ((void) 0); // no-warning
+
+  // FIXME: Ideally we need to warn after this invalidate(). The per-function
+  // retain-release contract is violated: the programmer should release
+  // the symbol after it was retained, within the same function.
+ 

[PATCH] D54463: [CMake] Support cross-compiling with Fuchsia toolchain build

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rC Clang

https://reviews.llvm.org/D54463



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

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, makes sense. Maybe `static bool BlahBlahChecker::isApplicable(const 
LangOpts &LO)` or something like that.

Btw, maybe instead of default constructor and `->setup(Args)` method we could 
stuff all of the `setup(Args)`'s arguments into the one-and-only constructor 
for the checker?


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D54493: [OPENMP] Check target architecture supports unified shared memory for requires directive

[Patrick Lyster via Phabricator via cfe-commits]

patricklyster created this revision.
patricklyster added reviewers: ABataev, Hahnfeld, RaviNarayanaswamy, mikerice, 
kkwli0, hfinkel, gtbercea.
patricklyster added projects: OpenMP, clang.
Herald added subscribers: cfe-commits, guansong, jholewinski.

Restriction on `unified_shared_memory` clause on OpenMP5.0 `requires` directive 
states that target architecture must support unified addressing (>=sm_70). This 
patch implements this restriction.


Repository:
  rC Clang

https://reviews.llvm.org/D54493

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  clang/test/OpenMP/requires_codegen.cpp

Index: clang/test/OpenMP/requires_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/requires_codegen.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -DREGION_HOST
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_20 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_21 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_30 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_32 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_35 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_37 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_50 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_52 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_53 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_60 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_61 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_62 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_70 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE_NO_ERR
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_72 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE_NO_ERR
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -target-cpu sm_75 -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t-out.ll -DREGION_DEVICE_NO_ERR
+
+#if defined(REGION_HOST) || defined(REGION_DEVICE_NO_ERR)
+// expected-no-diagnostics
+#pragma omp requires unified_shared_memory
+#endif
+
+#ifdef REGION_DEVICE
+#pragma omp requires unified_shared_memory // expected-error {{Target architecture does not support unified addressing}} 
+#endif
Index: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- clang/lib/CodeGen/CGOpenMPRun

[PATCH] D53717: [AST] Only store the needed data in ForStmt.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno abandoned this revision.
riccibruno added a comment.
Herald added a reviewer: shafik.

Let's close it for now. I agree that the complexity is probably not worth it. 
We can always
resurrect it in the future. I will submit a patch doing the move into a new 
`StmtBase.h/.cpp`
after I am done with packing the other expression classes.


Repository:
  rC Clang

https://reviews.llvm.org/D53717



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

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus added a comment.

Thanks you so much! One of the reasons why I love working on this is because 
the great feedback I get. I don't wanna seem annoyingly grateful, but it's been 
a very enjoyable experience so far.

> This should not cause a warnings because it should be fine to compile 
> different translation units with different flags within the same project, 
> while the decision to enable the checker explicitly is done once per project.

Okay, I don't insist on it :)

> Now, normally these decisions are handled by the Clang Driver - the gcc 
> compatibility wrapper for clang that converts usual gcc-compatible run-lines 
> into `-cc1` run-lines. Eg., the `unix` package is only enabled on UNIX 
> targets and the `osx` package is only enabled on macOS/iOS targets and the 
> `core` package is enabled on all targets and `security.insecureAPI` is 
> disabled on PS4 //because that's what the Driver automatically appends to the 
> -cc1 run-line//. Even though `scan-build` does not call the analyzer through 
> the Driver, it invokes a `-###` run-line to obtain flags first, so it still 
> indirectly consults the Driver.
> 
> But the problem with the Driver is that nobody knows about this layer of 
> decision-making (unlike the obvious `Checkers.td`) and therefore it's kinda 
> messy to have all checkers hardcoded separately within the Driver. It kinda 
> makes more sense to split the checkers into packages and let the Driver 
> choose which packages are enabled.
>  So, generally, i suggest not to jump onto this as long as the analyzer as a 
> whole works more or less correctly. Restricting functionality for the purpose 
> of avoiding mistakes should be done with extreme care because, well, it 
> restricts functionality. Before restricting funcitonality, we need to be more 
> or less certain that nobody will ever need such functionality or that we will 
> be able to provide a safe replacement when necessary without also introducing 
> too much complexity (aka bugs). Which is why i recommend not to bother too 
> much about it. There are much more terrible bugs all over the place, no need 
> to struggle that hard to prevent these small bugs.
> 
> Another approach to the original problem would be to replace the language 
> options check in registerBlahBlahChecker() with a set of language options 
> checks in every checker callback. It's annoying to write and clutters the 
> checker but it preserves all the functionality without messing with the 
> registration process. So if you simply want to move these checks out of the 
> way of your work, this is probably the way to go.

Do you mean something along the lines of supplying a boolean 
`shouldRegister##CHECKERNAME` function to checkers? That would essentially 
achieve the same thing, but still allows me to move 
`CheckerManager::registerChecker` ultimately out of checker registry function.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: include/clang/AST/Expr.h:908-912
+  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+if (ConstantExpr *CE = dyn_cast(E))
+  return CE;
+return new (Context) ConstantExpr(E);
+  }

rsmith wrote:
> If we're creating two `ConstantExpr` wrappers for the same expression, that 
> seems like a bug in the caller to me. When does this happen?
I saw a place that was trying to do that, but it may no longer be valid. I can 
remove the check for now.



Comment at: lib/AST/Expr.cpp:2915-2916
+  case ConstantExprClass: {
+const Expr *Exp = cast(this)->getSubExpr();
+return Exp->isConstantInitializer(Ctx, false, Culprit);
+  }

rsmith wrote:
> Can we just return `true` here? If not, please add a FIXME saying that we 
> should be able to.
To be honest, I don't know. Theoretically we should be able to. Let me give it 
a try and see how it goes.



Comment at: lib/Sema/SemaExpr.cpp:14156-14157
 
+  if (!CurContext->isFunctionOrMethod())
+E = ConstantExpr::Create(Context, E);
+

rsmith wrote:
> I don't understand why `CurContext` matters here. Can you explain the intent 
> of this check?
It may have been an artifact of development. I can remove it.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173922.
void marked an inline comment as done.
void added a comment.

Remove some extraneous checks.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastExpr *ICE = dyn_cast(E))
   E = ICE->getSubExpr();
+else if (const ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (const SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5444,7 +5444,7 @@
 
 if (Notes.empty()) {
   // It's a constant expression.
-  return new (S.Context) ConstantExpr(Result.get());
+  return ConstantExpr::Crea

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In https://reviews.llvm.org/D54356#1297543, @rsmith wrote:

> In https://reviews.llvm.org/D54356#1297522, @void wrote:
>
> > Okay. I'll revert this then.
>
>
> I don't think we necessarily need the change in the other patch that's based 
> on this one, but I still think this refactoring is an improvement :)


Thanks. I can resurrect this after these changes go in. This will keep the 
resulting changes small. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void abandoned this revision.
void added a comment.

This isn't necessary. We can assume it's in a constant context because it's 
checking for an ICE.


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

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

rsmith added a comment.

In https://reviews.llvm.org/D54356#1297522, @void wrote:

> Okay. I'll revert this then.


I don't think we necessarily need the change in the other patch that's based on 
this one, but I still think this refactoring is an improvement :)


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

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

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

In https://reviews.llvm.org/D54356#1297506, @void wrote:

> This code is called in over 90 places, so it's hard to tell if they all are 
> in a constant context. Though I suppose that what this code is supposed to 
> check for would work the same in a constant context as without one. I can 
> revert this if you want, but to be honest the original function was 
> terrible--it's huge and hard to understand what's going on. As for adding new 
> expressions, this isn't the only place where a `StmtVisitor` is used. One 
> still needs to go through all of those visitors to see if they need to add 
> their expression.


Thinking about this some more: in the case where adding a new `Stmt` node 
without considering this code is likely to result in a silent and 
initially-unnoticed bug, I think it's useful to use one of our 
covered-switch-like patterns. But I don't think this actually is such a case; 
the C ICE rules are pretty conservative in what they allow, and new `Stmt` 
nodes should nearly always be treated as non-ICE. Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Okay. I'll revert this then.


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

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

rsmith added a comment.

In https://reviews.llvm.org/D54356#1297470, @rsmith wrote:

> Can you explain more about the justification for this? The code today has a 
> covered switch, which is useful for maintainability -- anyone adding a new 
> `Expr` node gets told they need to think about and update this code. Are 
> there any cases where we check for an ICE and aren't in a constant context? I 
> would have expected that the fact we're asking implies that we are in a 
> constant context (at least when the answer is "yes").


Oh, I see, you want to temporarily set "IsConstantContext" to true while 
evaluating the subexpression of a `ConstantExpr`. I think that's unnecessary, 
because anywhere we ask "is this syntactically an ICE?", we always want to 
evaluate as if in a constant context (unlike places where we run the evaluator 
on an expression, where that doesn't necessarily imply anything in particular 
about the nature of the expression.)


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54489: Implement -frecord-gcc-switches

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In https://reviews.llvm.org/D54489#1297504, @troyj wrote:

> I realize that you're probably striving for option compatibility with gcc, 
> but continuing to name it -frecord-gcc-switches when it actually records 
> Clang switches seems weird to me.  It almost sounds like something that would 
> dump gcc equivalents of all Clang options, or maybe let you know which Clang 
> options you've used that match gcc options.  Either way, by the name -- if 
> you aren't familiar with the gcc option -- it doesn't read like it records 
> Clang options.
>
> Would it be that bad to name it -frecord-clang-switches?  Or just 
> -frecord-switches?


I agree, and this was my original plan, but then I noticed that Clang already 
implements -grecord-gcc-switches and so I decided to mirror the naming for the 
-f variant as well.

If anything I think dropping the -gcc- altogether would make the most sense. I 
don't understand why GCC includes it in the first place.


Repository:
  rC Clang

https://reviews.llvm.org/D54489



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

This code is called in over 90 places, so it's hard to tell if they all are in 
a constant context. Though I suppose that what this code is supposed to check 
for would work the same in a constant context as without one. I can revert this 
if you want, but to be honest the original function was terrible--it's huge and 
hard to understand what's going on. As for adding new expressions, this isn't 
the only place where a `StmtVisitor` is used. One still needs to go through all 
of those visitors to see if they need to add their expression.


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54489: Implement -frecord-gcc-switches

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

I realize that you're probably striving for option compatibility with gcc, but 
continuing to name it -frecord-gcc-switches when it actually records Clang 
switches seems weird to me.  It almost sounds like something that would dump 
gcc equivalents of all Clang options, or maybe let you know which Clang options 
you've used that match gcc options.  Either way, by the name -- if you aren't 
familiar with the gcc option -- it doesn't read like it records Clang options.

Would it be that bad to name it -frecord-clang-switches?  Or just 
-frecord-switches?


Repository:
  rC Clang

https://reviews.llvm.org/D54489



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

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

rsmith added inline comments.



Comment at: include/clang/AST/Expr.h:908-912
+  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+if (ConstantExpr *CE = dyn_cast(E))
+  return CE;
+return new (Context) ConstantExpr(E);
+  }

If we're creating two `ConstantExpr` wrappers for the same expression, that 
seems like a bug in the caller to me. When does this happen?



Comment at: lib/AST/Expr.cpp:2915-2916
+  case ConstantExprClass: {
+const Expr *Exp = cast(this)->getSubExpr();
+return Exp->isConstantInitializer(Ctx, false, Culprit);
+  }

Can we just return `true` here? If not, please add a FIXME saying that we 
should be able to.



Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+  Arg = ConstantExpr::Create(Context, Arg);
+

We should create this node as part of checking that the argument is an ICE (in 
`SemaBuiltinConstantArg`).



Comment at: lib/Sema/SemaExpr.cpp:14156-14157
 
+  if (!CurContext->isFunctionOrMethod())
+E = ConstantExpr::Create(Context, E);
+

I don't understand why `CurContext` matters here. Can you explain the intent of 
this check?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

r346793 - [AST][NFC] Order the bit-field classes of Stmt like in StmtNodes.td

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Nov 13 12:23:11 2018
New Revision: 346793

URL: http://llvm.org/viewvc/llvm-project?rev=346793&view=rev
Log:
[AST][NFC] Order the bit-field classes of Stmt like in StmtNodes.td

Reorder the bit-field classes and the members of the anonymous union
so that they both match the order in StmtNodes.td.

There is already a fair amount of them, and this is not going to
improve. Therefore lets try to keep some order here.

Strictly NFC.


Modified:
cfe/trunk/include/clang/AST/Stmt.h

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=346793&r1=346792&r2=346793&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Tue Nov 13 12:23:11 2018
@@ -332,12 +332,20 @@ protected:
 SourceLocation Loc;
   };
 
-  class CharacterLiteralBitfields {
-friend class CharacterLiteral;
+  class DeclRefExprBitfields {
+friend class ASTStmtReader; // deserialization
+friend class DeclRefExpr;
 
 unsigned : NumExprBits;
 
-unsigned Kind : 3;
+unsigned HasQualifier : 1;
+unsigned HasTemplateKWAndArgsInfo : 1;
+unsigned HasFoundDecl : 1;
+unsigned HadMultipleCandidates : 1;
+unsigned RefersToEnclosingVariableOrCapture : 1;
+
+/// The location of the declaration name itself.
+SourceLocation Loc;
   };
 
   enum APFloatSemantics {
@@ -358,6 +366,14 @@ protected:
 unsigned IsExact : 1;
   };
 
+  class CharacterLiteralBitfields {
+friend class CharacterLiteral;
+
+unsigned : NumExprBits;
+
+unsigned Kind : 3;
+  };
+
   class UnaryExprOrTypeTraitExprBitfields {
 friend class UnaryExprOrTypeTraitExpr;
 
@@ -367,20 +383,12 @@ protected:
 unsigned IsType : 1; // true if operand is a type, false if an expression.
   };
 
-  class DeclRefExprBitfields {
-friend class ASTStmtReader; // deserialization
-friend class DeclRefExpr;
+  class CallExprBitfields {
+friend class CallExpr;
 
 unsigned : NumExprBits;
 
-unsigned HasQualifier : 1;
-unsigned HasTemplateKWAndArgsInfo : 1;
-unsigned HasFoundDecl : 1;
-unsigned HadMultipleCandidates : 1;
-unsigned RefersToEnclosingVariableOrCapture : 1;
-
-/// The location of the declaration name itself.
-SourceLocation Loc;
+unsigned NumPreArgs : 1;
   };
 
   class CastExprBitfields {
@@ -394,24 +402,14 @@ protected:
 unsigned BasePathIsEmpty : 1;
   };
 
-  class CallExprBitfields {
-friend class CallExpr;
-
-unsigned : NumExprBits;
-
-unsigned NumPreArgs : 1;
-  };
-
-  class ExprWithCleanupsBitfields {
-friend class ASTStmtReader; // deserialization
-friend class ExprWithCleanups;
+  class InitListExprBitfields {
+friend class InitListExpr;
 
 unsigned : NumExprBits;
 
-// When false, it must not have side effects.
-unsigned CleanupsHaveSideEffects : 1;
-
-unsigned NumObjects : 32 - 1 - NumExprBits;
+/// Whether this initializer list originally had a GNU array-range
+/// designator in it. This is a temporary marker used by CodeGen.
+unsigned HadArrayRangeDesignator : 1;
   };
 
   class PseudoObjectExprBitfields {
@@ -426,33 +424,7 @@ protected:
 unsigned ResultIndex : 32 - 8 - NumExprBits;
   };
 
-  class OpaqueValueExprBitfields {
-friend class OpaqueValueExpr;
-
-unsigned : NumExprBits;
-
-/// The OVE is a unique semantic reference to its source expressio if this
-/// bit is set to true.
-unsigned IsUnique : 1;
-  };
-
-  class ObjCIndirectCopyRestoreExprBitfields {
-friend class ObjCIndirectCopyRestoreExpr;
-
-unsigned : NumExprBits;
-
-unsigned ShouldCopy : 1;
-  };
-
-  class InitListExprBitfields {
-friend class InitListExpr;
-
-unsigned : NumExprBits;
-
-/// Whether this initializer list originally had a GNU array-range
-/// designator in it. This is a temporary marker used by CodeGen.
-unsigned HadArrayRangeDesignator : 1;
-  };
+  //===--- C++ Expression bitfields classes ---===//
 
   class TypeTraitExprBitfields {
 friend class ASTStmtReader;
@@ -472,6 +444,20 @@ protected:
 unsigned NumArgs : 32 - 8 - 1 - NumExprBits;
   };
 
+  class ExprWithCleanupsBitfields {
+friend class ASTStmtReader; // deserialization
+friend class ExprWithCleanups;
+
+unsigned : NumExprBits;
+
+// When false, it must not have side effects.
+unsigned CleanupsHaveSideEffects : 1;
+
+unsigned NumObjects : 32 - 1 - NumExprBits;
+  };
+
+  //===--- C++ Coroutines TS bitfields classes ---===//
+
   class CoawaitExprBitfields {
 friend class CoawaitExpr;
 
@@ -480,7 +466,30 @@ protected:
 unsigned IsImplicit : 1;
   };
 
+  //===--- Obj-C Expression bitfields classes ---===//
+
+  class ObjCIndirectCopyRestoreExprBitfields {
+friend class ObjCIndirectCopyRestoreExpr;
+
+unsigned : NumExprBits;
+
+unsigned ShouldCopy 

[PATCH] D54356: Convert CheckICE into a statment visitor

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

rsmith added a comment.

Can you explain more about the justification for this? The code today has a 
covered switch, which is useful for maintainability -- anyone adding a new 
`Expr` node gets told they need to think about and update this code. Are there 
any cases where we check for an ICE and aren't in a constant context? I would 
have expected that the fact we're asking implies that we are in a constant 
context (at least when the answer is "yes").


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

This new flag inhibits the warnings from -Wuninitialized, right? 
While this is fine for experimenting (and I want to have this in ToT to enable 
wide experimentation)
we should clearly state (in the comments) that the final intent is to make the 
feature work together with -Wuninitialized.

Another thing that we will need (and not necessary in the first change) is to 
have fine grained controls over what we zero-initialize. 
We may want to make separate decisions for pointer/non-pointer scalars, PODs, 
arrays of {scalars, pointers, PODs}, etc.


Repository:
  rC Clang

https://reviews.llvm.org/D54473



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

[PATCH] D54243: DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Settled on renaming the flag for consistency with, say, -fdebug-types-section, 
to -fdebug-ranges-base-address (though 'debug' is sort of ambiguous (Clang can 
produce non-DWARF debug info) but this driver (as opposed to clang-cl) is 
intended for DWARF emission, not PDB emission - and the "debug-ranges" part of 
the flag names the literal debug_ranges section (like debug-types refers to the 
debug_types section)).


Repository:
  rL LLVM

https://reviews.llvm.org/D54243



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

[PATCH] D54243: DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

[David Blaikie via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346789: DebugInfo: Add a driver flag for DWARF debug_ranges 
base address specifier use. (authored by dblaikie, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54243?vs=173096&id=173905#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54243

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/debug-info-ranges-base-address.c
  cfe/trunk/test/Driver/debug-options.c

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -651,6 +651,7 @@
   : Args.hasArg(OPT_gpubnames)
 ? llvm::DICompileUnit::DebugNameTableKind::Default
 : llvm::DICompileUnit::DebugNameTableKind::None);
+  Opts.DebugRangesBaseAddress = Args.hasArg(OPT_fdebug_ranges_base_address);
 
   setPGOInstrumentor(Opts, Args, Diags);
   Opts.InstrProfileOutput =
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -586,7 +586,8 @@
   CGM.getTarget().getTriple().isNVPTX()
   ? llvm::DICompileUnit::DebugNameTableKind::None
   : static_cast(
-CGOpts.DebugNameTable));
+CGOpts.DebugNameTable),
+  CGOpts.DebugRangesBaseAddress);
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const BuiltinType *BT) {
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3188,6 +3188,11 @@
 ? "-gpubnames"
 : "-ggnu-pubnames");
 
+  if (Args.hasFlag(options::OPT_fdebug_ranges_base_address,
+   options::OPT_fno_debug_ranges_base_address, false)) {
+CmdArgs.push_back("-fdebug-ranges-base-address");
+  }
+
   // -gdwarf-aranges turns on the emission of the aranges section in the
   // backend.
   // Always enabled for SCE tuning.
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -331,6 +331,9 @@
 /// Whether to emit .debug_gnu_pubnames section instead of .debug_pubnames.
 CODEGENOPT(DebugNameTable, 2, 0)
 
+/// Whether to use DWARF base address specifiers in .debug_ranges.
+CODEGENOPT(DebugRangesBaseAddress, 1, 0)
+
 CODEGENOPT(NoPLT, 1, 0)
 
 /// Whether to embed source in DWARF debug line section.
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1780,6 +1780,10 @@
   Flags<[CC1Option]>, HelpText<"Place debug types in their own section (ELF Only)">;
 def fno_debug_types_section: Flag<["-"], "fno-debug-types-section">, Group,
   Flags<[CC1Option]>;
+def fdebug_ranges_base_address: Flag <["-"], "fdebug-ranges-base-address">, Group,
+  Flags<[CC1Option]>, HelpText<"Use DWARF base address selection entries in debug_ranges">;
+def fno_debug_ranges_base_address: Flag <["-"], "fno-debug-ranges-base-address">, Group,
+  Flags<[CC1Option]>;
 def fsplit_dwarf_inlining: Flag <["-"], "fsplit-dwarf-inlining">, Group,
   Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the object/executable to facilitate online symbolication/stack traces in the absence of .dwo/.dwp files when using Split DWARF">;
 def fno_split_dwarf_inlining: Flag<["-"], "fno-split-dwarf-inlining">, Group,
Index: cfe/trunk/test/Driver/debug-options.c
===
--- cfe/trunk/test/Driver/debug-options.c
+++ cfe/trunk/test/Driver/debug-options.c
@@ -165,6 +165,10 @@
 // RUN: %clang -### -c -gsplit-dwarf %s 2>&1 | FileCheck -check-prefix=GPUB %s
 // RUN: %clang -### -c -gsplit-dwarf -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 //
+// RUN: %clang -### -c -fdebug-ranges-base-address %s 2>&1 | FileCheck -check-prefix=RNGBSE %s
+// RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
+// RUN: %clang -### -c -fdebug-ranges-base-address -fno-debug-ranges-base-address %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
+//
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=GPUB %s
 // RUN: %clang -### -c -glld

r346789 - DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Tue Nov 13 12:08:13 2018
New Revision: 346789

URL: http://llvm.org/viewvc/llvm-project?rev=346789&view=rev
Log:
DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

Summary:
This saves a lot of relocations in optimized object files (at the cost
of some cost/increase in linked executable bytes), but gold's 32 bit
gdb-index support has a bug (
https://sourceware.org/bugzilla/show_bug.cgi?id=21894 ) so we can't
switch to this unconditionally. (& even if it weren't for that bug, one
might argue that some users would want to optimize in one direction or
the other - prioritizing object size or linked executable size)

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

Added:
cfe/trunk/test/CodeGen/debug-info-ranges-base-address.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/debug-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=346789&r1=346788&r2=346789&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Nov 13 12:08:13 2018
@@ -1780,6 +1780,10 @@ def fdebug_types_section: Flag <["-"], "
   Flags<[CC1Option]>, HelpText<"Place debug types in their own section (ELF 
Only)">;
 def fno_debug_types_section: Flag<["-"], "fno-debug-types-section">, 
Group,
   Flags<[CC1Option]>;
+def fdebug_ranges_base_address: Flag <["-"], "fdebug-ranges-base-address">, 
Group,
+  Flags<[CC1Option]>, HelpText<"Use DWARF base address selection entries in 
debug_ranges">;
+def fno_debug_ranges_base_address: Flag <["-"], 
"fno-debug-ranges-base-address">, Group,
+  Flags<[CC1Option]>;
 def fsplit_dwarf_inlining: Flag <["-"], "fsplit-dwarf-inlining">, 
Group,
   Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the 
object/executable to facilitate online symbolication/stack traces in the 
absence of .dwo/.dwp files when using Split DWARF">;
 def fno_split_dwarf_inlining: Flag<["-"], "fno-split-dwarf-inlining">, 
Group,

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=346789&r1=346788&r2=346789&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Tue Nov 13 12:08:13 2018
@@ -331,6 +331,9 @@ CODEGENOPT(PreserveVec3Type, 1, 0)
 /// Whether to emit .debug_gnu_pubnames section instead of .debug_pubnames.
 CODEGENOPT(DebugNameTable, 2, 0)
 
+/// Whether to use DWARF base address specifiers in .debug_ranges.
+CODEGENOPT(DebugRangesBaseAddress, 1, 0)
+
 CODEGENOPT(NoPLT, 1, 0)
 
 /// Whether to embed source in DWARF debug line section.

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=346789&r1=346788&r2=346789&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Nov 13 12:08:13 2018
@@ -586,7 +586,8 @@ void CGDebugInfo::CreateCompileUnit() {
   CGM.getTarget().getTriple().isNVPTX()
   ? llvm::DICompileUnit::DebugNameTableKind::None
   : static_cast(
-CGOpts.DebugNameTable));
+CGOpts.DebugNameTable),
+  CGOpts.DebugRangesBaseAddress);
 }
 
 llvm::DIType *CGDebugInfo::CreateType(const BuiltinType *BT) {

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=346789&r1=346788&r2=346789&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Nov 13 12:08:13 2018
@@ -3188,6 +3188,11 @@ static void RenderDebugOptions(const Too
 ? "-gpubnames"
 : "-ggnu-pubnames");
 
+  if (Args.hasFlag(options::OPT_fdebug_ranges_base_address,
+   options::OPT_fno_debug_ranges_base_address, false)) {
+CmdArgs.push_back("-fdebug-ranges-base-address");
+  }
+
   // -gdwarf-aranges turns on the emission of the aranges section in the
   // backend.
   // Always enabled for SCE tuning.

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=346789&r1=346788&r2=346789&view=diff
==

[PATCH] D54489: Implement -frecord-gcc-switches

[Scott Linder via Phabricator via cfe-commits]

scott.linder created this revision.
Herald added a subscriber: cfe-commits.

See the parent revision https://reviews.llvm.org/D54487 for differences between 
this implementation and the equivalent GCC option.


Repository:
  rC Clang

https://reviews.llvm.org/D54489

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -538,3 +538,10 @@
 // RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s
 // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
 // CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg'
+
+// RUN: %clang -### -S -frecord-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -fno-record-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -fno-record-gcc-switches -frecord-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES %s
+// RUN: %clang -### -S -frecord-gcc-switches -fno-record-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RECORD-GCC-SWITCHES %s
+// CHECK-RECORD-GCC-SWITCHES: "-record-command-line"
+// CHECK-NO-RECORD-GCC-SWITCHES-NOT: "-record-command-line"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -627,6 +627,7 @@
   Args.hasFlag(OPT_ffine_grained_bitfield_accesses,
OPT_fno_fine_grained_bitfield_accesses, false);
   Opts.DwarfDebugFlags = Args.getLastArgValue(OPT_dwarf_debug_flags);
+  Opts.RecordCommandLine = Args.getLastArgValue(OPT_record_command_line);
   Opts.MergeAllConstants = Args.hasArg(OPT_fmerge_all_constants);
   Opts.NoCommon = Args.hasArg(OPT_fno_common);
   Opts.NoImplicitFloat = Args.hasArg(OPT_no_implicit_float);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4985,14 +4985,18 @@
 
   const char *Exec = D.getClangProgramPath();
 
-  // Optionally embed the -cc1 level arguments into the debug info, for build
-  // analysis.
+  // Optionally embed the -cc1 level arguments into the debug info or a
+  // section, for build analysis.
   // Also record command line arguments into the debug info if
   // -grecord-gcc-switches options is set on.
   // By default, -gno-record-gcc-switches is set on and no recording.
-  if (TC.UseDwarfDebugFlags() ||
+  auto GRecordSwitches =
   Args.hasFlag(options::OPT_grecord_gcc_switches,
-   options::OPT_gno_record_gcc_switches, false)) {
+   options::OPT_gno_record_gcc_switches, false);
+  auto FRecordSwitches =
+  Args.hasFlag(options::OPT_frecord_gcc_switches,
+   options::OPT_fno_record_gcc_switches, false);
+  if (TC.UseDwarfDebugFlags() || GRecordSwitches || FRecordSwitches) {
 ArgStringList OriginalArgs;
 for (const auto &Arg : Args)
   Arg->render(Args, OriginalArgs);
@@ -5005,8 +5009,15 @@
   Flags += " ";
   Flags += EscapedArg;
 }
-CmdArgs.push_back("-dwarf-debug-flags");
-CmdArgs.push_back(Args.MakeArgString(Flags));
+auto FlagsArgString = Args.MakeArgString(Flags);
+if (TC.UseDwarfDebugFlags() || GRecordSwitches) {
+  CmdArgs.push_back("-dwarf-debug-flags");
+  CmdArgs.push_back(FlagsArgString);
+}
+if (FRecordSwitches) {
+  CmdArgs.push_back("-record-command-line");
+  CmdArgs.push_back(FlagsArgString);
+}
   }
 
   // Host-side cuda compilation receives all device-side outputs in a single
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1402,6 +1402,9 @@
   /// Emit the Clang version as llvm.ident metadata.
   void EmitVersionIdentMetadata();
 
+  /// Emit the Clang commandline as llvm.commandline metadata.
+  void EmitCommandLineMetadata();
+
   /// Emits target specific Metadata for global declarations.
   void EmitTargetMetadata();
 
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -587,6 +587,9 @@
   if (getCodeGenOpts().EmitVersionIdentMetadata)
 EmitVersionIdentMetadata();
 
+  if (!getCodeGenOpts().RecordCommandLine.empty())
+EmitCommandLineMetadata();
+
   EmitTargetMeta

[PATCH] D54488: [AST] [analyzer] NFC: Reuse code in stable ID dumping methods.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: george.karpenkov, rsmith.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

Use the new fancy method introduced in https://reviews.llvm.org/D54486 to 
simplify some code.


Repository:
  rC Clang

https://reviews.llvm.org/D54488

Files:
  lib/AST/DeclBase.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/Stmt.cpp
  lib/StaticAnalyzer/Core/ExplodedGraph.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -70,10 +70,7 @@
 }
 
 int64_t ProgramState::getID() const {
-  Optional Out = getStateManager().Alloc.identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(ProgramState) == 0 && "Wrong alignment information");
-  return *Out / alignof(ProgramState);
+  return 
getStateManager().Alloc.identifyKnownAlignedObject(this);
 }
 
 ProgramStateManager::ProgramStateManager(ASTContext &Ctx,
Index: lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -284,10 +284,7 @@
 }
 
 int64_t ExplodedNode::getID(ExplodedGraph *G) const {
-  Optional Out = G->getAllocator().identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(ExplodedNode) == 0 && "Wrong alignment information");
-  return *Out / alignof(ExplodedNode);
+  return G->getAllocator().identifyKnownAlignedObject(this);
 }
 
 bool ExplodedNode::isTrivial() const {
Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -303,10 +303,7 @@
 }
 
 int64_t Stmt::getID(const ASTContext &Context) const {
-  Optional Out = Context.getAllocator().identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(Stmt) == 0 && "Wrong alignment information");
-  return *Out / alignof(Stmt);
+  return Context.getAllocator().identifyKnownAlignedObject(this);
 }
 
 CompoundStmt::CompoundStmt(ArrayRef Stmts, SourceLocation LB,
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -2247,11 +2247,8 @@
   IsDelegating(true), IsVirtual(false), IsWritten(false), SourceOrder(0) {}
 
 int64_t CXXCtorInitializer::getID(const ASTContext &Context) const {
-  Optional Out = Context.getAllocator().identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(CXXCtorInitializer) == 0 &&
- "Wrong alignment information");
-  return *Out / alignof(CXXCtorInitializer);
+  return Context.getAllocator()
+.identifyKnownAlignedObject(this);
 }
 
 TypeLoc CXXCtorInitializer::getBaseClassLoc() const {
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -955,10 +955,7 @@
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
 int64_t Decl::getID() const {
-  Optional Out = getASTContext().getAllocator().identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(Decl) == 0 && "Wrong alignment information");
-  return *Out / alignof(Decl);
+  return getASTContext().getAllocator().identifyKnownAlignedObject(this);
 }
 
 const FunctionType *Decl::getFunctionType(bool BlocksToo) const {


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -70,10 +70,7 @@
 }
 
 int64_t ProgramState::getID() const {
-  Optional Out = getStateManager().Alloc.identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(ProgramState) == 0 && "Wrong alignment information");
-  return *Out / alignof(ProgramState);
+  return getStateManager().Alloc.identifyKnownAlignedObject(this);
 }
 
 ProgramStateManager::ProgramStateManager(ASTContext &Ctx,
Index: lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -284,10 +284,7 @@
 }
 
 int64_t ExplodedNode::getID(ExplodedGraph *G) const {
-  Optional Out = G->getAllocator().identifyObject(this);
-  assert(Out && "Wrong allocator used");
-  assert(*Out % alignof(ExplodedNode) == 0 && "Wrong alignment information");
-  return *Out / alignof(ExplodedNode);
+  return G->getAllocator().identifyKnownAlignedObject(this);
 }
 
 bool ExplodedNode::isTrivial() const {
Index: lib/AST/Stmt.cpp
=

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

The test case I've promised is

  touch a.h
  touch b.h
  touch c.h
  ln -s b.h d.h
  
  echo '#include "a.h"' > umbrella.h
  echo '#include "b.h"' >> umbrella.h
  
  echo '#include "b.h"' > test_case.c
  
  cat < module.modulemap
  module my_module {
umbrella header "umbrella.h" export *
header "c.h" export *
header "d.h" export *
  }
  EOF
  
  clang -cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=./cache 
./test_case.c -MT test_case.o -dependency-file -

Expected b.h to be present in `test_case.o` dependencies but it's not.

Keep in mind this test case can be unrelated to your change.


https://reviews.llvm.org/D53522



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

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D54438#1296832, @Szelethus wrote:

> - Some checkers do not register themselves in all cases[1], which should 
> probably be handled at a higher level, preferably in `Checkers.td`. Warnings 
> could be emitted when a checker is explicitly enabled but will not be 
> registered. [1]




In https://reviews.llvm.org/D54438#1296832, @Szelethus wrote:

>   void ento::registerObjCDeallocChecker(CheckerManager &Mgr) {
> const LangOptions &LangOpts = Mgr.getLangOpts();
> // These checker only makes sense under MRR.
> if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount)
>   return;
>  
> Mgr.registerChecker();
>   }
>


The can of worms here is as follows.

It is clear that not all checkers are applicable to all languages or all 
combinations of compiler flags. In this case, significantly different semantics 
are observed under different Objective-C memory management policies and the 
checker isn't designed to work correctly for anything but the "manual" memory 
management policy. The way the code is now written, it is impossible to 
register the checker for translation units that are compiled with incompatible 
flags.

This should not cause a warnings because it should be fine to compile different 
translation units with different flags within the same project, while the 
decision to enable the checker explicitly is done once per project.

Now, normally these decisions are handled by the Clang Driver - the gcc 
compatibility wrapper for clang that converts usual gcc-compatible run-lines 
into `-cc1` run-lines. Eg., the `unix` package is only enabled on UNIX targets 
and the `osx` package is only enabled on macOS/iOS targets and the `core` 
package is enabled on all targets and `security.insecureAPI` is disabled on PS4 
//because that's what the Driver automatically appends to the -cc1 run-line//. 
Even though `scan-build` does not call the analyzer through the Driver, it 
invokes a `-###` run-line to obtain flags first, so it still indirectly 
consults the Driver.

But the problem with the Driver is that nobody knows about this layer of 
decision-making (unlike the obvious `Checkers.td`) and therefore it's kinda 
messy to have all checkers hardcoded separately within the Driver. It kinda 
makes more sense to split the checkers into packages and let the Driver choose 
which packages are enabled.

But the problem with packages is that they are very hard to change because they 
are the UI that external GUIs rely upon. And right now we don't yet have 
separate packages for Objective-C MRR/ARC/GC modes (btw, GC needs to be 
removed).

And even if we were to go in that direction, the amount of packages would 
explode exponentially with the number of different independent flags we need to 
track. If only we had hashtags, that would have been a great direction to go.

So, generally, i suggest not to jump onto this as long as the analyzer as a 
whole works more or less correctly. Restricting functionality for the purpose 
of avoiding mistakes should be done with extreme care because, well, it 
restricts functionality. Before restricting funcitonality, we need to be more 
or less certain that nobody will ever need such functionality or that we will 
be able to provide a safe replacement when necessary without also introducing 
too much complexity (aka bugs). Which is why i recommend not to bother too much 
about it. There are much more terrible bugs all over the place, no need to 
struggle that hard to prevent these small bugs.

Another approach to the original problem would be to replace the language 
options check in registerBlahBlahChecker() with a set of language options 
checks in every checker callback. It's annoying to write and clutters the 
checker but it preserves all the functionality without messing with the 
registration process. So if you simply want to move these checks out of the way 
of your work, this is probably the way to go.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

Quuxplusone wrote:
> rjmccall wrote:
> > Quuxplusone wrote:
> > > @rjmccall wrote:
> > > > trivial_abi permits annotated types to be passed and returned in 
> > > > registers, which is ABI-breaking. Skimming the blog post, it looks like 
> > > > trivially_relocatable does not permit this — it merely signifies that 
> > > > destruction is a no-op after a move construction or assignment.
> > > 
> > > Not necessarily a "no-op"; my canonical example is a 
> > > CopyOnlyCXX03SharedPtr which increments a refcount on construction and 
> > > decrements on destruction. But move-construction plus destruction should 
> > > "balance out" and result in no observable side effects.
> > > 
> > > > This is usefully different in the design space, since it means you can 
> > > > safely add the attribute retroactively to e.g. std::unique_ptr, and 
> > > > other templates can then detect that std::unique_ptr is 
> > > > trivially-relocatable and optimize themselves to use memcpy or realloc 
> > > > or whatever it is that they want to do. So in that sense trivial_abi is 
> > > > a *stronger* attribute, not a *weaker* one: the property it determines 
> > > > ought to imply trivially_relocatable.
> > > 
> > > `trivial_abi` is an "orthogonal" attribute: you can have `trivial_abi` 
> > > types with non-trivial constructors and destructors, which can have 
> > > observable side effects. For example,
> > > ```
> > > struct [[clang::trivial_abi]] DestructionAnnouncer {
> > > ~DestructionAnnouncer() { puts("hello!"); }
> > > };
> > > ```
> > > is `trivial_abi` (because of the annotation) yet not trivially 
> > > relocatable, because its "move plus destroy" operation has observable 
> > > side effects.
> > > 
> > > > The only interesting question in the language design that I know of is 
> > > > what happens if you put the attribute on a template that's instantiated 
> > > > to contain a sub-object that is definitely not trivially relocatable / 
> > > > trivial-ABI. For trivial_abi, we decided that the attribute is simply 
> > > > ignored — it implicitly only applies to specializations where the 
> > > > attribute would be legal. I haven't dug into the design enough to know 
> > > > what trivially_relocatable decides in this situation, but the three 
> > > > basic options are:
> > > >
> > > > - the attribute always has effect and allows trivial relocation 
> > > > regardless of the subobject types; this is obviously unsafe, so it 
> > > > limits the safe applicability of the attribute to templates
> > > > - the attribute is ignored, like trivial_abi is
> > > > - the attribute is ill-formed, and you'll need to add a 
> > > > [[trivially_relocatable(bool)]] version to support templates
> > > 
> > > What happens is basically the first thing you said, except that I 
> > > disagree that it's "obviously unsafe." Right now, conditionally trivial 
> > > relocation is possible via template metaprogramming; see the libcxx patch 
> > > at e.g.
> > > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > > Since the attribute is an opt-in mechanism, it makes perfect sense to me 
> > > that if you put it on a class (or class template), then it applies to the 
> > > class, without any further sanity-checking by the compiler. The compiler 
> > > has no reason to second-guess the programmer here.
> > > 
> > > However, there's one more interesting case. Suppose the programmer puts 
> > > the attribute on a class that isn't relocatable at all! (For example, the 
> > > union case @erichkeane mentioned, or a class type with a deleted 
> > > destructor.) In that case, this patch *does* give an error... *unless* 
> > > the class was produced by instantiating a template, in which case we 
> > > *don't* give an error, because it's not the template-writer's fault.
> > > https://p1144.godbolt.org/z/wSZPba
> > > trivial_abi is an "orthogonal" attribute: you can have trivial_abi types 
> > > with non-trivial constructors and destructors, which can have observable 
> > > side effects. 
> > 
> > Let me cut this conversation short.  `trivial_abi` is not such an old and 
> > widely-established attribute that we are unable to revise its definition.  
> > I am comfortable making the same semantic guarantees for `trivial_abi` that 
> > you're making for `trivially_relocatable`, because I think it is in the 
> > language's interest for `trivial_abi` to be strictly stronger than 
> > `trivially_relocatable`.
> > 
> > > What happens is basically the first thing you said, except that I 
> > > disagree that it's "obviously unsafe." 
> > 
> > Under your semantics, the attribute is an unchecked assertion about all of 
> > a class's subobjects.  A class template which fails to correctl

r346781 - [AST][NFC] Style fixes for UnaryOperator

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Nov 13 11:27:39 2018
New Revision: 346781

URL: http://llvm.org/viewvc/llvm-project?rev=346781&view=rev
Log:
[AST][NFC] Style fixes for UnaryOperator

In preparation for the patch which will move some data to the bit-fields
of Stmt. In particular, rename the private variable "Val" -> "Operand"
since the substatement is the operand of the unary operator.
Run clang-format on UnaryOperator. NFC otherwise.


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Expr.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346781&r1=346780&r2=346781&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Nov 13 11:27:39 2018
@@ -1876,27 +1876,27 @@ private:
   unsigned Opc : 5;
   unsigned CanOverflow : 1;
   SourceLocation Loc;
-  Stmt *Val;
+  Stmt *Operand;
+
 public:
-  UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK,
-ExprObjectKind OK, SourceLocation l, bool CanOverflow)
-  : Expr(UnaryOperatorClass, type, VK, OK,
- input->isTypeDependent() || type->isDependentType(),
- input->isValueDependent(),
- (input->isInstantiationDependent() ||
-  type->isInstantiationDependentType()),
- input->containsUnexpandedParameterPack()),
-Opc(opc), CanOverflow(CanOverflow), Loc(l), Val(input) {}
+  UnaryOperator(Expr *Operand, Opcode Opc, QualType Ty, ExprValueKind VK,
+ExprObjectKind OK, SourceLocation Loc, bool CanOverflow)
+  : Expr(UnaryOperatorClass, Ty, VK, OK,
+ Operand->isTypeDependent() || Ty->isDependentType(),
+ Operand->isValueDependent(),
+ (Operand->isInstantiationDependent() ||
+  Ty->isInstantiationDependentType()),
+ Operand->containsUnexpandedParameterPack()),
+Opc(Opc), CanOverflow(CanOverflow), Loc(Loc), Operand(Operand) {}
 
   /// Build an empty unary operator.
-  explicit UnaryOperator(EmptyShell Empty)
-: Expr(UnaryOperatorClass, Empty), Opc(UO_AddrOf) { }
+  explicit UnaryOperator(EmptyShell Empty) : Expr(UnaryOperatorClass, Empty) {}
 
   Opcode getOpcode() const { return static_cast(Opc); }
   void setOpcode(Opcode O) { Opc = O; }
 
-  Expr *getSubExpr() const { return cast(Val); }
-  void setSubExpr(Expr *E) { Val = E; }
+  Expr *getSubExpr() const { return cast(Operand); }
+  void setSubExpr(Expr *E) { Operand = E; }
 
   /// getOperatorLoc - Return the location of the operator.
   SourceLocation getOperatorLoc() const { return Loc; }
@@ -1912,45 +1912,41 @@ public:
   void setCanOverflow(bool C) { CanOverflow = C; }
 
   /// isPostfix - Return true if this is a postfix operation, like x++.
-  static bool isPostfix(Opcode Op) {
-return Op == UO_PostInc || Op == UO_PostDec;
+  static bool isPostfix(Opcode Opc) {
+return Opc == UO_PostInc || Opc == UO_PostDec;
   }
 
   /// isPrefix - Return true if this is a prefix operation, like --x.
-  static bool isPrefix(Opcode Op) {
-return Op == UO_PreInc || Op == UO_PreDec;
+  static bool isPrefix(Opcode Opc) {
+return Opc == UO_PreInc || Opc == UO_PreDec;
   }
 
   bool isPrefix() const { return isPrefix(getOpcode()); }
   bool isPostfix() const { return isPostfix(getOpcode()); }
 
-  static bool isIncrementOp(Opcode Op) {
-return Op == UO_PreInc || Op == UO_PostInc;
-  }
-  bool isIncrementOp() const {
-return isIncrementOp(getOpcode());
+  static bool isIncrementOp(Opcode Opc) {
+return Opc == UO_PreInc || Opc == UO_PostInc;
   }
+  bool isIncrementOp() const { return isIncrementOp(getOpcode()); }
 
-  static bool isDecrementOp(Opcode Op) {
-return Op == UO_PreDec || Op == UO_PostDec;
-  }
-  bool isDecrementOp() const {
-return isDecrementOp(getOpcode());
+  static bool isDecrementOp(Opcode Opc) {
+return Opc == UO_PreDec || Opc == UO_PostDec;
   }
+  bool isDecrementOp() const { return isDecrementOp(getOpcode()); }
 
-  static bool isIncrementDecrementOp(Opcode Op) { return Op <= UO_PreDec; }
+  static bool isIncrementDecrementOp(Opcode Opc) { return Opc <= UO_PreDec; }
   bool isIncrementDecrementOp() const {
 return isIncrementDecrementOp(getOpcode());
   }
 
-  static bool isArithmeticOp(Opcode Op) {
-return Op >= UO_Plus && Op <= UO_LNot;
+  static bool isArithmeticOp(Opcode Opc) {
+return Opc >= UO_Plus && Opc <= UO_LNot;
   }
   bool isArithmeticOp() const { return isArithmeticOp(getOpcode()); }
 
   /// getOpcodeStr - Turn an Opcode enum value into the punctuation char it
   /// corresponds to, e.g. "sizeof" or "[pre]++"
-  static StringRef getOpcodeStr(Opcode Op);
+  static StringRef getOpcodeStr(Opcode Opc);
 
   /// Retrieve the unary opcode that corresponds to the given
   /// overloaded operator.
@@ -1961,21 +1957,21 @@ public:
   static Overloa

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/Type.cpp:2234
+bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
+  QualType T = Context.getBaseElementType(*this);
+  if (T->isIncompleteType())

Quuxplusone wrote:
> erichkeane wrote:
> > Quuxplusone wrote:
> > > erichkeane wrote:
> > > > You likely want to canonicalize here.
> > > You mean `QualType T = Context.getBaseElementType(getCanonicalType());`?
> > > I can do that. For my own edification (and/or a test case), in what way 
> > > does the current code fail?
> > More like: QualType T = 
> > Context.getBaseElementType(*this).getCanonicalType();
> > 
> > This desugars typedefs in some cases, which could make the below fail.  
> > 
> > Something like this: https://godbolt.org/z/-C-Onh
> > 
> > It MIGHT still pass here, but something else might canonicalize this along 
> > the way.
> > 
> > ADDITIONALLY, I wonder if this properly handles references?  I don't think 
> > getBaseElementType will de-reference. You might want to do that as well.
> > ADDITIONALLY, I wonder if this properly handles references? I don't think 
> > getBaseElementType will de-reference. You might want to do that as well.
> 
> I actually don't want references to be stripped here: `int&` should come out 
> as "not trivially relocatable" because it is not an object type.
> https://p1144.godbolt.org/z/TbSsOA
Ah, I see.  For some reason I had it in my mind that you wanted to follow 
references.


Repository:
  rC Clang

https://reviews.llvm.org/D50119



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

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

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

Quuxplusone added inline comments.



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

rjmccall wrote:
> Quuxplusone wrote:
> > @rjmccall wrote:
> > > trivial_abi permits annotated types to be passed and returned in 
> > > registers, which is ABI-breaking. Skimming the blog post, it looks like 
> > > trivially_relocatable does not permit this — it merely signifies that 
> > > destruction is a no-op after a move construction or assignment.
> > 
> > Not necessarily a "no-op"; my canonical example is a CopyOnlyCXX03SharedPtr 
> > which increments a refcount on construction and decrements on destruction. 
> > But move-construction plus destruction should "balance out" and result in 
> > no observable side effects.
> > 
> > > This is usefully different in the design space, since it means you can 
> > > safely add the attribute retroactively to e.g. std::unique_ptr, and other 
> > > templates can then detect that std::unique_ptr is trivially-relocatable 
> > > and optimize themselves to use memcpy or realloc or whatever it is that 
> > > they want to do. So in that sense trivial_abi is a *stronger* attribute, 
> > > not a *weaker* one: the property it determines ought to imply 
> > > trivially_relocatable.
> > 
> > `trivial_abi` is an "orthogonal" attribute: you can have `trivial_abi` 
> > types with non-trivial constructors and destructors, which can have 
> > observable side effects. For example,
> > ```
> > struct [[clang::trivial_abi]] DestructionAnnouncer {
> > ~DestructionAnnouncer() { puts("hello!"); }
> > };
> > ```
> > is `trivial_abi` (because of the annotation) yet not trivially relocatable, 
> > because its "move plus destroy" operation has observable side effects.
> > 
> > > The only interesting question in the language design that I know of is 
> > > what happens if you put the attribute on a template that's instantiated 
> > > to contain a sub-object that is definitely not trivially relocatable / 
> > > trivial-ABI. For trivial_abi, we decided that the attribute is simply 
> > > ignored — it implicitly only applies to specializations where the 
> > > attribute would be legal. I haven't dug into the design enough to know 
> > > what trivially_relocatable decides in this situation, but the three basic 
> > > options are:
> > >
> > > - the attribute always has effect and allows trivial relocation 
> > > regardless of the subobject types; this is obviously unsafe, so it limits 
> > > the safe applicability of the attribute to templates
> > > - the attribute is ignored, like trivial_abi is
> > > - the attribute is ill-formed, and you'll need to add a 
> > > [[trivially_relocatable(bool)]] version to support templates
> > 
> > What happens is basically the first thing you said, except that I disagree 
> > that it's "obviously unsafe." Right now, conditionally trivial relocation 
> > is possible via template metaprogramming; see the libcxx patch at e.g.
> > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > Since the attribute is an opt-in mechanism, it makes perfect sense to me 
> > that if you put it on a class (or class template), then it applies to the 
> > class, without any further sanity-checking by the compiler. The compiler 
> > has no reason to second-guess the programmer here.
> > 
> > However, there's one more interesting case. Suppose the programmer puts the 
> > attribute on a class that isn't relocatable at all! (For example, the union 
> > case @erichkeane mentioned, or a class type with a deleted destructor.) In 
> > that case, this patch *does* give an error... *unless* the class was 
> > produced by instantiating a template, in which case we *don't* give an 
> > error, because it's not the template-writer's fault.
> > https://p1144.godbolt.org/z/wSZPba
> > trivial_abi is an "orthogonal" attribute: you can have trivial_abi types 
> > with non-trivial constructors and destructors, which can have observable 
> > side effects. 
> 
> Let me cut this conversation short.  `trivial_abi` is not such an old and 
> widely-established attribute that we are unable to revise its definition.  I 
> am comfortable making the same semantic guarantees for `trivial_abi` that 
> you're making for `trivially_relocatable`, because I think it is in the 
> language's interest for `trivial_abi` to be strictly stronger than 
> `trivially_relocatable`.
> 
> > What happens is basically the first thing you said, except that I disagree 
> > that it's "obviously unsafe." 
> 
> Under your semantics, the attribute is an unchecked assertion about all of a 
> class's subobjects.  A class template which fails to correctly apply the 
> template metaprogramming trick to all of its dependently-typed subobjects — 
> which can be quite awkward because it creates an extra dimension of partial 
> specialization, a

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

Quuxplusone wrote:
> @rjmccall wrote:
> > trivial_abi permits annotated types to be passed and returned in registers, 
> > which is ABI-breaking. Skimming the blog post, it looks like 
> > trivially_relocatable does not permit this — it merely signifies that 
> > destruction is a no-op after a move construction or assignment.
> 
> Not necessarily a "no-op"; my canonical example is a CopyOnlyCXX03SharedPtr 
> which increments a refcount on construction and decrements on destruction. 
> But move-construction plus destruction should "balance out" and result in no 
> observable side effects.
> 
> > This is usefully different in the design space, since it means you can 
> > safely add the attribute retroactively to e.g. std::unique_ptr, and other 
> > templates can then detect that std::unique_ptr is trivially-relocatable and 
> > optimize themselves to use memcpy or realloc or whatever it is that they 
> > want to do. So in that sense trivial_abi is a *stronger* attribute, not a 
> > *weaker* one: the property it determines ought to imply 
> > trivially_relocatable.
> 
> `trivial_abi` is an "orthogonal" attribute: you can have `trivial_abi` types 
> with non-trivial constructors and destructors, which can have observable side 
> effects. For example,
> ```
> struct [[clang::trivial_abi]] DestructionAnnouncer {
> ~DestructionAnnouncer() { puts("hello!"); }
> };
> ```
> is `trivial_abi` (because of the annotation) yet not trivially relocatable, 
> because its "move plus destroy" operation has observable side effects.
> 
> > The only interesting question in the language design that I know of is what 
> > happens if you put the attribute on a template that's instantiated to 
> > contain a sub-object that is definitely not trivially relocatable / 
> > trivial-ABI. For trivial_abi, we decided that the attribute is simply 
> > ignored — it implicitly only applies to specializations where the attribute 
> > would be legal. I haven't dug into the design enough to know what 
> > trivially_relocatable decides in this situation, but the three basic 
> > options are:
> >
> > - the attribute always has effect and allows trivial relocation regardless 
> > of the subobject types; this is obviously unsafe, so it limits the safe 
> > applicability of the attribute to templates
> > - the attribute is ignored, like trivial_abi is
> > - the attribute is ill-formed, and you'll need to add a 
> > [[trivially_relocatable(bool)]] version to support templates
> 
> What happens is basically the first thing you said, except that I disagree 
> that it's "obviously unsafe." Right now, conditionally trivial relocation is 
> possible via template metaprogramming; see the libcxx patch at e.g.
> https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> Since the attribute is an opt-in mechanism, it makes perfect sense to me that 
> if you put it on a class (or class template), then it applies to the class, 
> without any further sanity-checking by the compiler. The compiler has no 
> reason to second-guess the programmer here.
> 
> However, there's one more interesting case. Suppose the programmer puts the 
> attribute on a class that isn't relocatable at all! (For example, the union 
> case @erichkeane mentioned, or a class type with a deleted destructor.) In 
> that case, this patch *does* give an error... *unless* the class was produced 
> by instantiating a template, in which case we *don't* give an error, because 
> it's not the template-writer's fault.
> https://p1144.godbolt.org/z/wSZPba
> trivial_abi is an "orthogonal" attribute: you can have trivial_abi types with 
> non-trivial constructors and destructors, which can have observable side 
> effects. 

Let me cut this conversation short.  `trivial_abi` is not such an old and 
widely-established attribute that we are unable to revise its definition.  I am 
comfortable making the same semantic guarantees for `trivial_abi` that you're 
making for `trivially_relocatable`, because I think it is in the language's 
interest for `trivial_abi` to be strictly stronger than `trivially_relocatable`.

> What happens is basically the first thing you said, except that I disagree 
> that it's "obviously unsafe." 

Under your semantics, the attribute is an unchecked assertion about all of a 
class's subobjects.  A class template which fails to correctly apply the 
template metaprogramming trick to all of its dependently-typed subobjects — 
which can be quite awkward because it creates an extra dimension of partial 
specialization, and which breaks ABI by adding extra template parameters — will 
be silently miscompiled to allow objects to be memcpy'ed when they're 
potentially not legal to memcpy.  That is a footg

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


https://reviews.llvm.org/D54245



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D51762: First part of the calendar stuff

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ugh i cannot close it :/


https://reviews.llvm.org/D51762



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

[PATCH] D51762: First part of the calendar stuff

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Whoops right sry again!


https://reviews.llvm.org/D51762



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

[PATCH] D54404: Exclude matchers which can have multiple results

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added a comment.

In https://reviews.llvm.org/D54404#1296224, @aaron.ballman wrote:

> In https://reviews.llvm.org/D54404#1295426, @steveire wrote:
>
> > I acknowledge and share the future-proofing concern.
> >
> > We could possibly use something trait-based instead and put the trait 
> > beside the matcher definition in ASTMatchers.h, but that doesn't really 
> > solve the problem. It only moves the problem.
>
>
> If we could somehow incorporate it into the matcher definition itself, 
> though, it means we don't have two lists of things to keep updated. You're 
> right that it doesn't eliminate the problem -- we still have to know to ask 
> the question when reviewing new matchers (so perhaps something that requires 
> you to explicitly opt-in/out would be better).
>
> Adding @sbenza in case he has ideas.


We could put something in `MatcherDescriptor` to indicate this. However, I am 
still not sure what property are we looking for here.




Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:606
+  // be used with particular arguments.
+  static std::vector ExcludedMatchers{
+  "allOf",

I don't think we are allowed to have non-trivial static storage duration 
objects like this.
You have to use llvm::ManagedStatic. Or just make it a raw array.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:624
+  "hasAnyDeclaration",
+  "hasAnyName",
+  "hasAnyParameter",

I'm not sure what goes in this list.
`hasAnyName` is here but not `hasName`.
What is ambiguous about `hasAnyName`?


Repository:
  rC Clang

https://reviews.llvm.org/D54404



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

[PATCH] D53934: [clangd] Improve code completion for ObjC methods

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

Since I don't have commit access, @sammccall will land this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53934



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

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

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

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



Comment at: docs/LanguageExtensions.rst:1096
+  equivalent to copying the underlying bytes and then dropping the source 
object
+  on the floor.
 * ``__is_destructible`` (MSVC 2013)

@rjmccall wrote:
> trivial_abi permits annotated types to be passed and returned in registers, 
> which is ABI-breaking. Skimming the blog post, it looks like 
> trivially_relocatable does not permit this — it merely signifies that 
> destruction is a no-op after a move construction or assignment.

Not necessarily a "no-op"; my canonical example is a CopyOnlyCXX03SharedPtr 
which increments a refcount on construction and decrements on destruction. But 
move-construction plus destruction should "balance out" and result in no 
observable side effects.

> This is usefully different in the design space, since it means you can safely 
> add the attribute retroactively to e.g. std::unique_ptr, and other templates 
> can then detect that std::unique_ptr is trivially-relocatable and optimize 
> themselves to use memcpy or realloc or whatever it is that they want to do. 
> So in that sense trivial_abi is a *stronger* attribute, not a *weaker* one: 
> the property it determines ought to imply trivially_relocatable.

`trivial_abi` is an "orthogonal" attribute: you can have `trivial_abi` types 
with non-trivial constructors and destructors, which can have observable side 
effects. For example,
```
struct [[clang::trivial_abi]] DestructionAnnouncer {
~DestructionAnnouncer() { puts("hello!"); }
};
```
is `trivial_abi` (because of the annotation) yet not trivially relocatable, 
because its "move plus destroy" operation has observable side effects.

> The only interesting question in the language design that I know of is what 
> happens if you put the attribute on a template that's instantiated to contain 
> a sub-object that is definitely not trivially relocatable / trivial-ABI. For 
> trivial_abi, we decided that the attribute is simply ignored — it implicitly 
> only applies to specializations where the attribute would be legal. I haven't 
> dug into the design enough to know what trivially_relocatable decides in this 
> situation, but the three basic options are:
>
> - the attribute always has effect and allows trivial relocation regardless of 
> the subobject types; this is obviously unsafe, so it limits the safe 
> applicability of the attribute to templates
> - the attribute is ignored, like trivial_abi is
> - the attribute is ill-formed, and you'll need to add a 
> [[trivially_relocatable(bool)]] version to support templates

What happens is basically the first thing you said, except that I disagree that 
it's "obviously unsafe." Right now, conditionally trivial relocation is 
possible via template metaprogramming; see the libcxx patch at e.g.
https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
Since the attribute is an opt-in mechanism, it makes perfect sense to me that 
if you put it on a class (or class template), then it applies to the class, 
without any further sanity-checking by the compiler. The compiler has no reason 
to second-guess the programmer here.

However, there's one more interesting case. Suppose the programmer puts the 
attribute on a class that isn't relocatable at all! (For example, the union 
case @erichkeane mentioned, or a class type with a deleted destructor.) In that 
case, this patch *does* give an error... *unless* the class was produced by 
instantiating a template, in which case we *don't* give an error, because it's 
not the template-writer's fault.
https://p1144.godbolt.org/z/wSZPba


Repository:
  rC Clang

https://reviews.llvm.org/D50119



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

[PATCH] D52296: [Clang] - Add '-gsplit-dwarf[=split, =single]' version for '-gsplit-dwarf' option.

[David Blaikie via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:813-830
+  if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
+if (StringRef(A->getValue()) == "single")
+  return Args.MakeArgString(Output.getFilename());
+
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
 SmallString<128> T(FinalOutput->getValue());

grimar wrote:
> grimar wrote:
> > dblaikie wrote:
> > > If this function now takes the output file name - could it be simplified 
> > > to just always use that, rather than the conditional code that follows 
> > > and tests whether -o is specified and builds up something like the output 
> > > file name & uses the dwo suffix?
> > I am investigating this.
> So what I see now is that when the function becomes:
> 
> ```
> const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
>   const InputInfo &Output) {
>   SmallString<128> T(Output.getFilename());
>   llvm::sys::path::replace_extension(T, "dwo");
>   return Args.MakeArgString(T);
> }
> ```
> 
> Then no clang tests (check-clang) fail. This does not seem normal to me.
> I would expect that such a change should break some `-fdebug-compilation-dir` 
> relative 
> logic at least and I suspect we just have no test(s) atm.
> 
> What about if I add test case(s) and post a follow-up refactor patch for this 
> place?
> (I started work on it, but it will take some time).
Sounds good - thanks!



Comment at: test/CodeGen/split-debug-single-file.c:12
+//  RUN:   -enable-split-dwarf=split -split-dwarf-file %t.o -emit-obj -o %t.o 
%s
+//  RUN: llvm-objdump -section-headers %t.o | FileCheck 
--check-prefix=MODE-SPLIT %s
+//  MODE-SPLIT-NOT: .dwo

grimar wrote:
> dblaikie wrote:
> > This looks like an end-to-end test, which we don't usually do in Clang (or 
> > in the LLVM project in general).
> > 
> > For example, the previous testing for split-dwarf had a driver test (that 
> > tested only that the driver produced the right cc1 invocation) and a 
> > CodeGen test (that tested that the right IR was produced), but I don't see 
> > any test like this (that tested the resulting object file)?
> > 
> > I know there's a narrow gap in IR testing - things that don't go in the IR, 
> > but instead go through MCOptions  & that the SplitDwarfFile is one of those.
> > 
> > So, yeah, in this case it's a bit of a test hole - if you're going to keep 
> > this test, perhaps it could test assembly, rather than the object file? 
> > Since it doesn't need complex parsing, etc, perhaps?
> > So, yeah, in this case it's a bit of a test hole - if you're going to keep 
> > this test, perhaps it could test assembly, rather than the object file? 
> > Since it doesn't need complex parsing, etc, perhaps?
> 
> I am not sure assembly can help here. For example
> `clang main.c -S -g -gsplit-dwarf` would create single asm file output anyways
> and what I tried to check here is that we can either place .dwo sections into 
> the
> same output or into a different one depending on new option.
> 
> > For example, the previous testing for split-dwarf had a driver test (that 
> > tested only that the driver produced the right cc1 invocation) and a 
> > CodeGen test (that tested that the right IR was produced), but I don't see 
> > any test like this (that tested the resulting object file)?
> 
> I think `split-debug-filename.c` do that?
> See: 
> https://github.com/llvm-mirror/clang/blob/master/test/CodeGen/split-debug-filename.c#L18
> 
> Also, `relax.c`: 
> https://github.com/llvm-mirror/clang/blob/master/test/CodeGen/relax.c
> And `mips-inline-asm-abi.c`:  
> https://github.com/llvm-mirror/clang/blob/master/test/CodeGen/mips-inline-asm-abi.c
> 
> Seems it is not common, but still acceptable?
Ah, I see/good point, split-debug-filename.c tests both the IR & then also 
tests the object headers.

Sounds good


https://reviews.llvm.org/D52296



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

r346770 - [AST][NFC] Pack DeclRefExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Tue Nov 13 09:56:44 2018
New Revision: 346770

URL: http://llvm.org/viewvc/llvm-project?rev=346770&view=rev
Log:
[AST][NFC] Pack DeclRefExpr

Move the SourceLocation to the bit-fields of Stmt + clang-format.
This saves one pointer per DeclRefExpr but otherwise NFC.


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/Expr.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346770&r1=346769&r2=346770&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Nov 13 09:56:44 2018
@@ -1038,61 +1038,60 @@ class DeclRefExpr final
   private llvm::TrailingObjects {
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
+  friend TrailingObjects;
+
   /// The declaration that we are referencing.
   ValueDecl *D;
 
-  /// The location of the declaration name itself.
-  SourceLocation Loc;
-
   /// Provides source/type location info for the declaration name
   /// embedded in D.
   DeclarationNameLoc DNLoc;
 
   size_t numTrailingObjects(OverloadToken) const {
-return hasQualifier() ? 1 : 0;
+return hasQualifier();
   }
 
   size_t numTrailingObjects(OverloadToken) const {
-return hasFoundDecl() ? 1 : 0;
+return hasFoundDecl();
   }
 
   size_t numTrailingObjects(OverloadToken) const {
-return hasTemplateKWAndArgsInfo() ? 1 : 0;
+return hasTemplateKWAndArgsInfo();
   }
 
   /// Test whether there is a distinct FoundDecl attached to the end of
   /// this DRE.
   bool hasFoundDecl() const { return DeclRefExprBits.HasFoundDecl; }
 
-  DeclRefExpr(const ASTContext &Ctx,
-  NestedNameSpecifierLoc QualifierLoc,
-  SourceLocation TemplateKWLoc,
-  ValueDecl *D, bool RefersToEnlosingVariableOrCapture,
-  const DeclarationNameInfo &NameInfo,
-  NamedDecl *FoundD,
-  const TemplateArgumentListInfo *TemplateArgs,
-  QualType T, ExprValueKind VK);
+  DeclRefExpr(const ASTContext &Ctx, NestedNameSpecifierLoc QualifierLoc,
+  SourceLocation TemplateKWLoc, ValueDecl *D,
+  bool RefersToEnlosingVariableOrCapture,
+  const DeclarationNameInfo &NameInfo, NamedDecl *FoundD,
+  const TemplateArgumentListInfo *TemplateArgs, QualType T,
+  ExprValueKind VK);
 
   /// Construct an empty declaration reference expression.
-  explicit DeclRefExpr(EmptyShell Empty)
-: Expr(DeclRefExprClass, Empty) { }
+  explicit DeclRefExpr(EmptyShell Empty) : Expr(DeclRefExprClass, Empty) {}
 
   /// Computes the type- and value-dependence flags for this
   /// declaration reference expression.
-  void computeDependence(const ASTContext &C);
+  void computeDependence(const ASTContext &Ctx);
 
 public:
   DeclRefExpr(ValueDecl *D, bool RefersToEnclosingVariableOrCapture, QualType 
T,
   ExprValueKind VK, SourceLocation L,
   const DeclarationNameLoc &LocInfo = DeclarationNameLoc())
-: Expr(DeclRefExprClass, T, VK, OK_Ordinary, false, false, false, false),
-  D(D), Loc(L), DNLoc(LocInfo) {
-DeclRefExprBits.HasQualifier = 0;
-DeclRefExprBits.HasTemplateKWAndArgsInfo = 0;
-DeclRefExprBits.HasFoundDecl = 0;
-DeclRefExprBits.HadMultipleCandidates = 0;
+  : Expr(DeclRefExprClass, T, VK, OK_Ordinary, false, false, false, false),
+D(D), DNLoc(LocInfo) {
+DeclRefExprBits.HasQualifier = false;
+DeclRefExprBits.HasTemplateKWAndArgsInfo = false;
+DeclRefExprBits.HasFoundDecl = false;
+DeclRefExprBits.HadMultipleCandidates = false;
 DeclRefExprBits.RefersToEnclosingVariableOrCapture =
 RefersToEnclosingVariableOrCapture;
+DeclRefExprBits.Loc = L;
 computeDependence(D->getASTContext());
   }
 
@@ -1112,8 +,7 @@ public:
  const TemplateArgumentListInfo *TemplateArgs = nullptr);
 
   /// Construct an empty declaration reference expression.
-  static DeclRefExpr *CreateEmpty(const ASTContext &Context,
-  bool HasQualifier,
+  static DeclRefExpr *CreateEmpty(const ASTContext &Context, bool HasQualifier,
   bool HasFoundDecl,
   bool HasTemplateKWAndArgsInfo,
   unsigned NumTemplateArgs);
@@ -1123,11 +1121,11 @@ public:
   void setDecl(ValueDecl *NewD) { D = NewD; }
 
   DeclarationNameInfo getNameInfo() const {
-return DeclarationNameInfo(getDecl()->getDeclName(), Loc, DNLoc);
+return DeclarationNameInfo(getDecl()->getDeclName(), getLocation(), DNLoc);
   }
 
-  SourceLocation getLocation() const { return Loc; }
-  void setLocation(SourceLocation L) { Loc = L; }
+  SourceLocation getLocation() const { return DeclRefExprBits.Loc; }
+  void setLocation(SourceLocation L) { Decl

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:4289
+ /*BasePath=*/nullptr, CCK)
+   .get();
 

Anastasia wrote:
> rjmccall wrote:
> > Okay.  But if `ToType` *isn't* a reference type, this will never be an 
> > address-space conversion.  I feel like this code could be written more 
> > clearly to express what it's trying to do.
> I hope it makes more sense now. Btw, it also applies to pointer type.
The logic is wrong for pointer types; if you're converting pointers, you need 
to be checking the address space of the pointee type of the from type.

It sounds like this is totally inadequately tested; please flesh out the test 
with all of these cases.  While you're at it, please ensure that there are 
tests verifying that we don't allowing address-space changes in nested 
positions.


https://reviews.llvm.org/D53764



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D54258#1296706, @richardmembarth wrote:

> CUDA maps `__shared__` internally also to `__attribute__((shared))`:
>
>   #define __annotate__(a) \
>   __attribute__((a))
>   #define __location__(a) \
>   __annotate__(a)
>   ...
>   #define __shared__ \
>   __location__(shared)
>
>
> My guess is that Clang does it just the same way and only converts to 
> `LangAS::cuda_shared` for code generation in `GetGlobalVarAddressSpace`:
>  https://clang.llvm.org/doxygen/CodeGenModule_8cpp_source.html#l03305
>  In contrast, OpenCL uses keywords that are mapped directly to 
> `LangAS::opencl_local` etc.


I agree with the change itself... but it's quite annoying that such things 
can't be tested. :(


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D54253: [OpenCL][NFC] Improve test coverage of test/Index/opencl-types.cl

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I am a bit confused about this testing since you are running OpenCL on 
architectures that aren't supposed to support it. Can you provide more details 
on why you are doing this?


Repository:
  rC Clang

https://reviews.llvm.org/D54253



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

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:4289
+ /*BasePath=*/nullptr, CCK)
+   .get();
 

rjmccall wrote:
> Okay.  But if `ToType` *isn't* a reference type, this will never be an 
> address-space conversion.  I feel like this code could be written more 
> clearly to express what it's trying to do.
I hope it makes more sense now. Btw, it also applies to pointer type.


https://reviews.llvm.org/D53764



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

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[Anastasia Stulova via Phabricator via cfe-commits]

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

Rewrite how CastKind is set for reference and pointer type.


https://reviews.llvm.org/D53764

Files:
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/address-space-deduction.cl

Index: test/CodeGenOpenCLCXX/address-space-deduction.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/address-space-deduction.cl
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -O0 -emit-llvm -o - | FileCheck %s -check-prefixes=COMMON,PTR
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -O0 -emit-llvm -o - -DREF | FileCheck %s -check-prefixes=COMMON,REF
+
+#ifdef REF
+#define PTR &
+#define ADR(x) x
+#else
+#define PTR *
+#define ADR(x) &x
+#endif
+
+//COMMON: @glob = addrspace(1) global i32
+int glob;
+//PTR: @glob_p = addrspace(1) global i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @glob to i32 addrspace(4)*)
+//REF: @glob_p = addrspace(1) global i32 addrspace(4)* null
+int PTR glob_p = ADR(glob);
+
+//COMMON: @_ZZ3fooi{{P|R}}U3AS4iE6loc_st = internal addrspace(1) global i32
+//PTR: @_ZZ3fooiPU3AS4iE8loc_st_p = internal addrspace(1) global i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @_ZZ3fooiPU3AS4iE6loc_st to i32 addrspace(4)*)
+//REF: @_ZZ3fooiRU3AS4iE8loc_st_p = internal addrspace(1) global i32 addrspace(4)* null
+//COMMON: @loc_ext_p = external addrspace(1) {{global|constant}} i32 addrspace(4)*
+//COMMON: @loc_ext = external addrspace(1) global i32
+
+//REF: store i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @glob to i32 addrspace(4)*), i32 addrspace(4)* addrspace(1)* @glob_p
+
+//COMMON: define spir_func i32 @_Z3fooi{{P|R}}U3AS4i(i32 %par, i32 addrspace(4)*{{.*}} %par_p)
+int foo(int par, int PTR par_p){
+//COMMON: %loc = alloca i32
+  int loc;
+//COMMON: %loc_p = alloca i32 addrspace(4)*
+//COMMON: [[GAS:%[0-9]+]] = addrspacecast i32* %loc to i32 addrspace(4)*
+//COMMON: store i32 addrspace(4)* [[GAS]], i32 addrspace(4)** %loc_p
+  int PTR loc_p = ADR(loc);
+
+// CHECK directives for the following code are located above.
+  static int loc_st;
+  static int PTR loc_st_p = ADR(loc_st);
+  extern int loc_ext;
+  extern int PTR loc_ext_p;
+  (void)loc_ext_p;
+  return loc_ext;
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7177,7 +7177,8 @@
   bool IsPointee =
   ChunkIndex > 0 &&
   (D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Pointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer);
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer ||
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference);
   bool IsFuncReturnType =
   ChunkIndex > 0 &&
   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Function;
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7209,12 +7209,20 @@
   return CreateMaterializeTemporaryExpr(E->getType(), E, false);
 }
 
-ExprResult
-InitializationSequence::Perform(Sema &S,
-const InitializedEntity &Entity,
-const InitializationKind &Kind,
-MultiExprArg Args,
-QualType *ResultType) {
+ExprResult Sema::PerformQualificationConversion(Expr *E, QualType Ty,
+ExprValueKind VK,
+CheckedConversionKind CCK) {
+  CastKind CK = (Ty.getAddressSpace() != E->getType().getAddressSpace())
+? CK_AddressSpaceConversion
+: CK_NoOp;
+  return ImpCastExprToType(E, Ty, CK, VK, /*BasePath=*/nullptr, CCK);
+}
+
+ExprResult InitializationSequence::Perform(Sema &S,
+   const InitializedEntity &Entity,
+   const InitializationKind &Kind,
+   MultiExprArg Args,
+   QualType *ResultType) {
   if (Failed()) {
 Diagnose(S, Entity, Kind, Args);
 return ExprError();
@@ -7603,12 +7611,11 @@
 case SK_QualificationConversionRValue: {
   // Perform a qualification conversion; these can never go wrong.
   ExprValueKind VK =
-  Step->Kind == SK_QualificationConversionLValue ?
-  VK_LValue :
-  (Step->Kind == SK_QualificationConversionXValue ?
-   VK_XValue :
-   VK_RValue);
-  CurInit = S.ImpCastExprToType(CurInit.get(), Step->Type, CK_NoOp, VK);
+ 

[PATCH] D54428: [clangd] XPC transport layer, framework, test-client

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

A question about the high-level build target setup (I don't know much about XPC 
services/frameworks, bear with me...):

This is set up so that the clangd binary (ClangdMain) can run unmodified as an 
XPC service, all flags and options are still respected etc.
At the same time, the framework plist seems like it is designed to invoke the 
clangd binary in a very specific configuration (no flags will be plumbed 
through).

Is it actually important that the `clangd` binary itself can be used with XPC, 
rather than defining another binary that spawns a `ClangdServer+XPCTransport` 
with the right config? If you strip out all of `ClangdMain` that's not related 
to flag parsing and options, there's almost nothing left...




Comment at: tool/ClangdMain.cpp:30
+#ifdef CLANGD_BUILD_XPC
+#include "xpc/XPCTransport.h"
+#endif

WDYT about putting `newXPCTransport()` in `Transport.h` behind an ifdef?

That will be consistent with JSON, allow the `XPCTransport.h` to be removed 
entirely, and remove one #ifdef here.

I find the #ifdefs easier to understand if they surround functional code, 
rather than #includes - might be just me.



Comment at: tool/ClangdMain.cpp:328
+#ifdef CLANGD_BUILD_XPC
+if (getenv("CLANGD_AS_XPC_SERVICE"))
+  return newXPCransport();

I'd consider putting this outside the #ifdef, so we can print an error and exit 
if it's requested but not built.

In fact, can this just be a regular flag instead? `-transport={xpc|json}`



Comment at: tool/ClangdMain.cpp:329
+if (getenv("CLANGD_AS_XPC_SERVICE"))
+  return newXPCransport();
+#endif

no support for `-input-mirror` or pretty-printing in the logs - deliberate?



Comment at: xpc/CMakeLists.txt:21
+
+add_clang_library(clangdXpcJsonConversions
+  Conversion.cpp

why is conversions a separate library from transport?

No problem with fine-grained targets per se, but it's inconsistent with much of 
the rest of clang-tools-extra.



Comment at: xpc/Conversion.cpp:16
+
+using namespace clang;
+using namespace clangd;

nit:
```
using namespace llvm;
namespace clang {
namespace clangd {
```
(we're finally consistent about this)



Comment at: xpc/Conversion.cpp:22
+xpc_object_t clangd::jsonToXpc(const json::Value &json) {
+  const char *const key = "LSP";
+  std::string payload_string;

Key, PayloadString, etc



Comment at: xpc/Conversion.cpp:23
+  const char *const key = "LSP";
+  std::string payload_string;
+  raw_string_ostream payload_stream(payload_string);

nit: you can #include ScopedPrinter, and then this is just llvm::to_string(json)



Comment at: xpc/Conversion.cpp:28
+  xpc_object_t payload_obj = xpc_string_create(payload_string.c_str());
+  return xpc_dictionary_create(&key, &payload_obj, 1);
+}

ah, this encoding is a little sad vs the "native" encoding of object -> 
xpc_dictionary, array -> xpc_array etc which looked promising...

Totally your call, but out of curiosity - why the change?



Comment at: xpc/Conversion.h:19
+
+xpc_object_t jsonToXpc(const llvm::json::Value &json);
+llvm::json::Value xpcToJson(const xpc_object_t &json);

param name json -> JSON



Comment at: xpc/XPCTransport.cpp:142
+
+namespace XPCClosure {
+// "owner" of this "closure object" - necessary for propagating connection to

lowercase namespace?



Comment at: xpc/XPCTransport.cpp:177
+if (!TransportObject->handleMessage(std::move(Doc), *HandlerPtr)) {
+  log("Received exit notification - cancelling connection.");
+  xpc_connection_cancel(xpc_dictionary_get_remote_connection(message));

I'm not clear on the relationship between XPC services and connections - if 
there are multiple connections, what happens? Do you want to check and close 
any subsequent ones? Can connections and/or messages arrive on multiple threads?



Comment at: xpc/XPCTransport.cpp:192
+  // exit of xpc_main().
+  XPCClosure::TransportObject = this;
+  XPCClosure::HandlerPtr = &Handler;

you could consider asserting that they were previously null



Comment at: xpc/XPCTransport.h:19
+
+std::unique_ptr newXPCransport();
+

as mentioned above, I think this may be more discoverable in Transport.h, with 
ifdef.
But if the layering/separate libraries are important, here seems OK too.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54428



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

[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D50119#1297070, @dblaikie wrote:

> Hey @rjmccall - I'm trying to remember, but can't get it quite clear in my 
> head. I seem to recall some discussion about trivial_abi not implying/being 
> strong enough for all the cases that trivial_relocatable sounds like it 
> covers? Do you happen to remember the distinction there (the summary in this 
> patch ("the warranting attribute [[trivially_relocatable]], which is similar 
> in spirit to [[trivial_abi]], in that it lets the programmer communicate back 
> to the compiler that a certain user-defined type should be assumed to have 
> this property even though it would not naturally have the property all else 
> being equal.") doesn't sound like what I remember - but my memory is hazy)?


`trivial_abi` permits annotated types to be passed and returned in registers, 
which is ABI-breaking.  Skimming the blog post, it looks like 
`trivially_relocatable` does not permit this — it merely signifies that 
destruction is a no-op after a move construction or assignment.  This is 
usefully different in the design space, since it means you can safely add the 
attribute retroactively to e.g. `std::unique_ptr`, and other templates can then 
detect that `std::unique_ptr` is trivially-relocatable and optimize themselves 
to use `memcpy` or `realloc` or whatever it is that they want to do.  So in 
that sense `trivial_abi` is a *stronger* attribute, not a *weaker* one: the 
property it determines ought to imply `trivially_relocatable`.

The only interesting question in the language design that I know of is what 
happens if you put the attribute on a template that's instantiated to contain a 
sub-object that is definitely not trivially relocatable / trivial-ABI.  For 
`trivial_abi`, we decided that the attribute is simply ignored — it implicitly 
only applies to specializations where the attribute would be legal.  I haven't 
dug into the design enough to know what `trivially_relocatable` decides in this 
situation, but the three basic options are:

- the attribute always has effect and allows trivial relocation regardless of 
the subobject types; this is obviously unsafe, so it limits the safe 
applicability of the attribute to templates
- the attribute is ignored, like `trivial_abi` is
- the attribute is ill-formed, and you'll need to add a 
`[[trivially_relocatable(bool)]]` version to support templates

If there are corner-case differences beyond that, I have no particular 
objection to unifying the semantics so that `trivial_abi` is strictly stronger, 
although we should talk about it case-by-case.




Comment at: test/SemaCXX/trivially-relocatable.cpp:42
+struct A6;
+struct [[trivially_relocatable]] A6 {};
+// expected-error@-1{{type A6 declared 'trivially_relocatable' after its first 
declaration}}

Rakete wrote:
> Quuxplusone wrote:
> > Rakete wrote:
> > > Why does this restriction exist? None of the existing attributes have it 
> > > and I don't see why it would make sense to disallow this.
> > `[[noreturn]]` has it, and I am pretty sure that `[[trivial_abi]]` *ought* 
> > to have it, even though it currently doesn't. The intent is to make it 
> > harder for people to create ODR violations by declaring a type, using it in 
> > some way, and then saying "oh by the way this type was x all along."
> > 
> > ```
> > struct S { S(S&&); ~S(); };
> > std::vector vec;
> > struct [[trivially_relocatable]] S;  // ha ha, now you have to re-do all of 
> > vector's codegen!
> > ```
> > 
> > Does that make sense as a reason it would be (mildly) beneficial to have 
> > this diagnostic?
> > You could still reasonably object to my assumptions that (A) the diagnostic 
> > is worth implementing in Clang and/or (B) the diagnostic is worth mandating 
> > in the Standard wording; but FWIW I do think it's very cheap both to 
> > implement and to specify.
> > 
> > (I will try to adjust the patch so that after the error is given, we remove 
> > the attribute from the class so as to make consistent any subsequent 
> > cascading errors. https://godbolt.org/g/nVRiow )
> Didn't know `[[noreturn]]` had it. Ah I see. Sold :)
I don't see any good reason to allow either this or `[[trivial_abi]]` on an 
arbitrary declaration of the type; it should have to be written on the 
definition.  This also gives sensible semantics for class template 
specializations.

The intrinsic obviously has to require its operand type to be complete, and 
therefore there is a unique declaration which can have the attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D50119



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