[PATCH] D91000: [clang-tidy] CERT MSC24-C Obsolescent Functions check

[Koller Tamás via Phabricator via cfe-commits]

ktomi996 added a comment.

In D91000#2382562 , @steakhal wrote:

> Quoting the revision summary:
>
>> This checker guards against using some vulnerable C functions which are 
>> mentioned in MSC24-C in obsolescent functions table.
>
> Why don't we check the rest of the functions as well?
> `asctime`, `atof`, `atoi`, `atol`, `atoll`, `ctime`, `fopen`, `freopen`, 
> `rewind`, `setbuf`
>
> Hm, I get that `cert-err34-c` will already diagnose the uses of `atof`, 
> `atoi`, `atol`, `atoll`, but then why do we check `vfscanf`, `vscanf` then?
> We should probably omit these, while documenting this.
> On the other hand, I would recommend checking `asctime`, `ctime`, `fopen`, 
> `freopen`, `rewind`, `setbuf` for the sake of completeness.

I only check functions which are in Unchecked Obsolescent Functions without 
setbuf because setbuf does not have a safer alternative in Annex K.
https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions

> What do you think?
>
> There is a mismatch between your code and docs too, regarding the function 
> you check for.
> In the code you match for `vsprintf`, `vsscanf`, `vswprintf`, `vswcanf`, 
> `wcrtomb`, `wcscat`, but none of these are mentioned in the docs.
>
>> The checker warns only if `STDC_LIB_EXT1` macro is defined and the value of 
>> `STDC_WANT_LIB_EXT1` macro is `1` in this case it suggests the corresponding 
>> functions from Annex K instead the vulnerable function.
>
> I would suggest mentioning these macros and their **purpose** in the docs.
> Eg. that the `STDC_WANT_LIB_EXT1` should be defined to `1` but the other is 
> left to the implementation.
>
> That being said, I would request more tests, demonstrating that this macro 
> detection works accordingly.
>
> This checker might be a bit noisy. Have you tried it on open-source projects?
> If it is, we should probably note that in the docs as well.
>
> In the tests, It is a good practice to demonstrate that the offered 
> recommendation does not trigger yet another warning.
> Don't forget to put a `no-warning` to highlight that.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000

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

[PATCH] D93597: [X86][SSE] Enable constexpr on some basic SSE intrinsics (RFC)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon updated this revision to Diff 313038.
RKSimon added a comment.

I've added the static_assert constexpr methods - this requires C++14 so I've 
left the existing constexpr wrapper tests with C++11 coverage as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93597

Files:
  clang/lib/Headers/xmmintrin.h
  clang/test/CodeGen/X86/sse-builtins.c

Index: clang/test/CodeGen/X86/sse-builtins.c
===
--- clang/test/CodeGen/X86/sse-builtins.c
+++ clang/test/CodeGen/X86/sse-builtins.c
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
-// RUN: %clang_cc1 -flax-vector-conversions=none -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
-
+// RUN: %clang_cc1 -x c -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c -flax-vector-conversions=none -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -flax-vector-conversions=none -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++14 -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++14 -flax-vector-conversions=none -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s
 
 #include 
 
@@ -807,3 +810,75 @@
   // CHECK: xor <4 x i32>
   return _mm_xor_ps(A, B);
 }
+
+// Test constexpr handling.
+#if defined(__cplusplus) && (__cplusplus >= 201103L)
+constexpr __m128 test_constexpr_mm_add_ps(__m128 A, __m128 B) {
+  return _mm_add_ps(A, B);
+}
+
+constexpr __m128 test_constexpr_mm_and_ps(__m128 A, __m128 B) {
+  return _mm_and_ps(A, B);
+}
+
+constexpr __m128 test_constexpr_mm_div_ps(__m128 A, __m128 B) {
+  return _mm_div_ps(A, B);
+}
+
+constexpr __m128 test_constexpr_mm_mul_ps(__m128 A, __m128 B) {
+  return _mm_mul_ps(A, B);
+}
+
+constexpr __m128 test_constexpr_mm_or_ps(__m128 A, __m128 B) {
+  return _mm_or_ps(A, B);
+}
+
+constexpr __m128 test_constexpr_mm_set_ss(float A) {
+  return _mm_set_ss(A);
+}
+
+constexpr __m128 test_constexpr_mm_set1_ps(float A) {
+  return _mm_set1_ps(A);
+}
+
+constexpr __m128 test_constexpr_mm_set_ps1(float A) {
+  return _mm_set_ps1(A);
+}
+
+constexpr __m128 test_constexpr_mm_set_ps(float A, float B, float C, float D) {
+  return _mm_set_ps(A, B, C, D);
+}
+
+constexpr __m128 test_constexpr_mm_setr_ps(float A, float B, float C, float D) {
+  return _mm_setr_ps(A, B, C, D);
+}
+
+constexpr __m128 test_constexpr_mm_setzero_ps() {
+  return _mm_setzero_ps();
+}
+
+constexpr __m128 test_constexpr_mm_xor_ps(__m128 A, __m128 B) {
+  return _mm_xor_ps(A, B);
+}
+#endif
+
+// Test constexpr method handling.
+#if defined(__cplusplus) && (__cplusplus >= 201402L)
+constexpr bool test() {
+  __m128 result =  _mm_setzero_ps();
+  result = _mm_add_ps(result, _mm_set_ss(1.0f));
+  result = _mm_div_ps(result, _mm_set1_ps(2.0f));
+  result = _mm_mul_ps(result, _mm_set_ps1(3.0f));
+  result = _mm_sub_ps(result, _mm_set_ps(4.0f, 5.0f, 6.0f, 7.0f));
+  result = _mm_and_ps(result, _mm_setr_ps(-1.0f, -2.0f, -3.0f, -4.0f));
+  result = _mm_or_ps(result, _mm_setr_ps(0.0f, 1.0f, 2.0f, 3.0f));
+  result = _mm_xor_ps(result, _mm_setr_ps(10.0f, 11.0f, 12.0f, 13.0f));
+  return true;
+}
+
+int main() {
+  test();
+  static_assert(test(), "SSE constexpr failure");
+  return 0;
+}
+#endif
Index: clang/lib/Headers/xmmintrin.h
===
--- clang/lib/Headers/xmmintrin.h
+++ clang/lib/Headers/xmmintrin.h
@@ -31,6 +31,12 @@
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("sse"), __min_vector_width__(128)))
 #define __DEFAULT_FN_ATTRS_MMX __attribute__((__always_inline__, __nodebug__, __target__("mmx,sse"), __min_vector_width__(64)))
 
+#if defined(__cplusplus) && (__cplusplus >= 201103L)
+#define __DEFAULT_FN_ATTRS_CONSTEXPR __DEFAULT_FN_ATTRS constexpr
+#else
+#define __DEFAULT_FN_ATTRS_CONSTEXPR __DEFAULT_FN_ATTRS
+#endif
+
 /// Adds the 32-bit float values in the low-order bits of the operands.
 ///
 /// \headerfile 
@@ -66,7 +72,7 @@
 ///

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

[Lubomir Litchev via Phabricator via cfe-commits]

llitchev updated this revision to Diff 313039.
llitchev marked 10 inline comments as done.
llitchev added a comment.

Addressed CR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91556

Files:
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/test/CodeGen/XCore/threads.ll
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
  mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir

Index: mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
===
--- /dev/null
+++ mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-translate  --mlir-to-llvmir %s | FileCheck %s
+
+module {
+  llvm.func @malloc(!llvm.i64) -> !llvm.ptr
+  llvm.func @main() {
+%0 = llvm.mlir.constant(4 : index) : !llvm.i64
+%1 = llvm.mlir.constant(4 : index) : !llvm.i64
+%2 = llvm.mlir.null : !llvm.ptr
+%3 = llvm.mlir.constant(1 : index) : !llvm.i64
+%4 = llvm.getelementptr %2[%3] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+%5 = llvm.ptrtoint %4 : !llvm.ptr to !llvm.i64
+%6 = llvm.mul %1, %5 : !llvm.i64
+%7 = llvm.call @malloc(%6) : (!llvm.i64) -> !llvm.ptr
+%8 = llvm.bitcast %7 : !llvm.ptr to !llvm.ptr
+%9 = llvm.mlir.undef : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%10 = llvm.insertvalue %8, %9[0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%11 = llvm.insertvalue %8, %10[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%12 = llvm.mlir.constant(0 : index) : !llvm.i64
+%13 = llvm.insertvalue %12, %11[2] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%14 = llvm.mlir.constant(1 : index) : !llvm.i64
+%15 = llvm.insertvalue %1, %13[3, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%16 = llvm.insertvalue %14, %15[4, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%17 = llvm.mlir.constant(4.20e+01 : f32) : !llvm.float
+// CHECK: %CaptureStructAlloca = alloca %CapturedStructType
+// CHECK: %{{.*}} = insertvalue %CapturedStructType undef, {{.*}}, 0
+// CHECK: store %CapturedStructType %{{.*}}, %CapturedStructType* %CaptureStructAlloca
+omp.parallel num_threads(%0 : !llvm.i64) {
+  // CHECK: %{{.*}} = load %CapturedStructType, %CapturedStructType* %CaptureStructAlloca
+  // CHECK: %{{.*}} = extractvalue %CapturedStructType %{{.*}}, 0
+  %27 = llvm.mlir.constant(1 : i64) : !llvm.i64
+  %28 = llvm.extractvalue %16[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+  %29 = llvm.mlir.constant(0 : index) : !llvm.i64
+  %30 = llvm.mlir.constant(1 : index) : !llvm.i64
+  %31 = llvm.mul %27, %30 : !llvm.i64
+  %32 = llvm.add %29, %31 : !llvm.i64
+  %33 = llvm.getelementptr %28[%32] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+  llvm.store %17, %33 : !llvm.ptr
+  omp.terminator
+}
+llvm.return
+  }
+}
Index: mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
===
--- mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -6,7 +6,7 @@
   %end = constant 0 : index
   // CHECK: omp.parallel
   omp.parallel {
-// CHECK-NEXT: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
+// CHECK: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
 br ^bb1(%start, %end : index, index)
   // CHECK-NEXT: ^[[BB1]](%[[ARG1:[0-9]+]]: !llvm.i64, %[[ARG2:[0-9]+]]: !llvm.i64):{{.*}}
   ^bb1(%0: index, %1: index):
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -60,25 +60,6 @@
   DebugLoc DL;
 };
 
-// Returns the value stored in the given allocation. Returns null if the given
-// value is not a result of an allocation, if no value is stored or if there is
-// more than one store.
-static Value *findStoredValue(Value *AllocaValue) {
-  Instruction *Alloca = dyn_cast(AllocaValue);
-  if (!Alloca)
-return nullptr;
-  StoreInst *Store = nullptr;
-  for (Use &U : Alloca->uses()) {
-if (auto *CandidateStore = dyn_cast(U.getUser())) {
-  EXPECT_EQ(Store, nullptr);
-  Store = CandidateStore;
-}
-  }
-  if (!Store)
-return nullptr;
-  return Store->getValueOperand();
-}
-
 TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
@@ -425,7 +406,6 @@
   EXPECT_EQ(ForkCI->getArgOperand(1),
 ConstantInt::get(Type::getInt32Ty(Ctx)

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

[Lubomir Litchev via Phabricator via cfe-commits]

llitchev added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:119
   ///  should be placed.
-  /// \param Original The value being copied/created, should not be used in the
-  /// generated IR.
-  /// \param Inner The equivalent of \p Original that should be used in the
-  ///  generated IR; this is equal to \p Original if the value is
-  ///  a pointer and can thus be passed directly, otherwise it is
-  ///  an equivalent but different value.
+  /// \param \param Val The value beeing copied/created.
   /// \param ReplVal The replacement value, thus a copy or new created version

jdoerfert wrote:
> wasn't that a spelling error before? and one `\param` is enough ;)
Fixed.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:678
+  ///
+  /// \param CaptureAllocaInsPoint Insertion point for the alloca-ed struct.
+  /// \param OuterFn The function containing the omp::Parallel.

ftynse wrote:
> Nit: it looks like this file uses IP rather than InsPoint for names related 
> to insertion points
No need to store this value anymore. Used the InsertBB->getTerminator(), thus 
guaranteeing the alloca and stores are just before the fork call (they were 
before that call too, since the ThreadID was called last), so even if more 
codegen is introduced in the future the logic deals with it.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:441
+  IRBuilder<>::InsertPoint CaptureAllocaInsPoint(Builder.GetInsertBlock(),
+ --Builder.GetInsertPoint());
+

jdoerfert wrote:
> How do we know that `--` is valid here? Couldn't `Loc` point to the begin of 
> a function? If possible, let's just use `Loc.IP`.
There is always an instruction before - the ThreadID was always generated, and 
that is what -- points to. Changed it to use InsertBB->getTerminator(). It is 
much more sturdy this way. Even if the codegen is changed, the alloca, insert 
and store will be generated always just before the forkCall.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:760
+  SetVector BlockParents;
+  for (unsigned Counter = 0; Counter < Blocks.size(); Counter++) {
+BasicBlock *ParallelRegionBlock = Blocks[Counter];

ftynse wrote:
> Nit:  I think 
> https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop
>  applies to `.size()` the same way it applies to `.end()`
Fixed.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:807-808
+  llvm::IRBuilder<>::InsertPointGuard Guard(Builder);
+  Builder.SetInsertPoint(CaptureAllocaInsPoint.getBlock(),
+ CaptureAllocaInsPoint.getPoint());
+  // Allocate and populate the capture struct.

ftynse wrote:
> Nit: `Builder.restoreIP(CaptureAllocaInsPoint)` looks shorter
Refactored. No need to store the InsertPoint.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:810-815
+  AllocaInst = Builder.CreateAlloca(CaptureStructType, nullptr,
+"CaptureStructAlloca");
+  Value *InsertValue = UndefValue::get(CaptureStructType);
+  for (auto SrcIdx : enumerate(CapturedValues))
+InsertValue = Builder.CreateInsertValue(InsertValue, SrcIdx.value(),
+SrcIdx.index());

ftynse wrote:
> I suppose you may want to have `alloca` inserted in a block (function entry) 
> different from the one where you store into the memory. You need to store 
> just before calling the fork function (or, at least, so that the store 
> postdominates all stored values). Looking at the function API, I would have 
> assumed `CaptureAllocaInsPoint` to be an insertion point at the function 
> entry block specifically for `alloca`s, where these `insertvalue`s are 
> invalid.
Now it is guaranteed that. the codegen of the alloca, insert, and stores are 
done just before the forkCall. Even if the codegen changes in the future. It 
was the case before because the code was generated after the ThreadID getting 
call (which was just before the fork).



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:826-829
+  for (unsigned Counter = 0; Counter < Blocks.size(); Counter++)
+for (auto I = Blocks[Counter]->begin(), E = Blocks[Counter]->end();
+ I != E; ++I)
+  for (Use &U : I->operands())

ftynse wrote:
> Can we rather take each captured value and enumerate its uses, replacing 
> those within the parallel block set?
That was the first implementation I had. The issues was that the uses() was not 
returning all the uses (particularly the ones introduced by the loop unroller - 
spent bunch of time debugging it). Iterating to all the instruction parameter

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

[Lubomir Litchev via Phabricator via cfe-commits]

llitchev updated this revision to Diff 313041.
llitchev added a comment.

Removed the changes from threads.ll.

I'll pull this is a new Diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91556

Files:
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
  mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir

Index: mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
===
--- /dev/null
+++ mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-translate  --mlir-to-llvmir %s | FileCheck %s
+
+module {
+  llvm.func @malloc(!llvm.i64) -> !llvm.ptr
+  llvm.func @main() {
+%0 = llvm.mlir.constant(4 : index) : !llvm.i64
+%1 = llvm.mlir.constant(4 : index) : !llvm.i64
+%2 = llvm.mlir.null : !llvm.ptr
+%3 = llvm.mlir.constant(1 : index) : !llvm.i64
+%4 = llvm.getelementptr %2[%3] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+%5 = llvm.ptrtoint %4 : !llvm.ptr to !llvm.i64
+%6 = llvm.mul %1, %5 : !llvm.i64
+%7 = llvm.call @malloc(%6) : (!llvm.i64) -> !llvm.ptr
+%8 = llvm.bitcast %7 : !llvm.ptr to !llvm.ptr
+%9 = llvm.mlir.undef : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%10 = llvm.insertvalue %8, %9[0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%11 = llvm.insertvalue %8, %10[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%12 = llvm.mlir.constant(0 : index) : !llvm.i64
+%13 = llvm.insertvalue %12, %11[2] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%14 = llvm.mlir.constant(1 : index) : !llvm.i64
+%15 = llvm.insertvalue %1, %13[3, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%16 = llvm.insertvalue %14, %15[4, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%17 = llvm.mlir.constant(4.20e+01 : f32) : !llvm.float
+// CHECK: %CaptureStructAlloca = alloca %CapturedStructType
+// CHECK: %{{.*}} = insertvalue %CapturedStructType undef, {{.*}}, 0
+// CHECK: store %CapturedStructType %{{.*}}, %CapturedStructType* %CaptureStructAlloca
+omp.parallel num_threads(%0 : !llvm.i64) {
+  // CHECK: %{{.*}} = load %CapturedStructType, %CapturedStructType* %CaptureStructAlloca
+  // CHECK: %{{.*}} = extractvalue %CapturedStructType %{{.*}}, 0
+  %27 = llvm.mlir.constant(1 : i64) : !llvm.i64
+  %28 = llvm.extractvalue %16[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+  %29 = llvm.mlir.constant(0 : index) : !llvm.i64
+  %30 = llvm.mlir.constant(1 : index) : !llvm.i64
+  %31 = llvm.mul %27, %30 : !llvm.i64
+  %32 = llvm.add %29, %31 : !llvm.i64
+  %33 = llvm.getelementptr %28[%32] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+  llvm.store %17, %33 : !llvm.ptr
+  omp.terminator
+}
+llvm.return
+  }
+}
Index: mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
===
--- mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -6,7 +6,7 @@
   %end = constant 0 : index
   // CHECK: omp.parallel
   omp.parallel {
-// CHECK-NEXT: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
+// CHECK: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
 br ^bb1(%start, %end : index, index)
   // CHECK-NEXT: ^[[BB1]](%[[ARG1:[0-9]+]]: !llvm.i64, %[[ARG2:[0-9]+]]: !llvm.i64):{{.*}}
   ^bb1(%0: index, %1: index):
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -60,25 +60,6 @@
   DebugLoc DL;
 };
 
-// Returns the value stored in the given allocation. Returns null if the given
-// value is not a result of an allocation, if no value is stored or if there is
-// more than one store.
-static Value *findStoredValue(Value *AllocaValue) {
-  Instruction *Alloca = dyn_cast(AllocaValue);
-  if (!Alloca)
-return nullptr;
-  StoreInst *Store = nullptr;
-  for (Use &U : Alloca->uses()) {
-if (auto *CandidateStore = dyn_cast(U.getUser())) {
-  EXPECT_EQ(Store, nullptr);
-  Store = CandidateStore;
-}
-  }
-  if (!Store)
-return nullptr;
-  return Store->getValueOperand();
-}
-
 TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
@@ -425,7 +406,6 @@
   EXPECT_EQ(ForkCI->getArgOperand(1),
 ConstantInt::get(Type::getInt32Ty(Ctx), 1U));
   EXPECT_EQ(ForkCI

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

[Lubomir Litchev via Phabricator via cfe-commits]

llitchev updated this revision to Diff 313048.
llitchev added a comment.

Some minor optimizations related to CR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91556

Files:
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
  mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir

Index: mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
===
--- /dev/null
+++ mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-translate  --mlir-to-llvmir %s | FileCheck %s
+
+module {
+  llvm.func @malloc(!llvm.i64) -> !llvm.ptr
+  llvm.func @main() {
+%0 = llvm.mlir.constant(4 : index) : !llvm.i64
+%1 = llvm.mlir.constant(4 : index) : !llvm.i64
+%2 = llvm.mlir.null : !llvm.ptr
+%3 = llvm.mlir.constant(1 : index) : !llvm.i64
+%4 = llvm.getelementptr %2[%3] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+%5 = llvm.ptrtoint %4 : !llvm.ptr to !llvm.i64
+%6 = llvm.mul %1, %5 : !llvm.i64
+%7 = llvm.call @malloc(%6) : (!llvm.i64) -> !llvm.ptr
+%8 = llvm.bitcast %7 : !llvm.ptr to !llvm.ptr
+%9 = llvm.mlir.undef : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%10 = llvm.insertvalue %8, %9[0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%11 = llvm.insertvalue %8, %10[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%12 = llvm.mlir.constant(0 : index) : !llvm.i64
+%13 = llvm.insertvalue %12, %11[2] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%14 = llvm.mlir.constant(1 : index) : !llvm.i64
+%15 = llvm.insertvalue %1, %13[3, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%16 = llvm.insertvalue %14, %15[4, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%17 = llvm.mlir.constant(4.20e+01 : f32) : !llvm.float
+// CHECK: %CaptureStructAlloca = alloca %CapturedStructType
+// CHECK: %{{.*}} = insertvalue %CapturedStructType undef, {{.*}}, 0
+// CHECK: store %CapturedStructType %{{.*}}, %CapturedStructType* %CaptureStructAlloca
+omp.parallel num_threads(%0 : !llvm.i64) {
+  // CHECK: %{{.*}} = load %CapturedStructType, %CapturedStructType* %CaptureStructAlloca
+  // CHECK: %{{.*}} = extractvalue %CapturedStructType %{{.*}}, 0
+  %27 = llvm.mlir.constant(1 : i64) : !llvm.i64
+  %28 = llvm.extractvalue %16[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+  %29 = llvm.mlir.constant(0 : index) : !llvm.i64
+  %30 = llvm.mlir.constant(1 : index) : !llvm.i64
+  %31 = llvm.mul %27, %30 : !llvm.i64
+  %32 = llvm.add %29, %31 : !llvm.i64
+  %33 = llvm.getelementptr %28[%32] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+  llvm.store %17, %33 : !llvm.ptr
+  omp.terminator
+}
+llvm.return
+  }
+}
Index: mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
===
--- mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -6,7 +6,7 @@
   %end = constant 0 : index
   // CHECK: omp.parallel
   omp.parallel {
-// CHECK-NEXT: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
+// CHECK: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
 br ^bb1(%start, %end : index, index)
   // CHECK-NEXT: ^[[BB1]](%[[ARG1:[0-9]+]]: !llvm.i64, %[[ARG2:[0-9]+]]: !llvm.i64):{{.*}}
   ^bb1(%0: index, %1: index):
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -60,25 +60,6 @@
   DebugLoc DL;
 };
 
-// Returns the value stored in the given allocation. Returns null if the given
-// value is not a result of an allocation, if no value is stored or if there is
-// more than one store.
-static Value *findStoredValue(Value *AllocaValue) {
-  Instruction *Alloca = dyn_cast(AllocaValue);
-  if (!Alloca)
-return nullptr;
-  StoreInst *Store = nullptr;
-  for (Use &U : Alloca->uses()) {
-if (auto *CandidateStore = dyn_cast(U.getUser())) {
-  EXPECT_EQ(Store, nullptr);
-  Store = CandidateStore;
-}
-  }
-  if (!Store)
-return nullptr;
-  return Store->getValueOperand();
-}
-
 TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
@@ -425,7 +406,6 @@
   EXPECT_EQ(ForkCI->getArgOperand(1),
 ConstantInt::get(Type::getInt32Ty(Ctx), 1U));
   EXPECT_EQ(ForkCI->getArgOperand(2), Usr);
- 

[PATCH] D87587: [clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2463
 
+**MaxUnwrappedLinesForShortNamespace** (``unsigned``)
+  The maximal number of unwrapped lines that a short namespace spans.

This is quite a mouthful before this lands do you want to consider shortening 
it?

`ShortNamespaceLength` ?

I'm not sure non clang-formast developers will know what an UnwrappedLine is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3080
   }
   Opts.setFPExceptionMode(FPEB);
 

jansvoboda11 wrote:
> The parsing of `OPT_ftrapping_math` and `OPT_fno_trapping_math` is 
> immediately overwritten here.
Yeah, I did some work in this area some years ago, but that's a few years ago, 
and then in addition to that, we are talking about option handling in Clang 
which I always find very confusing... Just saying I can't remember too many 
details at this point.

But starting with a first observation, you're saying that things are 
overwritten here, but they are different options. I.e., the deleted part 
honours `OPT_ftrapping_math`, and here exception modes are set based on 
`OPT_ffp_exception_behavior`. So, looks like there is some interaction here... 
Do we know how this should work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93395

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

[clang] 27b7d64 - [clang][cli] Streamline MarshallingInfoFlag description

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2020-12-21T11:32:46+01:00
New Revision: 27b7d646886d499c70dec3481dfc3c82dfc43dd7

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

LOG: [clang][cli] Streamline MarshallingInfoFlag description

This replaces the existing `MarshallingInfoFlag<...>, IsNegative` with simpler 
`MarshallingInfoNegativeFlag`.

Reviewed By: dexonsmith

Original patch by Daniel Grumberg.

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
llvm/include/llvm/Option/OptParser.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 63a5b5484f0f..29ee948f1849 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -696,7 +696,7 @@ def O_flag : Flag<["-"], "O">, Flags<[CC1Option]>, 
Alias, AliasArgs<["1"]>;
 def Ofast : Joined<["-"], "Ofast">, Group, Flags<[CC1Option]>;
 def P : Flag<["-"], "P">, Flags<[CC1Option]>, Group,
   HelpText<"Disable linemarker output in -E mode">,
-  MarshallingInfoFlag<"PreprocessorOutputOpts.ShowLineMarkers", "true">, 
IsNegative;
+  MarshallingInfoNegativeFlag<"PreprocessorOutputOpts.ShowLineMarkers">;
 def Qy : Flag<["-"], "Qy">, Flags<[CC1Option]>,
   HelpText<"Emit metadata containing compiler name and version">;
 def Qn : Flag<["-"], "Qn">, Flags<[CC1Option]>,
@@ -1212,7 +1212,7 @@ def : Flag<["-"], "frecord-gcc-switches">, 
Alias;
 def : Flag<["-"], "fno-record-gcc-switches">, Alias;
 def fcommon : Flag<["-"], "fcommon">, Group,
   Flags<[CoreOption, CC1Option]>, HelpText<"Place uninitialized global 
variables in a common block">,
-  MarshallingInfoFlag<"CodeGenOpts.NoCommon", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"CodeGenOpts.NoCommon">;
 def fcompile_resource_EQ : Joined<["-"], "fcompile-resource=">, Group;
 defm complete_member_pointers : BoolOption<"complete-member-pointers",
   "LangOpts->CompleteMemberPointers", DefaultsToFalse,
@@ -1856,7 +1856,7 @@ def fmodules_validate_once_per_build_session : 
Flag<["-"], "fmodules-validate-on
 def fmodules_disable_diagnostic_validation : Flag<["-"], 
"fmodules-disable-diagnostic-validation">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disable validation of the diagnostic options when loading the 
module">,
-  MarshallingInfoFlag<"HeaderSearchOpts->ModulesValidateDiagnosticOptions", 
"true">, IsNegative;
+  
MarshallingInfoNegativeFlag<"HeaderSearchOpts->ModulesValidateDiagnosticOptions">;
 defm modules_validate_system_headers : 
BoolOption<"modules-validate-system-headers",
   "HeaderSearchOpts->ModulesValidateSystemHeaders", DefaultsToFalse,
   ChangedBy,
@@ -1944,7 +1944,7 @@ def fno_asm : Flag<["-"], "fno-asm">, Group;
 def fno_asynchronous_unwind_tables : Flag<["-"], 
"fno-asynchronous-unwind-tables">, Group;
 def fno_assume_sane_operator_new : Flag<["-"], 
"fno-assume-sane-operator-new">, Group,
   HelpText<"Don't assume that C++'s global operator new can't alias any 
pointer">,
-  Flags<[CC1Option]>, MarshallingInfoFlag<"CodeGenOpts.AssumeSaneOperatorNew", 
"true">, IsNegative;
+  Flags<[CC1Option]>, 
MarshallingInfoNegativeFlag<"CodeGenOpts.AssumeSaneOperatorNew">;
 def fno_builtin : Flag<["-"], "fno-builtin">, Group, 
Flags<[CC1Option, CoreOption]>,
   HelpText<"Disable implicit builtin knowledge of functions">;
 def fno_builtin_ : Joined<["-"], "fno-builtin-">, Group, 
Flags<[CC1Option, CoreOption]>,
@@ -2022,7 +2022,7 @@ def fno_strict_overflow : Flag<["-"], 
"fno-strict-overflow">, Group;
 def fno_temp_file : Flag<["-"], "fno-temp-file">, Group,
   Flags<[CC1Option, CoreOption]>, HelpText<
   "Directly create compilation output files. This may lead to incorrect 
incremental builds if the compiler crashes">,
-  MarshallingInfoFlag<"FrontendOpts.UseTemporary", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"FrontendOpts.UseTemporary">;
 defm use_cxa_atexit : BoolFOption<"use-cxa-atexit",
   "CodeGenOpts.CXAAtExit", DefaultsToTrue,
   ChangedBy,
@@ -2030,7 +2030,7 @@ defm use_cxa_atexit : BoolFOption<"use-cxa-atexit",
 def fno_unit_at_a_time : Flag<["-"], "fno-unit-at-a-time">, Group;
 def fno_unwind_tables : Flag<["-"], "fno-unwind-tables">, Group;
 def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group, 
Flags<[CC1Option]>,
-  MarshallingInfoFlag<"CodeGenOpts.AsmVerbose", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"CodeGenOpts.AsmVerbose">;
 def fno_working_directory : Flag<["-"], "fno-working-directory">, 
Group;
 def fno_wrapv : Flag<["-"], "fno-wrapv">, Group;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group, Flags<[CC1Option]>,
@@ -3313,7 +3313,7 @@ def no_pedantic : Flag<["-", "--"], "no-pedantic">, 
Group;
 def no__dead__strip__inits__and__terms : Flag<["-"], 
"no_dead_strip_inits_and_ter

[clang] 70410a2 - [clang][cli] Let denormalizer decide how to render the option based on the option class

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2020-12-21T11:32:47+01:00
New Revision: 70410a264949101ced3ce3458f37dd4cc2f5af85

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

LOG: [clang][cli] Let denormalizer decide how to render the option based on the 
option class

Before this patch, you needed to use `AutoNormalizeEnumJoined` whenever you 
wanted to **de**normalize joined enum.
Besides the naming confusion, this means the fact the option is joined is 
specified in two places: in the normalization multiclass and in the 
`Joined<["-"], ...>` multiclass.
This patch makes this work automatically, taking into account the `OptionClass` 
of options.

Also, the enum denormalizer now just looks up the spelling of the present enum 
case in a table and forwards it to the string denormalizer.

I also added more tests that exercise this.

Reviewed By: dexonsmith

Original patch by Daniel Grumberg.

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/unittests/Frontend/CompilerInvocationTest.cpp
llvm/include/llvm/Option/OptParser.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 29ee948f1849..82c4e9399d9d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4882,7 +4882,7 @@ def arcmt_action_EQ : Joined<["-"], "arcmt-action=">, 
Flags<[CC1Option, NoDriver
   NormalizedValuesScope<"FrontendOptions">,
   NormalizedValues<["ARCMT_Check", "ARCMT_Modify", "ARCMT_Migrate"]>,
   MarshallingInfoString<"FrontendOpts.ARCMTAction", "ARCMT_None">,
-  AutoNormalizeEnumJoined;
+  AutoNormalizeEnum;
 
 def opt_record_file : Separate<["-"], "opt-record-file">,
   HelpText<"File name to use for YAML optimization record output">,

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 00615a70d730..f71b14eabc49 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -152,8 +152,8 @@ static Optional 
normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned,
 /// argument.
 static void denormalizeSimpleFlag(SmallVectorImpl &Args,
   const char *Spelling,
-  CompilerInvocation::StringAllocator, 
unsigned,
-  /*T*/...) {
+  CompilerInvocation::StringAllocator,
+  Option::OptionClass, unsigned, /*T*/...) {
   Args.push_back(Spelling);
 }
 
@@ -200,12 +200,41 @@ static auto makeBooleanOptionNormalizer(bool Value, bool 
OtherValue,
 
 static auto makeBooleanOptionDenormalizer(bool Value) {
   return [Value](SmallVectorImpl &Args, const char *Spelling,
- CompilerInvocation::StringAllocator, unsigned, bool KeyPath) {
+ CompilerInvocation::StringAllocator, Option::OptionClass,
+ unsigned, bool KeyPath) {
 if (KeyPath == Value)
   Args.push_back(Spelling);
   };
 }
 
+static void denormalizeStringImpl(SmallVectorImpl &Args,
+  const char *Spelling,
+  CompilerInvocation::StringAllocator SA,
+  Option::OptionClass OptClass, unsigned,
+  Twine Value) {
+  switch (OptClass) {
+  case Option::SeparateClass:
+  case Option::JoinedOrSeparateClass:
+Args.push_back(Spelling);
+Args.push_back(SA(Value));
+break;
+  case Option::JoinedClass:
+Args.push_back(SA(Twine(Spelling) + Value));
+break;
+  default:
+llvm_unreachable("Cannot denormalize an option with option class "
+ "incompatible with string denormalization.");
+  }
+}
+
+template 
+static void
+denormalizeString(SmallVectorImpl &Args, const char *Spelling,
+  CompilerInvocation::StringAllocator SA,
+  Option::OptionClass OptClass, unsigned TableIndex, T Value) {
+  denormalizeStringImpl(Args, Spelling, SA, OptClass, TableIndex, 
Twine(Value));
+}
+
 static Optional
 findValueTableByName(const SimpleEnumValueTable &Table, StringRef Name) {
   for (int I = 0, E = Table.Size; I != E; ++I)
@@ -247,12 +276,13 @@ static llvm::Optional 
normalizeSimpleEnum(OptSpecifier Opt,
 static void denormalizeSimpleEnumImpl(SmallVectorImpl &Args,
   const char *Spelling,
   CompilerInvocation::StringAllocator SA,
+  Option::OptionClass OptClass,
   unsigned TableIndex, unsigned Value) {
   assert(TableIndex < SimpleEnu

[PATCH] D84675: [clang][cli] Streamline MarhsallingInfoFlag description

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG27b7d646886d: [clang][cli] Streamline MarshallingInfoFlag 
description (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84675

Files:
  clang/include/clang/Driver/Options.td
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -173,6 +173,12 @@
   code Denormalizer = "denormalizeSimpleFlag";
 }
 
+class MarshallingInfoNegativeFlag
+  : MarshallingInfo {
+  code Normalizer = "normalizeSimpleNegativeFlag";
+  code Denormalizer = "denormalizeSimpleFlag";
+}
+
 class MarshallingInfoBitfieldFlag
   : MarshallingInfoFlag {
   code Normalizer = "makeFlagToValueNormalizer("#value#")";
@@ -190,9 +196,6 @@
 
 // Mixins for additional marshalling attributes.
 
-class IsNegative {
-  code Normalizer = "normalizeSimpleNegativeFlag";
-}
 class AlwaysEmit { bit ShouldAlwaysEmit = true; }
 class Normalizer { code Normalizer = normalizer; }
 class Denormalizer { code Denormalizer = denormalizer; }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -696,7 +696,7 @@
 def Ofast : Joined<["-"], "Ofast">, Group, Flags<[CC1Option]>;
 def P : Flag<["-"], "P">, Flags<[CC1Option]>, Group,
   HelpText<"Disable linemarker output in -E mode">,
-  MarshallingInfoFlag<"PreprocessorOutputOpts.ShowLineMarkers", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"PreprocessorOutputOpts.ShowLineMarkers">;
 def Qy : Flag<["-"], "Qy">, Flags<[CC1Option]>,
   HelpText<"Emit metadata containing compiler name and version">;
 def Qn : Flag<["-"], "Qn">, Flags<[CC1Option]>,
@@ -1212,7 +1212,7 @@
 def : Flag<["-"], "fno-record-gcc-switches">, Alias;
 def fcommon : Flag<["-"], "fcommon">, Group,
   Flags<[CoreOption, CC1Option]>, HelpText<"Place uninitialized global variables in a common block">,
-  MarshallingInfoFlag<"CodeGenOpts.NoCommon", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"CodeGenOpts.NoCommon">;
 def fcompile_resource_EQ : Joined<["-"], "fcompile-resource=">, Group;
 defm complete_member_pointers : BoolOption<"complete-member-pointers",
   "LangOpts->CompleteMemberPointers", DefaultsToFalse,
@@ -1856,7 +1856,7 @@
 def fmodules_disable_diagnostic_validation : Flag<["-"], "fmodules-disable-diagnostic-validation">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disable validation of the diagnostic options when loading the module">,
-  MarshallingInfoFlag<"HeaderSearchOpts->ModulesValidateDiagnosticOptions", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"HeaderSearchOpts->ModulesValidateDiagnosticOptions">;
 defm modules_validate_system_headers : BoolOption<"modules-validate-system-headers",
   "HeaderSearchOpts->ModulesValidateSystemHeaders", DefaultsToFalse,
   ChangedBy,
@@ -1944,7 +1944,7 @@
 def fno_asynchronous_unwind_tables : Flag<["-"], "fno-asynchronous-unwind-tables">, Group;
 def fno_assume_sane_operator_new : Flag<["-"], "fno-assume-sane-operator-new">, Group,
   HelpText<"Don't assume that C++'s global operator new can't alias any pointer">,
-  Flags<[CC1Option]>, MarshallingInfoFlag<"CodeGenOpts.AssumeSaneOperatorNew", "true">, IsNegative;
+  Flags<[CC1Option]>, MarshallingInfoNegativeFlag<"CodeGenOpts.AssumeSaneOperatorNew">;
 def fno_builtin : Flag<["-"], "fno-builtin">, Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Disable implicit builtin knowledge of functions">;
 def fno_builtin_ : Joined<["-"], "fno-builtin-">, Group, Flags<[CC1Option, CoreOption]>,
@@ -2022,7 +2022,7 @@
 def fno_temp_file : Flag<["-"], "fno-temp-file">, Group,
   Flags<[CC1Option, CoreOption]>, HelpText<
   "Directly create compilation output files. This may lead to incorrect incremental builds if the compiler crashes">,
-  MarshallingInfoFlag<"FrontendOpts.UseTemporary", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"FrontendOpts.UseTemporary">;
 defm use_cxa_atexit : BoolFOption<"use-cxa-atexit",
   "CodeGenOpts.CXAAtExit", DefaultsToTrue,
   ChangedBy,
@@ -2030,7 +2030,7 @@
 def fno_unit_at_a_time : Flag<["-"], "fno-unit-at-a-time">, Group;
 def fno_unwind_tables : Flag<["-"], "fno-unwind-tables">, Group;
 def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group, Flags<[CC1Option]>,
-  MarshallingInfoFlag<"CodeGenOpts.AsmVerbose", "true">, IsNegative;
+  MarshallingInfoNegativeFlag<"CodeGenOpts.AsmVerbose">;
 def fno_working_directory : Flag<["-"], "fno-working-directory">, Group;
 def fno_wrapv : Flag<["-"], "fno-wrapv">, Group;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group, Flags<[CC1Option]>,
@@ -3313,7 +3313,7 @@
 def no__dead__s

[clang] 5a85526 - [clang] Use enum for LangOptions::SYCLVersion instead of unsigned

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2020-12-21T11:32:47+01:00
New Revision: 5a85526728c9e57efe26f322e4718fffd2634d23

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

LOG: [clang] Use enum for LangOptions::SYCLVersion instead of unsigned

`LangOptions::SYCLVersion` can only have two values. This patch introduces an 
enum that allows us to reduce the member size from 32 bits to 1 bit.

Consequently, this also makes marshalling of this option fit into our model for 
enums: D84674.

Reviewed By: bader

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Frontend/InitPreprocessor.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 251fd68f4df8..cc5eb939dbd2 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -246,7 +246,7 @@ LANGOPT(GPUExcludeWrongSideOverloads, 1, 0, "always exclude 
wrong side overloads
 
 LANGOPT(SYCL  , 1, 0, "SYCL")
 LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
-LANGOPT(SYCLVersion   , 32, 0, "Version of the SYCL standard used")
+ENUM_LANGOPT(SYCLVersion  , SYCLMajorVersion, 1, SYCL_None, "Version of the 
SYCL standard used")
 
 LANGOPT(HIPUseNewLaunchAPI, 1, 0, "Use new kernel launching API for HIP")
 

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index ed9f729417af..8b3fb562561f 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -125,6 +125,11 @@ class LangOptions : public LangOptionsBase {
 MSVC2019 = 1920,
   };
 
+  enum SYCLMajorVersion {
+SYCL_None,
+SYCL_2017,
+  };
+
   /// Clang versions with 
diff erent platform ABI conformance.
   enum class ClangABI {
 /// Attempt to be ABI-compatible with code generated by Clang 3.8.x

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f71b14eabc49..fc5fd1547599 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2277,11 +2277,13 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
 // -sycl-std applies to any SYCL source, not only those containing kernels,
 // but also those using the SYCL API
 if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
-  Opts.SYCLVersion = llvm::StringSwitch(A->getValue())
- .Cases("2017", "1.2.1", "121", "sycl-1.2.1", 2017)
- .Default(0U);
+  Opts.setSYCLVersion(
+  llvm::StringSwitch(A->getValue())
+  .Cases("2017", "1.2.1", "121", "sycl-1.2.1",
+ LangOptions::SYCL_2017)
+  .Default(LangOptions::SYCL_None));
 
-  if (Opts.SYCLVersion == 0U) {
+  if (Opts.getSYCLVersion() == LangOptions::SYCL_None) {
 // User has passed an invalid value to the flag, this is an error
 Diags.Report(diag::err_drv_invalid_value)
 << A->getAsString(Args) << A->getValue();

diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index d4b77a65aa63..87af9247b91c 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -476,7 +476,7 @@ static void InitializeStandardPredefinedMacros(const 
TargetInfo &TI,
 
   if (LangOpts.SYCL) {
 // SYCL Version is set to a value when building SYCL applications
-if (LangOpts.SYCLVersion == 2017)
+if (LangOpts.getSYCLVersion() == LangOptions::SYCL_2017)
   Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
   }
 



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

[PATCH] D93540: [clang] Use enum for LangOptions::SYCLVersion instead of unsigned

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a85526728c9: [clang] Use enum for LangOptions::SYCLVersion 
instead of unsigned (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93540

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -476,7 +476,7 @@
 
   if (LangOpts.SYCL) {
 // SYCL Version is set to a value when building SYCL applications
-if (LangOpts.SYCLVersion == 2017)
+if (LangOpts.getSYCLVersion() == LangOptions::SYCL_2017)
   Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
   }
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2277,11 +2277,13 @@
 // -sycl-std applies to any SYCL source, not only those containing kernels,
 // but also those using the SYCL API
 if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
-  Opts.SYCLVersion = llvm::StringSwitch(A->getValue())
- .Cases("2017", "1.2.1", "121", "sycl-1.2.1", 2017)
- .Default(0U);
+  Opts.setSYCLVersion(
+  llvm::StringSwitch(A->getValue())
+  .Cases("2017", "1.2.1", "121", "sycl-1.2.1",
+ LangOptions::SYCL_2017)
+  .Default(LangOptions::SYCL_None));
 
-  if (Opts.SYCLVersion == 0U) {
+  if (Opts.getSYCLVersion() == LangOptions::SYCL_None) {
 // User has passed an invalid value to the flag, this is an error
 Diags.Report(diag::err_drv_invalid_value)
 << A->getAsString(Args) << A->getValue();
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -125,6 +125,11 @@
 MSVC2019 = 1920,
   };
 
+  enum SYCLMajorVersion {
+SYCL_None,
+SYCL_2017,
+  };
+
   /// Clang versions with different platform ABI conformance.
   enum class ClangABI {
 /// Attempt to be ABI-compatible with code generated by Clang 3.8.x
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -246,7 +246,7 @@
 
 LANGOPT(SYCL  , 1, 0, "SYCL")
 LANGOPT(SYCLIsDevice  , 1, 0, "Generate code for SYCL device")
-LANGOPT(SYCLVersion   , 32, 0, "Version of the SYCL standard used")
+ENUM_LANGOPT(SYCLVersion  , SYCLMajorVersion, 1, SYCL_None, "Version of the 
SYCL standard used")
 
 LANGOPT(HIPUseNewLaunchAPI, 1, 0, "Use new kernel launching API for HIP")
 


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -476,7 +476,7 @@
 
   if (LangOpts.SYCL) {
 // SYCL Version is set to a value when building SYCL applications
-if (LangOpts.SYCLVersion == 2017)
+if (LangOpts.getSYCLVersion() == LangOptions::SYCL_2017)
   Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
   }
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2277,11 +2277,13 @@
 // -sycl-std applies to any SYCL source, not only those containing kernels,
 // but also those using the SYCL API
 if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
-  Opts.SYCLVersion = llvm::StringSwitch(A->getValue())
- .Cases("2017", "1.2.1", "121", "sycl-1.2.1", 2017)
- .Default(0U);
+  Opts.setSYCLVersion(
+  llvm::StringSwitch(A->getValue())
+  .Cases("2017", "1.2.1", "121", "sycl-1.2.1",
+ LangOptions::SYCL_2017)
+  .Default(LangOptions::SYCL_None));
 
-  if (Opts.SYCLVersion == 0U) {
+  if (Opts.getSYCLVersion() == LangOptions::SYCL_None) {
 // User has passed an invalid value to the flag, this is an error
 Diags.Report(diag::err_drv_invalid_value)
 << A->getAsString(Args) << A->getValue();
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangO

[PATCH] D84189: [clang][cli] Let denormalizer decide how to render the option based on the option class

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG70410a264949: [clang][cli] Let denormalizer decide how to 
render the option based on the… (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84189

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -205,9 +205,6 @@
   code Normalizer = "normalizeSimpleEnum";
   code Denormalizer = "denormalizeSimpleEnum";
 }
-class AutoNormalizeEnumJoined : AutoNormalizeEnum {
-  code Denormalizer = "denormalizeSimpleEnumJoined";
-}
 class ValueMerger { code ValueMerger = merger; }
 class ValueExtractor { code ValueExtractor = extractor; }
 
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -342,30 +342,70 @@
   ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str(;
 }
 
-TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnumNonDefault) {
+TEST_F(CommandLineTest, SeparateEnumNonDefault) {
   const char *Args[] = {"-mrelocation-model", "static"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().RelocationModel, Reloc::Model::Static);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Non default relocation model.
+  ASSERT_THAT(GeneratedArgs, Contains(StrEq("-mrelocation-model")));
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("static")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-mrelocation-model=static";
 }
 
-TEST_F(CommandLineTest, CanGenerateCC1COmmandLineSeparateEnumDefault) {
+TEST_F(CommandLineTest, SeparateEnumDefault) {
   const char *Args[] = {"-mrelocation-model", "pic"};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().RelocationModel, Reloc::Model::PIC_);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   // Default relocation model.
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-mrelocation-model";
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-mrelocation-model=pic";
+}
+
+TEST_F(CommandLineTest, JoinedEnumNonDefault) {
+  const char *Args[] = {"-fobjc-dispatch-method=non-legacy"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().getObjCDispatchMethod(),
+CodeGenOptions::NonLegacy);
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs,
+  Contains(StrEq("-fobjc-dispatch-method=non-legacy")));
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fobjc-dispatch-method=";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("non-legacy";
+}
+
+TEST_F(CommandLineTest, JoinedEnumDefault) {
+  const char *Args[] = {"-fobjc-dispatch-method=legacy"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getCodeGenOpts().getObjCDispatchMethod(),
+CodeGenOptions::Legacy);
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs,
+  Not(Contains(StrEq("-fobjc-dispatch-method=legacy";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fobjc-dispatch-method=";
+  ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("legacy";
 }
 
 // Tree of boolean options that can be (directly or transitively) implied by
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -152,8 +152,8 @@
 /// argument.
 static void denormalizeSimpleFlag(SmallVectorImpl &Args,
   const char *Spelling,
-  CompilerInvocation::StringAllocator, unsigned,
-  /*T*/...) {
+  CompilerInvocation::StringAllocator,
+  Option::OptionClass, unsigned, /*T*/...) {
   Args.push_back(Spelling);
 }
 
@@ -200,12 +200,41 @@
 
 static auto makeBooleanOptionDenormalizer(bool Value) {
   return [Value](SmallVectorImpl &Args, cons

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

@SjoerdMeijer If you're not comfortable reviewing this, do you know of anyone 
who might want to take a look?




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3080
   }
   Opts.setFPExceptionMode(FPEB);
 

SjoerdMeijer wrote:
> jansvoboda11 wrote:
> > The parsing of `OPT_ftrapping_math` and `OPT_fno_trapping_math` is 
> > immediately overwritten here.
> Yeah, I did some work in this area some years ago, but that's a few years 
> ago, and then in addition to that, we are talking about option handling in 
> Clang which I always find very confusing... Just saying I can't remember too 
> many details at this point.
> 
> But starting with a first observation, you're saying that things are 
> overwritten here, but they are different options. I.e., the deleted part 
> honours `OPT_ftrapping_math`, and here exception modes are set based on 
> `OPT_ffp_exception_behavior`. So, looks like there is some interaction 
> here... Do we know how this should work?
I see. The thing is, before this patch, we call `Opts.setFPExceptionMode` 
whenever we see either `OPT_ftrapping_math` or `OPT_fno_trapping_math`.

But then, we **unconditionally** call `Opts.setFPExceptionMode(FPEB)` again 
here. That **always** overwrites what we previously did for 
`OPT_f[no_]trapping_math`, making that a dead code.

`Opts.setFPExceptionMode()` doesn't do anything fancy, just assigns the 
argument to the `Opts.FPExceptionModel` member.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93395

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

[PATCH] D87587: [clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2463
 
+**MaxUnwrappedLinesForShortNamespace** (``unsigned``)
+  The maximal number of unwrapped lines that a short namespace spans.

MyDeveloperDay wrote:
> This is quite a mouthful before this lands do you want to consider shortening 
> it?
> 
> `ShortNamespaceLength` ?
> 
> I'm not sure non clang-formast developers will know what an UnwrappedLine is?
I agree that one needs to wrap its head around to digest this name. I'd vote 
for what @mydeveloperday proposed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3080
   }
   Opts.setFPExceptionMode(FPEB);
 

jansvoboda11 wrote:
> SjoerdMeijer wrote:
> > jansvoboda11 wrote:
> > > The parsing of `OPT_ftrapping_math` and `OPT_fno_trapping_math` is 
> > > immediately overwritten here.
> > Yeah, I did some work in this area some years ago, but that's a few years 
> > ago, and then in addition to that, we are talking about option handling in 
> > Clang which I always find very confusing... Just saying I can't remember 
> > too many details at this point.
> > 
> > But starting with a first observation, you're saying that things are 
> > overwritten here, but they are different options. I.e., the deleted part 
> > honours `OPT_ftrapping_math`, and here exception modes are set based on 
> > `OPT_ffp_exception_behavior`. So, looks like there is some interaction 
> > here... Do we know how this should work?
> I see. The thing is, before this patch, we call `Opts.setFPExceptionMode` 
> whenever we see either `OPT_ftrapping_math` or `OPT_fno_trapping_math`.
> 
> But then, we **unconditionally** call `Opts.setFPExceptionMode(FPEB)` again 
> here. That **always** overwrites what we previously did for 
> `OPT_f[no_]trapping_math`, making that a dead code.
> 
> `Opts.setFPExceptionMode()` doesn't do anything fancy, just assigns the 
> argument to the `Opts.FPExceptionModel` member.
Ah yes, I actually missed that we always set `Opts.setFPExceptionMode(FPEB)` 
here! So that's clear now.

But looks like my previous question is still relevant how these options should 
work together? I now see that 
`-ffp-exception-behavior` is a intel/microsoft special introduced in  D62731.

And while currently we ignore `-fftrapping-math` here, but that just seems like 
a bug, it looks like we actually need to handle it here too?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93395

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

[PATCH] D93626: [clang-format] PR48535 clang-format Incorrectly Removes Space After C Style Cast When Type Is Not a Pointer

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, krasimir, JakeMerdichAMD.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

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

using `SpaceAfterCStyleCast: true`

  size_t idx = (size_t) a;
  size_t idx = (size_t) (a - 1);

is formatted as:

  size_t idx = (size_t) a;
  size_t idx = (size_t)(a - 1);

This revision aims to improve that by improving the function which tries to 
identify a CastRParen


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93626

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11989,6 +11989,20 @@
"  do_something((int) i);\n"
"} while (something( ));",
Spaces);
+
+  verifyFormat("size_t idx = (size_t) (ptr - ((char *) file));", Spaces);
+  verifyFormat("size_t idx = (size_t) a;", Spaces);
+  verifyFormat("size_t idx = (size_t) (a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->*foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
+  Spaces.SpaceAfterCStyleCast = false;
+  verifyFormat("size_t idx = (size_t)(ptr - ((char *)file));", Spaces);
+  verifyFormat("size_t idx = (size_t)a;", Spaces);
+  verifyFormat("size_t idx = (size_t)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->*foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1915,6 +1915,13 @@
 if (Tok.Next->isOneOf(tok::identifier, tok::kw_this))
   return true;
 
+if (Tok.Next->is(tok::l_paren) &&
+!(Tok.Previous && Tok.Previous->is(tok::identifier) &&
+  Tok.Previous->Previous &&
+  Tok.Previous->Previous->isOneOf(tok::arrowstar, tok::arrow,
+  tok::star)))
+  return true;
+
 if (!Tok.Next->Next)
   return false;
 
@@ -1964,9 +1971,9 @@
 
 if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
tok::comma, tok::semi, tok::kw_return, tok::colon,
-   tok::kw_co_return, tok::kw_co_await, 
tok::kw_co_yield,
-   tok::equal, tok::kw_delete, tok::kw_sizeof,
-   tok::kw_throw) ||
+   tok::kw_co_return, tok::kw_co_await,
+   tok::kw_co_yield, tok::equal, tok::kw_delete,
+   tok::kw_sizeof, tok::kw_throw) ||
 PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
TT_UnaryOperator, TT_CastRParen))
   return TT_UnaryOperator;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11989,6 +11989,20 @@
"  do_something((int) i);\n"
"} while (something( ));",
Spaces);
+
+  verifyFormat("size_t idx = (size_t) (ptr - ((char *) file));", Spaces);
+  verifyFormat("size_t idx = (size_t) a;", Spaces);
+  verifyFormat("size_t idx = (size_t) (a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->*foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
+  Spaces.SpaceAfterCStyleCast = false;
+  verifyFormat("size_t idx = (size_t)(ptr - ((char *)file));", Spaces);
+  verifyFormat("size_t idx = (size_t)a;", Spaces);
+  verifyFormat("size_t idx = (size_t)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->*foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
+  verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1915,6 +1915,13 @@
 if (Tok.Next->isOneOf(tok::identifier, tok::kw_this))
   return true;
 
+if (Tok.Next->is(tok::l_paren) &&
+!(Tok.Previous && Tok.Previous->is(tok::identifier) &&
+  Tok.Previous->Previous &&
+  Tok.Previous->Previous->isOneOf(tok::arrowstar, tok::arrow,
+  tok::star)))
+  return true;
+
 if (!Tok.Next->Next)
   return f

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

[Lubomir Litchev via Phabricator via cfe-commits]

llitchev updated this revision to Diff 313077.
llitchev added a comment.

Fixed a casing issue with a local var.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91556

Files:
  clang/test/OpenMP/parallel_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
  mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir

Index: mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
===
--- /dev/null
+++ mlir/test/Conversion/OpenMPToLLVM/openmp_float-parallel_param.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-translate  --mlir-to-llvmir %s | FileCheck %s
+
+module {
+  llvm.func @malloc(!llvm.i64) -> !llvm.ptr
+  llvm.func @main() {
+%0 = llvm.mlir.constant(4 : index) : !llvm.i64
+%1 = llvm.mlir.constant(4 : index) : !llvm.i64
+%2 = llvm.mlir.null : !llvm.ptr
+%3 = llvm.mlir.constant(1 : index) : !llvm.i64
+%4 = llvm.getelementptr %2[%3] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+%5 = llvm.ptrtoint %4 : !llvm.ptr to !llvm.i64
+%6 = llvm.mul %1, %5 : !llvm.i64
+%7 = llvm.call @malloc(%6) : (!llvm.i64) -> !llvm.ptr
+%8 = llvm.bitcast %7 : !llvm.ptr to !llvm.ptr
+%9 = llvm.mlir.undef : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%10 = llvm.insertvalue %8, %9[0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%11 = llvm.insertvalue %8, %10[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%12 = llvm.mlir.constant(0 : index) : !llvm.i64
+%13 = llvm.insertvalue %12, %11[2] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%14 = llvm.mlir.constant(1 : index) : !llvm.i64
+%15 = llvm.insertvalue %1, %13[3, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%16 = llvm.insertvalue %14, %15[4, 0] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+%17 = llvm.mlir.constant(4.20e+01 : f32) : !llvm.float
+// CHECK: %CaptureStructAlloca = alloca %CapturedStructType
+// CHECK: %{{.*}} = insertvalue %CapturedStructType undef, {{.*}}, 0
+// CHECK: store %CapturedStructType %{{.*}}, %CapturedStructType* %CaptureStructAlloca
+omp.parallel num_threads(%0 : !llvm.i64) {
+  // CHECK: %{{.*}} = load %CapturedStructType, %CapturedStructType* %CaptureStructAlloca
+  // CHECK: %{{.*}} = extractvalue %CapturedStructType %{{.*}}, 0
+  %27 = llvm.mlir.constant(1 : i64) : !llvm.i64
+  %28 = llvm.extractvalue %16[1] : !llvm.struct<(ptr, ptr, i64, array<1 x i64>, array<1 x i64>)>
+  %29 = llvm.mlir.constant(0 : index) : !llvm.i64
+  %30 = llvm.mlir.constant(1 : index) : !llvm.i64
+  %31 = llvm.mul %27, %30 : !llvm.i64
+  %32 = llvm.add %29, %31 : !llvm.i64
+  %33 = llvm.getelementptr %28[%32] : (!llvm.ptr, !llvm.i64) -> !llvm.ptr
+  llvm.store %17, %33 : !llvm.ptr
+  omp.terminator
+}
+llvm.return
+  }
+}
Index: mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
===
--- mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -6,7 +6,7 @@
   %end = constant 0 : index
   // CHECK: omp.parallel
   omp.parallel {
-// CHECK-NEXT: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
+// CHECK: llvm.br ^[[BB1:.*]](%{{[0-9]+}}, %{{[0-9]+}} : !llvm.i64, !llvm.i64
 br ^bb1(%start, %end : index, index)
   // CHECK-NEXT: ^[[BB1]](%[[ARG1:[0-9]+]]: !llvm.i64, %[[ARG2:[0-9]+]]: !llvm.i64):{{.*}}
   ^bb1(%0: index, %1: index):
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -60,25 +60,6 @@
   DebugLoc DL;
 };
 
-// Returns the value stored in the given allocation. Returns null if the given
-// value is not a result of an allocation, if no value is stored or if there is
-// more than one store.
-static Value *findStoredValue(Value *AllocaValue) {
-  Instruction *Alloca = dyn_cast(AllocaValue);
-  if (!Alloca)
-return nullptr;
-  StoreInst *Store = nullptr;
-  for (Use &U : Alloca->uses()) {
-if (auto *CandidateStore = dyn_cast(U.getUser())) {
-  EXPECT_EQ(Store, nullptr);
-  Store = CandidateStore;
-}
-  }
-  if (!Store)
-return nullptr;
-  return Store->getValueOperand();
-}
-
 TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
@@ -425,7 +406,6 @@
   EXPECT_EQ(ForkCI->getArgOperand(1),
 ConstantInt::get(Type::getInt32Ty(Ctx), 1U));
   EXPECT_EQ(ForkCI->getArgOperand(2), Usr);
-  

[PATCH] D93546: [clangd][NFC] Improve clangd status messages

[Quentin Chateau via Phabricator via cfe-commits]

qchateau updated this revision to Diff 313079.
qchateau added a comment.

- dont capitalize first letter of status


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93546

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1206,7 +1206,7 @@
   }
   if (Result.empty())
 return "idle";
-  return llvm::join(Result, ",");
+  return llvm::join(Result, ", ");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -621,7 +621,7 @@
 File));
   };
 
-  WorkScheduler.runWithAST("Type Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("TypeHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::resolveTypeHierarchy(
@@ -642,7 +642,7 @@
   return CB(InpAST.takeError());
 CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
   };
-  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("CallHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::incomingCalls(
@@ -678,7 +678,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getDocumentSymbols(InpAST->AST));
   };
-  WorkScheduler.runWithAST("documentSymbols", File, std::move(Action),
+  WorkScheduler.runWithAST("DocumentSymbols", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
@@ -690,7 +690,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getFoldingRanges(InpAST->AST));
   };
-  WorkScheduler.runWithAST("foldingRanges", File, std::move(Action),
+  WorkScheduler.runWithAST("FoldingRanges", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1206,7 +1206,7 @@
   }
   if (Result.empty())
 return "idle";
-  return llvm::join(Result, ",");
+  return llvm::join(Result, ", ");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -621,7 +621,7 @@
 File));
   };
 
-  WorkScheduler.runWithAST("Type Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("TypeHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::resolveTypeHierarchy(
@@ -642,7 +642,7 @@
   return CB(InpAST.takeError());
 CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
   };
-  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("CallHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::incomingCalls(
@@ -678,7 +678,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getDocumentSymbols(InpAST->AST));
   };
-  WorkScheduler.runWithAST("documentSymbols", File, std::move(Action),
+  WorkScheduler.runWithAST("DocumentSymbols", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
@@ -690,7 +690,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getFoldingRanges(InpAST->AST));
   };
-  WorkScheduler.runWithAST("foldingRanges", File, std::move(Action),
+  WorkScheduler.runWithAST("FoldingRanges", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84924: [clang-tidy][WIP] Added command line option `fix-notes`

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 313080.
njames93 marked 2 inline comments as done.
njames93 added a comment.

Use enum for handling fix behaviour.
Fix tests using `--fix-notes` instead of `-fix-notes`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84924

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t
+// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t -- --fix-notes
 void foo(int a) {
   if (a = 1) {
   // CHECK-NOTES: [[@LINE-1]]:9: warning: using the result of an assignment as a condition without parentheses [clang-diagnostic-parentheses]
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
@@ -81,7 +81,6 @@
 // eol-comments aren't removed (yet)
 using n::A; // A
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
-// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using
 // CHECK-FIXES: {{^}}// A
 using n::B;
 using n::C;
Index: clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- --fix-notes
 
 int f() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -173,6 +173,10 @@
  errors were found. If compiler errors have
  attached fix-its, clang-tidy will apply them as
  well.
+--fix-notes- 
+ If a warning has no fix, but has notes attached
+ which contain fixes, apply the first fix found
+ in any notes.
 --format-style=-
  Style for formatting code around applied fixes:
- 'none' (default) turns off formatting
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,10 @@
 Improvements to clang-tidy
 --
 
+ - Added command line option `--fix-notes` to apply fixes found in notes
+   attached to warnings. These are typically cases where we are less confident
+   the fix will have the desired effect.
+
 - Checks that allow configuring names of headers to include now support wrapping
   the include in angle brackets to create a system include. For example,
   :doc:`cppcoreguidelines-init-variables
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -127,6 +127,13 @@
 )"),
cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt FixNotes("fix-notes", cl::desc(R"(
+If a warning has no fix, but has notes attached
+which contain fixes,

[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D84924#2446075 , @aaron.ballman 
wrote:

> In D84924#2184132 , @njames93 wrote:
>
>> This is very much a work in progress
>> Another direction I was thinking was only apply the fixes found in notes if 
>> there is exactly one fix attached to the notes in a diagnostic, instead of 
>> just applying the first one we find
>
> I was wondering the same thing here myself. If there's exactly one fix, then 
> it's unambiguous as to what behavior you get. One (minor) concern I have 
> about the current approach is with nondeterminism in diagnostic ordering 
> where different runs may pick different fixes for the same code. I don't 
> *think* we have any instances of this in Clang or clang-tidy, but users can 
> add their own plugins (for instance to the clang static analyzer) that may 
> introduce some small risk there. Do you have a reason why you picked one 
> approach over the other?

Part of the reason for this approach is from this bug report 
https://bugs.llvm.org/show_bug.cgi?id=47971, where its pointed out that clang 
diags gets fix-its added in notes if the fixes will change behaviour or they 
aren't sure its going to actually fix the issue.
As clang-tidy also applies fixes reported from clang, it is wise to adhere to a 
similar level caution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84924

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

[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation

[Jan Svoboda via Phabricator via cfe-commits]

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

Change `makeFlagToValueNormalizer` so that one specialization converts all 
integral/enum arguments into `uint64_t` and forwards them to the more generic 
version.

This makes it easy to replace the custom `FlagToValueNormalizer` struct with a 
lambda, which is the common approach in other (de)normalizers.

Finally, drop custom `is_int_convertbile` in favor of 
`llvm::is_integral_or_enum`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93628

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,23 @@
   Args.push_back(Spelling);
 }
 
-namespace {
-template  struct FlagToValueNormalizer {
-  T Value;
-
-  Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template ::value &&
+ llvm::is_integral_or_enum::value),
+   bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+  return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
 if (Args.hasArg(Opt))
   return Value;
 return None;
-  }
-};
-} // namespace
-
-template  static constexpr bool is_int_convertible() {
-  return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
-  return FlagToValueNormalizer{Value};
+  };
 }
 
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
-  return FlagToValueNormalizer{std::move(Value)};
+template ::value &&
+llvm::is_integral_or_enum::value),
+   bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+  return makeFlagToValueNormalizer(uint64_t(Value));
 }
 
 static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -157,33 +157,23 @@
   Args.push_back(Spelling);
 }
 
-namespace {
-template  struct FlagToValueNormalizer {
-  T Value;
-
-  Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template ::value &&
+ llvm::is_integral_or_enum::value),
+   bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+  return [Value](OptSpecifier Opt, unsigned, const ArgList &Args,
+ DiagnosticsEngine &) -> Optional {
 if (Args.hasArg(Opt))
   return Value;
 return None;
-  }
-};
-} // namespace
-
-template  static constexpr bool is_int_convertible() {
-  return sizeof(T) <= sizeof(uint64_t) &&
- std::is_trivially_constructible::value &&
- std::is_trivially_constructible::value;
-}
-
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
-  return FlagToValueNormalizer{Value};
+  };
 }
 
-template (), bool> = false>
-static FlagToValueNormalizer makeFlagToValueNormalizer(T Value) {
-  return FlagToValueNormalizer{std::move(Value)};
+template ::value &&
+llvm::is_integral_or_enum::value),
+   bool> = false>
+static auto makeFlagToValueNormalizer(T Value) {
+  return makeFlagToValueNormalizer(uint64_t(Value));
 }
 
 static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93543: clang-tidy: Leave the possibility of opting out having coloured diagnostic messages.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Would it be wise to use `os.isatty(sys.stdout.fileno())` as the value if left 
unspecified.
As we capture stdout from clang-tidy, clang-tidy assumes we aren't connected to 
a terminal and thus disables colours, unless explicitly enabled.
Therefore if our python script is known to be running from a terminal and we 
haven't specified to use colours. We should enable `--use-color`.

Based off this, I think the argument should be a carbon copy of the 
`--use-color` option from clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93543

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

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
Herald added a subscriber: jdoerfert.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before this commit, expression statements could not be annotated
with statement attributes.  Whenever parser found attribute, it
unconditionally assumed that it was followed by a declaration.
This not only doesn't allow expression attributes to have attributes,
but also produces spurious error diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93630

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/Parser/stmt-attributes.c
  clang/test/Sema/address_spaces.c
  clang/test/Sema/block-literal.c
  clang/test/SemaTemplate/address_space-dependent.cpp

Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -9,7 +9,8 @@
   __attribute__((address_space(J))) int array[5]; // expected-error {{automatic variable qualified with an address space}}
   __attribute__((address_space(I))) int arrarr[5][5]; // expected-error {{automatic variable qualified with an address space}}
 
-  __attribute__((address_space(J))) * x; // expected-error {{C++ requires a type specifier for all declarations}}
+  __attribute__((address_space(J))) * x; // expected-error {{use of undeclared identifier 'x'}}
+  // expected-error@-1 {{'address_space' attribute cannot be applied to a statement}}
 
   __attribute__((address_space(I))) float *B;
 
Index: clang/test/Sema/block-literal.c
===
--- clang/test/Sema/block-literal.c
+++ clang/test/Sema/block-literal.c
@@ -41,8 +41,11 @@
 
   foo:
   takeblock(^{ x = 4; });  // expected-error {{variable is not assignable (missing __block type specifier)}}
-  __block y = 7;// expected-warning {{type specifier missing, defaults to 'int'}}
-  takeblock(^{ y = 8; });
+  __block y = 7;   // expected-error {{use of undeclared identifier 'y'}}
+  __block int z = 7;
+  takeblock(^{
+z = 8;
+  });
 }
 
 
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -9,7 +9,8 @@
 void foo(_AS3 float *a,
  _AS1 float b) // expected-error {{parameter may not be qualified with an address space}}
 {
-  _AS2 *x;// expected-warning {{type specifier missing, defaults to 'int'}}
+  _AS2 *x; // expected-error {{'address_space' attribute cannot be applied to a statement}}
+  // expected-error@-1 {{use of undeclared identifier 'x'}}
   _AS1 float * _AS2 *B;
 
   int _AS1 _AS2 *Y;   // expected-error {{multiple address spaces specified for type}}
Index: clang/test/Parser/stmt-attributes.c
===
--- /dev/null
+++ clang/test/Parser/stmt-attributes.c
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void foo(int i) {
+
+  __attribute__((unknown_attribute));// expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  __attribute__((unknown_attribute)) {}  // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  __attribute__((unknown_attribute)) if (0) {}   // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  __attribute__((unknown_attribute)) for (;;);   // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  __attribute__((unknown_attribute)) do {// expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+__attribute__((unknown_attribute)) continue; // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  }
+  while (0)
+;
+  __attribute__((unknown_attribute)) while (0); // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+
+  __attribute__((unknown_attribute)) switch (i) {  // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+__attribute__((unknown_attribute)) case 0 :// expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+__attribute__((unknown_attribute)) default :   // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+ __attribute__((unknown_attribute)) break; // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  }
+
+  __attribute__((unknown_attribute)) goto here; // expected-warning {{unknown attribute 'unknown_attribute' ignored}}
+  __attribute__((unknown_attribute)) here :   

[clang] b2ba686 - Refactoring the attribute plugin example to fit the new API

[Aaron Ballman via cfe-commits]

Author: Yafei Liu
Date: 2020-12-21T08:24:09-05:00
New Revision: b2ba6867eac10874bd279c739639bdb9e60c1996

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

LOG: Refactoring the attribute plugin example to fit the new API

Make the example compile and the test case pass.

Added: 


Modified: 
clang/examples/Attribute/Attribute.cpp
clang/test/Frontend/plugin-attribute.cpp

Removed: 




diff  --git a/clang/examples/Attribute/Attribute.cpp 
b/clang/examples/Attribute/Attribute.cpp
index 998f175dae54..159b09e4b154 100644
--- a/clang/examples/Attribute/Attribute.cpp
+++ b/clang/examples/Attribute/Attribute.cpp
@@ -23,9 +23,10 @@ namespace {
 
 struct ExampleAttrInfo : public ParsedAttrInfo {
   ExampleAttrInfo() {
-// Can take an optional string argument (the check that the argument
-// actually is a string happens in handleDeclAttribute).
-OptArgs = 1;
+// Can take up to 15 optional arguments, to emulate accepting a variadic
+// number of arguments. This just illustrates how many arguments a
+// `ParsedAttrInfo` can hold, we will not use that much in this example.
+OptArgs = 15;
 // GNU-style __attribute__(("example")) and C++-style [[example]] and
 // [[plugin::example]] supported.
 static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
@@ -39,7 +40,7 @@ struct ExampleAttrInfo : public ParsedAttrInfo {
 // This attribute appertains to functions only.
 if (!isa(D)) {
   S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
-<< Attr << "functions";
+  << Attr << "functions";
   return false;
 }
 return true;
@@ -55,23 +56,39 @@ struct ExampleAttrInfo : public ParsedAttrInfo {
   S.Diag(Attr.getLoc(), ID);
   return AttributeNotApplied;
 }
-// Check if we have an optional string argument.
-StringRef Str = "";
+// We make some rules here:
+// 1. Only accept at most 3 arguments here.
+// 2. The first argument must be a string literal if it exists.
+if (Attr.getNumArgs() > 3) {
+  unsigned ID = S.getDiagnostics().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "'example' attribute only accepts at most three arguments");
+  S.Diag(Attr.getLoc(), ID);
+  return AttributeNotApplied;
+}
+// If there are arguments, the first argument should be a string literal.
 if (Attr.getNumArgs() > 0) {
-  Expr *ArgExpr = Attr.getArgAsExpr(0);
+  auto *Arg0 = Attr.getArgAsExpr(0);
   StringLiteral *Literal =
-  dyn_cast(ArgExpr->IgnoreParenCasts());
-  if (Literal) {
-Str = Literal->getString();
-  } else {
-S.Diag(ArgExpr->getExprLoc(), diag::err_attribute_argument_type)
-<< Attr.getAttrName() << AANT_ArgumentString;
+  dyn_cast(Arg0->IgnoreParenCasts());
+  if (!Literal) {
+unsigned ID = S.getDiagnostics().getCustomDiagID(
+DiagnosticsEngine::Error, "first argument to the 'example' "
+  "attribute must be a string literal");
+S.Diag(Attr.getLoc(), ID);
 return AttributeNotApplied;
   }
+  SmallVector ArgsBuf;
+  for (unsigned i = 0; i < Attr.getNumArgs(); i++) {
+ArgsBuf.push_back(Attr.getArgAsExpr(i));
+  }
+  D->addAttr(AnnotateAttr::Create(S.Context, "example", ArgsBuf.data(),
+  ArgsBuf.size(), Attr.getRange()));
+} else {
+  // Attach an annotate attribute to the Decl.
+  D->addAttr(AnnotateAttr::Create(S.Context, "example", nullptr, 0,
+  Attr.getRange()));
 }
-// Attach an annotate attribute to the Decl.
-D->addAttr(AnnotateAttr::Create(S.Context, "example(" + Str.str() + ")",
-Attr.getRange()));
 return AttributeApplied;
   }
 };

diff  --git a/clang/test/Frontend/plugin-attribute.cpp 
b/clang/test/Frontend/plugin-attribute.cpp
index 571ede3dc0b1..969105927be5 100644
--- a/clang/test/Frontend/plugin-attribute.cpp
+++ b/clang/test/Frontend/plugin-attribute.cpp
@@ -1,25 +1,22 @@
-// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o 
- 2>&1 | FileCheck %s --check-prefix=ATTRIBUTE
-// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm 
-DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// RUN: split-file %s %t
+// RUN: %clang -cc1 -load %llvmshlibdir/Attribute%pluginext -fsyntax-only 
-ast-dump -verify %t/good_attr.cpp | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -fsyntax-only 
-Xclang -verify %t/bad_attr.cpp
 // REQUIRES: plugins, examples
+//--- good_attr.cpp
+// expected-no-diagnostics
+void fn1a() __attribute__((example)) 

[PATCH] D92006: Refactoring the attribute plugin example to fit the new API

[Aaron Ballman via Phabricator via cfe-commits]

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

In D92006#2465239 , @psionic12 wrote:

> Hi, could anyone help to commit this?

I'm happy to do so, sorry about not asking earlier. I've commit on your behalf 
in b2ba6867eac10874bd279c739639bdb9e60c1996 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

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

[PATCH] D93631: [clang][cli] Implement `getAllArgValues` marshalling

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This infrastructure can be used ~30 more command line options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93631

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -167,6 +167,12 @@
   code Denormalizer = "denormalizeString";
 }
 
+class MarshallingInfoStringVector
+  : MarshallingInfo({})"> {
+  code Normalizer = "normalizeStringVector";
+  code Denormalizer = "denormalizeStringVector";
+}
+
 class MarshallingInfoFlag
   : MarshallingInfo {
   code Normalizer = "normalizeSimpleFlag";
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -18,6 +18,7 @@
 using namespace clang;
 
 using ::testing::Contains;
+using ::testing::HasSubstr;
 using ::testing::StrEq;
 
 namespace {
@@ -408,6 +409,45 @@
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("legacy";
 }
 
+TEST_F(CommandLineTest, StringVectorEmpty) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getFrontendOpts().ModuleMapFiles.empty());
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-fmodule-map-file=";
+}
+
+TEST_F(CommandLineTest, StringVectorSingle) {
+  const char *Args[] = {"-fmodule-map-file=a"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getFrontendOpts().ModuleMapFiles,
+std::vector({"a"}));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_EQ(count(GeneratedArgs, StringRef("-fmodule-map-file=a")), 1);
+}
+
+TEST_F(CommandLineTest, StringVectorMultiple) {
+  const char *Args[] = {"-fmodule-map-file=a", "-fmodule-map-file=b"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getFrontendOpts().ModuleMapFiles ==
+  std::vector({"a", "b"}));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+  ASSERT_EQ(count(GeneratedArgs, StringRef("-fmodule-map-file=a")), 1);
+  ASSERT_EQ(count(GeneratedArgs, StringRef("-fmodule-map-file=b")), 1);
+}
+
 // Tree of boolean options that can be (directly or transitively) implied by
 // their parent:
 //
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -323,6 +323,23 @@
   return Res;
 }
 
+static Optional>
+normalizeStringVector(OptSpecifier Opt, int, const ArgList &Args,
+  DiagnosticsEngine &) {
+  return Args.getAllArgValues(Opt);
+}
+
+static void denormalizeStringVector(SmallVectorImpl &Args,
+const char *Spelling,
+CompilerInvocation::StringAllocator SA,
+Option::OptionClass OptClass,
+unsigned TableIndex,
+const std::vector &Values) {
+  for (const std::string& Value : Values) {
+denormalizeString(Args, Spelling, SA, OptClass, TableIndex, Value);
+  }
+}
+
 static Optional normalizeTriple(OptSpecifier Opt, int TableIndex,
  const ArgList &Args,
  DiagnosticsEngine &Diags) {
@@ -1715,7 +1732,6 @@
   Opts.LLVMArgs = Args.getAllArgValues(OPT_mllvm);
   Opts.ASTDumpDecls = Args.hasArg(OPT_ast_dump, OPT_ast_dump_EQ);
   Opts.ASTDumpAll = Args.hasArg(OPT_ast_dump_all, OPT_ast_dump_all_EQ);
-  Opts.ModuleMapFiles = Args.getAllArgValues(OPT_fmodule_map_file);
   // Only the -fmodule-file= form.
   for (const auto *A : Args.filtered(OPT_fmodule_file)) {
 StringRef Val = A->getValue();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1915,7 +1915,8 @@
   MarshallingInfoFlag<"FrontendOpts.IsSystemModule">;
 def fmodule_map_file : Joined<["-"], "fmodule-map-file=">,
   Group, Flags<[NoXarchOption,C

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/IR/ConstantFold.cpp:539
+  if (V->isNullValue() && !DestTy->isX86_MMXTy() && !DestTy->isX86_AMXTy()
+  && opc != Instruction::AddrSpaceCast)
 return Constant::getNullValue(DestTy);

Operation should at the end of the line.



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:55
+  switch (II->getIntrinsicID()) {
+  default: {
+Row = II->getArgOperand(0);

I think we'd better to check exceptions. E.g.
```
default:
  llvm_unreachable("");
case Intrinsic::x86_tileloadd64_internal:
case Intrinsic::x86_tdpbssd_internal:
case Intrinsic::x86_tilestored64_internal:
  Row = II->getArgOperand(0);
  Col = II->getArgOperand(1);
  break;
```



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:114
+  Value *Row = nullptr, *Col = nullptr;
+  Use &U = *(Bitcast->use_begin());
+  unsigned OpNo = U.getOperandNo();

Why don't check empty like line 157?



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:193
+// %2 = load <256 x i32>, <256 x i32>* %addr, align 1024
+auto *II = cast(Src);
+Value *Row = II->getOperand(0);

Is it possible the x86_amx operand isn't from AMX intrinsic, e.g.
```
%src = bitcast <256 x i32> %xxx to x86_amx
%2 = bitcast x86_amx %src to <256 x i32>
```



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:233
 bool X86LowerAMXType::visit() {
   bool C;
+  SmallVector DeadInsts;

Better move it to line 310.



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:238
 for (Instruction &Inst : BB) {
-  LoadInst *LD = dyn_cast(&Inst);
-  // Check load instruction.
-  // %3 = load <256 x i32>, <256 x i32>* %1, align 64
-  if (LD) {
-FixedVectorType *VTy = dyn_cast(Inst.getType());
-if (!IsAMXType(VTy))
-  continue;
-LDSet.insert(&Inst);
+  if (!dyn_cast(&Inst))
 continue;

Better to reuse the cast result, e.g.
```
BitCastInst *BInst = dyn_cast(&Inst);
if (!BInst )
```
You can save several `cast(&Inst)` below.



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:265
+// If the dst type is <256 x i32>*, it is valid intruction.
+// %0 = bitcast x86_amx* %tile to <256 x i32>*
+// %1 = load <256 x i32>, <256 x i32>* %0, align 64

Where's `x86_amx* %tile` from? Shouldn't been transfered to `x86_amx` before 
this bitcast if it exists?



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:271
+LoadInst *LD = dyn_cast(Src);
+if (!LD || !LD->hasOneUser()) {
+  transformBitcast(cast(&Inst));

Maybe better to keep a duplicated `load` that calling `transformBitcast`. The 
same for line 285.



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:286
+if (!Inst.hasOneUser()) {
+  transformBitcast(cast(&Inst));
+  DeadInsts.push_back(&Inst);

Why we need to consider <256 x i32> has more than one use?



Comment at: llvm/test/CodeGen/X86/AMX/amx-across-func.ll:89-91
 attributes #2 = { nounwind uwtable 
"correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" 
"frame-pointer"="none" "less-precise-fpmad"="false" 
"min-legal-vector-width"="8192" "no-infs-fp-math"="false" 
"no-jump-tables"="false" "no-nans-fp-math"="false" 
"no-signed-zeros-fp-math"="false" "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" "target-cpu"="x86-64" 
"target-features"="+amx-int8,+amx-tile,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" 
"tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #3 = { "correctly-rounded-divide-sqrt-fp-math"="false" 
"disable-tail-calls"="false" "frame-pointer"="none" 
"less-precise-fpmad"="false" "no-infs-fp-math"="false" 
"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"target-cpu"="x86-64" 
"target-features"="+amx-int8,+amx-tile,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" 
"tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #4 = { nounwind }

Better to remove these unused attributes. The same to other tests.



Comment at: llvm/test/CodeGen/X86/AMX/amx-type.ll:67
+
+define dso_local void @test_src_add(<256 x i32> %x, <256 x i32> %y, i16 %r, 
i16 %c, i8* %buf, i64 %s) #2 {
+; CHECK-LABEL: @test_src_add(

For this and the next test, we have chances to optimize to memcpy if we can 
make sure %s is constant 64.



Comment at: llvm/test/CodeGen/X86/AMX/amx-type.ll:103
+
 define dso_local void @test_load(i8* %in, i8* %out) local_unnamed_addr #2 {
 ; CHECK-LABEL: @test_load(

We don't need to check this case now, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
 

[PATCH] D93633: [format] Add overload to parseConfiguration that accept llvm::MemoryBufferRef

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: MyDeveloperDay, fodinabor, JakeMerdichAMD.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This overload should be used for better diagnostics when parsing configurations.
Now a failure to parse will list the filename (or ) instead of 
just `YAML`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93633

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/test/Format/error-config.cpp

Index: clang/test/Format/error-config.cpp
===
--- clang/test/Format/error-config.cpp
+++ clang/test/Format/error-config.cpp
@@ -1,10 +1,10 @@
 // RUN: clang-format %s --Wno-error=unknown --style="{UnknownKey: true}" 2>&1 | FileCheck %s -check-prefix=CHECK
 // RUN: not clang-format %s --style="{UnknownKey: true}" 2>&1 | FileCheck %s -check-prefix=CHECK-FAIL
 
-// CHECK: YAML:1:2: warning: unknown key 'UnknownKey'
+// CHECK: :1:2: warning: unknown key 'UnknownKey'
 // CHECK-NEXT: {UnknownKey: true}
 // CHECK-NEXT: ^~
-// CHECK-FAIL: YAML:1:2: error: unknown key 'UnknownKey'
+// CHECK-FAIL: :1:2: error: unknown key 'UnknownKey'
 // CHECK-FAIL-NEXT: {UnknownKey: true}
 // CHECK-FAIL-NEXT: ^~
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -35,6 +35,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/VirtualFileSystem.h"
@@ -1327,16 +1328,17 @@
   return true;
 }
 
-std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
+   FormatStyle *Style,
bool AllowUnknownOptions) {
   assert(Style);
   FormatStyle::LanguageKind Language = Style->Language;
   assert(Language != FormatStyle::LK_None);
-  if (Text.trim().empty())
+  if (Config.getBuffer().trim().empty())
 return make_error_code(ParseError::Error);
   Style->StyleSet.Clear();
   std::vector Styles;
-  llvm::yaml::Input Input(Text);
+  llvm::yaml::Input Input(Config);
   // DocumentListTraits> uses the context to get default
   // values for the fields, keys for which are missing from the configuration.
   // Mapping also uses the context to get the language to find the correct
@@ -2864,8 +2866,9 @@
 
   if (StyleName.startswith("{")) {
 // Parse YAML/JSON style from the command line.
-if (std::error_code ec =
-parseConfiguration(StyleName, &Style, AllowUnknownOptions))
+if (std::error_code ec = parseConfiguration(
+llvm::MemoryBufferRef(StyleName, ""), &Style,
+AllowUnknownOptions))
   return make_string_error("Error parsing -style: " + ec.message());
 return Style;
   }
@@ -2909,8 +2912,8 @@
 FS->getBufferForFile(ConfigFile.str());
 if (std::error_code EC = Text.getError())
   return make_string_error(EC.message());
-if (std::error_code ec = parseConfiguration(
-Text.get()->getBuffer(), &Style, AllowUnknownOptions)) {
+if (std::error_code ec =
+parseConfiguration(*Text.get(), &Style, AllowUnknownOptions)) {
   if (ec == ParseError::Unsuitable) {
 if (!UnsuitableConfigFiles.empty())
   UnsuitableConfigFiles.append(", ");
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2879,7 +2879,8 @@
 private:
   FormatStyleSet StyleSet;
 
-  friend std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+  friend std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
+FormatStyle *Style,
 bool AllowUnknownOptions);
 };
 
@@ -2938,9 +2939,17 @@
 ///
 /// If AllowUnknownOptions is true, no errors are emitted if unknown
 /// format options are occured.
-std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
+   FormatStyle *Style,
bool AllowUnknownOptions = false);
 
+/// Like above but accepts an unnamed buffer.
+inline std::error_code parseConfiguration(StringRef Config, FormatStyle *Style,
+  bool AllowUnknownOptions = false) {
+  return parseConfiguration(llvm::MemoryBufferRef(Config, "YAML"), Style,
+AllowUnknownOptions);
+}
+
 /// Gets configuration in a

[PATCH] D77056: [Sema] Allow non-member operators for sizeless SVE types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

Thanks for the reviews.

In D77056#2463959 , @rsmith wrote:

> That said, I'm concerned that the design of this feature will severely limit 
> its utility. For classes and enums, operators are typically defined in an 
> associated namespace so that they can be found by ADL. For these types, there 
> are no associated namespaces, so user-defined operators for them can never be 
> found by ADL, and such operators are essentially never going to work in 
> generic code. Maybe that's not something you care too much about for the 
> intended use cases?

Yeah, we don't expect the intended use cases would depend on ADL.  The main aim 
is to support a style of SIMD framework in which all operations are performed 
using operators or using non-member functions.  The SIMD framework would then 
have two choices:

1. If all those non-member functions belong to a namespace `foo` and if the 
SIMD framework expects users to be `using namespace foo`, the non-member 
operator overloads can simply be defined in `foo` and be brough into scope that 
way.
2. Otherwise, the framework could handle non-member operator overloads in one 
of the following ways:
  - define the overloads to be static (perhaps the least likely)
  - define the overloads in an anonymous namespace
  - define the overloads in an internal namespace and make the header file use 
that namespace at global scope

The second choice would be awkward if a header file wanted to use the SIMD 
framework internally but leave the user free to use a different SIMD framework. 
 That sort of use case would require the first choice instead.  However, we 
expect in practice this feature will mostly be used in well-controlled 
environments where there is no risk of a clash between SIMD frameworks within a 
package.  Requiring the overloads to be static or defined within a namespace is 
just to prevent accidental ODR violations between packages that are linked 
together.

Either way, I realise this isn't great style.  It just seems like a practical 
compromise between the rock of classes having a constant size and the hard 
place of vectors having a variable length.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77056

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

[PATCH] D87587: [clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option

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

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2463
 
+**MaxUnwrappedLinesForShortNamespace** (``unsigned``)
+  The maximal number of unwrapped lines that a short namespace spans.

curdeius wrote:
> MyDeveloperDay wrote:
> > This is quite a mouthful before this lands do you want to consider 
> > shortening it?
> > 
> > `ShortNamespaceLength` ?
> > 
> > I'm not sure non clang-formast developers will know what an UnwrappedLine 
> > is?
> I agree that one needs to wrap its head around to digest this name. I'd vote 
> for what @mydeveloperday proposed.
Although I already made some changes inside clang-format and single stepped 
through some formations, I still can not explain what those UnwrappedLines are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

Ping 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D93636: [clang][cli] Implement ContainsN Google Test matcher

[Jan Svoboda via Phabricator via cfe-commits]

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

This allows us to verify that we don't emit options multiple times.

In most cases, that would be benign, but for options with 
`MarshallingInfoVectorString`, emitting wrong number of arguments might change 
the semantics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93636

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -39,6 +39,61 @@
   }
 };
 
+template 
+std::string describeContainsN(M InnerMatcher, unsigned N, bool Negation) {
+  StringRef Contains = Negation ? "doesn't contain" : "contains";
+  StringRef Instance = N == 1 ? " instance " : " instances ";
+  StringRef Element = "of element that ";
+
+  std::ostringstream Inner;
+  InnerMatcher.impl().DescribeTo(&Inner);
+
+  return (Contains + " exactly " + Twine(N) + Instance + Element + Inner.str())
+  .str();
+}
+
+MATCHER_P2(ContainsN, InnerMatcher, N,
+   describeContainsN(InnerMatcher, N, negation)) {
+  auto InnerMatches = [this](const auto &Element) {
+::testing::internal::DummyMatchResultListener InnerListener;
+return InnerMatcher.impl().MatchAndExplain(Element, &InnerListener);
+  };
+
+  return count_if(arg, InnerMatches) == N;
+}
+
+TEST(ContainsN, Empty) {
+  const char *Array[] = {""};
+
+  ASSERT_THAT(Array, ContainsN(StrEq("x"), 0));
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 1)));
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 2)));
+}
+
+TEST(ContainsN, Zero) {
+  const char *Array[] = {"y"};
+
+  ASSERT_THAT(Array, ContainsN(StrEq("x"), 0));
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 1)));
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 2)));
+}
+
+TEST(ContainsN, One) {
+  const char *Array[] = {"a", "b", "x", "z"};
+
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 0)));
+  ASSERT_THAT(Array, ContainsN(StrEq("x"), 1));
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 2)));
+}
+
+TEST(ContainsN, Two) {
+  const char *Array[] = {"x", "a", "b", "x"};
+
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 0)));
+  ASSERT_THAT(Array, Not(ContainsN(StrEq("x"), 1)));
+  ASSERT_THAT(Array, ContainsN(StrEq("x"), 2));
+}
+
 // Boolean option with a keypath that defaults to true.
 // The only flag with a negative spelling can set the keypath to false.
 
@@ -270,7 +325,7 @@
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
-  ASSERT_EQ(count(GeneratedArgs, StringRef("-fdebug-pass-manager")), 1);
+  ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-fdebug-pass-manager"), 1));
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fno-debug-pass-manager";
 }
 
@@ -418,7 +473,8 @@
   ASSERT_TRUE(Invocation.getFrontendOpts().ModuleMapFiles.empty());
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-fmodule-map-file=";
+
+  ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-fmodule-map-file";
 }
 
 TEST_F(CommandLineTest, StringVectorSingle) {
@@ -431,7 +487,9 @@
 std::vector({"a"}));
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_EQ(count(GeneratedArgs, StringRef("-fmodule-map-file=a")), 1);
+
+  ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-fmodule-map-file=a"), 1));
+  ASSERT_THAT(GeneratedArgs, ContainsN(HasSubstr("-fmodule-map-file"), 1));
 }
 
 TEST_F(CommandLineTest, StringVectorMultiple) {
@@ -444,8 +502,10 @@
   std::vector({"a", "b"}));
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
-  ASSERT_EQ(count(GeneratedArgs, StringRef("-fmodule-map-file=a")), 1);
-  ASSERT_EQ(count(GeneratedArgs, StringRef("-fmodule-map-file=b")), 1);
+
+  ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-fmodule-map-file=a"), 1));
+  ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-fmodule-map-file=b"), 1));
+  ASSERT_THAT(GeneratedArgs, ContainsN(HasSubstr("-fmodule-map-file"), 2));
 }
 
 // Tree of boolean options that can be (directly or transitively) implied by
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93636: [clang][cli] Implement ContainsN Google Test matcher

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:55
+
+MATCHER_P2(ContainsN, InnerMatcher, N,
+   describeContainsN(InnerMatcher, N, negation)) {

I'm not sure where custom Google Test matchers go. I guess this might be useful 
for other LLVM projects. Any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93636

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

[PATCH] D93626: [clang-format] PR48535 clang-format Incorrectly Removes Space After C Style Cast When Type Is Not a Pointer

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1976
+   tok::kw_co_yield, tok::equal, tok::kw_delete,
+   tok::kw_sizeof, tok::kw_throw) ||
 PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,

What's the policy towards unrelated changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93626

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

[PATCH] D93401: [flang][driver] Add support for `-D`, `-U`

[Faris via Phabricator via cfe-commits]

FarisRehman updated this revision to Diff 313108.
FarisRehman edited the summary of this revision.
FarisRehman added a comment.

Address review comments

Address comments with the following changes:

- Add a dedicated method to adding preprocessing options in Flang.cpp
- Change preprocessorOpts to use std::shared_ptr
- Separate SetDefaultFortranOpts from SetFortranOpts in CompilerInvocation
- Fix method and variable naming in CompilerInvocation.cpp
- Add comments to new methods and fields
- Add missing file header to PreprocessorOptions.h
- Remove unnecessary changes and method calls
- Add a regression test to check end-of-line character behaviour in a macro 
definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93401

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/PreprocessorOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/macro_def_undef.f90
  flang/test/Flang-Driver/macro_multiline.f90

Index: flang/test/Flang-Driver/macro_multiline.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/macro_multiline.f90
@@ -0,0 +1,26 @@
+! Ensure \n and anything that follows after in a macro definition (-D) is ignored.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %flang-new -E %s  2>&1 | FileCheck -strict-whitespace %s
+
+!-
+!   FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %flang-new -fc1 -E %s  2>&1 | FileCheck -strict-whitespace %s
+
+
+!---
+! EXPECTED OUTPUT FOR MACRO 'X'
+!---
+! printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs flang-new -E %s
+! CHECK:program a
+! CHECK-NOT:program x
+! CHECK-NEXT:end
+
+! Macro.F:
+program X
+end
\ No newline at end of file
Index: flang/test/Flang-Driver/macro_def_undef.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/macro_def_undef.f90
@@ -0,0 +1,43 @@
+! Ensure arguments -D and -U work as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang-new -E -DX=A %s  2>&1 | FileCheck %s --check-prefix=DEFINED
+! RUN: %flang-new -E -DX=A -UX %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+
+!-
+!   FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: %flang-new -fc1 -E %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+! RUN: %flang-new -fc1 -E -DX=A %s  2>&1 | FileCheck %s --check-prefix=DEFINED
+! RUN: %flang-new -fc1 -E -DX -UX %s  2>&1 | FileCheck %s --check-prefix=UNDEFINED
+
+
+!
+! EXPECTED OUTPUT FOR AN UNDEFINED MACRO
+!
+! flang-new -E %s
+! flang-new -E -DX=A -UX %s
+! UNDEFINED:program b
+! UNDEFINED-NOT:program x
+! UNDEFINED-NEXT:end
+
+!
+! EXPECTED OUTPUT FOR MACRO 'X' DEFINED AS A
+!
+! flang-new -E -DX=A %s
+! DEFINED:program a
+! DEFINED-NOT:program b
+! DEFINED-NEXT:end
+
+! Macro.F:
+#ifdef X
+program X
+#else
+program B
+#endif
+end
\ No newline at end of file
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -19,11 +19,13 @@
 ! HELP-EMPTY:
 ! HELP-NEXT:OPTIONS:
 ! HELP-NEXT: -###   Print (but do not run) the commands to run for this compilation
+! HELP-NEXT: -D = Define  to  (or 1 if  omitted)
 ! HELP-NEXT: -E Only run the preprocessor
 ! HELP-NEXT: -fcolor-diagnosticsEnable colors in diagnostics
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! HELP-NEXT: -help  Display available options
 ! HELP-NEXT: -o   Write output to 
+! HELP-NEXT: -U  Undefine macro 
 ! HELP-NEXT: --version  Print version information
 
 !-
@@ -32,10 +34,12 @@
 ! HELP-FC1:USAGE: flang-new
 ! HELP-FC

[PATCH] D93490: [clang-format] PR48539 ReflowComments breaks Qt translation comments

[JVApen via Phabricator via cfe-commits]

JVApen added a comment.

In D93490#2463068 , @MyDeveloperDay 
wrote:

> Maybe @JVApen can help as they logged 
> https://bugs.llvm.org/show_bug.cgi?id=48539
>
> I'm not a Qt developer so I can't really comment beyond fixing what was in 
> the bug rpeort

We actually only use //: in our code, I found the others when looking at the 
docs. Maybe it's better to skip //~ and add a to-do with the open question if 
we even can correctly reflow this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93490

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

[PATCH] D93401: [flang][driver] Add support for `-D`, `-U`

[Faris via Phabricator via cfe-commits]

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



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:228
+  // Note: GCC drops anything following an end-of-line character.
+  llvm::StringRef::size_type End = MacroBody.find_first_of("\n\r");
+  MacroBody = MacroBody.substr(0, End);

tskeith wrote:
> This is a change in behavior from f18, right? If so, the commit message 
> should mention it and why it's changing.
Updated the summary to address this, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93401

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

[PATCH] D93637: [libTooling] Add support for smart pointers to releveant Transformer `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: tdl-g, sbenza.
ymandel requested review of this revision.
Herald added a project: clang.

Stencils `maybeDeref` and `maybeAddressOf` are designed to handle nodes that may
be pointers. Currently, they only handle native pointers. This patch extends the
support to recognize smart pointers and handle them as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93637

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
@@ -30,11 +31,17 @@
 
 // Create a valid translation-unit from a statement.
 static std::string wrapSnippet(StringRef StatementCode) {
-  return ("namespace N { class C {}; } "
-  "namespace { class AnonC {}; } "
-  "struct S { int field; }; auto stencil_test_snippet = []{" +
-  StatementCode + "};")
-  .str();
+  constexpr char Preface[] = R"cc(
+namespace N { class C {}; }
+namespace { class AnonC {}; }
+struct S { int Field; };
+struct Smart {
+  S* operator->() const;
+  S& operator*() const;
+};
+auto stencil_test_snippet = []{
+  )cc";
+  return (Preface + StatementCode + "};").str();
 }
 
 static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
@@ -260,6 +267,25 @@
   testExpr(Id, "int x; &x;", maybeDeref(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeDerefSmartPointer) {
+  StringRef Id = "id";
+  std::string Snippet = R"cc(
+Smart x;
+x;
+  )cc";
+  testExpr(Id, Snippet, maybeDeref(Id), "*x");
+}
+
+TEST_F(StencilTest, MaybeDerefSmartPointerFromMemberExpr) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+  matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id;
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeDeref(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("*x"));
+}
+
 TEST_F(StencilTest, MaybeAddressOfPointer) {
   StringRef Id = "id";
   testExpr(Id, "int *x; x;", maybeAddressOf(Id), "x");
@@ -280,6 +306,26 @@
   testExpr(Id, "int *x; *x;", addressOf(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeAddressOfSmartPointer) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; x;", maybeAddressOf(Id), "x");
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerFromMemberCall) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+  matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id;
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeAddressOf(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("x"));
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerDerefNoCancel) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; *x;", maybeAddressOf(Id), "&*x");
+}
+
 TEST_F(StencilTest, AccessOpValue) {
   StringRef Snippet = R"cc(
 S x;
Index: clang/lib/Tooling/Transformer/Stencil.cpp
===
--- clang/lib/Tooling/Transformer/Stencil.cpp
+++ clang/lib/Tooling/Transformer/Stencil.cpp
@@ -195,6 +195,22 @@
   return printNode(Data.Id, Match, Result);
 }
 
+// FIXME: Consider memoizing this function using the `ASTContext`.
+static bool isSmartPointerType(QualType Ty, ASTContext &Context) {
+  using namespace ::clang::ast_matchers;
+
+  auto KnownSmartPointer =
+  cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+  const auto QuacksLikeASmartPointer = cxxRecordDecl(
+  hasMethod(cxxMethodDecl(hasOverloadedOperatorName("->"),
+  returns(qualType(pointsTo(type()),
+  hasMethod(cxxMethodDecl(hasOverloadedOperatorName("*"),
+  returns(qualType(references(type()));
+  const auto SmartPointer = qualType(hasDeclaration(
+  cxxRecordDecl(anyOf(KnownSmartPointer, QuacksLikeASmartPointer;
+  return match(SmartPointer, Ty, Context).size() > 0;
+}
+
 Error evalData(const UnaryOperationData &Data,
const MatchFinder::MatchResult &Match, std::string *Result) {
   // The `Describe` operation can be applied to any node, not just expressions,
@@ -215,17 +231,37 @@
 Source = tooling::buildDereference(*E, *Match.Context);
 break;
   case UnaryNodeOperator::MaybeDeref:
-if (!E->getType()->isAnyPointerType()) {
-  *Result += tooling::getText(*E, *Match.Context);
-  return Error::success();
+if (E->getType()->isAnyPointerType() ||
+isSmartPointerTy

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: tra, yaxunl.
hliao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- MSVC has different `` implementation which calls into functions 
declared in ``. Instead of through builtins or inline functions, 
`` functions are provided through external libraries. To get 
`` compiled with HIP on MSVC, we need to + Wrap `` to force 
its functions to be `__host__ __device__`. + Provide its function definitions 
for the device compilation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93638

Files:
  clang/lib/Headers/__clang_hip_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/ymath.h


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,36 @@
+/*=== complex - CUDA wrapper for  -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_COMPLEX
+#define __CLANG_CUDA_WRAPPERS_COMPLEX
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,18 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,36 @@
+/*=== complex - CUDA wrapper for  -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ *===--

[PATCH] D93401: [flang][driver] Add support for `-D`, `-U`

[Faris via Phabricator via cfe-commits]

FarisRehman marked 2 inline comments as done.
FarisRehman added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:205
+/// Collect the macro definitions provided by the given preprocessor
+/// options into the parser options.
+void collectMacroDefinitions(

awarzynski wrote:
> This is not quite true, because you're collecting them into preprocessor 
> options :)
Added a comment to clarify the input and output arguments in the latest 
revision 👍 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93401

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

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan updated this revision to Diff 313112.
abhina.sreeskantharajan added a comment.

Thanks for your patience, I've addressed some more comments. Here is the 
summary of the changes in this patch:

- add translation for UCN strings, update testcase
- fix helptext for fexec-charset option in Options.td
- check for invalid charsets when parsing driver options.
- fix up char conversion code




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Lex/LiteralTranslator.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/CMakeLists.txt
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/LiteralTranslator.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CodeGen/systemz-charset.c
  clang/test/CodeGen/systemz-charset.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Support/Triple.cpp

Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -1023,6 +1023,13 @@
   return Tmp.split('-').second;  // Strip second component
 }
 
+// System charset on z/OS is IBM-1047 and UTF-8 otherwise
+StringRef Triple::getSystemCharset() const {
+  if (getOS() == llvm::Triple::ZOS)
+return "IBM-1047";
+  return "UTF-8";
+}
+
 static unsigned EatNumber(StringRef &Str) {
   assert(!Str.empty() && Str[0] >= '0' && Str[0] <= '9' && "Not a number");
   unsigned Result = 0;
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -390,6 +390,9 @@
   /// if the environment component is present).
   StringRef getOSAndEnvironmentName() const;
 
+  /// getSystemCharset - Get the system charset of the triple.
+  StringRef getSystemCharset() const;
+
   /// @}
   /// @name Convenience Predicates
   /// @{
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -209,8 +209,8 @@
 // RUN: %clang -### -S -finput-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-CHARSET %s
 // CHECK-INVALID-CHARSET: error: invalid value 'iso-8859-1' in '-finput-charset=iso-8859-1'
 
-// RUN: %clang -### -S -fexec-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
-// CHECK-INVALID-INPUT-CHARSET: error: invalid value 'iso-8859-1' in '-fexec-charset=iso-8859-1'
+// RUN: %clang -### -S -fexec-charset=invalid-charset -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
+// CHECK-INVALID-INPUT-CHARSET: error: invalid value 'invalid-charset' in '-fexec-charset'
 
 // Test that we don't error on these.
 // RUN: %clang -### -S -Werror\
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -209,10 +209,11 @@
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
 // source-charset-utf-16: invalid value 'utf-16' in '/source-charset:utf-16'
 
-// /execution-charset: should warn on everything except UTF-8.
-// RUN: %clang_cl /execution-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=execution-charset-utf-16 %s
-// execution-charset-utf-16: invalid value 'utf-16' in '/execution-charset:utf-16'
+// /execution-charset: should warn on invalid charsets.
+// RUN: %clang_cl /execution-charset:invalid-charset -### -- %s 2>&1 | FileCheck -check-prefix=execution-charset-invalid %s
+// execution-charset-invalid: invalid value 'invalid-charset' in '-fexec-charset'
 //
+
 // RUN: %clang_cl /Umymacro -### -- %s 2>&1 | FileCheck -check-prefix=U %s
 // RUN: %clang_cl /U mymacro -### -- %s 2>&1 | FileCheck -check-prefix=U %s
 // U: "-U" "mymacro"
Index: clang/test/CodeGen/systemz-charset.cpp
===
--- /dev/null
+++ clang/test/CodeGen/systemz-charset.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang %s -std=c++17 -emit-llvm -S -target s390x-ibm-zos -o - | FileCheck %s
+
+const char *RawString = R"(Hello\n)";
+//CHECK: c"\C8\85\93\93\96\E0\95\00"
+
+char UnicodeChar8 = u8'1';
+//CHECK: i8 49
+char16_t UnicodeChar16 = u'1';
+//CHECK: i16 49
+char32_t UnicodeChar32 = U'1';
+//CHECK: i32 49
+
+const char *UnicodeString8 = u8"Hello";
+//CHECK: c"Hello\

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan marked 11 inline comments as done.
abhina.sreeskantharajan added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3583-3584
 
+def fexec_charset : Separate<["-"], "fexec-charset">, 
MetaVarName<"">,
+  HelpText<"Set the execution  for string and character literals">;
 def target_cpu : Separate<["-"], "target-cpu">,

tahonermann wrote:
> abhina.sreeskantharajan wrote:
> > tahonermann wrote:
> > > How about substituting "character set", "character encoding", or 
> > > "charset" for "codepage"?
> > > 
> > > This doesn't state what names are recognized.  The ones provided by the 
> > > system iconv() implementation (as is the case for gcc)?  Or all names and 
> > > aliases specified by the [[ 
> > > https://www.iana.org/assignments/character-sets/character-sets.xhtml | 
> > > IANA character set registry ]]?
> > > 
> > > The set of recognized names can be a superset of the names that are 
> > > actually supported.
> > I've updated the description from codepage to charset.
> > 
> > It's hard to specify what charsets are supported because iconv library 
> > differs between targets, so the list will not be the same on every platform.
> Being dependent on the host iconv library seems fine by me; that is the case 
> for gcc today.  I suggest making that explicit here:
>   def fexec_charset : Separate<["-"], "fexec-charset">, 
> MetaVarName<"">,
> HelpText<"Set the execution  for string and character literals.  
> Supported character encodings include XXX and those supported by the host 
> iconv library.">;
I've updated the HelpText with your suggested description.



Comment at: clang/include/clang/Lex/LiteralSupport.h:192
+ConversionState translationState = TranslateToExecCharset);
+  ConversionState TranslationState;
 

tahonermann wrote:
> Does the conversion state need to be persisted as a data member?  The literal 
> is consumed in the constructor.
Thanks, I've removed this.



Comment at: clang/include/clang/Lex/LiteralSupport.h:244
   bool Pascal;
+  ConversionState TranslationState;
+

tahonermann wrote:
> Same concern here with respect to persisting the conversion state as a data 
> member.
If this member is removed in StringLiteralParser, we will need to pass the 
State to multiple functions in StringLiteralParser like init(). Would this 
solution be preferable to keeping a data member?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5969-5977
+  // Pass all -fexec-charset options to cc1.
+  std::vector vList =
+  Args.getAllArgValues(options::OPT_fexec_charset_EQ);
+  // Set the default fexec-charset as the system charset.
+  CmdArgs.push_back("-fexec-charset");
+  CmdArgs.push_back(Args.MakeArgString(Triple.getSystemCharset()));
+  for (auto it = vList.begin(), ie = vList.end(); it != ie; ++it) {

tahonermann wrote:
> abhina.sreeskantharajan wrote:
> > tahonermann wrote:
> > > I think it would be preferable to diagnose an unrecognized character 
> > > encoding name here if possible.  The current changes will result in an 
> > > unrecognized name (as opposed to one that is unsupported for the target) 
> > > being diagnosed for each compiler instance.
> > Since we do not know what charsets are supported by the iconv library on 
> > the target platform, we don't know what charsets are actually invalid until 
> > we try creating a CharSetConverter.
> Understood, but what would be the harm in performing a lookup (constructing a 
> `CharSetConverter`) here?
I initially thought it will be a performance issue if we are creating the 
Converter twice, once here and once in the Preprocessor. But I do think its a 
good idea to diagnose this early. I've modified the code to diagnose and error 
here.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3573
+StringRef Value = ExecCharset->getValue();
+Opts.ExecCharset = (std::string)Value;
+  }

tahonermann wrote:
> abhina.sreeskantharajan wrote:
> > tahonermann wrote:
> > > I wouldn't expect the cast to `std::string` to be needed here.
> > Without that cast, I get the following build error:
> > ```
> >  error: no viable overloaded '='
> > ```
> Ok, rather than a cast, I suggest:
>   Opts.ExecCharset = Value.str();
Thanks, I've applied this change.



Comment at: clang/lib/Lex/LiteralSupport.cpp:231-239
+  if (Translate && Converter) {
+// ResultChar is either UTF-8 or ASCII literal and can only be converted
+// to EBCDIC on z/OS if the character can be represented in one byte.
+if (ResultChar < 0x100) {
+  SmallString<8> ResultCharConv;
+  Converter->convert(StringRef((char *)&ResultChar), ResultCharConv);
+  void *Pointer = &ResultChar;

tahonermann wrote:
> rsmith wrote:
> > Is it correct, in general, to do character-at-a-time translation here, when 
> > processing

[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

[Denys Petrov via Phabricator via cfe-commits]

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

@steakhal 
I've precisely inspected your reports and find the bug. I've fixed it. I also 
verified all the rest and find some other vulnerabilities and fixed them as 
well.
Thank you for your testing, and I would ask you to run these tests again on my 
fix.


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

https://reviews.llvm.org/D90157

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -530,108 +530,184 @@
   return evalCast(val, castTy, originalTy);
 }
 
-// FIXME: should rewrite according to the cast kind.
-SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
-  castTy = Context.getCanonicalType(castTy);
-  originalTy = Context.getCanonicalType(originalTy);
-  if (val.isUnknownOrUndef() || castTy == originalTy)
-return val;
+//===--===//
+// Cast methods.
+// `evalCast` is the main method
+// `evalCastKind` and `evalCastSubKind` are helpers
+//===--===//
 
-  if (castTy->isBooleanType()) {
-if (val.isUnknownOrUndef())
-  return val;
-if (val.isConstant())
-  return makeTruthVal(!val.isZeroConstant(), castTy);
-if (!Loc::isLocType(originalTy) &&
-!originalTy->isIntegralOrEnumerationType() &&
-!originalTy->isMemberPointerType())
-  return UnknownVal();
-if (SymbolRef Sym = val.getAsSymbol(true)) {
-  BasicValueFactory &BVF = getBasicValueFactory();
-  // FIXME: If we had a state here, we could see if the symbol is known to
-  // be zero, but we don't.
-  return makeNonLoc(Sym, BO_NE, BVF.getValue(0, Sym->getType()), castTy);
-}
-// Loc values are not always true, they could be weakly linked functions.
-if (Optional L = val.getAs())
-  return evalCastFromLoc(*L, castTy);
+SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
+  CastTy = Context.getCanonicalType(CastTy);
+  OriginalTy = Context.getCanonicalType(OriginalTy);
+  if (CastTy == OriginalTy)
+return V;
 
-Loc L = val.castAs().getLoc();
-return evalCastFromLoc(L, castTy);
+  // FIXME: Move this check to the most appropriate evalCastKind/evalCastSubKind
+  // function.
+  // For const casts, casts to void, just propagate the value.
+  if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType())
+if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy),
+Context.getPointerType(OriginalTy)))
+  return V;
+
+  // Cast SVal according to kinds.
+  switch (V.getBaseKind()) {
+  case SVal::UndefinedValKind:
+return evalCastKind(V.castAs(), CastTy, OriginalTy);
+  case SVal::UnknownValKind:
+return evalCastKind(V.castAs(), CastTy, OriginalTy);
+  case SVal::LocKind:
+return evalCastKind(V.castAs(), CastTy, OriginalTy);
+  case SVal::NonLocKind:
+return evalCastKind(V.castAs(), CastTy, OriginalTy);
+  default:
+llvm_unreachable("Unknown SVal kind");
   }
+}
 
-  // For const casts, casts to void, just propagate the value.
-  if (!castTy->isVariableArrayType() && !originalTy->isVariableArrayType())
-if (shouldBeModeledWithNoOp(Context, Context.getPointerType(castTy),
- Context.getPointerType(originalTy)))
-  return val;
+SVal SValBuilder::evalCastKind(UndefinedVal V, QualType CastTy,
+   QualType OriginalTy) {
+  return V;
+}
 
-  // Check for casts from pointers to integers.
-  if (castTy->isIntegralOrEnumerationType() && Loc::isLocType(originalTy))
-return evalCastFromLoc(val.castAs(), castTy);
-
-  // Check for casts from integers to pointers.
-  if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) {
-if (Optional LV = val.getAs()) {
-  if (const MemRegion *R = LV->getLoc().getAsRegion()) {
-StoreManager &storeMgr = StateMgr.getStoreManager();
-R = storeMgr.castRegion(R, castTy);
-return R ? SVal(loc::MemRegionVal(R)) : UnknownVal();
-  }
-  return LV->getLoc();
-}
-return dispatchCast(val, castTy);
+SVal SValBuilder::evalCastKind(UnknownVal V, QualType CastTy,
+   QualType OriginalTy) {
+  return V;
+}
+
+SVal SValBuilder::evalCastKind(Loc V, QualType CastTy, QualType OriginalTy) {
+  switch (V.getSubKind()) {
+  case loc::ConcreteIntKind:
+return evalCastSubKind(V.castAs(), CastTy, OriginalTy);
+  case loc::GotoLabelKind:
+return evalCastSubKind(V.castAs(), CastTy, OriginalTy);
+  case loc::MemRegionValKind:
+return evalCas

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

Beyond the enabling of the compilation with `` on Windows, I really 
have the concern on the current approach supporting `` compilation in 
the device compilation. The device compilation should not relies on the host 
STL implementation. That results in inconsistent compilation results across 
various platforms, especially Linux vs. Windows.
BTW, the use of `` in CUDA cannot be compiled with NVCC directly even 
with `--expt-relaxed-constexpr`, c.f. https://godbolt.org/z/3f79co


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 313116.
hliao added a comment.

Fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

Files:
  clang/lib/Headers/__clang_hip_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/ymath.h


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,36 @@
+/*=== complex - CUDA wrapper for  -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,18 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,36 @@
+/*=== complex - CUDA wrapper for  -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
==

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1322-1323
+  TranslationState = translationState;
+  if (Kind == tok::wide_string_literal)
+TranslationState = TranslateToSystemCharset;
+  else if (isUTFLiteral(Kind))

tahonermann wrote:
> Converting wide character literals to the system encoding doesn't seem right 
> to me.  For z/OS, this should presumably convert to the wide EBCDIC encoding, 
> but for all other supported platforms, the wide execution character set is 
> either UTF-16 or UTF-32 depending on the size of `wchar_t` (which may be 
> influenced by the `-fshort-wchar` option).
Since we don't implement -fwide-exec-charset yet, what do you think should be 
the default behaviour for the interim?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

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

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

[David Sherwood via Phabricator via cfe-commits]

david-arm added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3356
   EnumArgument<"State", "LoopHintState",
-   ["enable", "disable", "numeric", "assume_safety", 
"full"],
-   ["Enable", "Disable", "Numeric", "AssumeSafety", 
"Full"]>,
+   ["enable", "disable", "numeric", "fixed_width", 
"scalable_width", "assume_safety", "full"],
+   ["Enable", "Disable", "Numeric", "FixedWidth", 
"ScalableWidth", "AssumeSafety", "Full"]>,

aaron.ballman wrote:
> Should the documentation in AttrDocs.td be updated for this change?
Hi @aaron.ballman I had a look at LoopHintDocs in AttrDocs.td and it didn't 
explicitly mention these states, i.e. "assume_safety", "numeric", etc., so I'm 
not sure if it's necessary to add anything there?


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

https://reviews.llvm.org/D89031

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Johannes Doerfert via Phabricator via cfe-commits]

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

Disclaimer: I request changes because of the next sentence, other than that I 
have no objection but also cannot review this.
All `cuda_wrapper` headers say something about complex in the first row, copy & 
paste error. All have the wrong license text (I think).

---

> The device compilation should not relies on the host STL implementation.

The OpenMP one doesn't, at least not as much as the CUDA one. Unsure how that 
works on windows though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

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

[clang] 9a93f95 - [clang] Fix expected errors in plugin attribute example

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2020-12-21T16:47:23Z
New Revision: 9a93f95fce91fb4616cee0f307b564b253789282

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

LOG: [clang] Fix expected errors in plugin attribute example

b2ba6867eac10874bd279c739639bdb9e60c1996 was landed
with updated error messages in the example file
but not in the test file.

Added: 


Modified: 
clang/test/Frontend/plugin-attribute.cpp

Removed: 




diff  --git a/clang/test/Frontend/plugin-attribute.cpp 
b/clang/test/Frontend/plugin-attribute.cpp
index 969105927be5..f02932d56c68 100644
--- a/clang/test/Frontend/plugin-attribute.cpp
+++ b/clang/test/Frontend/plugin-attribute.cpp
@@ -18,5 +18,5 @@ int var1 __attribute__((example("otherstring"))) = 1; // 
expected-warning {{'exa
 class Example {
   void __attribute__((example)) fn3(); // expected-error {{'example' attribute 
only allowed at file scope}}
 };
-void fn4() __attribute__((example(123))) { } // expected-error {{'example's 
first argument should be a string literal}}
-void fn5() __attribute__((example("a","b", 3, 4.0))) { } // expected-error 
{{'example' attribute only allowed at most three arguments}}
+void fn4() __attribute__((example(123))) { } // expected-error {{first 
argument to the 'example' attribute must be a string literal}}
+void fn5() __attribute__((example("a","b", 3, 4.0))) { } // expected-error 
{{'example' attribute only accepts at most three arguments}}



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

[PATCH] D92006: Refactoring the attribute plugin example to fit the new API

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

The last update didn't update the expected errors. I have done so in 
9a93f95fce91fb4616cee0f307b564b253789282 
 and it's 
now passing. (I think it missed the bots since they don't build examples)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

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

[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

[Alex Orlov via Phabricator via cfe-commits]

aorlov added a comment.

Hi, community. I kindly ask you to review this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92024

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

[PATCH] D92006: Refactoring the attribute plugin example to fit the new API

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D92006#2466114 , @DavidSpickett 
wrote:

> The last update didn't update the expected errors. I have done so in 
> 9a93f95fce91fb4616cee0f307b564b253789282 
>  and 
> it's now passing. (I think it missed the bots since they don't build examples)

Oh, thank you for this! I'm guessing my local build also missed this for the 
same reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92006

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

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D93586#2464841 , @spatel wrote:

> Besides changing IRBuilder and shufflevector's definition, I think we'll also 
> need updates in the vectorizers, InstSimplify, and other places in 
> InstCombine that use UndefValue for InsertElement and shuffle transforms.

Thank you for the pointer!

> Do you have an estimate of how many tests are out there? If it's a temporary 
> increase and not huge, I don't think there is much downside. But if we think 
> the transition will take a long time, then maybe we don't want the 
> duplication.

Using this command (which counts `insertelement <..> undef` in non-checks):

  grep -R -E '^[^;]*insertelement <.*> undef,' . | cut -d":" -f1 | uniq | wc -l

There are 792 files in llvm/test, most of which are in test/Transform (119) and 
test/CodeGen(655).
The transition will be swiftly done (if there's no other issue hopefully) by 
the next weekend.

My concern is that some tests aren't using `utils/update_test_checks.py`, and 
this makes things complicated. :(
Simply replacing `insertelement undef` at `CHECK:` isn't enough (ex: 
Transforms/SLPVectorizer/X86/alternate-cast.ll).
What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93586

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

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

In D93586#2466284 , @aqjune wrote:

> The transition will be swiftly done (if there's no other issue hopefully) by 
> the next weekend.

Oops, I meant this weekend hopefully.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93586

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 313136.
hliao added a comment.

Fix typo again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

Files:
  clang/lib/Headers/__clang_hip_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/ymath.h


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,36 @@
+/*=== ymath.h - CUDA wrapper for  -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,18 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,36 @@
+/*=== ymath.h - CUDA wrapper for  -===
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h

[PATCH] D93648: Revert "[AMDGPU][HIP] Switch default DWARF version to 5"

[Scott Linder via Phabricator via cfe-commits]

scott.linder created this revision.
Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, yaxunl, nhaehnle, 
jvesely, kzhuravl.
scott.linder requested review of this revision.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.

This reverts commit c4d10e7e9bb47b77fad43d8ddcfa328298f36c88 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93648

Files:
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.h
  clang/test/Driver/amdgpu-toolchain.c
  clang/test/Driver/hip-toolchain-dwarf.hip


Index: clang/test/Driver/hip-toolchain-dwarf.hip
===
--- clang/test/Driver/hip-toolchain-dwarf.hip
+++ clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
 
   const ToolChain &HostTC;
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 


Index: clang/test/Driver/hip-toolchain-dwarf.hip
===
--- clang/test/Driver/hip-toolchain-dwarf.hip
+++ clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
 
   const ToolChain &HostTC;
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 313138.
hliao added a comment.

Fix license.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

Files:
  clang/lib/Headers/__clang_hip_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/ymath.h


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,22 @@
+/*=== ymath.h - HIP wrapper for  --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,18 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,22 @@
+/*=== ymath.h - HIP wrapper for  --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,18 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D93638#2466097 , @jdoerfert wrote:

> Disclaimer: I request changes because of the next sentence, other than that I 
> have no objection but also cannot review this.
> All `cuda_wrapper` headers say something about complex in the first row, copy 
> & paste error. All have the wrong license text (I think).

Fixed.

> ---
>
>> The device compilation should not relies on the host STL implementation.
>
> The OpenMP one doesn't, at least not as much as the CUDA one. Unsure how that 
> works on windows though.

This patch doesn't apply to OpenMP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

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

[clang] bb8d20d - [cuda][hip] Fix typoes in header wrappers.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2020-12-21T13:02:47-05:00
New Revision: bb8d20d9f3bb955ae6f6143d24749faf61d573a9

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

LOG: [cuda][hip] Fix typoes in header wrappers.

Added: 


Modified: 
clang/lib/Headers/cuda_wrappers/algorithm
clang/lib/Headers/cuda_wrappers/new

Removed: 




diff  --git a/clang/lib/Headers/cuda_wrappers/algorithm 
b/clang/lib/Headers/cuda_wrappers/algorithm
index 01af18360d8d..f14a0b00bb04 100644
--- a/clang/lib/Headers/cuda_wrappers/algorithm
+++ b/clang/lib/Headers/cuda_wrappers/algorithm
@@ -1,4 +1,4 @@
-/*=== complex - CUDA wrapper for  
===
+/*=== algorithm - CUDA wrapper for  -===
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal

diff  --git a/clang/lib/Headers/cuda_wrappers/new 
b/clang/lib/Headers/cuda_wrappers/new
index 7f255314056a..d5fb3b7011de 100644
--- a/clang/lib/Headers/cuda_wrappers/new
+++ b/clang/lib/Headers/cuda_wrappers/new
@@ -1,4 +1,4 @@
-/*=== complex - CUDA wrapper for  --===
+/*=== new - CUDA wrapper for  -===
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal



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

[PATCH] D93452: [clangd] Trim memory periodically

[Sam McCall via Phabricator via cfe-commits]

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

Awesome! Some nits on naming and stuff, but the only real substantial question 
for me is the exact availability/behavior of the command-line flag. Even there, 
as long as the default behavior is to trim when build with glibc, the details 
are less important.
Let me know when you want me to land it!

In D93452#2465076 , @qchateau wrote:

> In the meantime, I've done some testing with tcmalloc and jemalloc.
> `jemalloc` does very good in this situation, the RSS goes down as soon as the 
> memory is not required anymore:
> `tcmalloc` does fine, but is not ideal, the RSS does not grow to an 
> unreasonable size, but does not go down when we release resources.

Thanks for this! I agree this is basically tcmalloc working as designed, it 
tends to sit at the high-water mark.

In D93452#2465117 , @qchateau wrote:

> Let me know if I should do anything concerning these files.
>
> - clang-tools-extra/clangd/test/lit.site.cfg.py.in

This is only needed if we want to make lit tests run conditionally on whether 
the CMake variable is enabled. (For various reasons, I think testing this at 
all is probably more trouble than it's worth).

> - llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn
> - llvm/utils/gn/secondary/clang-tools-extra/clangd/test/BUILD.gn

These are for the alternate `gn` build system, which is maintained by its users 
(commits are not expected to keep it working). It would be reasonable/friendly 
to blindly add `"CLANGD_ENABLE_MEMORY_CLEANUP=1"` to both files so that gn 
always builds in that configuration. FWIW I almost always forget/don't bother, 
and that's totally acceptable too.




Comment at: clang-tools-extra/clangd/CMakeLists.txt:17
 
+option(CLANGD_ENABLE_MEMORY_CLEANUP "Enable active memory cleanup for Clangd." 
ON)
+

In keeping with reducing the abstraction level, maybe we could call this 
CLANGD_MALLOC_TRIM and note "(only takes affect when using glibc)"?



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:504
 
+#if CLANGD_ENABLE_MEMORY_CLEANUP
+opt EnableMemoryCleanup{

currently we have the flag or not depending on the cmake flag, and it does 
something or not depending on whether we're using glibc.

I'd suggest rather we should have the flag if we have glibc (i.e. if it does 
something), and its *default* should depend on the cmake flag.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:504
 
+#if CLANGD_ENABLE_MEMORY_CLEANUP
+opt EnableMemoryCleanup{

sammccall wrote:
> currently we have the flag or not depending on the cmake flag, and it does 
> something or not depending on whether we're using glibc.
> 
> I'd suggest rather we should have the flag if we have glibc (i.e. if it does 
> something), and its *default* should depend on the cmake flag.
It would also be nice to reduce the number of separate #ifdef sections 
(currently 4).

I'd suggest something like:

```
#ifdef __GLIBC__
opt EnableMallocTrim{
...
   init(CLANGD_MALLOC_TRIM),
};
static std::function MallocTrimFunc() {
  if (!EnableMallocTrim)
return nullptr;
  return []{ ... };
}
#else
static std::function MallocTrimFunc() {
  return nullptr;
}
#endif
```

then later we can assign to opts without an ifdef or other condition



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:600
+  constexpr size_t MallocTrimPad = 20'000'000;
+  return []() { malloc_trim(MallocTrimPad); };
+#else

now we don't have the abstraction, we can have our logging back:

```
if (malloc_trim(MallocTrimPad))
  vlog("Released memory via malloc_trim");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93452

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

[PATCH] D92221: Don't delete default constructor of PathDiagnosticConsumerOptions

[Moritz Sichert via Phabricator via cfe-commits]

MoritzS updated this revision to Diff 313145.
MoritzS added a comment.

Rebased onto master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92221

Files:
  clang/include/clang/Analysis/PathDiagnostic.h


Index: clang/include/clang/Analysis/PathDiagnostic.h
===
--- clang/include/clang/Analysis/PathDiagnostic.h
+++ clang/include/clang/Analysis/PathDiagnostic.h
@@ -95,8 +95,6 @@
   /// Whether the consumer should present the name of the entity that emitted
   /// the diagnostic (eg., a checker) so that the user knew how to disable it.
   bool ShouldDisplayDiagnosticName;
-
-  PathDiagnosticConsumerOptions() = delete;
 };
 
 class PathDiagnosticConsumer {


Index: clang/include/clang/Analysis/PathDiagnostic.h
===
--- clang/include/clang/Analysis/PathDiagnostic.h
+++ clang/include/clang/Analysis/PathDiagnostic.h
@@ -95,8 +95,6 @@
   /// Whether the consumer should present the name of the entity that emitted
   /// the diagnostic (eg., a checker) so that the user knew how to disable it.
   bool ShouldDisplayDiagnosticName;
-
-  PathDiagnosticConsumerOptions() = delete;
 };
 
 class PathDiagnosticConsumer {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93600: [clangd] When querying drivers by binary, look in PATH too

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, this looks like a good idea to me, but some quibbles with the details:

- for any string, we statically know whether it's path or WD-relative
- we should make sure all the IO gets cached




Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:334
 
 llvm::SmallString<128> Driver(Cmd->CommandLine.front());
 llvm::sys::fs::make_absolute(Cmd->Directory, Driver);

Oops, this has come up before (in CommandMangler).
Your idea here makes sense (if the driver is "clang" we should look it up on 
the PATH) and the approach is backwards-compatible.

However when we think more carefully, by allowing the driver to be both 
WD-relative and dir-relative, we're emulating shell behavior (which makes 
sense, we have an argv!).
But shells don't generally try one then the other, they instead distinguish 
three cases:
 - bare name `clang` is looked up on PATH
 - relative path `bin/clang` is resolved against WD
 - absolute path `/usr/bin/clang` is used as-is

So I think we'd rather have something like:
```
if (llvm::none_of(Driver,
 [](char C) { return llvm::sys::path::is_separator(C); })) {
  // resolve against path
} else {
  make_absolute(Cmd->Directory, Driver);
}
```



Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:337
 
+if (!llvm::sys::fs::exists(Driver)) {
+  auto DriverProgram =

I'm not sure it's reasonable to do IO like this before we have a chance to hit 
the cache (but see above comment, I don't think we need the `exists()` at all)



Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:339
+  auto DriverProgram =
+  llvm::sys::findProgramByName(Cmd->CommandLine.front());
+  if (DriverProgram) {

hmm, this also does IO, and we obviously do need this.

I think we need to move this into the cached lookup, so:
 - if the path is "bin/clangd" we resolve it to absolute before querying the 
cache
 - if the path is "/usr/bin/clangd" we pass that absolute path to cache
 - but if the path is "clangd" we pass that exact string into 
extractSystemIncludesAndTarget(), which notices that it's relative and looks it 
up on PATH. Fortunately we can assume PATH doesn't change while running, so the 
cache is still correct. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93600

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

[PATCH] D93546: [clangd][NFC] Improve clangd status messages

[Sam McCall via Phabricator via cfe-commits]

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

Thanks, LG - want me to land it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93546

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

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

[Sam McCall via Phabricator via cfe-commits]

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

Thanks, this version seems just as clear as the original to me, and fewer 
allocations is nice :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93531

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

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3356
   EnumArgument<"State", "LoopHintState",
-   ["enable", "disable", "numeric", "assume_safety", 
"full"],
-   ["Enable", "Disable", "Numeric", "AssumeSafety", 
"Full"]>,
+   ["enable", "disable", "numeric", "fixed_width", 
"scalable_width", "assume_safety", "full"],
+   ["Enable", "Disable", "Numeric", "FixedWidth", 
"ScalableWidth", "AssumeSafety", "Full"]>,

david-arm wrote:
> aaron.ballman wrote:
> > Should the documentation in AttrDocs.td be updated for this change?
> Hi @aaron.ballman I had a look at LoopHintDocs in AttrDocs.td and it didn't 
> explicitly mention these states, i.e. "assume_safety", "numeric", etc., so 
> I'm not sure if it's necessary to add anything there?
Oh, I see now, we're deferring to the documentation in the language extensions 
document. I suppose that's fine as far as this patch goes, sorry for the noise.


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

https://reviews.llvm.org/D89031

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

[PATCH] D93546: [clangd][NFC] Improve clangd status messages

[Quentin Chateau via Phabricator via cfe-commits]

qchateau added a comment.

Sure !
Email: quentin.chat...@gmail.com

I guess I'll need to ask for commit access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93546

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

[PATCH] D93653: [clangd] Avoid reallocating buffers for each message read:

[Sam McCall via Phabricator via cfe-commits]

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

- reuse std::string we read messages into
- when reading line-wise, use SmallVector<128> and read in chunks of 128 (this 
affects headers, which are short, and tests, which don't matter)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93653

Files:
  clang-tools-extra/clangd/JSONTransport.cpp

Index: clang-tools-extra/clangd/JSONTransport.cpp
===
--- clang-tools-extra/clangd/JSONTransport.cpp
+++ clang-tools-extra/clangd/JSONTransport.cpp
@@ -10,6 +10,7 @@
 #include "support/Cancellation.h"
 #include "support/Logger.h"
 #include "support/Shutdown.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/Error.h"
 #include 
@@ -99,6 +100,7 @@
   }
 
   llvm::Error loop(MessageHandler &Handler) override {
+std::string JSON; // Messages may be large, reuse same big buffer.
 while (!feof(In)) {
   if (shutdownRequested())
 return error(std::make_error_code(std::errc::operation_canceled),
@@ -106,14 +108,14 @@
   if (ferror(In))
 return llvm::errorCodeToError(
 std::error_code(errno, std::system_category()));
-  if (auto JSON = readRawMessage()) {
-if (auto Doc = llvm::json::parse(*JSON)) {
+  if (readRawMessage(JSON)) {
+if (auto Doc = llvm::json::parse(JSON)) {
   vlog(Pretty ? "<<< {0:2}\n" : "<<< {0}\n", *Doc);
   if (!handleMessage(std::move(*Doc), Handler))
 return llvm::Error::success(); // we saw the "exit" notification.
 } else {
   // Parse error. Log the raw message.
-  vlog("<<< {0}\n", *JSON);
+  vlog("<<< {0}\n", JSON);
   elog("JSON parse error: {0}", llvm::toString(Doc.takeError()));
 }
   }
@@ -136,12 +138,12 @@
   }
 
   // Read raw string messages from input stream.
-  llvm::Optional readRawMessage() {
-return Style == JSONStreamStyle::Delimited ? readDelimitedMessage()
-   : readStandardMessage();
+  bool readRawMessage(std::string &JSON) {
+return Style == JSONStreamStyle::Delimited ? readDelimitedMessage(JSON)
+   : readStandardMessage(JSON);
   }
-  llvm::Optional readDelimitedMessage();
-  llvm::Optional readStandardMessage();
+  bool readDelimitedMessage(std::string &JSON);
+  bool readStandardMessage(std::string &JSON);
 
   std::FILE *In;
   llvm::raw_ostream &Out;
@@ -190,8 +192,10 @@
 
 // Tries to read a line up to and including \n.
 // If failing, feof(), ferror(), or shutdownRequested() will be set.
-bool readLine(std::FILE *In, std::string &Out) {
-  static constexpr int BufSize = 1024;
+bool readLine(std::FILE *In, llvm::SmallVectorImpl &Out) {
+  // Big enough to hold any reasonable header line. May not fit content lines
+  // in delimited mode, but performance doesn't matter for that mode.
+  static constexpr int BufSize = 128;
   size_t Size = 0;
   Out.clear();
   for (;;) {
@@ -215,14 +219,14 @@
 // Returns None when:
 //  - ferror(), feof(), or shutdownRequested() are set.
 //  - Content-Length is missing or empty (protocol error)
-llvm::Optional JSONTransport::readStandardMessage() {
+bool JSONTransport::readStandardMessage(std::string &JSON) {
   // A Language Server Protocol message starts with a set of HTTP headers,
   // delimited  by \r\n, and terminated by an empty line (\r\n).
   unsigned long long ContentLength = 0;
-  std::string Line;
+  llvm::SmallString<128> Line;
   while (true) {
 if (feof(In) || ferror(In) || !readLine(In, Line))
-  return llvm::None;
+  return false;
 InMirror << Line;
 
 llvm::StringRef LineRef(Line);
@@ -257,14 +261,14 @@
 elog("Refusing to read message with long Content-Length: {0}. "
  "Expect protocol errors",
  ContentLength);
-return llvm::None;
+return false;
   }
   if (ContentLength == 0) {
 log("Warning: Missing Content-Length header, or zero-length message.");
-return llvm::None;
+return false;
   }
 
-  std::string JSON(ContentLength, '\0');
+  JSON.resize(ContentLength);
   for (size_t Pos = 0, Read; Pos < ContentLength; Pos += Read) {
 // Handle EINTR which is sent when a debugger attaches on some platforms.
 Read = retryAfterSignalUnlessShutdown(0, [&]{
@@ -273,24 +277,24 @@
 if (Read == 0) {
   elog("Input was aborted. Read only {0} bytes of expected {1}.", Pos,
ContentLength);
-  return llvm::None;
+  return false;
 }
 InMirror << llvm::StringRef(&JSON[Pos], Read);
 clearerr(In); // If we're done, the error was transient. If we're not done,
   // either it was transient or we'll 

[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I've put a version of the reading part in D93653 
. This seems a bit less invasive to me - I'm 
still not sure if it's worth the readability hit though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93531

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

[PATCH] D93546: [clangd][NFC] Improve clangd status messages

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

It's up to you - it's easy for me/others to land them on your behalf. Having 
your own commit access gives you more control, ability to deal with build 
breakages quickly, fix trivial things without review etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93546

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

[clang-tools-extra] 3fa2d37 - [clangd][NFC] Improve clangd status messages

[Sam McCall via cfe-commits]

Author: Quentin Chateau
Date: 2020-12-21T20:19:25+01:00
New Revision: 3fa2d37eb3f8acddcfde749ca822f2cc7d900cbb

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

LOG: [clangd][NFC] Improve clangd status messages

clangd actions have various naming schemes, the most
common being PascalCase. This commit applies PascalCase
to all clangd actions, and fix the status rendering
in `renderTUAction` to look more consistent.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/TUScheduler.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 8b0d0591abe7..b760b31c0b87 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -621,7 +621,7 @@ void ClangdServer::typeHierarchy(PathRef File, Position 
Pos, int Resolve,
 File));
   };
 
-  WorkScheduler.runWithAST("Type Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("TypeHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::resolveTypeHierarchy(
@@ -642,7 +642,7 @@ void ClangdServer::prepareCallHierarchy(
   return CB(InpAST.takeError());
 CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
   };
-  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("CallHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::incomingCalls(
@@ -678,7 +678,7 @@ void ClangdServer::documentSymbols(llvm::StringRef File,
   return CB(InpAST.takeError());
 CB(clangd::getDocumentSymbols(InpAST->AST));
   };
-  WorkScheduler.runWithAST("documentSymbols", File, std::move(Action),
+  WorkScheduler.runWithAST("DocumentSymbols", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
@@ -690,7 +690,7 @@ void ClangdServer::foldingRanges(llvm::StringRef File,
   return CB(InpAST.takeError());
 CB(clangd::getFoldingRanges(InpAST->AST));
   };
-  WorkScheduler.runWithAST("foldingRanges", File, std::move(Action),
+  WorkScheduler.runWithAST("FoldingRanges", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 813a000b41a5..7a858664faa5 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -1220,7 +1220,7 @@ std::string renderTUAction(const PreambleAction PA, const 
ASTAction &AA) {
   }
   if (Result.empty())
 return "idle";
-  return llvm::join(Result, ",");
+  return llvm::join(Result, ", ");
 }
 
 } // namespace



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

[PATCH] D93546: [clangd][NFC] Improve clangd status messages

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3fa2d37eb3f8: [clangd][NFC] Improve clangd status messages 
(authored by qchateau, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93546

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1220,7 +1220,7 @@
   }
   if (Result.empty())
 return "idle";
-  return llvm::join(Result, ",");
+  return llvm::join(Result, ", ");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -621,7 +621,7 @@
 File));
   };
 
-  WorkScheduler.runWithAST("Type Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("TypeHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::resolveTypeHierarchy(
@@ -642,7 +642,7 @@
   return CB(InpAST.takeError());
 CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
   };
-  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("CallHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::incomingCalls(
@@ -678,7 +678,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getDocumentSymbols(InpAST->AST));
   };
-  WorkScheduler.runWithAST("documentSymbols", File, std::move(Action),
+  WorkScheduler.runWithAST("DocumentSymbols", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
@@ -690,7 +690,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getFoldingRanges(InpAST->AST));
   };
-  WorkScheduler.runWithAST("foldingRanges", File, std::move(Action),
+  WorkScheduler.runWithAST("FoldingRanges", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 


Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1220,7 +1220,7 @@
   }
   if (Result.empty())
 return "idle";
-  return llvm::join(Result, ",");
+  return llvm::join(Result, ", ");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -621,7 +621,7 @@
 File));
   };
 
-  WorkScheduler.runWithAST("Type Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("TypeHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::resolveTypeHierarchy(
@@ -642,7 +642,7 @@
   return CB(InpAST.takeError());
 CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
   };
-  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+  WorkScheduler.runWithAST("CallHierarchy", File, std::move(Action));
 }
 
 void ClangdServer::incomingCalls(
@@ -678,7 +678,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getDocumentSymbols(InpAST->AST));
   };
-  WorkScheduler.runWithAST("documentSymbols", File, std::move(Action),
+  WorkScheduler.runWithAST("DocumentSymbols", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
@@ -690,7 +690,7 @@
   return CB(InpAST.takeError());
 CB(clangd::getFoldingRanges(InpAST->AST));
   };
-  WorkScheduler.runWithAST("foldingRanges", File, std::move(Action),
+  WorkScheduler.runWithAST("FoldingRanges", File, std::move(Action),
TUScheduler::InvalidateOnUpdate);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93596: [ASTMatchers] Traverse-ignore range-for implementation details

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93596

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert resigned from this revision.
jdoerfert added a comment.

In D93638#2466333 , @hliao wrote:

> In D93638#2466097 , @jdoerfert wrote:
>
>>> The device compilation should not relies on the host STL implementation.
>>
>> The OpenMP one doesn't, at least not as much as the CUDA one. Unsure how 
>> that works on windows though.
>
> This patch doesn't apply to OpenMP.

I'm aware.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

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

[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D84924#2465745 , @njames93 wrote:

> In D84924#2446075 , @aaron.ballman 
> wrote:
>
>> In D84924#2184132 , @njames93 wrote:
>>
>>> This is very much a work in progress
>>> Another direction I was thinking was only apply the fixes found in notes if 
>>> there is exactly one fix attached to the notes in a diagnostic, instead of 
>>> just applying the first one we find
>>
>> I was wondering the same thing here myself. If there's exactly one fix, then 
>> it's unambiguous as to what behavior you get. One (minor) concern I have 
>> about the current approach is with nondeterminism in diagnostic ordering 
>> where different runs may pick different fixes for the same code. I don't 
>> *think* we have any instances of this in Clang or clang-tidy, but users can 
>> add their own plugins (for instance to the clang static analyzer) that may 
>> introduce some small risk there. Do you have a reason why you picked one 
>> approach over the other?
>
> Part of the reason for this approach is from this bug report 
> https://bugs.llvm.org/show_bug.cgi?id=47971, where its pointed out that clang 
> diags gets fix-its added in notes if the fixes will change behaviour or they 
> aren't sure its going to actually fix the issue.
> As clang-tidy also applies fixes reported from clang, it is wise to adhere to 
> a similar level caution.

Agreed, but I don't think the current behavior in this patch is cautious 
enough. I like that we have to specify "please apply fixits from notes" 
explicitly. I think that's the right behavior we want. But if there are 
multiple fixits attached to the notes for a diagnostic, I don't think we should 
assume that the first fix-it is the correct one to apply -- I think the user 
should have to manually pick the variant they want in that case. So I think the 
behavior should be to apply the fixits from notes if there's only a single 
fixit to be applied. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84924

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

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Should this have some tests? Even if guarded by `REQUIRES:` if some feature is 
needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90159

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy created this revision.
Herald added subscribers: dexonsmith, wenlei, hiraditya.
hoy requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

`UniqueInternalLinkageNamesPass` is useful to CSSPGO, especially when pseudo 
probe is used. It solves naming conflict for static functions which otherwise 
will share a merged profile and likely have a profile quality issue with 
mismatched CFG checksums. Since the pseudo probe instrumentation happens very 
early in the pipeline, I'm moving `UniqueInternalLinkageNamesPass` right before 
it. This is being done only to the new pass manager. In addition, funtions that 
are renamed have their debug linkage name and an AutoFDO attribute updated 
optionally.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93656

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/unique-internal-linkage-names.cpp
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp

Index: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp
===
--- llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp
+++ llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp
@@ -13,13 +13,20 @@
 
 #include "llvm/Transforms/Utils/UniqueInternalLinkageNames.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
 #include "llvm/InitializePasses.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
 using namespace llvm;
 
+static cl::opt UniqueDebugAndProfileNames(
+"unqiue-debug-profile-names", cl::init(false), cl::Hidden,
+cl::desc("Uniqueify debug and profile symbol Names"));
+
 static bool uniqueifyInternalLinkageNames(Module &M) {
   llvm::MD5 Md5;
   Md5.update(M.getSourceFileName());
@@ -31,11 +38,19 @@
   // this symbol is of internal linkage type.
   std::string ModuleNameHash = (Twine(".__uniq.") + Twine(Str)).str();
   bool Changed = false;
+  MDBuilder MDB(M.getContext());
 
   // Append the module hash to all internal linkage functions.
   for (auto &F : M) {
 if (F.hasInternalLinkage()) {
   F.setName(F.getName() + ModuleNameHash);
+  if (UniqueDebugAndProfileNames) {
+F.addFnAttr("sample-profile-suffix-elision-policy", "selected");
+if (DISubprogram *SP = F.getSubprogram()) {
+  auto Name = MDB.createString(F.getName());
+  SP->replaceRawLinkageName(Name);
+}
+  }
   Changed = true;
 }
   }
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -434,8 +434,10 @@
 
 PassBuilder::PassBuilder(bool DebugLogging, TargetMachine *TM,
  PipelineTuningOptions PTO, Optional PGOOpt,
- PassInstrumentationCallbacks *PIC)
-: DebugLogging(DebugLogging), TM(TM), PTO(PTO), PGOOpt(PGOOpt), PIC(PIC) {
+ PassInstrumentationCallbacks *PIC,
+ bool UniqueLinkageNames)
+: DebugLogging(DebugLogging), UniqueLinkageNames(UniqueLinkageNames),
+  TM(TM), PTO(PTO), PGOOpt(PGOOpt), PIC(PIC) {
   if (TM)
 TM->registerPassBuilderCallbacks(*this, DebugLogging);
   if (PIC && shouldPopulateClassToPassNames()) {
@@ -1001,6 +1003,11 @@
ThinLTOPhase Phase) {
   ModulePassManager MPM(DebugLogging);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage
+  // symbols with unique names.
+  if (UniqueLinkageNames)
+MPM.addPass(UniqueInternalLinkageNamesPass());
+
   // Place pseudo probe instrumentation as the first pass of the pipeline to
   // minimize the impact of optimization changes.
   if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
@@ -1764,6 +1771,11 @@
 
   ModulePassManager MPM(DebugLogging);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage
+  // symbols with unique names.
+  if (UniqueLinkageNames)
+MPM.addPass(UniqueInternalLinkageNamesPass());
+
   if (PGOOpt && (PGOOpt->Action == PGOOptions::IRInstr ||
  PGOOpt->Action == PGOOptions::IRUse))
 addPGOInstrPassesForO0(
Index: llvm/lib/IR/DebugInfoMetadata.cpp
===
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -877,6 +877,10 @@
   return F->getSubprogram() == this;
 }
 
+void DISubprogram::replaceRawLinkageName(MDString *LinkageName) {
+  replaceOperandWith(3, LinkageName);
+}
+
 DILexicalBlock *DILexicalBlock::getImpl(LLVMContext &Context, Metadata *Scope,
 

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

I too was currently working on changing the raw linkage names, 
DW_AT_linkage_name, and was about to send out a patch along these lines :).  
Thanks for doing this!  I will take a look at it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D72235: [clang-tidy] new altera unroll loops check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 313174.
ffrankies added a comment.

- Addressed comments from @njames93
- Rebased with latest master branch to make sure there are no merge issues with 
the latest committed altera check


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

https://reviews.llvm.org/D72235

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.cpp
  clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-unroll-loops.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/altera-unroll-loops.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera-unroll-loops.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-unroll-loops.cpp
@@ -0,0 +1,359 @@
+// RUN: %check_clang_tidy %s altera-unroll-loops %t -- -config="{CheckOptions: [{key: "altera-unroll-loops.MaxLoopIterations", value: 50}]}" -header-filter=.* "--" --include opencl-c.h -cl-std=CL1.2 -c
+
+// Inner loops should be unrolled
+__kernel void nested_simple_loops(__global int *A) {
+for (int i = 0; i < 1000; ++i) {
+for (int j = 0; j < 2000; ++j) { 
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[0] += i + j;
+}
+}
+
+for (int i = 0; i < 1000; ++i) {
+int j = 0;
+while (j < 2000) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[1] += i + j;
+j++;
+}
+}
+
+for (int i = 0; i < 1000; ++i) {
+int j = 0; 
+do {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[2] += i + j;
+j++;
+} while (j < 2000);
+}
+
+int i = 0;
+while (i < 1000) {
+for (int j = 0; j < 2000; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[3] += i + j;
+}
+i++;
+}
+
+i = 0;
+while (i < 1000) {
+int j = 0;
+while (j < 2000) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[4] += i + j;
+j++;
+}
+i++;
+}
+
+i = 0;
+while (i < 1000) {
+int j = 0;
+do {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[5] += i + j;
+j++;
+} while (j < 2000);
+i++;
+}
+
+i = 0;
+do {
+for (int j = 0; j < 2000; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[6] += i + j;
+}
+i++;
+} while (i < 1000);
+
+i = 0;
+do {
+int j = 0;
+while (j < 2000) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[7] += i + j;
+j++;
+}
+i++;
+} while (i < 1000);
+
+i = 0;
+do {
+int j = 0;
+do {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+A[8] += i + j;
+j++;
+} while (j < 2000);
+i++;
+} while (i < 1000);
+
+for (int i = 0; i < 100; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+printf("Hello");
+}
+
+i = 0;
+while (i < 100) {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+i++;
+}
+
+i = 0;
+do {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: kernel performance could be improved by unrolling this loop with a #pragma unroll directive [altera-unroll-loops]
+i++;
+} while (i < 100);
+}
+
+// These loops are all correctly unrolled
+__kernel void unrolled_nested_simple_loops(__global int *A) {
+for (int i = 0; i < 1000; ++i) {
+#pragma unro

[PATCH] D90329: [PowerPC] Fix va_arg in C++, Objective-C on 32-bit ELF targets

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai added a comment.

LGTM aside from a minor nit.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:4722
   bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64;
-  bool isInt =
-  Ty->isIntegerType() || Ty->isPointerType() || Ty->isAggregateType();
+  bool isInt = !Ty->isFloatingType();
   bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64;

nit (feel free to ignore): seems like a better name might be something like 
`isSingleGPR` since it would appear that this is for types that go into a 
(single) general purpose register.


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

https://reviews.llvm.org/D90329

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

https://reviews.llvm.org/D73307#1932131  rnk@ mentioned this :  "At a higher 
level, should this just be an IR pass that clang adds into the pipeline when 
the flag is set? It should be safe to rename internal functions and give 
private functions internal linkage. It would be less invasive to clang and have 
better separation of concerns. As written, this is based on the source filename 
on the module, which is accessible from IR. The only reason I can think of 
against this is that the debug info might refer to the function linkage name, 
but maybe that is calculated later during codegen."

I just wanted to mention it  here that this was anticipated and was missed in 
the original patch, my bad as I didnt think about DebugInfo change.  However, I 
think it is pretty straightforward to change the linkage name so I would still 
keep the pass approach.




Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:27
+static cl::opt UniqueDebugAndProfileNames(
+"unqiue-debug-profile-names", cl::init(false), cl::Hidden,
+cl::desc("Uniqueify debug and profile symbol Names"));

Can we make it true by default?  Atleast the DW_AT_linkage_name must reflect 
the new name by default IMO.



Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:51
+  auto Name = MDB.createString(F.getName());
+  SP->replaceRawLinkageName(Name);
+}

Do we need to check if it had a rawLinkageName before replacing it?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:27
+static cl::opt UniqueDebugAndProfileNames(
+"unqiue-debug-profile-names", cl::init(false), cl::Hidden,
+cl::desc("Uniqueify debug and profile symbol Names"));

tmsriram wrote:
> Can we make it true by default?  Atleast the DW_AT_linkage_name must reflect 
> the new name by default IMO.
Sounds good.



Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:51
+  auto Name = MDB.createString(F.getName());
+  SP->replaceRawLinkageName(Name);
+}

tmsriram wrote:
> Do we need to check if it had a rawLinkageName before replacing it?  
Good point. `rawLinkageName` is missing for C programs. I think it might still 
be appropriate to assign a new linkage name in the case so as to differ from 
the original source function name. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added a comment.

In D93656#2466689 , @tmsriram wrote:

> https://reviews.llvm.org/D73307#1932131  rnk@ mentioned this :  "At a higher 
> level, should this just be an IR pass that clang adds into the pipeline when 
> the flag is set? It should be safe to rename internal functions and give 
> private functions internal linkage. It would be less invasive to clang and 
> have better separation of concerns. As written, this is based on the source 
> filename on the module, which is accessible from IR. The only reason I can 
> think of against this is that the debug info might refer to the function 
> linkage name, but maybe that is calculated later during codegen."
>
> I just wanted to mention it  here that this was anticipated and was missed in 
> the original patch, my bad as I didnt think about DebugInfo change.  However, 
> I think it is pretty straightforward to change the linkage name so I would 
> still keep the pass approach.

Thanks for the information. Yeah it's pretty straightforward hook it up with 
debug info as an IR pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D93631: [clang][cli] Implement `getAllArgValues` marshalling

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:338
+const std::vector &Values) {
+  for (const std::string& Value : Values) {
+denormalizeString(Args, Spelling, SA, OptClass, TableIndex, Value);

Nit: we usually put the `&` next to the variable name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93631

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

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

[Renato Golin via Phabricator via cfe-commits]

rengolin added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2089
+  static const char *const M68kTriples[] = {
+  "m68k-linux-gnu", "m68k-unknown-linux-gnu", "m68k-suse-linux"};
+

jrtc27 wrote:
> rengolin wrote:
> > The front-end supports FreeBSD, too.
> Aren't these arrays only used on multiarch systems so irrelevant for BSDs? 
> There aren't FreeBSD triples listed for X86/X86_64 for example.
Makes sense.


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

https://reviews.llvm.org/D88394

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

[PATCH] D93490: [clang-format] PR48539 ReflowComments breaks Qt translation comments

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

HazardyKnusperkeks added a comment.

So, I've tested a bit:

  //= Die ID
  //~ foo bar
  //~ EinWort Ein langer Text der fortgesetzt wird
  //: Dies ist ein langer Kommentar
  //: der umgebrochen wird
  tr("Foo");

Results in

  
  
  Foo
  Dies ist ein langer Kommentar der umgebrochen 
wird
  
  Ein langer Text der fortgesetzt wird
  bar
  

And breaking `//=` or `//~` as followed

  //= Die
  //= ID
  //~ foo bar
  //~ EinWort Ein langer Text der
  //~ fortgesetzt wird
  //: Dies ist ein langer Kommentar
  //: der umgebrochen wird
  tr("Foo");

changes the result

  
  
  Foo
  Dies ist ein langer Kommentar der umgebrochen 
wird
  
  Ein langer Text der
  bar
  wird
  

So my conclusion is `//:` can and should be wrapped like in this patch, but 
`//=` and `//~` should not be touched anyhow, this can be accomplished with the 
`CommentPragmas` Setting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93490

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

[PATCH] D93628: [clang] NFC: Refactor custom class into a lambda in CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

This looks like an improvement, so LGTM, but I have a couple of comments.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:160-161
 
-namespace {
-template  struct FlagToValueNormalizer {
-  T Value;
-
-  Optional operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
- DiagnosticsEngine &) {
+template ::value &&
+ llvm::is_integral_or_enum::value),
+   bool> = false>

I missed the double negation here for a while; might be simpler as:
```
::value ||
 !llvm::is_integral_or_neum::value>
```




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:172-174
+template ::value &&
+llvm::is_integral_or_enum::value),
+   bool> = false>

Ah, I guess you want this to match the above. I guess that's the benefit of the 
old `is_int_convertible` -- it didn't need double-negation to make it clear 
that the two functions were alternatives. I might prefer the previous option of 
having a stand-alone function (possibly renamed if you think the old name isn't 
correct anymore). (Regardless, you can drop the unnecessary parentheses.)



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:176
+static auto makeFlagToValueNormalizer(T Value) {
+  return makeFlagToValueNormalizer(uint64_t(Value));
 }

(not a behaviour change but) I wonder if this correct for signed types, where 
the conversion back to `T` could change sign if it's a negative value. Should 
there be an assertion that the value is `>= 0`? (Probably to do separately / 
outside this patch)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93628

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added inline comments.



Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:881
+void DISubprogram::replaceRawLinkageName(MDString *LinkageName) {
+  replaceOperandWith(3, LinkageName);
+}

Nit, Move the body to DebugInfoMetadata.h itself? 



Comment at: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp:51
+  auto Name = MDB.createString(F.getName());
+  SP->replaceRawLinkageName(Name);
+}

hoy wrote:
> tmsriram wrote:
> > Do we need to check if it had a rawLinkageName before replacing it?  
> Good point. `rawLinkageName` is missing for C programs. I think it might 
> still be appropriate to assign a new linkage name in the case so as to differ 
> from the original source function name. What do you think?
@dblaikie Adding the expert here.  As far as I understand, the linkage name is 
the mangled name and this would capture this too correctly. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D93565: scan-view: Remove Reporter.py and associated AppleScript files

[Artem Dergachev via Phabricator via cfe-commits]

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

Uhm, yeah, indeed. Let's remove it, thanks!

I doubt people even use scan-view at this point (but i'd rather keep it) 
(scan-build is definitely used).




Comment at: clang/tools/scan-view/share/Reporter.py:154
-def fileReport(self, report, parameters):
-raise NotImplementedError
- 

Looks like there was an attempt to support llvm.org bugzilla as well, but it 
never got anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93565

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy marked 2 inline comments as done.
hoy added inline comments.



Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:881
+void DISubprogram::replaceRawLinkageName(MDString *LinkageName) {
+  replaceOperandWith(3, LinkageName);
+}

tmsriram wrote:
> Nit, Move the body to DebugInfoMetadata.h itself? 
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy updated this revision to Diff 313187.
hoy marked an inline comment as done.
hoy added a comment.

Addressing feedbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/unique-internal-linkage-names.cpp
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp

Index: llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp
===
--- llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp
+++ llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp
@@ -13,13 +13,20 @@
 
 #include "llvm/Transforms/Utils/UniqueInternalLinkageNames.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
 #include "llvm/InitializePasses.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
 using namespace llvm;
 
+static cl::opt UniqueDebugAndProfileNames(
+"unqiue-debug-profile-names", cl::init(true), cl::Hidden,
+cl::desc("Uniqueify debug and profile symbol Names"));
+
 static bool uniqueifyInternalLinkageNames(Module &M) {
   llvm::MD5 Md5;
   Md5.update(M.getSourceFileName());
@@ -31,11 +38,19 @@
   // this symbol is of internal linkage type.
   std::string ModuleNameHash = (Twine(".__uniq.") + Twine(Str)).str();
   bool Changed = false;
+  MDBuilder MDB(M.getContext());
 
   // Append the module hash to all internal linkage functions.
   for (auto &F : M) {
 if (F.hasInternalLinkage()) {
   F.setName(F.getName() + ModuleNameHash);
+  if (UniqueDebugAndProfileNames) {
+F.addFnAttr("sample-profile-suffix-elision-policy", "selected");
+if (DISubprogram *SP = F.getSubprogram()) {
+  auto *Name = MDB.createString(F.getName());
+  SP->replaceRawLinkageName(Name);
+}
+  }
   Changed = true;
 }
   }
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -434,8 +434,10 @@
 
 PassBuilder::PassBuilder(bool DebugLogging, TargetMachine *TM,
  PipelineTuningOptions PTO, Optional PGOOpt,
- PassInstrumentationCallbacks *PIC)
-: DebugLogging(DebugLogging), TM(TM), PTO(PTO), PGOOpt(PGOOpt), PIC(PIC) {
+ PassInstrumentationCallbacks *PIC,
+ bool UniqueLinkageNames)
+: DebugLogging(DebugLogging), UniqueLinkageNames(UniqueLinkageNames),
+  TM(TM), PTO(PTO), PGOOpt(PGOOpt), PIC(PIC) {
   if (TM)
 TM->registerPassBuilderCallbacks(*this, DebugLogging);
   if (PIC && shouldPopulateClassToPassNames()) {
@@ -1001,6 +1003,11 @@
ThinLTOPhase Phase) {
   ModulePassManager MPM(DebugLogging);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage
+  // symbols with unique names.
+  if (UniqueLinkageNames)
+MPM.addPass(UniqueInternalLinkageNamesPass());
+
   // Place pseudo probe instrumentation as the first pass of the pipeline to
   // minimize the impact of optimization changes.
   if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
@@ -1764,6 +1771,11 @@
 
   ModulePassManager MPM(DebugLogging);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage
+  // symbols with unique names.
+  if (UniqueLinkageNames)
+MPM.addPass(UniqueInternalLinkageNamesPass());
+
   if (PGOOpt && (PGOOpt->Action == PGOOptions::IRInstr ||
  PGOOpt->Action == PGOOptions::IRUse))
 addPGOInstrPassesForO0(
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -137,6 +137,7 @@
 /// construction.
 class PassBuilder {
   bool DebugLogging;
+  bool UniqueLinkageNames;
   TargetMachine *TM;
   PipelineTuningOptions PTO;
   Optional PGOOpt;
@@ -281,7 +282,8 @@
   explicit PassBuilder(bool DebugLogging = false, TargetMachine *TM = nullptr,
PipelineTuningOptions PTO = PipelineTuningOptions(),
Optional PGOOpt = None,
-   PassInstrumentationCallbacks *PIC = nullptr);
+   PassInstrumentationCallbacks *PIC = nullptr,
+   bool UniqueLinkageNames = false);
 
   /// Cross register the analysis managers through their proxies.
   ///
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -20

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

In D85788#2449299 , @aqjune wrote:

> In D85788#2444136 , @guiand wrote:
>
>> IMO it's better to just one-and-done programatically add `-Xclang 
>> -disable-noundef-analysis` to all the tests' RUN lines. That way there are 
>> no hidden flags.
>
> If a script for this can be written and people who are working for the 
> downstream project are happy with the script, this is the best approach IMO. 
> It is clear and explicit.

Yeah, I also think this would be the best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85788

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

[PATCH] D93648: Revert "[AMDGPU][HIP] Switch default DWARF version to 5"

[Tony Tye via Phabricator via cfe-commits]

t-tye accepted this revision.
t-tye added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93648

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

[clang] ffba47d - Revert "[AMDGPU][HIP] Switch default DWARF version to 5"

[Scott Linder via cfe-commits]

Author: Scott Linder
Date: 2020-12-21T21:43:51Z
New Revision: ffba47df76460905965df4b54cf6ba945d2eb1ce

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

LOG: Revert "[AMDGPU][HIP] Switch default DWARF version to 5"

This reverts commit c4d10e7e9bb47b77fad43d8ddcfa328298f36c88.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.h
clang/lib/Driver/ToolChains/HIP.h
clang/test/Driver/amdgpu-toolchain.c
clang/test/Driver/hip-toolchain-dwarf.hip

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.h 
b/clang/lib/Driver/ToolChains/AMDGPU.h
index f5448b76aee5..55ef6e01967e 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public 
Generic_ELF {
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 

diff  --git a/clang/lib/Driver/ToolChains/HIP.h 
b/clang/lib/Driver/ToolChains/HIP.h
index ff58c5451b0b..5e2be7138579 100644
--- a/clang/lib/Driver/ToolChains/HIP.h
+++ b/clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@ class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public 
ROCMToolChain {
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
 
   const ToolChain &HostTC;
 

diff  --git a/clang/test/Driver/amdgpu-toolchain.c 
b/clang/test/Driver/amdgpu-toolchain.c
index cb92744eee6a..ac558e0e26eb 100644
--- a/clang/test/Driver/amdgpu-toolchain.c
+++ b/clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s

diff  --git a/clang/test/Driver/hip-toolchain-dwarf.hip 
b/clang/test/Driver/hip-toolchain-dwarf.hip
index c853d5cf07cf..44d66fe52e04 100644
--- a/clang/test/Driver/hip-toolchain-dwarf.hip
+++ b/clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"



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

[PATCH] D93648: Revert "[AMDGPU][HIP] Switch default DWARF version to 5"

[Scott Linder via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGffba47df7646: Revert "[AMDGPU][HIP] Switch default 
DWARF version to 5" (authored by scott.linder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93648

Files:
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.h
  clang/test/Driver/amdgpu-toolchain.c
  clang/test/Driver/hip-toolchain-dwarf.hip


Index: clang/test/Driver/hip-toolchain-dwarf.hip
===
--- clang/test/Driver/hip-toolchain-dwarf.hip
+++ clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
 
   const ToolChain &HostTC;
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 


Index: clang/test/Driver/hip-toolchain-dwarf.hip
===
--- clang/test/Driver/hip-toolchain-dwarf.hip
+++ clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=5"
+// DWARF_VER: "-dwarf-version=4"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
 
   const ToolChain &HostTC;
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 5; }
+  unsigned GetDefaultDwarfVersion() const override { return 4; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D52050#2441164 , @glaubitz wrote:

> In D52050#2441141 , @jrtc27 wrote:
>
>> What gets done currently for i386? That suffers potentially the same problem 
>> given both /usr/lib/gcc/x86_64-linux-gnu/$V/32 and 
>> /usr/lib/gcc/i386-linux-gnu/$V are possible locations for an i386 GCC 
>> toolchain.
>
> Very good suggestion. I will look into that tomorrow. Thanks for the pointer!

I have been wrapping my head around this for some time and I have run into one 
problem trying to apply the suggested approach.

The problem is that I don't know how to tell whether I'm on an x86_64 system or 
an x32 system there is no ```case llvm::Triple::x32:``` which would be needed 
here (we have it for x86).

x32 takes the switch case for x86_64, so x32 and x86_64 targeting x32 are 
identical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D52050

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

[PATCH] D87587: [clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2463
 
+**MaxUnwrappedLinesForShortNamespace** (``unsigned``)
+  The maximal number of unwrapped lines that a short namespace spans.

HazardyKnusperkeks wrote:
> curdeius wrote:
> > MyDeveloperDay wrote:
> > > This is quite a mouthful before this lands do you want to consider 
> > > shortening it?
> > > 
> > > `ShortNamespaceLength` ?
> > > 
> > > I'm not sure non clang-formast developers will know what an UnwrappedLine 
> > > is?
> > I agree that one needs to wrap its head around to digest this name. I'd 
> > vote for what @mydeveloperday proposed.
> Although I already made some changes inside clang-format and single stepped 
> through some formations, I still can not explain what those UnwrappedLines 
> are.
Just found a precedence for this: 
https://clang.llvm.org/extra/clang-tidy/checks/llvm-namespace-comment.html. 
This clang-tidy check uses `ShortNamespaceLines` for the very same thing so 
maybe we should match?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87587

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 313199.
hliao added a comment.

These functions are pure C functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

Files:
  clang/lib/Headers/__clang_hip_runtime_wrapper.h
  clang/lib/Headers/cuda_wrappers/ymath.h


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,22 @@
+/*=== ymath.h - HIP wrapper for  --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,24 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+#if defined(__cplusplus)
+extern "C" {
+#endif // defined(__cplusplus)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#if defined(__cplusplus)
+}
+#endif // defined(__cplusplus)
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1


Index: clang/lib/Headers/cuda_wrappers/ymath.h
===
--- /dev/null
+++ clang/lib/Headers/cuda_wrappers/ymath.h
@@ -0,0 +1,22 @@
+/*=== ymath.h - HIP wrapper for  --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_CUDA_WRAPPERS_YMATH_H
+#define __CLANG_CUDA_WRAPPERS_YMATH_H
+
+// Wrapper around  that forces its functions to be __host__
+// __device__.
+
+#pragma clang force_cuda_host_device begin
+
+#include_next 
+
+#pragma clang force_cuda_host_device end
+
+#endif // include guard
Index: clang/lib/Headers/__clang_hip_runtime_wrapper.h
===
--- clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -61,6 +61,24 @@
 #include 
 #include 
 #include 
+
+// Define math functions from  on MSVC for the device compilation
+// only to avoid conflicts with MSVC runtime in the host compilation.
+#if defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+#if defined(__cplusplus)
+extern "C" {
+#endif // defined(__cplusplus)
+__host__ __device__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__host__ __device__ float _FCosh(float x, float y) { return coshf(x) * y; }
+__host__ __device__ short _Dtest(double *p) { return fpclassify(*p); }
+__host__ __device__ short _FDtest(float *p) { return fpclassify(*p); }
+__host__ __device__ double _Sinh(double x, double y) { return sinh(x) * y; }
+__host__ __device__ float _FSinh(float x, float y) { return sinhf(x) * y; }
+#if defined(__cplusplus)
+}
+#endif // defined(__cplusplus)
+#endif // defined(_MSC_VER) && defined(__HIP_DEVICE_COMPILE__)
+
 #endif // !_OPENMP || __HIP_ENABLE_CUDA_WRAPPER_FOR_OPENMP__
 
 #define __CLANG_HIP_RUNTIME_WRAPPER_INCLUDED__ 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92054: [Driver] Default Generic_GCC ppc/ppc64/ppc64le to -fasynchronous-unwind-tables

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/test/Driver/ppc-features.cpp:45
+// PPC64:  "-munwind-tables"
+// PPC64-SAME: "-mfloat-abi" "hard"
 

Curious - how come no check for `PPC64BE`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92054

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