[PATCH] D69697: [WebAssembly] Add experimental SIMD dot product instruction

[Thomas Lively via Phabricator via cfe-commits]

tlively updated this revision to Diff 227393.
tlively added a comment.

- Add second argument


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69697

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -571,4 +571,7 @@
 # CHECK: v128.andnot # encoding: [0xfd,0xd8,0x01]
 v128.andnot
 
+# CHECK: i32x4.dot_i16x8_s # encoding: [0xfd,0xd9,0x01]
+i32x4.dot_i16x8_s
+
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -387,6 +387,16 @@
   ret <4 x i32> %a
 }
 
+; CHECK-LABEL: dot:
+; SIMD128-NEXT: .functype dot (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i32x4.dot_i16x8_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <4 x i32> @llvm.wasm.dot(<8 x i16>, <8 x i16>)
+define <4 x i32> @dot(<8 x i16> %x, <8 x i16> %y) {
+  %a = call <4 x i32> @llvm.wasm.dot(<8 x i16> %x, <8 x i16> %y)
+  ret <4 x i32> %a
+}
+
 ; CHECK-LABEL: any_v4i32:
 ; SIMD128-NEXT: .functype any_v4i32 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i32x4.any_true $push[[R:[0-9]+]]=, $0{{$}}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -738,6 +738,13 @@
 defm MAX_U : SIMDBinaryIntNoI64x2;
 } // isCommutable = 1
 
+// Widening dot product: i32x4.dot_i16x8_s
+let isCommutable = 1 in
+defm DOT : SIMD_I<(outs V128:$dst), (ins V128:$lhs, V128:$rhs), (outs), (ins),
+  [(set V128:$dst, (int_wasm_dot V128:$lhs, V128:$rhs))],
+  "i32x4.dot_i16x8_s\t$dst, $lhs, $rhs", "i32x4.dot_i16x8_s",
+  217>;
+
 //===--===//
 // Floating-point unary arithmetic
 //===--===//
Index: llvm/include/llvm/IR/IntrinsicsWebAssembly.td
===
--- llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ llvm/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -152,6 +152,10 @@
   Intrinsic<[llvm_anyvector_ty],
 [LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>],
 [IntrNoMem, IntrSpeculatable]>;
+def int_wasm_dot :
+  Intrinsic<[llvm_v4i32_ty],
+[llvm_v8i16_ty, llvm_v8i16_ty],
+[IntrNoMem, IntrSpeculatable]>;
 def int_wasm_narrow_signed :
   Intrinsic<[llvm_anyvector_ty],
 [llvm_anyvector_ty, LLVMMatchType<1>],
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -436,6 +436,12 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
+i32x4 dot_i16x8_s(i16x8 x, i16x8 y) {
+  return __builtin_wasm_dot_s_i32x4_i16x8(x, y);
+  // WEBASSEMBLY: call <4 x i32> @llvm.wasm.dot(<8 x i16> %x, <8 x i16> %y)
+  // WEBASSEMBLY-NEXT: ret
+}
+
 i32x4 bitselect(i32x4 x, i32x4 y, i32x4 c) {
   return __builtin_wasm_bitselect(x, y, c);
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.bitselect.v4i32(
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -14360,6 +14360,12 @@
 Function *Callee = CGM.getIntrinsic(IntNo, ConvertType(E->getType()));
 return Builder.CreateCall(Callee, {LHS, RHS});
   }
+  case WebAssembly::BI__builtin_wasm_dot_s_i32x4_i16x8: {
+Value *LHS = EmitScalarExpr(E->getArg(0));
+Value *RHS = EmitScalarExpr(E->getArg(1));
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_dot);
+return Builder.CreateCall(Callee, {LHS, RHS});
+  }
   case WebAssembly::BI__builtin_wasm_any_true_i8x16:
   case WebAssembly::BI__builtin_wasm_any_true_i16x8:
   case WebAssembly::BI__builtin_wasm_any_true_i32x4:
Index: clang/include/clang/Basic/BuiltinsWebAssembly.def
===
--- clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -132,6 +132,8 @@
 TARGET_BUILTIN(__builtin_wasm_min_f64x2, "V2dV2dV2d", "nc", "unimplemented-simd128")
 

LLVM buildmaster will be updated and restarted soon

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted in the nearest hour.

Thanks

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

[PATCH] D69696: [WebAssembly] SIMD integer min and max instructions

[Thomas Lively via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa07019a275cd: [WebAssembly] SIMD integer min and max 
instructions (authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69696

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -283,6 +283,18 @@
 # CHECK: i8x16.mul # encoding: [0xfd,0x5d]
 i8x16.mul
 
+# CHECK: i8x16.min_s # encoding: [0xfd,0x5e]
+i8x16.min_s
+
+# CHECK: i8x16.min_u # encoding: [0xfd,0x5f]
+i8x16.min_u
+
+# CHECK: i8x16.max_s # encoding: [0xfd,0x60]
+i8x16.max_s
+
+# CHECK: i8x16.max_u # encoding: [0xfd,0x61]
+i8x16.max_u
+
 # CHECK: i16x8.neg # encoding: [0xfd,0x62]
 i16x8.neg
 
@@ -322,6 +334,18 @@
 # CHECK: i16x8.mul # encoding: [0xfd,0x6e]
 i16x8.mul
 
+# CHECK: i16x8.min_s # encoding: [0xfd,0x6f]
+i16x8.min_s
+
+# CHECK: i16x8.min_u # encoding: [0xfd,0x70]
+i16x8.min_u
+
+# CHECK: i16x8.max_s # encoding: [0xfd,0x71]
+i16x8.max_s
+
+# CHECK: i16x8.max_u # encoding: [0xfd,0x72]
+i16x8.max_u
+
 # CHECK: i32x4.neg # encoding: [0xfd,0x73]
 i32x4.neg
 
@@ -349,6 +373,18 @@
 # CHECK: i32x4.mul # encoding: [0xfd,0x7f]
 i32x4.mul
 
+# CHECK: i32x4.min_s # encoding: [0xfd,0x80,0x01]
+i32x4.min_s
+
+# CHECK: i32x4.min_u # encoding: [0xfd,0x81,0x01]
+i32x4.min_u
+
+# CHECK: i32x4.max_s # encoding: [0xfd,0x82,0x01]
+i32x4.max_s
+
+# CHECK: i32x4.max_u # encoding: [0xfd,0x83,0x01]
+i32x4.max_u
+
 # CHECK: i64x2.neg # encoding: [0xfd,0x84,0x01]
 i64x2.neg
 
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -65,6 +65,46 @@
   ret <16 x i8> %a
 }
 
+; CHECK-LABEL: min_s_v16i8:
+; SIMD128-NEXT: .functype min_s_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.min_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.min.signed.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @min_s_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.min.signed.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: min_u_v16i8:
+; SIMD128-NEXT: .functype min_u_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.min_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.min.unsigned.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @min_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.min.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: max_s_v16i8:
+; SIMD128-NEXT: .functype max_s_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.max_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.max.signed.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @max_s_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.max.signed.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: max_u_v16i8:
+; SIMD128-NEXT: .functype max_u_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.max_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.max.unsigned.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @max_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.max.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
 ; CHECK-LABEL: any_v16i8:
 ; SIMD128-NEXT: .functype any_v16i8 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i8x16.any_true $push[[R:[0-9]+]]=, $0{{$}}
@@ -168,6 +208,46 @@
   ret <8 x i16> %a
 }
 
+; CHECK-LABEL: min_s_v8i16:
+; SIMD128-NEXT: .functype min_s_v8i16 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.min_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.min.signed.v8i16(<8 x i16>, <8 x i16>)
+define <8 x i16> @min_s_v8i16(<8 x i16> %x, <8 x i16> %y) {
+  %a = call <8 x i16> @llvm.wasm.min.signed.v8i16(<8 x i16> %x, <8 x i16> %y)
+  ret <8 x i16> %a
+}
+
+; CHECK-LABEL: min_u_v8i16:
+; SIMD128-NEXT: .functype min_u_v8i16 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.min_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x 

[PATCH] D69697: [WebAssembly] Add experimental SIMD dot product instruction

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

https://github.com/WebAssembly/simd/pull/127


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69697



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

[clang] c6da9ec - clang: Fix assert on void pointer arithmetic with address_space

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2019-10-31T20:07:23-07:00
New Revision: c6da9ec0e90ea8798ecae583bb8d26bdf6b9b79f

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

LOG: clang: Fix assert on void pointer arithmetic with address_space

This attempted to always use the default address space void pointer
type instead of preserving the source address space.

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/address-space.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 55a413a2a717..c1391d46f60c 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3258,7 +3258,7 @@ static Value *emitPointerArithmetic(CodeGenFunction ,
   // GNU void* casts amount to no-ops since our void* type is i8*, but this is
   // future proof.
   if (elementType->isVoidType() || elementType->isFunctionType()) {
-Value *result = CGF.Builder.CreateBitCast(pointer, CGF.VoidPtrTy);
+Value *result = CGF.EmitCastToVoidPtr(pointer);
 result = CGF.Builder.CreateGEP(result, index, "add.ptr");
 return CGF.Builder.CreateBitCast(result, pointer->getType());
   }

diff  --git a/clang/test/CodeGen/address-space.c 
b/clang/test/CodeGen/address-space.c
index d6a40f33029f..a76d2e743e6e 100644
--- a/clang/test/CodeGen/address-space.c
+++ b/clang/test/CodeGen/address-space.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck 
-check-prefixes=CHECK,AMDGCN %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-enable-var-scope -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck 
-enable-var-scope -check-prefixes=CHECK,AMDGCN %s
 
 // CHECK: @foo = common addrspace(1) global
 int foo __attribute__((address_space(1)));
@@ -10,11 +10,11 @@ int ban[10] __attribute__((address_space(1)));
 // CHECK: @a = common global
 int a __attribute__((address_space(0)));
 
-// CHECK-LABEL: define i32 @test1() 
+// CHECK-LABEL: define i32 @test1()
 // CHECK: load i32, i32 addrspace(1)* @foo
 int test1() { return foo; }
 
-// CHECK-LABEL: define i32 @test2(i32 %i) 
+// CHECK-LABEL: define i32 @test2(i32 %i)
 // CHECK: load i32, i32 addrspace(1)*
 // CHECK-NEXT: ret i32
 int test2(int i) { return ban[i]; }
@@ -45,3 +45,17 @@ void test4(MyStruct __attribute__((address_space(2))) *pPtr) 
{
   MyStruct s = pPtr[0];
   pPtr[0] = s;
 }
+
+// Make sure the right address space is used when doing arithmetic on a void
+// pointer. Make sure no invalid bitcast is introduced.
+
+// CHECK-LABEL: @void_ptr_arithmetic_test(
+// X86: [[ALLOCA:%.*]] = alloca i8 addrspace(1)*
+// X86-NEXT: store i8 addrspace(1)* %arg, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: load i8 addrspace(1)*, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: getelementptr i8, i8 addrspace(1)*
+// X86-NEXT: ret i8 addrspace(1)*
+void __attribute__((address_space(1)))*
+void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
+return arg + 4;
+}



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

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

c6da9ec0e90ea8798ecae583bb8d26bdf6b9b79f


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

https://reviews.llvm.org/D69666



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I see. Some decoupling is desirable, I agree. Maybe move `StencilInterface` and 
`Stencil` into a separate header that `RewriteRule` can depend on, and then 
define the library of stencils in another header?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D69625: [libTooling] Support implicit coercions in Stencil's `access` combinator.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

> Which idiom do you think we should encourage, then, for text and 
> range-selectors -- the named combinator or the single-argument cat? That is

You are making a very interesting point!

It sounds very appealing to me to remove the special `text` and `selection` 
functions in favor of `cat`. Users of the library (both readers and writers) 
will have to know about `cat`. So the advantage of having special functions 
`text` and `selection` is really limited I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69625



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

[PATCH] D69697: [WebAssembly] Add experimental SIMD dot product instruction

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:135
 
+TARGET_BUILTIN(__builtin_wasm_dot_s_i32x4_i16x8, "V4iV8s", "nc", "simd128")
+

We talked offline. There should be two arguments :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69697



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

[PATCH] D69699: [clang][driver] Add ProfileData to LLVM_LINK_COMPONENTS

[Heejin Ahn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9903ec8979f: [clang][driver] Add ProfileData to 
LLVM_LINK_COMPONENTS (authored by aheejin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69699

Files:
  clang/lib/Driver/CMakeLists.txt


Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Option
+  ProfileData
   Support
   )
 


Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Option
+  ProfileData
   Support
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69699: [clang][driver] Add ProfileData to LLVM_LINK_COMPONENTS

[Thomas Lively via Phabricator via cfe-commits]

tlively accepted this revision.
tlively 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/D69699/new/

https://reviews.llvm.org/D69699



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

[PATCH] D69699: [clang][driver] Add ProfileData to LLVM_LINK_COMPONENTS

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added reviewers: vsk, tlively.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.

LGTM!


After D68351  we need this to make builds with 
`-DBUILD_SHARED_LIB=ON`
work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69699

Files:
  clang/lib/Driver/CMakeLists.txt


Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Option
+  ProfileData
   Support
   )
 


Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Option
+  ProfileData
   Support
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b9903ec - [clang][driver] Add ProfileData to LLVM_LINK_COMPONENTS

[Heejin Ahn via cfe-commits]

Author: Heejin Ahn
Date: 2019-10-31T19:52:41-07:00
New Revision: b9903ec8979fc43f1484e1ee8749c7d18ce90bf0

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

LOG: [clang][driver] Add ProfileData to LLVM_LINK_COMPONENTS

Summary:
After D68351 we need this to make builds with `-DBUILD_SHARED_LIB=ON`
work.

Reviewers: tlively

Subscribers: mgorny, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Driver/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index e175e88c4ebb..eb76cd3dcade 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Option
+  ProfileData
   Support
   )
 



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

[PATCH] D69697: [WebAssembly] Add experimental SIMD dot product instruction

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

This is https://github.com/WebAssembly/simd/pull/127


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69697



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

[PATCH] D69696: [WebAssembly] SIMD integer min and max instructions

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D69696#1729676 , @aheejin wrote:

> Is this https://github.com/WebAssembly/simd/pull/27 ? Can you please include 
> the spec (even if it's still an unmerged PR) in the CL description next time? 
> LGTM.


Yes, that's right. Will do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69696



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

[PATCH] D69696: [WebAssembly] SIMD integer min and max instructions

[Heejin Ahn via Phabricator via cfe-commits]

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

Is this https://github.com/WebAssembly/simd/pull/27 ? Can you please include 
the spec (even if it's still an unmerged PR) in the CL description next time? 
LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69696



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

[PATCH] D69678: [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb1616ba4726: [CodeGen] Fix invalid llvm.linker.options 
about pragma detect_mismatch (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69678

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/pragma-detect_mismatch.c


Index: clang/test/CodeGen/pragma-detect_mismatch.c
===
--- clang/test/CodeGen/pragma-detect_mismatch.c
+++ clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1933,6 +1933,8 @@
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }


Index: clang/test/CodeGen/pragma-detect_mismatch.c
===
--- clang/test/CodeGen/pragma-detect_mismatch.c
+++ clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1933,6 +1933,8 @@
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bb1616b - [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2019-10-31T22:27:35-04:00
New Revision: bb1616ba47261a0767063e85718d546165972120

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

LOG: [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

When a target does not support pragma detect_mismatch, an llvm.linker.options
metadata with an empty entry is created, which causes diagnostic in backend
since backend expects name/value pair in llvm.linker.options entries.

This patch fixes that.

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/pragma-detect_mismatch.c

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 480a33f27285..49df82cea42b 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1933,6 +1933,8 @@ void CodeGenModule::AppendLinkerOptions(StringRef Opts) {
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }

diff  --git a/clang/test/CodeGen/pragma-detect_mismatch.c 
b/clang/test/CodeGen/pragma-detect_mismatch.c
index 066183d31264..145b28572cc7 100644
--- a/clang/test/CodeGen/pragma-detect_mismatch.c
+++ b/clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options



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

[PATCH] D69625: [libTooling] Support implicit coercions in Stencil's `access` combinator.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D69625#1729634 , @gribozavr2 wrote:

> I fully agree about passing a Stencil to `access`. However whether to call 
> `makeStencil` inside is an interesting question. On one hand, such implicit 
> conversions increase the convenience. On the other, they increase the API 
> surface (more possible argument types), and makes the API harder to read. 
> What does `access` take? "I don't know, some T" vs. "A Stencil".
>
> I think that implicit conversions for `cat` arguments can be justified 
> because it seems like `cat` will be used frequently; however, `access` won't 
> be as frequently called.
>
> What do you think?


This seems reasonable, particularly the problem caused by using a template. I'd 
consider having three explicit overloads, but this doesn't scale in general 
(especially once you have a combinator with 2+ Stencil args).

Which idiom do you think we should encourage, then, for text and 
range-selectors -- the named combinator or the single-argument `cat`? That is

  access("object", text("field"))
  access("object", selection(member("e")))

versus

  access("object", cat("field"))
  access("object", cat(member("e")))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69625



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 8 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:154
+/// for more details.
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);

gribozavr2 wrote:
> ymandel wrote:
> > gribozavr2 wrote:
> > > I'm a bit confused by the name. The function returns a MatchConsumer, but 
> > > it is called "stencil". What is the intended usage?
> > Intended use is something like `makeRule(matcher, change(stencil(...)))`.
> > 
> > The naming is the same as we previously had for `text()`. Since 
> > `MatchConsumer` is so general, the function name describes its argument, 
> > not its particular type.  But, I'm open to other suggestions.
> I see -- thanks. `change()` takes a `TextGenerator`. Aside from stencils, 
> what are the other text generators? If those are not important, could we 
> change `change()` to take a stencil directly? Or if those other text 
> generators are important, we could add overloads to `change()` that accept a 
> stencil.
> 
> My thinking here is that the user is not really concerned with the fact that 
> Stencil-to-TextGenerator conversion is happening here. I think the more 
> complicated operation is the combining of stencils -- and we already have 
> `cat` for that; so adding another way to spell that is better avoided. So, 
> not emphasizing the conversion by making it a part of the `change()` seems 
> fine, and spelling the operation as `cat` seems to be an improvement to me.
> I see -- thanks. change() takes a TextGenerator. Aside from stencils, what 
> are the other text generators? If those are not important, could we change 
> change() to take a stencil directly?

Actually, that was the original design of the library.  We changed it in the 
first submitted version on Ilya's suggestion to avoid the dependency between 
Transformer and Stencil.  In fact, the reason TextGenerator exists is to 
abstract away that dependency.

Looking back, I think it was a reasonable choice at the time, keeping the 
commits, etc. independent. 
But, based on my experience with the library over the last six months, I'd say 
these are clearly a single package and we'd be better off re-introducing the 
dependency in exactly the way you describe.

I think we can start (here) by taking the Stencil as an argument and wrapping 
it as a TextGenerator. But, I should follow up with a cleanup to actually store 
the value as a Stencil rather than wrapped as a TextGenerator.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D69697: [WebAssembly] Add experimental SIMD dot product instruction

[Thomas Lively via Phabricator via cfe-commits]

tlively created this revision.
tlively added a reviewer: aheejin.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
jgravelle-google, sbc100, dschuff.
Herald added projects: clang, LLVM.

This instruction is not merged to the spec proposal, but we need it to
be implemented in the toolchain to experiment with it. It is available
only on an opt-in basis through a clang builtin.

Depends on D69696 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69697

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -571,4 +571,7 @@
 # CHECK: v128.andnot # encoding: [0xfd,0xd8,0x01]
 v128.andnot
 
+# CHECK: i32x4.dot_i16x8_s # encoding: [0xfd,0xd9,0x01]
+i32x4.dot_i16x8_s
+
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -407,6 +407,16 @@
   ret i32 %a
 }
 
+; CHECK-LABEL: dot:
+; SIMD128-NEXT: .functype dot (v128) -> (v128){{$}}
+; SIMD128-NEXT: i32x4.dot_i16x8_s $push[[R:[0-9]+]]=, $0{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <4 x i32> @llvm.wasm.dot(<8 x i16>)
+define <4 x i32> @dot(<8 x i16> %x) {
+  %a = call <4 x i32> @llvm.wasm.dot(<8 x i16> %x)
+  ret <4 x i32> %a
+}
+
 ; CHECK-LABEL: bitselect_v4i32:
 ; SIMD128-NEXT: .functype bitselect_v4i32 (v128, v128, v128) -> (v128){{$}}
 ; SIMD128-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -639,6 +639,11 @@
   (i32 (!cast(reduction[1]#"_"#ty) (ty V128:$x)))>;
 }
 
+// Widening dot product: i32x4.dot_i16x8_s
+defm DOT : SIMD_I<(outs V128:$dst), (ins V128:$vec), (outs), (ins),
+  [(set V128:$dst, (v4i32 (int_wasm_dot V128:$vec)))],
+  "i32x4.dot_i16x8_s\t$dst, $vec", "i32x4.dot_i16x8_s", 217>;
+
 //===--===//
 // Bit shifts
 //===--===//
Index: llvm/include/llvm/IR/IntrinsicsWebAssembly.td
===
--- llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ llvm/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -152,6 +152,10 @@
   Intrinsic<[llvm_anyvector_ty],
 [LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>],
 [IntrNoMem, IntrSpeculatable]>;
+def int_wasm_dot :
+  Intrinsic<[llvm_v4i32_ty],
+[llvm_v8i16_ty],
+[IntrNoMem, IntrSpeculatable]>;
 def int_wasm_narrow_signed :
   Intrinsic<[llvm_anyvector_ty],
 [llvm_anyvector_ty, LLVMMatchType<1>],
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -436,6 +436,12 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
+i32x4 dot_i16x8_s(i16x8 x) {
+  return __builtin_wasm_dot_s_i32x4_i16x8(x);
+  // WEBASSEMBLY: call <4 x i32> @llvm.wasm.dot(<8 x i16> %x)
+  // WEBASSEMBLY-NEXT: ret
+}
+
 i32x4 bitselect(i32x4 x, i32x4 y, i32x4 c) {
   return __builtin_wasm_bitselect(x, y, c);
   // WEBASSEMBLY: call <4 x i32> @llvm.wasm.bitselect.v4i32(
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -14360,6 +14360,11 @@
 Function *Callee = CGM.getIntrinsic(IntNo, ConvertType(E->getType()));
 return Builder.CreateCall(Callee, {LHS, RHS});
   }
+  case WebAssembly::BI__builtin_wasm_dot_s_i32x4_i16x8: {
+Value *Vec = EmitScalarExpr(E->getArg(0));
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_dot);
+return Builder.CreateCall(Callee, {Vec});
+  }
   case WebAssembly::BI__builtin_wasm_any_true_i8x16:
   case WebAssembly::BI__builtin_wasm_any_true_i16x8:
   case WebAssembly::BI__builtin_wasm_any_true_i32x4:
Index: clang/include/clang/Basic/BuiltinsWebAssembly.def
===
--- clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -132,6 +132,8 @@
 

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D69666



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:389-392
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+  JA.isDeviceOffloading(Action::OFK_HIP))
+Default = false;

tra wrote:
> Shouldn't it be `true`, considering that we do want to **disable** 
> autolinking by default for device-side CUDA/HIP?
> 
> If we don't support autolinking at all for CUDA/HIP, perhaps we should just 
> return `true` here.
This variable Default is to be used as the default value of OPT_fautolink. For 
device compilation we want the default value to be false. However if users want 
to force it to be true, we may still want to respect it.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Communication problem:

> This attempted to always use the default address space void pointer type 
> instead of preserving the source address space.

Where "This" is the old code. Replace "This" in the commit message with some 
more descriptive wording and the confusion will go away :)


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

https://reviews.llvm.org/D69666



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

[PATCH] D69625: [libTooling] Support implicit coercions in Stencil's `access` combinator.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I fully agree about passing a Stencil to `access`. However whether to call 
`makeStencil` inside is an interesting question. On one hand, such implicit 
conversions increase the convenience. On the other, they increase the API 
surface (more possible argument types), and makes the API harder to read. What 
does `access` take? "I don't know, some T" vs. "A Stencil".

I think that implicit conversions for `cat` arguments can be justified because 
it seems like `cat` will be used frequently; however, `access` won't be as 
frequently called.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69625



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

[PATCH] D69696: [WebAssembly] SIMD integer min and max instructions

[Thomas Lively via Phabricator via cfe-commits]

tlively created this revision.
tlively added a reviewer: aheejin.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
jgravelle-google, sbc100, dschuff.
Herald added projects: clang, LLVM.

Introduces a clang builtins and LLVM intrinsics representing integer
min/max instructions. These instructions have not been merged to the
SIMD spec proposal yet, so they are currently opt-in only via builtins
and not produced by general pattern matching. If these instructions
are accepted into the spec proposal the builtins and intrinsics will
be replaced with normal pattern matching.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69696

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -283,6 +283,18 @@
 # CHECK: i8x16.mul # encoding: [0xfd,0x5d]
 i8x16.mul
 
+# CHECK: i8x16.min_s # encoding: [0xfd,0x5e]
+i8x16.min_s
+
+# CHECK: i8x16.min_u # encoding: [0xfd,0x5f]
+i8x16.min_u
+
+# CHECK: i8x16.max_s # encoding: [0xfd,0x60]
+i8x16.max_s
+
+# CHECK: i8x16.max_u # encoding: [0xfd,0x61]
+i8x16.max_u
+
 # CHECK: i16x8.neg # encoding: [0xfd,0x62]
 i16x8.neg
 
@@ -322,6 +334,18 @@
 # CHECK: i16x8.mul # encoding: [0xfd,0x6e]
 i16x8.mul
 
+# CHECK: i16x8.min_s # encoding: [0xfd,0x6f]
+i16x8.min_s
+
+# CHECK: i16x8.min_u # encoding: [0xfd,0x70]
+i16x8.min_u
+
+# CHECK: i16x8.max_s # encoding: [0xfd,0x71]
+i16x8.max_s
+
+# CHECK: i16x8.max_u # encoding: [0xfd,0x72]
+i16x8.max_u
+
 # CHECK: i32x4.neg # encoding: [0xfd,0x73]
 i32x4.neg
 
@@ -349,6 +373,18 @@
 # CHECK: i32x4.mul # encoding: [0xfd,0x7f]
 i32x4.mul
 
+# CHECK: i32x4.min_s # encoding: [0xfd,0x80,0x01]
+i32x4.min_s
+
+# CHECK: i32x4.min_u # encoding: [0xfd,0x81,0x01]
+i32x4.min_u
+
+# CHECK: i32x4.max_s # encoding: [0xfd,0x82,0x01]
+i32x4.max_s
+
+# CHECK: i32x4.max_u # encoding: [0xfd,0x83,0x01]
+i32x4.max_u
+
 # CHECK: i64x2.neg # encoding: [0xfd,0x84,0x01]
 i64x2.neg
 
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -65,6 +65,46 @@
   ret <16 x i8> %a
 }
 
+; CHECK-LABEL: min_s_v16i8:
+; SIMD128-NEXT: .functype min_s_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.min_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.min.signed.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @min_s_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.min.signed.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: min_u_v16i8:
+; SIMD128-NEXT: .functype min_u_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.min_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.min.unsigned.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @min_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.min.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: max_s_v16i8:
+; SIMD128-NEXT: .functype max_s_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.max_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.max.signed.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @max_s_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.max.signed.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: max_u_v16i8:
+; SIMD128-NEXT: .functype max_u_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.max_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.max.unsigned.v16i8(<16 x i8>, <16 x i8>)
+define <16 x i8> @max_u_v16i8(<16 x i8> %x, <16 x i8> %y) {
+  %a = call <16 x i8> @llvm.wasm.max.unsigned.v16i8(<16 x i8> %x, <16 x i8> %y)
+  ret <16 x i8> %a
+}
+
 ; CHECK-LABEL: any_v16i8:
 ; SIMD128-NEXT: .functype any_v16i8 (v128) -> (i32){{$}}
 ; SIMD128-NEXT: i8x16.any_true $push[[R:[0-9]+]]=, $0{{$}}
@@ -168,6 +208,46 @@
   ret <8 x i16> %a
 }
 
+; CHECK-LABEL: min_s_v8i16:
+; SIMD128-NEXT: .functype min_s_v8i16 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.min_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.min.signed.v8i16(<8 x i16>, <8 x i16>)
+define <8 x i16> 

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:82
+Stencil makeStencil(RangeSelector Selector);
+inline Stencil makeStencil(Stencil S) { return S; }
 

ymandel wrote:
> gribozavr2 wrote:
> > I feel like this should be an implementation detail of `cat` (or other 
> > stencil combinators who want to support such implicit conversions). Users 
> > writing stencils should use concrete factory functions below.
> > 
> > 
> Agreed, but I wasn't sure if it was worth introducing a "detail" namespace 
> for just one (overloaded) function. I liked when these were private methods 
> of Stencil, but we don't have that option now. Should I use a `namespace 
> detail`? something else? 
I don't have a strong opinion, but `namespace detail` should be fine.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:137
+/// number.
+Stencil sequence(std::vector Parts);
 

ymandel wrote:
> gribozavr2 wrote:
> > It is the same operation as `cat`, right? So WDYT about `cat_vector`?
> not quite. `cat` is "smart" and optimizes case of one argument to just return 
> that argument. in the future, it might also handle the case of 0 arguments 
> specially. `sequence` just blindly constructs a sequence stencil.
> 
> that said, i can see the argument that the user doesn't care about such 
> details and instead of the three functions:
> ```
> cat(...)
> cat(vector)
> sequence(vector)
> ```
> 
> we should just have the two
> ```
> cat(...)
> catVector(vector)
> ```
> with no "plain" sequence constructor. WDYT?
> i can see the argument that the user doesn't care about such details

I agree, and I would also say that we probably don't even want the users of the 
library to depend on such details either. So I agree, `cat(...)` and 
`catVector(vector)` look like the best option to me.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:154
+/// for more details.
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);

ymandel wrote:
> gribozavr2 wrote:
> > I'm a bit confused by the name. The function returns a MatchConsumer, but 
> > it is called "stencil". What is the intended usage?
> Intended use is something like `makeRule(matcher, change(stencil(...)))`.
> 
> The naming is the same as we previously had for `text()`. Since 
> `MatchConsumer` is so general, the function name describes its argument, not 
> its particular type.  But, I'm open to other suggestions.
I see -- thanks. `change()` takes a `TextGenerator`. Aside from stencils, what 
are the other text generators? If those are not important, could we change 
`change()` to take a stencil directly? Or if those other text generators are 
important, we could add overloads to `change()` that accept a stencil.

My thinking here is that the user is not really concerned with the fact that 
Stencil-to-TextGenerator conversion is happening here. I think the more 
complicated operation is the combining of stencils -- and we already have `cat` 
for that; so adding another way to spell that is better avoided. So, not 
emphasizing the conversion by making it a part of the `change()` seems fine, 
and spelling the operation as `cat` seems to be an improvement to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Alright then. (Although I don't know whether the redeclaration chain always 
models that wording in the standard, and what that wording means when we don't 
have a linear ordering of text -- that is, in modules.)

Do you have commit access or do you need me to commit for you?


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

https://reviews.llvm.org/D69435



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

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 227374.
nridge marked 3 inline comments as done.
nridge added a comment.

Addressed some nits and got existing tests to pass

Will follow up with new tests in the next update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -140,7 +140,7 @@
   }
   for (auto  : ExpectedLines)
 ExpectedLinePairHighlighting.push_back(
-{LineTokens.first, LineTokens.second});
+{LineTokens.first, LineTokens.second, /*IsInactive = */ false});
 
   std::vector ActualDiffed =
   diffHighlightings(NewTokens, OldTokens);
@@ -656,10 +656,12 @@
{{HighlightingKind::Variable,
  Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
 {HighlightingKind::Function,
- Range{CreatePosition(3, 4), CreatePosition(3, 7),
+ Range{CreatePosition(3, 4), CreatePosition(3, 7)}}},
+   /* IsInactive = */ false},
   {1,
{{HighlightingKind::Variable,
- Range{CreatePosition(1, 1), CreatePosition(1, 5)};
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)}}},
+   /* IsInactive = */ true}};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -57,6 +57,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"meta.disabled"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
@@ -66,6 +69,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -81,10 +85,12 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 0,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -100,6 +106,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": "BAABAAA="
 # CHECK-NEXT:  }
@@ -115,6 +122,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"isInactive": false,
 # CHECK-NEXT:"line": 1,
 # CHECK-NEXT:"tokens": ""
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -44,7 +44,11 @@
   Primitive,
   Macro,
 
-  LastKind = Macro
+  // This one is different from the other kinds as it's a line style
+  // rather than a token style.
+  InactiveCode,
+
+  LastKind = InactiveCode
 };
 llvm::raw_ostream <<(llvm::raw_ostream , HighlightingKind K);
 
@@ -61,6 +65,7 @@
 struct LineHighlightings {
   int Line;
   std::vector Tokens;
+  bool IsInactive;
 };
 
 bool operator==(const LineHighlightings , const LineHighlightings );
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -158,6 +158,27 @@
   // the end of the Tokens).
   TokRef = TokRef.drop_front(Conflicting.size());
 }
+// Add inactive highlighting tokens.
+const SourceManager  = AST.getSourceManager();
+for (const SourceRange  : AST.getSkippedRanges()) {
+  if (isInsideMainFile(R.getBegin(), SM)) {
+// Create one token for each line in the skipped range, so it works
+// with 

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  auto  = DiffedLines.back();
+  for (auto Iter = AddedLine.Tokens.begin();
+   Iter != AddedLine.Tokens.end();) {

hokein wrote:
> it took me a while to understand this code,
> 
> If the NewLine is `IsInactive`, it just contains a single token whose range 
> is [0, 0), can't we just?
> 
> ```
> 
> if (NewLine.back().Tokens.empty()) continue;
> 
> bool InactiveLine = NewLine.back().Tokens.front().Kind == InactiveCode;
> assert(InactiveLine && NewLine.back().Tokens.size() == 1 && "IncativeCode 
> must have a single token");
> DiffedLines.back().IsInactive = true;
> ```
An inactive line can contain token highlightings as well. For example, we 
highlight macro references in the condition of an `#ifdef`, and that line is 
also inactive if the condition is false. Clients can merge the line style 
(which is typically a background color) with the token styles (typically a 
foreground color).

I did expand the comment to explain what the loop is doing more clearly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67536



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

[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

[Vedant Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd889d1efefe9: [profile] Add a mode to continuously sync 
counter updates to a file (authored by vsk).
Herald added projects: clang, Sanitizers.
Herald added subscribers: Sanitizers, cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68351?vs=226991=227369#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68351

Files:
  clang/docs/SourceBasedCodeCoverage.rst
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld.c
  compiler-rt/lib/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfiling.h
  compiler-rt/lib/profile/InstrProfilingBuffer.c
  compiler-rt/lib/profile/InstrProfilingFile.c
  compiler-rt/lib/profile/InstrProfilingPort.h
  compiler-rt/lib/profile/InstrProfilingRuntime.cpp
  compiler-rt/lib/profile/InstrProfilingWriter.c
  compiler-rt/test/profile/ContinuousSyncMode/basic.c
  compiler-rt/test/profile/ContinuousSyncMode/darwin-proof-of-concept.c
  compiler-rt/test/profile/ContinuousSyncMode/lit.local.cfg.py
  compiler-rt/test/profile/ContinuousSyncMode/multiple-DSOs.c
  compiler-rt/test/profile/ContinuousSyncMode/pid-substitution.c
  compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
  compiler-rt/test/profile/ContinuousSyncMode/set-filename.c
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/ProfileData/InstrProfReader.cpp
  llvm/test/tools/llvm-profdata/Inputs/c-general.profraw
  llvm/test/tools/llvm-profdata/c-general.test
  llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
  llvm/test/tools/llvm-profdata/raw-32-bits-be.test
  llvm/test/tools/llvm-profdata/raw-32-bits-le.test
  llvm/test/tools/llvm-profdata/raw-64-bits-be.test
  llvm/test/tools/llvm-profdata/raw-64-bits-le.test
  llvm/test/tools/llvm-profdata/raw-two-profiles.test

Index: llvm/test/tools/llvm-profdata/raw-two-profiles.test
===
--- llvm/test/tools/llvm-profdata/raw-two-profiles.test
+++ llvm/test/tools/llvm-profdata/raw-two-profiles.test
@@ -1,7 +1,9 @@
 RUN: printf '\201rforpl\377' > %t-foo.profraw
-RUN: printf '\4\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\5\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-foo.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\10\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\4\0\1\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\4\0\2\0\0\0' >> %t-foo.profraw
@@ -18,9 +20,11 @@
 RUN: printf '\3\0foo\0\0\0' >> %t-foo.profraw
 
 RUN: printf '\201rforpl\377' > %t-bar.profraw
-RUN: printf '\4\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\5\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\1\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t-bar.profraw
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\10\0\0\0\0\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\6\0\1\0\0\0' >> %t-bar.profraw
 RUN: printf '\0\0\6\0\2\0\0\0' >> %t-bar.profraw
Index: llvm/test/tools/llvm-profdata/raw-64-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-le.test
@@ -1,7 +1,9 @@
 RUN: printf '\201rforpl\377' > %t
-RUN: printf '\4\0\0\0\0\0\0\0' >> %t
+RUN: printf '\5\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\20\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\4\0\1\0\0\0' >> %t
 RUN: printf '\0\0\4\0\2\0\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-64-bits-be.test
===
--- llvm/test/tools/llvm-profdata/raw-64-bits-be.test
+++ llvm/test/tools/llvm-profdata/raw-64-bits-be.test
@@ -1,7 +1,9 @@
 RUN: printf '\377lprofr\201' > %t
-RUN: printf '\0\0\0\0\0\0\0\4' >> %t
+RUN: printf '\0\0\0\0\0\0\0\5' >> %t
 RUN: printf '\0\0\0\0\0\0\0\2' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\3' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\20' >> %t
 RUN: printf '\0\0\0\1\0\4\0\0' >> %t
 RUN: printf '\0\0\0\2\0\4\0\0' >> %t
Index: llvm/test/tools/llvm-profdata/raw-32-bits-le.test
===
--- llvm/test/tools/llvm-profdata/raw-32-bits-le.test
+++ llvm/test/tools/llvm-profdata/raw-32-bits-le.test
@@ -1,7 +1,9 @@
 RUN: printf '\201Rforpl\377' > %t
-RUN: printf '\4\0\0\0\0\0\0\0' >> %t
+RUN: printf '\5\0\0\0\0\0\0\0' >> %t
 RUN: printf '\2\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
 RUN: printf '\3\0\0\0\0\0\0\0' >> %t
+RUN: printf 

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69666#1728946 , @yaxunl wrote:

> Is the description reversed?
>
> This attempts to preserve the source address space instead of always using 
> the default address space for void pointer type.


I don't think so? This now preserves it, it did not before


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

https://reviews.llvm.org/D69666



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

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

[Eric Astor via Phabricator via cfe-commits]

epastor added a comment.

Thanks, Reid; I'm not 100% sure I've checked all the corner cases either, but 
this at least seems like a step forward.

As a reminder: I don't have commit access. Could someone else commit this for 
me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69626



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

[PATCH] D69582: Let clang driver support parallel jobs

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

This is somehow similar to what I was proposing in D52193 
.

Would you possibly provide tests and/or an example of your usage please?




Comment at: clang/lib/Driver/Compilation.cpp:303
+}
+std::thread Th(Work);
+Th.detach();

thakis wrote:
> Maybe a select() / fork() loop is a better approach than spawning one thread 
> per subprocess? This is doing thread-level parallelism in addition to 
> process-level parallelism :)
> 
> If llvm doesn't have a subprocess pool abstraction yet, ninja's is pretty 
> small, self-contained, battle-tested and open-source, maybe you could copy 
> that over (and remove bits you don't need)?
> 
> https://github.com/ninja-build/ninja/blob/master/src/subprocess.h
> https://github.com/ninja-build/ninja/blob/master/src/subprocess-win32.cc
> https://github.com/ninja-build/ninja/blob/master/src/subprocess-posix.cc
@thakis How would this new `Subprocess` interface with the existing 
`llvm/include/llvm/Support/Program.h` APIs? Wouldn't be better to simply extend 
what is already there with a `WaitMany()` and a `Terminate()` API like I was 
suggesting in D52193? That would cover all that's needed. Or are you suggesting 
to further stub `ExecuteAndWait()` by this new `Subprocess` API?



Comment at: clang/lib/Driver/Compilation.cpp:332
+if (!Next) {
+  std::this_thread::yield();
   continue;

In addition to what @thakis said above, yielding here is maybe not a good idea. 
This causes the process to spin, and remain in the OS' active process list, 
which uselessly eats cpu cycles. This can become significant over the course of 
several minutes of compilation.

Here's a //tiny// example of what happens when threads are waiting for 
something to happen:
(the top parts yields frequently; the bottom part does not yield - see D68820)
{F10592208}

You would need here to go through a OS primitive that suspends the process 
until at least one job in the pool completes. On Windows this can be achieved 
through `WaitForMultipleObjects()` or I/O completion ports like provided by 
@thakis. You can take a look at `Compilation::executeJobs()` in D52193 and 
further down the line, `WaitMany()` which waits for at least one job/process to 
complete.



Comment at: clang/lib/Driver/Compilation.cpp:354
+};
+JS.launch(Work);
   }

It's a waste to start a new thread here just because `ExecuteAndWait()` is used 
inside `Command::Execute()`. An async mechanism would be a lot better like 
stated above.


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

https://reviews.llvm.org/D69582



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

Re: [clang] 33a745e - [clang][clang-scan-deps] Add support for extracting full module dependencies.

[Reid Kleckner via cfe-commits]

You could hack it to standardize the yaml output to forward slashes with
sed, or clang might wish to do it internally.

My current position is that we should pick the path style and slash
direction closest to the point at which the path will be emitted. In the
past we've tried to make arguments that slashes should be internally one
way or another depending on if it's Windows, MinGW, MSVC, or some other
criteria, but the codebase is inconsistent. Now I lean towards
canonicalizing when printing based on the format in use.
- For example, if emitting codeview, try to standardize to backslashes to
match MSVC.
- If emitting DWARF, maybe use forward slashes since the consumer will
probably be a gnu tool.
- When implementing -print-*-path in the driver, the user is probably a
Makefile, so use forward slashes.
- When emitting this YAML stuff, it's our own format, so do whatever is
convenient.

On Wed, Oct 30, 2019 at 4:03 PM Michael Spencer 
wrote:

> On Wed, Oct 30, 2019 at 3:54 PM Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I XFAILED the test on Windows in 52194350cfe. The [/\\] regexes
>> might need to match two slashes, and the PREFIX variable ends up having
>> inconsistent slash direction.
>>
>
> Hmm, fixing the slashes should be easy, but I'm not sure there's a good
> way to get the PREFIX right. I feel like FileCheck needs an ignore slash
> direction mode.
>
> - Michael Spencer
>
>
>>
>> On Wed, Oct 30, 2019 at 3:27 PM Michael Spencer via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>> Author: Michael Spencer
>>> Date: 2019-10-30T15:27:27-07:00
>>> New Revision: 33a745e6fe7e81d3793f7831d2832aa0785ef327
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/33a745e6fe7e81d3793f7831d2832aa0785ef327
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/33a745e6fe7e81d3793f7831d2832aa0785ef327.diff
>>>
>>> LOG: [clang][clang-scan-deps] Add support for extracting full module
>>> dependencies.
>>>
>>> This is a recommit of d8a4ef0e685c with the nondeterminism fixed.
>>>
>>> This adds experimental support for extracting a Clang module dependency
>>> graph
>>> from a compilation database. The output format is experimental and will
>>> change.
>>> It is currently a concatenation of JSON outputs for each compilation.
>>> Future
>>> patches will change this to deduplicate modules between compilations.
>>>
>>> Differential Revision: https://reviews.llvm.org/D69420
>>>
>>> Added:
>>> clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
>>> clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
>>> clang/test/ClangScanDeps/modules-full.cpp
>>>
>>> Modified:
>>>
>>> clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>>>
>>> clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
>>>
>>> clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
>>> clang/lib/Tooling/DependencyScanning/CMakeLists.txt
>>> clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
>>> clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
>>> clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
>>> clang/tools/clang-scan-deps/ClangScanDeps.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>>
>>> 
>>> diff  --git
>>> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>>> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>>> index fd8ed80b143c..76edf150dbee 100644
>>> ---
>>> a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>>> +++
>>> b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
>>> @@ -30,15 +30,30 @@ enum class ScanningMode {
>>>MinimizedSourcePreprocessing
>>>  };
>>>
>>> +/// The format that is output by the dependency scanner.
>>> +enum class ScanningOutputFormat {
>>> +  /// This is the Makefile compatible dep format. This will include all
>>> of the
>>> +  /// deps necessary for an implicit modules build, but won't include
>>> any
>>> +  /// intermodule dependency information.
>>> +  Make,
>>> +
>>> +  /// This outputs the full module dependency graph suitable for use for
>>> +  /// explicitly building modules.
>>> +  Full,
>>> +};
>>> +
>>>  /// The dependency scanning service contains the shared state that is
>>> used by
>>>  /// the invidual dependency scanning workers.
>>>  class DependencyScanningService {
>>>  public:
>>> -  DependencyScanningService(ScanningMode Mode, bool ReuseFileManager =
>>> true,
>>> +  DependencyScanningService(ScanningMode Mode, ScanningOutputFormat
>>> Format,
>>> +bool ReuseFileManager = true,
>>>  bool SkipExcludedPPRanges = true);
>>>
>>>ScanningMode getMode() const { return Mode; }
>>>
>>> +  ScanningOutputFormat getFormat() const { return Format; }
>>> +
>>>bool 

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6429cdd65ff: Refactor getDeclAtPosition() to use 
SelectionTree + targetDecl() (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -490,7 +490,7 @@
 struct Foo {
   Foo();
   Foo(Foo&&);
-  Foo(const char*);
+  $ConstructorLoc[[Foo]](const char*);
 };
 
 Foo f();
@@ -507,6 +507,8 @@
   Foo ab$7^c;
   Foo ab$8^cd("asdf");
   Foo foox = Fo$9^o("asdf");
+  Foo abcde$10^("asdf");
+  Foo foox2 = Foo$11^("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
@@ -517,12 +519,16 @@
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  // FIXME: Target the constructor as well.
   EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
-  ElementsAre(Sym("Foo"), Sym("abcd")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
-  // First one is class definition, second is the constructor.
-  ElementsAre(Sym("Foo"), Sym("Foo")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")), ElementsAre(Sym("abcd")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("10")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
@@ -2192,7 +2198,7 @@
 const char *DeducedType;
   } Tests[] = {
   {"^auto i = 0;", "int"},
-  {"^auto f(){ return 1;};", "int"}
+  {"^auto f(){ return 1;};", "int"},
   };
   for (Test T : Tests) {
 Annotations File(T.AnnotatedCode);
Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -60,6 +60,8 @@
   int c;
 };
 
+using A^lias = Child2;
+
 int main() {
   Ch^ild2 ch^ild2;
   ch^ild2.c = 1;
Index: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
@@ -24,7 +24,7 @@
 namespace clangd {
 namespace {
 
-using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 auto CreateExpectedSymbolDetails = [](const std::string ,
   const std::string ,
@@ -329,7 +329,7 @@
 auto AST = TestTU::withCode(TestInput.code()).build();
 
 EXPECT_THAT(getSymbolInfo(AST, TestInput.point()),
-ElementsAreArray(T.second))
+UnorderedElementsAreArray(T.second))
 << T.first;
   }
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,18 @@
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation 
-
-public:
-  DeclarationFinder(const SourceLocation )
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  SourceLocation Loc,
-  

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 12 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1978
+   DSAStack->getDefaultDMIBAtLevel(Level, OMPC_DEFAULTMAP_scalar) ==
+   DMIB_firstprivate) ||
   DSAStack->hasExplicitDSA(

ABataev wrote:
> If the scalars are mapped as firstprivates by default, they must be captured 
> by value.
I somehow forgot about this, will fix this soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

Based on the comments it looks like you confirmed this matches MSVC's behavior, 
at least in this regard. I haven't stared at this deeply to think of all the 
corner cases, though.

To Nico's point, I think this change doesn't move us away from conformance, 
it's a bug in our compatibility. We've created a bad situation here were we are 
buggily implementing MSVC pre-processor rules, and nobody should have to add a 
third set of ifdefs to deal with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69626



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

[clang-tools-extra] b6429cd - Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2019-10-31T17:37:27-04:00
New Revision: b6429cdd65ffa28591c5b0da37244ab66d0b1785

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

LOG: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

Summary: This fixes issue #163, among other improvements to go-to-definition.

Reviewers: sammccall

Subscribers: jkorous, mgrang, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 3ee04f031795..0cfa53558114 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,18 @@ SymbolLocation getPreferredLocation(const Location 
,
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation 
-
-public:
-  DeclarationFinder(const SourceLocation )
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  SourceLocation Loc,
-  index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-// Skip non-semantic references.
-if (Roles & static_cast(index::SymbolRole::NameReference))
-  return true;
-
-if (Loc == SearchedLocation) {
-  auto IsImplicitExpr = [](const Expr *E) {
-if (!E)
-  return false;
-// We assume that a constructor expression is implict (was inserted by
-// clang) if it has an invalid paren/brace location, since such
-// experssion is impossible to write down.
-if (const auto *CtorExpr = dyn_cast(E))
-  return CtorExpr->getParenOrBraceRange().isInvalid();
-// Ignore implicit conversion-operator AST node.
-if (const auto *ME = dyn_cast(E)) {
-  if (isa(ME->getMemberDecl()))
-return ME->getMemberLoc().isInvalid();
-}
-return isa(E);
-  };
-
-  if (IsImplicitExpr(ASTNode.OrigE))
-return true;
-  // Find and add definition declarations (for GoToDefinition).
-  // We don't use parameter `D`, as Parameter `D` is the canonical
-  // declaration, which is the first declaration of a redeclarable
-  // declaration, and it could be a forward declaration.
-  if (const auto *Def = getDefinition(D)) {
-Decls.insert(Def);
-  } else {
-// Couldn't find a definition, fall back to use `D`.
-Decls.insert(D);
-  }
-}
-return true;
+std::vector getDeclAtPosition(ParsedAST , SourceLocation Pos,
+DeclRelationSet Relations) {
+  FileID FID;
+  unsigned Offset;
+  std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+  std::vector Result;
+  if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+auto Decls = targetDecl(N->ASTNode, Relations);
+Result.assign(Decls.begin(), Decls.end());
   }
-};
-
-std::vector getDeclAtPosition(ParsedAST ,
-SourceLocation Pos) {
-  DeclarationFinder Finder(Pos);
-  index::IndexingOptions IndexOpts;
-  IndexOpts.SystemSymbolFilter =
-  index::IndexingOptions::SystemSymbolFilterKind::All;
-  IndexOpts.IndexFunctionLocals = true;
-  IndexOpts.IndexParametersInDeclarations = true;
-  IndexOpts.IndexTemplateParameters = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
- AST.getLocalTopLevelDecls(), Finder, IndexOpts);
-
-  return Finder.getFoundDecls();
+  return Result;
 }
 
 llvm::Optional makeLocation(ASTContext , SourceLocation TokLoc,
@@ -258,14 +194,13 @@ std::vector locateSymbolAt(ParsedAST , 
Position Pos,
 }
   }
 
-  

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 227352.
nridge marked 12 inline comments as done.
nridge added a comment.

Address latest comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -490,7 +490,7 @@
 struct Foo {
   Foo();
   Foo(Foo&&);
-  Foo(const char*);
+  $ConstructorLoc[[Foo]](const char*);
 };
 
 Foo f();
@@ -507,6 +507,8 @@
   Foo ab$7^c;
   Foo ab$8^cd("asdf");
   Foo foox = Fo$9^o("asdf");
+  Foo abcde$10^("asdf");
+  Foo foox2 = Foo$11^("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
@@ -517,12 +519,16 @@
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  // FIXME: Target the constructor as well.
   EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
-  ElementsAre(Sym("Foo"), Sym("abcd")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
-  // First one is class definition, second is the constructor.
-  ElementsAre(Sym("Foo"), Sym("Foo")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")), ElementsAre(Sym("abcd")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("10")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
@@ -2192,7 +2198,7 @@
 const char *DeducedType;
   } Tests[] = {
   {"^auto i = 0;", "int"},
-  {"^auto f(){ return 1;};", "int"}
+  {"^auto f(){ return 1;};", "int"},
   };
   for (Test T : Tests) {
 Annotations File(T.AnnotatedCode);
Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -60,6 +60,8 @@
   int c;
 };
 
+using A^lias = Child2;
+
 int main() {
   Ch^ild2 ch^ild2;
   ch^ild2.c = 1;
Index: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
@@ -24,7 +24,7 @@
 namespace clangd {
 namespace {
 
-using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 auto CreateExpectedSymbolDetails = [](const std::string ,
   const std::string ,
@@ -329,7 +329,7 @@
 auto AST = TestTU::withCode(TestInput.code()).build();
 
 EXPECT_THAT(getSymbolInfo(AST, TestInput.point()),
-ElementsAreArray(T.second))
+UnorderedElementsAreArray(T.second))
 << T.first;
   }
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,18 @@
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation 
-
-public:
-  DeclarationFinder(const SourceLocation )
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  SourceLocation Loc,
-  index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-// 

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1133
+  DeclRelationSet Relations =
+  DeclRelation::TemplatePattern | DeclRelation::Alias;
+  auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);

sammccall wrote:
> Are you sure you don't want type hierarchy to work on aliases to record 
> types? (i.e. specify underlying rather than alias)
Good catch! I added a test case to `TypeHierarchyTests` for this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237



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

[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

[Michael Spencer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd816d9bdc585: [clang][ScanDeps] Fix issue with multiple 
commands with the same input. (authored by Bigcheese).

Changed prior to commit:
  https://reviews.llvm.org/D69643?vs=227178=227347#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69643

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/error.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -108,6 +108,24 @@
   return ObjFileName.str();
 }
 
+class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
+public:
+  SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
+  : Command(std::move(Cmd)) {}
+
+  virtual std::vector
+  getCompileCommands(StringRef FilePath) const {
+return {Command};
+  }
+
+  virtual std::vector getAllCompileCommands() const {
+return {Command};
+  }
+
+private:
+  tooling::CompileCommand Command;
+};
+
 /// Takes the result of a dependency scan and prints error / dependency files
 /// based on the result.
 ///
@@ -147,11 +165,6 @@
 
   llvm::cl::PrintOptionValues();
 
-  // By default the tool runs on all inputs in the CDB.
-  std::vector> Inputs;
-  for (const auto  : Compilations->getAllCompileCommands())
-Inputs.emplace_back(Command.Filename, Command.Directory);
-
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
   std::make_unique(
@@ -221,8 +234,12 @@
 #endif
   std::vector> WorkerTools;
   for (unsigned I = 0; I < NumWorkers; ++I)
-WorkerTools.push_back(std::make_unique(
-Service, *AdjustingCompilations));
+WorkerTools.push_back(std::make_unique(Service));
+
+  std::vector Inputs;
+  for (tooling::CompileCommand Cmd :
+   AdjustingCompilations->getAllCompileCommands())
+Inputs.emplace_back(Cmd);
 
   std::vector WorkerThreads;
   std::atomic HadErrors(false);
@@ -237,20 +254,22 @@
 auto Worker = [I, , , , , ,
, ]() {
   while (true) {
-std::string Input;
-StringRef CWD;
+const SingleCommandCompilationDatabase *Input;
+std::string Filename;
+std::string CWD;
 // Take the next input.
 {
   std::unique_lock LockGuard(Lock);
   if (Index >= Inputs.size())
 return;
-  const auto  = Inputs[Index++];
-  Input = Compilation.first;
-  CWD = Compilation.second;
+  Input = [Index++];
+  tooling::CompileCommand Cmd = Input->getAllCompileCommands()[0];
+  Filename = std::move(Cmd.Filename);
+  CWD = std::move(Cmd.Directory);
 }
 // Run the tool on it.
-auto MaybeFile = WorkerTools[I]->getDependencyFile(Input, CWD);
-if (handleDependencyToolResult(Input, MaybeFile, DependencyOS, Errs))
+auto MaybeFile = WorkerTools[I]->getDependencyFile(*Input, CWD);
+if (handleDependencyToolResult(Filename, MaybeFile, DependencyOS, Errs))
   HadErrors = true;
   }
 };
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===
--- clang/test/ClangScanDeps/regular_cdb.cpp
+++ clang/test/ClangScanDeps/regular_cdb.cpp
@@ -9,11 +9,11 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources \
-// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 //
 // Make sure we didn't produce any dependency files!
 // RUN: not cat %t.dir/regular_cdb.d
@@ -40,6 +40,9 @@
 // CHECK1-NEXT: Inputs{{/|\\}}header.h
 // CHECK1-NEXT: Inputs{{/|\\}}header2.h
 
+// CHECK3: regular_cdb_input.o
 // CHECK2: regular_cdb_input.cpp
 // CHECK2-NEXT: Inputs{{/|\\}}header.h
 // CHECK2NO-NOT: header2
+
+// CHECK3: adena.o
Index: clang/test/ClangScanDeps/error.cpp

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:45
+  isDefaulted(),
+  unless(anyOf(isFirstDecl(), isVirtual(),
+   ofClass(cxxRecordDecl(

gribozavr2 wrote:
> The "isFirstDecl" part probably will always work, however, it is semantically 
> not really the right predicate to check. What we want to check is where the 
> constructor is declared -- in the class or outside the class, correct? If so, 
> then the matcher should be looking at the DeclContext of the constructor decl.
Thanks for the comment.
> What we want to check is where the constructor is declared -- in the class or 
> outside the class, correct? 
I was following the Standard's wording, which says that trivial destructor is 
not user-provided, where [[ 
http://eel.is/c++draft/dcl.fct.def.default#def:user-provided | user-provided ]] 
means
> user-declared and not explicitly defaulted or deleted on its **first 
> declaration**.
Technically, I think both ways should work as C++ doesn't allow more than 2 
special function declarations.


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

https://reviews.llvm.org/D69435



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

[clang] d816d9b - [clang][ScanDeps] Fix issue with multiple commands with the same input.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-31T14:22:01-07:00
New Revision: d816d9bdc585bbf77a7a1c47a7199fd9e0c34402

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

LOG: [clang][ScanDeps] Fix issue with multiple commands with the same input.

Previously, given a CompilationDatabase with two commands for the same
source file we would report that file twice with the union of the
dependencies for each command both times.

This was due to the way `ClangTool` runs actions given an input source
file (see the comment in `DependencyScanningTool.cpp`). This commit adds
a `SingleCommandCompilationDatabase` that is created with each
`CompileCommand` in the original CDB, which is then used for each
`ClangTool` invocation. This gives us a single run of
`DependencyScanningAction` per `CompileCommand`.

I looked at using `AllTUsToolExecutor` which is a parallel tool
executor, but I'm not sure it's suitable for `clang-scan-deps` as it
does a lot more sharing of state than `AllTUsToolExecutor` expects.

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

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/test/ClangScanDeps/Inputs/regular_cdb.json
clang/test/ClangScanDeps/error.cpp
clang/test/ClangScanDeps/regular_cdb.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 78b49e4fa0c5..a3a8c6988819 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -26,9 +26,7 @@ class DependencyScanningTool {
   ///
   /// \param Compilations The reference to the compilation database that's
   /// used by the clang tool.
-  DependencyScanningTool(
-  DependencyScanningService ,
-  const clang::tooling::CompilationDatabase );
+  DependencyScanningTool(DependencyScanningService );
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
@@ -36,13 +34,13 @@ class DependencyScanningTool {
   ///
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, dependency file contents otherwise.
-  llvm::Expected getDependencyFile(const std::string ,
-StringRef CWD);
+  llvm::Expected
+  getDependencyFile(const tooling::CompilationDatabase ,
+StringRef CWD);
 
 private:
   const ScanningOutputFormat Format;
   DependencyScanningWorker Worker;
-  const tooling::CompilationDatabase 
 };
 
 } // end namespace dependencies

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 4b10f24167a8..f643c538f8f9 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -23,14 +23,12 @@ namespace tooling{
 namespace dependencies{
 
 DependencyScanningTool::DependencyScanningTool(
-DependencyScanningService ,
-const tooling::CompilationDatabase )
-: Format(Service.getFormat()), Worker(Service), Compilations(Compilations) 
{
+DependencyScanningService )
+: Format(Service.getFormat()), Worker(Service) {
 }
 
-llvm::Expected
-DependencyScanningTool::getDependencyFile(const std::string ,
-  StringRef CWD) {
+llvm::Expected DependencyScanningTool::getDependencyFile(
+const tooling::CompilationDatabase , StringRef CWD) {
   /// Prints out all of the gathered dependencies into a string.
   class MakeDependencyPrinterConsumer : public DependencyConsumer {
   public:
@@ -140,6 +138,16 @@ DependencyScanningTool::getDependencyFile(const 
std::string ,
 std::string ContextHash;
   };
 
+  
+  // We expect a single command here because if a source file occurs multiple
+  // times in the original CDB, then `computeDependencies` would run the
+  // `DependencyScanningAction` once for every time the input occured in the
+  // CDB. Instead we split up the CDB into single command chunks to avoid this
+  // behavior.
+  assert(Compilations.getAllCompileCommands().size() == 1 &&
+ "Expected a compilation database with a single command!");
+  std::string Input = Compilations.getAllCompileCommands().front().Filename;
+  
   if (Format == ScanningOutputFormat::Make) {
 MakeDependencyPrinterConsumer Consumer;
 auto Result =

diff  --git 

[clang] 19f1dc7 - Remove unneeded template alias, causes issues with some MSVC version

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-10-31T14:20:54-07:00
New Revision: 19f1dc7b527eade11dae9425c420cc9f450393b6

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

LOG: Remove unneeded template alias, causes issues with some MSVC version

I built locally with the latest MSVC in c++14 and c++17, but it does not
complain for me. Osman Zakir on llvm-dev reports that they run into
compile errors here.

In any case, it seems prefereable to reuse clang's LLVM.h header to
bring in llvm::Optional and Expected.

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeStmtGen.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp 
b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index c71301598bde..5b47489e65e0 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -14,13 +14,11 @@
 #include "PrimType.h"
 #include "Program.h"
 #include "State.h"
+#include "clang/Basic/LLVM.h"
 
 using namespace clang;
 using namespace clang::interp;
 
-template  using Expected = llvm::Expected;
-template  using Optional = llvm::Optional;
-
 namespace clang {
 namespace interp {
 



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:389-392
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+  JA.isDeviceOffloading(Action::OFK_HIP))
+Default = false;

Shouldn't it be `true`, considering that we do want to **disable** autolinking 
by default for device-side CUDA/HIP?

If we don't support autolinking at all for CUDA/HIP, perhaps we should just 
return `true` here.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69678: [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

[John McCall via Phabricator via cfe-commits]

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

Seems reasonable.


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

https://reviews.llvm.org/D69678



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D57829#1729094 , @tra wrote:

> Could you, please, give us a bit more context and provide more info for the 
> questions @rjmccall and I asked before?
>
> Is the problem specifically because autolink is not supported on device side? 
> Or is disabling autolink just a convoluted way to avoid calling 
> EmitModuleLinkOptions()?
>  If it's the former, then we should just disable it unconditionally -- we 
> already filter out some other flags  (e.g. asan).
>  If it's the latter, then tweaking autolink option behavior is just covering 
> the problem. Sooner or later EmitModuleLinkOptions() will be called by 
> something else and we'll be back to where we are now.


we need to disable autolink for device compilation because the linker options 
are intended for host compilation.

Previously I said the backend does not support linker option, which was 
incorrect. The diagnostic about invalid linker option was due to a clang 
codegen bug due to detect_mismatch, which I have a fix

https://reviews.llvm.org/D69678

Even with that fix, we still need to disable auto link for device compilation, 
since it is intended for host compilation.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69678: [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

When a target does not support pragma detect_mismatch, an llvm.linker.options
metadata with an empty entry is created, which causes diagnostic in backend
since backend expects name/value pair in llvm.linker.options entries.

This patch fixes that.


https://reviews.llvm.org/D69678

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/pragma-detect_mismatch.c


Index: clang/test/CodeGen/pragma-detect_mismatch.c
===
--- clang/test/CodeGen/pragma-detect_mismatch.c
+++ clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1930,6 +1930,8 @@
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }


Index: clang/test/CodeGen/pragma-detect_mismatch.c
===
--- clang/test/CodeGen/pragma-detect_mismatch.c
+++ clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1930,6 +1930,8 @@
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:1152
+def frounding_math : Flag<["-"], "frounding-math">, Group, 
Flags<[CC1Option]>;
+def fno_rounding_math : Flag<["-"], "fno-rounding-math">, Group, 
Flags<[CC1Option]>;
 def ftrapping_math : Flag<["-"], "ftrapping-math">, Group, 
Flags<[CC1Option]>;

rjmccall wrote:
> It looks like both of these can now be written with `BooleanFFlag`.
BooleanFFlag doesn't work, there's a FIXME message saying that prefixes don't 
work, currently they are only being used for unimplemented options.
llvm/clang/lib/Driver/ToolChains/Clang.cpp:2301:17: error: ‘OPT_frounding_math’ 
is not a member of ‘clang::driver::options’
 optID = options::OPT_frounding_math;
 ^



Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

LGTM modulo the isFirstDeclComment. Will approve after we resolve that 
discussion.




Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:45
+  isDefaulted(),
+  unless(anyOf(isFirstDecl(), isVirtual(),
+   ofClass(cxxRecordDecl(

The "isFirstDecl" part probably will always work, however, it is semantically 
not really the right predicate to check. What we want to check is where the 
constructor is declared -- in the class or outside the class, correct? If so, 
then the matcher should be looking at the DeclContext of the constructor decl.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 5 inline comments as done.
ymandel added a comment.

Thanks for the review! I agreed w/ all the comments, just responding early w/ 
those I had further thoughts/questions on.




Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:82
+Stencil makeStencil(RangeSelector Selector);
+inline Stencil makeStencil(Stencil S) { return S; }
 

gribozavr2 wrote:
> I feel like this should be an implementation detail of `cat` (or other 
> stencil combinators who want to support such implicit conversions). Users 
> writing stencils should use concrete factory functions below.
> 
> 
Agreed, but I wasn't sure if it was worth introducing a "detail" namespace for 
just one (overloaded) function. I liked when these were private methods of 
Stencil, but we don't have that option now. Should I use a `namespace detail`? 
something else? 



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:137
+/// number.
+Stencil sequence(std::vector Parts);
 

gribozavr2 wrote:
> It is the same operation as `cat`, right? So WDYT about `cat_vector`?
not quite. `cat` is "smart" and optimizes case of one argument to just return 
that argument. in the future, it might also handle the case of 0 arguments 
specially. `sequence` just blindly constructs a sequence stencil.

that said, i can see the argument that the user doesn't care about such details 
and instead of the three functions:
```
cat(...)
cat(vector)
sequence(vector)
```

we should just have the two
```
cat(...)
catVector(vector)
```
with no "plain" sequence constructor. WDYT?



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:150
 }
-} // namespace stencil
-} // namespace tooling
+Stencil cat(std::vector Parts);
+

gribozavr2 wrote:
> Like I said in the other review, I don't feel great about an overload set 
> with universal references and anything else.
Sorry, I'd forgotten we'd discussed this before. I see your point -- I was 
surprised it worked, but when it did just barelled along. That should have been 
a red flag. :)  I'm happy to rework this, per the comment above,.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:154
+/// for more details.
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);

gribozavr2 wrote:
> I'm a bit confused by the name. The function returns a MatchConsumer, but it 
> is called "stencil". What is the intended usage?
Intended use is something like `makeRule(matcher, change(stencil(...)))`.

The naming is the same as we previously had for `text()`. Since `MatchConsumer` 
is so general, the function name describes its argument, not its particular 
type.  But, I'm open to other suggestions.



Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:262
+
+template  class StencilImpl : public StencilInterface {
   T Data;

gribozavr2 wrote:
> There isn't much logic left in StencilImpl.
> 
> Would the code be simpler if we made the various Data classes implement 
> StencilInterface directly?
Yes! I plan to do that in a followup revision, though I was really tempted to 
do it here already. :)

That said, we could also use a `llvm::PointerSumType` and implement it in 
functional style, fwiw. I'm fine either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Could you, please, give us a bit more context and provide more info for the 
questions @rjmccall and I asked before?

Is the problem specifically because autolink is not supported on device side? 
Or is disabling autolink just a convoluted way to avoid calling 
EmitModuleLinkOptions()?
If it's the former, then we should just disable it unconditionally -- we 
already filter out some other flags  (e.g. asan).
If it's the latter, then tweaking autolink option behavior is just covering the 
problem. Sooner or later EmitModuleLinkOptions() will be called by something 
else and we'll be back to where we are now.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 227317.
MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added a comment.

Add additional test case


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

https://reviews.llvm.org/D69577

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
@@ -4977,6 +4977,25 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> A>)>;");
+  verifyFormat("A() -> Afoo<1>)>;");
+  verifyFormat("A() -> A<(3 < 2)>;");
+  verifyFormat("A() -> A<((3) < (2))>;");
+
+  // Ensure not deduction guides.
+  verifyFormat("c()->f();");
+  verifyFormat("x()->foo<1>;");
+  verifyFormat("x = p->foo<3>();");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,6 +1350,58 @@
 }
   }
 
+  static FormatToken *untilMatchingParen(FormatToken *Current) {
+// for when MatchingParen is not yet established
+int ParenLevel = 0;
+while (Current) {
+  if (Current->is(tok::l_paren))
+ParenLevel++;
+  if (Current->is(tok::r_paren))
+ParenLevel--;
+  if (ParenLevel < 1)
+break;
+  Current = Current->Next;
+}
+return Current;
+  }
+
+  static bool isDeductionGuide(FormatToken ) {
+// Look for a deduction guide A(...) -> A<...>;
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.
+  FormatToken *TemplateCloser = Current.Next->Next;
+  int NestingLevel = 0;
+  while (TemplateCloser) {
+// Skip over an expressions in parens  A<(3 < 2)>;
+if (TemplateCloser->is(tok::l_paren)) {
+  // No Matching Paren yet so skip to matching paren
+  TemplateCloser = untilMatchingParen(TemplateCloser);
+}
+if (TemplateCloser->is(tok::less))
+  NestingLevel++;
+if (TemplateCloser->is(tok::greater))
+  NestingLevel--;
+if (NestingLevel < 1)
+  break;
+TemplateCloser = TemplateCloser->Next;
+  }
+  // Assuming we have found the end of the template ensure its followed
+  // with a ;
+  if (TemplateCloser && TemplateCloser->Next &&
+  TemplateCloser->Next->is(tok::semi) &&
+  Current.Previous->MatchingParen) {
+// Determine if the identifier `A` prior to the A<..>; is the same as
+// prior to the A(..)
+FormatToken *LeadingIdentifier =
+Current.Previous->MatchingParen->Previous;
+return (LeadingIdentifier &&
+LeadingIdentifier->TokenText == Current.Next->TokenText);
+  }
+}
+return false;
+  }
+
   void determineTokenType(FormatToken ) {
 if (!Current.is(TT_Unknown))
   // The token type is already known.
@@ -1397,6 +1449,10 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
+
+} else if (isDeductionGuide(Current)) {
+  // Deduction guides trailing arrow " A(...) -> A;".
+  Current.Type = TT_TrailingReturnArrow;
 } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1371
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.

lichray wrote:
> Maybe make use of some `TT_TemplateOpener`?
We can't use TT_TemplateOpener  because like MatchParen it hasn't been set yet 
on the downstream tokens



Comment at: clang/unittests/Format/FormatTest.cpp:4987
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> Afoo<1>)>;");

lichray wrote:
> Does `A() -> A>)>` (C++11 `>>`) work?
this should work because we are skipping everything in between the `()`


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

https://reviews.llvm.org/D69577



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

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:67-70
+// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to
+// return CtorExpr->getSourceRange(). However, starting with c++17, parsing
+// the expression 'std::string Name = ""' results in a CtorExpr whose
+// SourceRange includes just '""' rather than the previous 'Name = ""'.

gribozavr2 wrote:
> poelmanc wrote:
> > aaron.ballman wrote:
> > > Are you sure that this behavioral difference isn't just a bug in Clang? I 
> > > can't think of why the source range should differ based on language mode, 
> > > so I wonder if the correct thing to do is fix how we calculate the source 
> > > range in Clang?
> > Thanks and no, I'm not sure at all. At the end of my summary paragraph I 
> > wrote:
> > 
> > > Or, is it possible that this change in SourceRange is an unintentional 
> > > difference in the parsing code? If so fixing that might make more sense.
> > 
> > Before starting this patch, I built a Debug build of LLVM and stepped 
> > through LLVM parsing code while running clang-tidy on the 
> > readability-redundant-string-init.cpp file. I set breakpoints and inspected 
> > variable values under both -std c++14 and -std c++17. The call stacks were 
> > clearly different. I could sometimes inspect character positions in the 
> > file and see where an expression's SourceLoc was set. However, a SourceLoc 
> > is a single character position; I couldn't figure out where an expression's 
> > SourceRange was even set. So I gave up and went down the current approach.
> > 
> > Should we ping the LLVM mailing list and see if this change rings a bell 
> > with anyone there?
> Yes, I believe that changing Clang to produce consistent source ranges in 
> C++14 and C++17 is the best way to go.
> 
> Manual parsing below is not a very maintainable solution.
> Yes, I believe that changing Clang to produce consistent source ranges in 
> C++14 and C++17 is the best way to go.

+1, if this is possible to do.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69238



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:955
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type = TT_OverloadedOperator;

MyDeveloperDay wrote:
> MyDeveloperDay wrote:
> > sammccall wrote:
> > > I'm confused about this: ISTM that where we were previously always 
> > > treating star as a pointer, now we're always treating it as an operator 
> > > name.
> > > Whereas it's sometimes an operator name (immediately after the `operator` 
> > > keyword) and sometimes a pointer (following a type name).
> > > 
> > > Maybe we should shift the OverloadedOperator labelling outside the loop 
> > > (since AFAICT it should only apply to the first token) and then keep the 
> > > loop to mark stars/amps elsewhere in the operator name as 
> > > PointerOrReference?
> > Let me take another look..
> @sammccall  In trying to understand what you were suggesting I tried the 
> following:
> 
> ```
> bool
> Foo::operator*(void *) const {
>   return true;
> }
> ```
> 
> The second `*` will be as seen correctly below a PointerOrReference by the 
> nature that we already hit the OverloadedOperatorLParen as the loop only goes 
> until it see a `(` `)` or `;`
> 
> I'm trying to think of a case where that perhaps wouldn't be the case? such 
> that we might get 2 OverloadedOperators
> 
> ```
> M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=bool L=4 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x1cea7c36c40 Text='bool'
>  M=1 C=1 T=FunctionDeclarationName S=1 B=0 BK=0 P=80 Name=identifier L=84 
> PPK=2 FakeLParens= FakeRParens=0 II=0x1cea7c42298 Text='Foo'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=coloncolon L=86 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='::'
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=520 Name=operator L=94 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x1cea7c36ed8 Text='operator'
>  M=0 C=0 T=OverloadedOperator S=0 B=0 BK=0 P=34 Name=star L=95 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='*'
>  M=0 C=0 T=OverloadedOperatorLParen S=0 B=0 BK=0 P=34 Name=l_paren L=96 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='('
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=void L=100 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x1cea7c368e0 Text='void'
>  M=0 C=1 T=PointerOrReference S=1 B=0 BK=0 P=230 Name=star L=102 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='*'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=54 Name=r_paren L=103 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=')'
>  M=0 C=1 T=TrailingAnnotation S=1 B=0 BK=0 P=170 Name=const L=109 PPK=2 
> FakeLParens= FakeRParens=0 II=0x1cea7c363e8 Text='const'
>  M=0 C=0 T=FunctionLBrace S=1 B=0 BK=1 P=23 Name=l_brace L=111 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='{'
> ```
I mean something like a conversion operator to pointer: `bool Foo::operator 
void *();`
The star appears before the operator arg list, but it's still a pointer.

So the testcases I'd try here would be:

```
Foo::operator*(); // should be OverloadedOperator
Foo::operator void *(); // should be PointerOrReference
Foo::operator()(void *); // should be PointerOrReference
Foo::operator*(void *); // should be OverloadedOperator, PointerOrReference
operator*(int(*)(), class Foo); // OverloadedOperator, PointerOrReference (this 
one is silly and doesn't need to work)
```



Comment at: clang/lib/Format/TokenAnnotator.cpp:2105
+  if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) &&
+  Next->Next && Next->Next->is(tok::star)) {
+// For operator void*(), operator char*(), operator Foo*().

I'm suspicious of code that handles star but not amp, though maybe I shouldn't 
be.


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

https://reviews.llvm.org/D69573



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:57
+  /// Constructs a string representation of the StencilInterfacePart.
+  /// StencilInterfaceParts generated by the `selection` and `run` functions do
+  /// not have a unique string representation.

s/Part// (2x)



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:82
+Stencil makeStencil(RangeSelector Selector);
+inline Stencil makeStencil(Stencil S) { return S; }
 

I feel like this should be an implementation detail of `cat` (or other stencil 
combinators who want to support such implicit conversions). Users writing 
stencils should use concrete factory functions below.





Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:137
+/// number.
+Stencil sequence(std::vector Parts);
 

It is the same operation as `cat`, right? So WDYT about `cat_vector`?



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:144
 
-namespace tooling {
-// DEPRECATED: These are temporary aliases supporting client migration to the
-// `transformer` namespace.
-using Stencil = transformer::Stencil;
-using StencilPart = transformer::StencilPart;
-namespace stencil {
-using transformer::access;
-using transformer::addressOf;
-using transformer::cat;
-using transformer::deref;
-using transformer::dPrint;
-using transformer::expression;
-using transformer::ifBound;
-using transformer::run;
-using transformer::selection;
-using transformer::text;
-/// \returns the source corresponding to the identified node.
-/// FIXME: Deprecated. Write `selection(node(Id))` instead.
-inline transformer::StencilPart node(llvm::StringRef Id) {
-  return selection(tooling::node(Id));
+/// General-purpose Stencil factory function. Concatenates 0+ stencil pieces
+/// into a single stencil. Arguments can be raw text, ranges in the matched 
code

I think it is better to remove the first sentence.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:150
 }
-} // namespace stencil
-} // namespace tooling
+Stencil cat(std::vector Parts);
+

Like I said in the other review, I don't feel great about an overload set with 
universal references and anything else.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:152
+
+/// Convenience function for creating a stencil-based MatchConsumer. See `cat`
+/// for more details.

"Creates a MatchConsumer that evaluates a given Stencil."



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:154
+/// for more details.
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);

I'm a bit confused by the name. The function returns a MatchConsumer, but it is 
called "stencil". What is the intended usage?



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:155
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);
+  return [S](const ast_matchers::MatchFinder::MatchResult ) {

I'm not sure the convenience of being able to avoid to say "cat(...)" at the 
callsite is worth the API complexity. In other words, my suggestion is to make 
this function take a single stencil.



Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:97
+// StencilInteface or just Stencil?
+// unique_ptr or shared_ptr?
+

I think these comments can be deleted now.



Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:262
+
+template  class StencilImpl : public StencilInterface {
   T Data;

There isn't much logic left in StencilImpl.

Would the code be simpler if we made the various Data classes implement 
StencilInterface directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable()

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

A few comments inline.




Comment at: clang/tools/driver/CMakeLists.txt:34
   ${tablegen_deps}
+  SUPPORT_PLUGINS
   )

This is now a behavior change because you're always passing this.

You'll want to bring back the conditional statement with a body like:
```
set(support_plugins SUPPORT_PLUGINS)
```

Then instead of passing `SUPPORT_PLUGINS` here you pass `${support_plugins}`



Comment at: llvm/cmake/modules/AddLLVM.cmake:787
+  if (ARG_SUPPORT_PLUGINS)
+set(LLVM_NO_DEAD_STRIP 1)
+  endif()

By convention in LLVM we use `On` rather than `1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69638



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

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:67-70
+// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to
+// return CtorExpr->getSourceRange(). However, starting with c++17, parsing
+// the expression 'std::string Name = ""' results in a CtorExpr whose
+// SourceRange includes just '""' rather than the previous 'Name = ""'.

poelmanc wrote:
> aaron.ballman wrote:
> > Are you sure that this behavioral difference isn't just a bug in Clang? I 
> > can't think of why the source range should differ based on language mode, 
> > so I wonder if the correct thing to do is fix how we calculate the source 
> > range in Clang?
> Thanks and no, I'm not sure at all. At the end of my summary paragraph I 
> wrote:
> 
> > Or, is it possible that this change in SourceRange is an unintentional 
> > difference in the parsing code? If so fixing that might make more sense.
> 
> Before starting this patch, I built a Debug build of LLVM and stepped through 
> LLVM parsing code while running clang-tidy on the 
> readability-redundant-string-init.cpp file. I set breakpoints and inspected 
> variable values under both -std c++14 and -std c++17. The call stacks were 
> clearly different. I could sometimes inspect character positions in the file 
> and see where an expression's SourceLoc was set. However, a SourceLoc is a 
> single character position; I couldn't figure out where an expression's 
> SourceRange was even set. So I gave up and went down the current approach.
> 
> Should we ping the LLVM mailing list and see if this change rings a bell with 
> anyone there?
Yes, I believe that changing Clang to produce consistent source ranges in C++14 
and C++17 is the best way to go.

Manual parsing below is not a very maintainable solution.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69238



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

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Gentle ping.

I'm open to suggestions to simplify the condition, but I think templates, 
partial specializations and instantiations are three different things that we 
all need to exclude here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:21
 
+const char DefaultStringNames[] = "basic_string";
+

poelmanc wrote:
> aaron.ballman wrote:
> > I think the default should probably be `::std::basic_string` to avoid 
> > getting other things named `basic_string`?
> The prior code had `basic_string` hard-coded at lines 27, 31, and 50 so I 
> initially set the default to `basic_string` just to keep the default behavior 
> unchanged.
> 
> I just now tried setting it to each of `std::basic_string`, 
> `::std::basic_string`, and even `std::string;std::wstring` and the 
> `readability-redundant-string-init.cpp` tests failed with any of those 
> settings, so I've left it set to `basic_string`.
I think we should change the checker behavior so that it checks against the 
fully-qualified name.

We are defining a user-settable option, and changing its behavior later to 
allow fully-qualified names could be difficult.

The reason why tests failed for you is because below the name "basic_string" is 
used as a method name (the name of the constructor). That checker should be 
rewritten to check the name of the type.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69548



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+llvm::Expected
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+  llvm::StringRef NewName, const SymbolIndex *Index,

hokein wrote:
> ilya-biryukov wrote:
> > NIT: Maybe call it `renameWithIndex` instead?
> > It should capture what this function is doing better...
> > 
> > But up to you
> I would keep the current name, as it is symmetrical to the `renameWithinFile`.
Yeah, those go together. Using `renameWithIndex` would mean the other one 
should be `renameWithAST` (or similar).
Feel free to keep as is too.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:216
+// Return all rename occurrences (per the index) outside of the main file,
+// grouped by the file name.
+llvm::StringMap>

NIT: grouped by the **absolute** file name? Or is there a better term than 
"absolute" to describe this?



Comment at: clang-tools-extra/clangd/refactor/Rename.h:23
 
+using DirtyBufferGetter =
+std::function(PathRef Path)>;

Could you document what does it mean for this function to return `llvm::None`?
I assume we read the contents from disk instead.

Also worth documenting what should be returned for `MainFilePath`? 
`llvm::None`? same value as `MainFileCode`?



Comment at: clang-tools-extra/clangd/refactor/Rename.h:24
+using DirtyBufferGetter =
+std::function(PathRef Path)>;
+

NIT: maybe use `function_ref`? We definitely don't store this function anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69673: [clangd] Implement semantic highlightings via findExplicitReferences

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59818 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69673



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

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Is the description reversed?

This attempts to preserve the source address space instead of always using the 
default address space for void pointer type.


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

https://reviews.llvm.org/D69666



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

[PATCH] D69673: [clangd] Implement semantic highlightings via findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

To keep the logic of finding locations of interesting AST nodes in one
place.

The advantage is better coverage of various AST nodes, both now and in
the future: as new nodes get added to `findExplicitReferences`, semantic
highlighting will automatically pick them up.

The drawback of this change is that we have to traverse declarations
inside our file twice in order to highlight dependent names, 'auto'
and 'decltype'. Hopefully, this should not affect the actual latency
too much, most time should be spent in building the AST and not
traversing it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69673

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -589,6 +589,15 @@
   R"cpp(
   void $Function[[foo]]();
   using ::$Function[[foo]];
+)cpp",
+  // Highlighting of template template arguments.
+  R"cpp(
+  template  class $TemplateParameter[[TT]],
+template  class ...$TemplateParameter[[TTs]]>
+  struct $Class[[Foo]] {
+$Class[[Foo]]<$TemplateParameter[[TT]], $TemplateParameter[[TTs]]...>
+  *$Field[[t]];
+  }
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "SemanticHighlighting.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
@@ -15,10 +16,16 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Casting.h"
 #include 
 
 namespace clang {
@@ -103,12 +110,12 @@
 return kindForDecl(TD);
   return llvm::None;
 }
-// Given a set of candidate declarations, if the declarations all have the same
-// highlighting kind, return that highlighting kind, otherwise return None.
-template 
-llvm::Optional kindForCandidateDecls(IteratorRange Decls) {
+
+llvm::Optional kindForReference(const ReferenceLoc ) {
   llvm::Optional Result;
-  for (NamedDecl *Decl : Decls) {
+  for (const NamedDecl *Decl : R.Targets) {
+if (!canHighlightName(Decl->getDeclName()))
+  return llvm::None;
 auto Kind = kindForDecl(Decl);
 if (!Kind || (Result && Kind != Result))
   return llvm::None;
@@ -117,27 +124,48 @@
   return Result;
 }
 
-// Collects all semantic tokens in an ASTContext.
-class HighlightingTokenCollector
-: public RecursiveASTVisitor {
-  std::vector Tokens;
-  ParsedAST 
-
+/// Consumes source locations and maps them to text ranges for highlightings.
+class HighlightingsBuilder {
 public:
-  HighlightingTokenCollector(ParsedAST ) : AST(AST) {}
-
-  std::vector collectTokens() {
-Tokens.clear();
-TraverseAST(AST.getASTContext());
-// Add highlightings for macro expansions as they are not traversed by the
-// visitor.
-for (const auto  : AST.getMacros().Ranges)
-  Tokens.push_back({HighlightingKind::Macro, M});
+  HighlightingsBuilder(const SourceManager , const LangOptions )
+  : SM(SM), LO(LO) {}
+
+  void addToken(HighlightingToken T) { Tokens.push_back(T); }
+
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isInvalid())
+  return;
+if (Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
+
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from
+// another file for a class from another file).
+// There are also cases with macros where the spelling loc will not be in
+// the main file and the highlighting would be incorrect.
+if (!isInsideMainFile(Loc, SM))
+  return;
+
+auto 

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

The functionality looks acceptable.  Trying to parse the whole thing still 
looks fragile to me.  I expect code owner to take a look at this change.




Comment at: clang/lib/Format/TokenAnnotator.cpp:1371
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.

Maybe make use of some `TT_TemplateOpener`?



Comment at: clang/unittests/Format/FormatTest.cpp:4987
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> Afoo<1>)>;");

Does `A() -> A>)>` (C++11 `>>`) work?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69577



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

[PATCH] D69657: [AArch64][SVE] Implement several floating-point arithmetic intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin updated this revision to Diff 227299.
kmclaughlin added a comment.

- Removed duplicate //AdvSIMD_Pred2VectorArg_Intrinsic// class after rebase


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

https://reviews.llvm.org/D69657

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll
@@ -0,0 +1,530 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; FABD
+;
+
+define  @fabd_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_h:
+; CHECK: fabd z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fabd_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_s:
+; CHECK: fabd z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fabd_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_d:
+; CHECK: fabd z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FADD
+;
+
+define  @fadd_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_h:
+; CHECK: fadd z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fadd_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_s:
+; CHECK: fadd z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fadd_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_d:
+; CHECK: fadd z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FDIV
+;
+
+define  @fdiv_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_h:
+; CHECK: fdiv z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fdiv_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_s:
+; CHECK: fdiv z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fdiv_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_d:
+; CHECK: fdiv z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FDIVR
+;
+
+define  @fdivr_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_h:
+; CHECK: fdivr z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv8f16( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fdivr_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_s:
+; CHECK: fdivr z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv4f32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @fdivr_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_d:
+; CHECK: fdivr z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv2f64( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+;
+; FMAX
+;
+
+define  @fmax_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fmax_h:
+; CHECK: fmax z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmax.nxv8f16( %pg,
+  

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Yes, thanks, looks a lot better.  Just a few tweaks now.




Comment at: clang/include/clang/Basic/LangOptions.h:348
   VersionTuple getOpenCLVersionTuple() const;
+
 };

Spurious change.



Comment at: clang/include/clang/Driver/Options.td:1152
+def frounding_math : Flag<["-"], "frounding-math">, Group, 
Flags<[CC1Option]>;
+def fno_rounding_math : Flag<["-"], "fno-rounding-math">, Group, 
Flags<[CC1Option]>;
 def ftrapping_math : Flag<["-"], "ftrapping-math">, Group, 
Flags<[CC1Option]>;

It looks like both of these can now be written with `BooleanFFlag`.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:148
+llvm_unreachable("Unsupported FP Exception Behavior");
+  }
+

Please make functions that do these translations, and please make them use 
exhaustive switches with `llvm_unreachable` at the end.



Comment at: clang/test/Driver/clang_f_opts.c:323
 // RUN: -fprofile-values  \
-// RUN: -frounding-math   \
 // RUN: -fschedule-insns  \

Looks like the intent of this test is that you pull this to the lines above, to 
test that we don't emit an error on it.  You should also test `-ffp-model`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> There is no difference in perf for GCC.

Yes, but that's because gcc still optimizes the call to noinlined function. In 
the common scenario which this check addresses (destructor defaulted in .cpp 
file) gcc is not able to optimize the call.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, jdoerfert, Anastasia.
Herald added a subscriber: wdng.

This attempted to always use the default address space void pointer
type instead of preserving the source address space.


https://reviews.llvm.org/D69666

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/address-space.c


Index: clang/test/CodeGen/address-space.c
===
--- clang/test/CodeGen/address-space.c
+++ clang/test/CodeGen/address-space.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck 
-check-prefixes=CHECK,AMDGCN %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-enable-var-scope -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck 
-enable-var-scope -check-prefixes=CHECK,AMDGCN %s
 
 // CHECK: @foo = common addrspace(1) global
 int foo __attribute__((address_space(1)));
@@ -10,11 +10,11 @@
 // CHECK: @a = common global
 int a __attribute__((address_space(0)));
 
-// CHECK-LABEL: define i32 @test1() 
+// CHECK-LABEL: define i32 @test1()
 // CHECK: load i32, i32 addrspace(1)* @foo
 int test1() { return foo; }
 
-// CHECK-LABEL: define i32 @test2(i32 %i) 
+// CHECK-LABEL: define i32 @test2(i32 %i)
 // CHECK: load i32, i32 addrspace(1)*
 // CHECK-NEXT: ret i32
 int test2(int i) { return ban[i]; }
@@ -45,3 +45,17 @@
   MyStruct s = pPtr[0];
   pPtr[0] = s;
 }
+
+// Make sure the right address space is used when doing arithmetic on a void
+// pointer. Make sure no invalid bitcast is introduced.
+
+// CHECK-LABEL: @void_ptr_arithmetic_test(
+// X86: [[ALLOCA:%.*]] = alloca i8 addrspace(1)*
+// X86-NEXT: store i8 addrspace(1)* %arg, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: load i8 addrspace(1)*, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: getelementptr i8, i8 addrspace(1)*
+// X86-NEXT: ret i8 addrspace(1)*
+void __attribute__((address_space(1)))*
+void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
+return arg + 4;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3258,7 +3258,7 @@
   // GNU void* casts amount to no-ops since our void* type is i8*, but this is
   // future proof.
   if (elementType->isVoidType() || elementType->isFunctionType()) {
-Value *result = CGF.Builder.CreateBitCast(pointer, CGF.VoidPtrTy);
+Value *result = CGF.EmitCastToVoidPtr(pointer);
 result = CGF.Builder.CreateGEP(result, index, "add.ptr");
 return CGF.Builder.CreateBitCast(result, pointer->getType());
   }


Index: clang/test/CodeGen/address-space.c
===
--- clang/test/CodeGen/address-space.c
+++ clang/test/CodeGen/address-space.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck -check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck -check-prefixes=CHECK,AMDGCN %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck -enable-var-scope -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck -enable-var-scope -check-prefixes=CHECK,AMDGCN %s
 
 // CHECK: @foo = common addrspace(1) global
 int foo __attribute__((address_space(1)));
@@ -10,11 +10,11 @@
 // CHECK: @a = common global
 int a __attribute__((address_space(0)));
 
-// CHECK-LABEL: define i32 @test1() 
+// CHECK-LABEL: define i32 @test1()
 // CHECK: load i32, i32 addrspace(1)* @foo
 int test1() { return foo; }
 
-// CHECK-LABEL: define i32 @test2(i32 %i) 
+// CHECK-LABEL: define i32 @test2(i32 %i)
 // CHECK: load i32, i32 addrspace(1)*
 // CHECK-NEXT: ret i32
 int test2(int i) { return ban[i]; }
@@ -45,3 +45,17 @@
   MyStruct s = pPtr[0];
   pPtr[0] = s;
 }
+
+// Make sure the right address space is used when doing arithmetic on a void
+// pointer. Make sure no invalid bitcast is introduced.
+
+// CHECK-LABEL: @void_ptr_arithmetic_test(
+// X86: [[ALLOCA:%.*]] = alloca i8 addrspace(1)*
+// X86-NEXT: store i8 addrspace(1)* %arg, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: load i8 addrspace(1)*, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: getelementptr i8, i8 addrspace(1)*
+// X86-NEXT: ret i8 addrspace(1)*
+void __attribute__((address_space(1)))*
+void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
+return arg + 4;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3258,7 +3258,7 @@
   // GNU void* casts amount to no-ops since our void* type is i8*, but this is
   // future proof.
   if (elementType->isVoidType() || 

[PATCH] D69632: [libTooling] Add Stencil constructor.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov 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/D69632/new/

https://reviews.llvm.org/D69632



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

[PATCH] D69664: [Diagnostics] Teach -Wnull-dereference about address_space attribute

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

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



Comment at: clang/lib/Sema/SemaExpr.cpp:486
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) 
&&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&

With this patch, we no longer warn for:

*(int __attribute__((address_space(0))) *) 0;

Worth to handle this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69664



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

[PATCH] D69664: [Diagnostics] Teach -Wnull-dereference about address_space attribute

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

xbolva00 updated this revision to Diff 227286.
xbolva00 added a comment.

Added testcase with typedefed int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69664

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/exprs.c


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,14 @@
   test18_e(); // expected-error {{too few arguments to function call, expected 
at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_AS256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null 
pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
+   *(int_AS256*)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null 
pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) 
&&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_through_null));
+}
 }
 
 static void DiagnoseDirectIsaAccess(Sema , const ObjCIvarRefExpr *OIRE,


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,14 @@
   test18_e(); // expected-error {{too few arguments to function call, expected at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_AS256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
+   *(int_AS256*)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) &&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_through_null));
+}
 }
 
 static void DiagnoseDirectIsaAccess(Sema , const ObjCIvarRefExpr *OIRE,
___
cfe-commits mailing list

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

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

xbolva00 added a comment.

In D69435#1728674 , @AntonBikineev 
wrote:

> BTW, this very contrived benchmark shows 5x: 
> http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew


There is no difference in perf for GCC.


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

https://reviews.llvm.org/D69435



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

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 5 inline comments as done.
mibintc added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:238
 CODEGENOPT(UnsafeFPMath  , 1, 0) ///< Allow unsafe floating point optzns.
+CODEGENOPT(RoundingFPMath, 1, 0) ///< Rounding floating point optzns.
 CODEGENOPT(UnwindTables  , 1, 0) ///< Emit unwind tables.

mibintc wrote:
> rjmccall wrote:
> > Why do we need both a code-gen option and a language option?
> The main reason i added it to LangOptions.h is because I saw the FPContract 
> support in there and I thought I'd get on that bandwagon.  My ultimate goal, 
> after committing the command line options, is to add support for controlling 
> rounding mode and exception behavior with pragma's embedded in the functions, 
> similar to https://reviews.llvm.org/D69272.  
> 
> There's a patch here that I like, to add rounding-mode and exception-behavior 
> to FPOptions https://reviews.llvm.org/D65994, but it hasn't been committed 
> yet.
> 
I dropped the code-gen option.



Comment at: clang/include/clang/Basic/LangOptions.h:366
+  FPEB = Value;
+}
+

rjmccall wrote:
> Everything here is a "setting", and in the context of this type they're all 
> FP.  Please name these methods something like `getRoundingMode()`.
> 
> Does this structure really need to exist as opposed to tracking the 
> dimensions separately?  Don't we already track some of this somewhere?  We 
> should subsume that state into these values rather than tracking them 
> separately.
I fixed the spelling, I also dropped the structure and used the ENUM_OPT macro 
instead of writing out the setter and getter. Look OK now?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D69664: [Diagnostics] Teach -Wnull-dereference about address_space attribute

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

xbolva00 created this revision.
xbolva00 added reviewers: aaron.ballman, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang should not warn for:
test.c:2:12: warning: indirection of non-volatile null pointer will be deleted,

not trap [-Wnull-dereference]
  return *(int __attribute__((address_space(256))) *) 0;
 ^~

Solves PR42292.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69664

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/exprs.c


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,13 @@
   test18_e(); // expected-error {{too few arguments to function call, expected 
at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_as_256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null 
pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null 
pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) 
&&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_through_null));
+}
 }
 
 static void DiagnoseDirectIsaAccess(Sema , const ObjCIvarRefExpr *OIRE,


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,13 @@
   test18_e(); // expected-error {{too few arguments to function call, expected at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_as_256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) &&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I have just updated D69383 .

It now defaults to `rv{XLEN}imac` on ELF, rather than `rv{XLEN}gc`. This is to 
help this multilib issue.

In the future, we want to implement MULTILIB_REUSE properly. Hopefully before 
Clang 10.0.0 is released.


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

https://reviews.llvm.org/D67508



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

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 227283.
mibintc added a comment.

I followed up on some code review remarks from @rjmccall. I dropped the CODEGEN 
option and fixed some code formatting. I changed the spelling of the 
enumeration values for RoundingMode and ExceptionMode to match those proposed 
in https://reviews.llvm.org/D65994


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
Index: clang/test/Driver/fp-model.c
===
--- /dev/null
+++ clang/test/Driver/fp-model.c
@@ -0,0 +1,130 @@
+// Test that incompatible combinations of -ffp-model= options
+// and other floating point options get a warning diagnostic.
+//
+// REQUIRES: clang-driver
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN %s
+// WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN1 %s
+// WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN2 %s
+// WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN3 %s
+// WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN4 %s
+// WARN4: warning: overriding '-ffp-model=strict' option with '-ffinite-math-only' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN5 %s
+// WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN6 %s
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN7 %s
+// WARN7: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN8 %s
+// WARN8: warning: overriding '-ffp-model=strict' option with '-fno-honor-infinities' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN9 %s
+// WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNa %s
+// WARNa: warning: overriding '-ffp-model=strict' option with '-fno-rounding-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-signed-zeros -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNb %s
+// WARNb: warning: overriding '-ffp-model=strict' option with '-fno-signed-zeros' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-trapping-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNc %s
+// WARNc: warning: overriding '-ffp-model=strict' option with '-fno-trapping-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -freciprocal-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNd %s
+// WARNd: warning: 

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 227284.
lenary added a comment.

- More conservative march/mabi defaults on riscv-unknown-elf
- Add clang release notes about this change to -march/-mabi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-gnutools.c

Index: clang/test/Driver/riscv-gnutools.c
===
--- clang/test/Driver/riscv-gnutools.c
+++ clang/test/Driver/riscv-gnutools.c
@@ -1,19 +1,19 @@
 // Check gnutools are invoked with propagated values for -mabi and -march.
 
 // RUN: %clang -target riscv32 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D %s
 
 // RUN: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D-MARCH-G %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D-MARCH-G %s
 
-// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
-// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32gc"
+// MABI-ILP32D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32g"
 
-// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
-// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
+// MABI-ILP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d"
+// MABI-ILP64D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64g"
Index: clang/test/Driver/riscv-abi.c
===
--- clang/test/Driver/riscv-abi.c
+++ clang/test/Driver/riscv-abi.c
@@ -1,9 +1,5 @@
-// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
-// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
@@ -14,6 +10,10 @@
 
 // CHECK-ILP32F: "-target-abi" "ilp32f"
 
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd -mabi=ilp32d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 
@@ -24,12 +24,8 @@
 
 // CHECK-RV32-LP64: error: unknown target ABI 'lp64'
 
-// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
-// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
 // RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
@@ -40,6 +36,10 @@
 
 // CHECK-LP64F: "-target-abi" "lp64f"
 
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D  %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d -mabi=lp64d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -709,11 +709,9 @@
 StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
 CmdArgs.push_back("-mabi");
 CmdArgs.push_back(ABIName.data());
-if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-  StringRef MArch = A->getValue();
-  CmdArgs.push_back("-march");
-  CmdArgs.push_back(MArch.data());
-}
+StringRef MArchName = 

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Never mind, I got it resolved and pushed. Sorry for the noise.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69649



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

BTW, this very contrived benchmark shows 5x: 
http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew


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

https://reviews.llvm.org/D69435



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

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "" Parameter

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d7bd5752648: [clang-format] Fix SpacesInSquareBrackets for 
Lambdas with Initial ref… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69649

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
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [  ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =,  ]() {};", Spaces);
+  verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [  ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =,  ]() {};", Spaces);
+  verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous &&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8d7bd57 - [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "" Parameter

[Mitchell Balan via cfe-commits]

Author: Mitchell Balan
Date: 2019-10-31T11:08:05-04:00
New Revision: 8d7bd57526486cab9e3daba9934042c405d7946b

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

LOG: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "" 
Parameter

Summary:
This fixes an edge case in the `SpacesInSquareBrackets` option where an initial 
`` lambda parameter is not padded with an initial space.

`int foo = [ ]() {}` is fixed to give `int foo = [  ]() {}`

Reviewers: MyDeveloperDay, klimek, sammccall

Reviewed by: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 1ed35597d075..98b87c0f5a25 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine ,
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index e0ebef1f7ced..ea03ee1c3cc9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@ TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) 
{
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [  ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =,  ]() {};", Spaces);
+  verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {



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

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

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

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a project: clang.

Use of evalCall is replaced by preCall and postCall.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69662

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -49,22 +49,21 @@
   }
 };
 
-class StreamChecker : public Checker {
+class StreamChecker
+: public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
-  bool evalCall(const CallEvent , CheckerContext ) const;
+  void checkPreCall(const CallEvent , CheckerContext ) const;
+  void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 
 private:
   using FnCheck = std::function;
 
-  CallDescriptionMap Callbacks = {
-  {{"fopen"}, ::evalFopen},
-  {{"tmpfile"}, ::evalFopen},
+  CallDescriptionMap PreCallbacks = {
   {{"fclose", 1}, ::evalFclose},
   {{"fread", 4},
std::bind(::checkArgNullStream, _1, _2, _3, 3)},
@@ -89,10 +88,17 @@
std::bind(::checkArgNullStream, _1, _2, _3, 0)},
   };
 
+  CallDescriptionMap PostCallbacks = {
+  {{"fopen"}, ::evalFopen},
+  {{"tmpfile"}, ::evalFopen},
+  };
+
   void evalFopen(const CallEvent , CheckerContext ) const;
   void evalFclose(const CallEvent , CheckerContext ) const;
   void evalFseek(const CallEvent , CheckerContext ) const;
 
+  void checkCall(const CallEvent , CheckerContext ,
+ const CallDescriptionMap ) const;
   void checkArgNullStream(const CallEvent , CheckerContext ,
   unsigned ArgI) const;
   bool checkNullStream(SVal SV, CheckerContext ,
@@ -107,29 +113,38 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
+void StreamChecker::checkPreCall(const CallEvent ,
+ CheckerContext ) const {
+  checkCall(Call, C, PreCallbacks);
+}
+
+void StreamChecker::checkPostCall(const CallEvent ,
+  CheckerContext ) const {
+  checkCall(Call, C, PostCallbacks);
+}
 
-bool StreamChecker::evalCall(const CallEvent , CheckerContext ) const {
+void StreamChecker::checkCall(
+const CallEvent , CheckerContext ,
+const CallDescriptionMap ) const {
   const auto *FD = dyn_cast_or_null(Call.getDecl());
   if (!FD || FD->getKind() != Decl::Function)
-return false;
+return;
 
   // Recognize "global C functions" with only integral or pointer arguments
   // (and matching name) as stream functions.
   if (!Call.isGlobalCFunction())
-return false;
+return;
   for (auto P : Call.parameters()) {
 QualType T = P->getType();
 if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
-  return false;
+  return;
   }
 
   const FnCheck *Callback = Callbacks.lookup(Call);
   if (!Callback)
-return false;
+return;
 
   (*Callback)(this, Call, C);
-
-  return C.isDifferent();
 }
 
 void StreamChecker::evalFopen(const CallEvent , CheckerContext ) const {


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -49,22 +49,21 @@
   }
 };
 
-class StreamChecker : public Checker {
+class StreamChecker
+: public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
-  bool evalCall(const CallEvent , CheckerContext ) const;
+  void checkPreCall(const CallEvent , CheckerContext ) const;
+  void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 
 private:
   using FnCheck = std::function;
 
-  CallDescriptionMap Callbacks = {
-  {{"fopen"}, ::evalFopen},
-  {{"tmpfile"}, ::evalFopen},
+  CallDescriptionMap PreCallbacks = {
   {{"fclose", 1}, ::evalFclose},
   {{"fread", 4},
std::bind(::checkArgNullStream, _1, _2, _3, 3)},
@@ -89,10 +88,17 @@
std::bind(::checkArgNullStream, _1, _2, _3, 0)},
   };
 
+  CallDescriptionMap PostCallbacks = {
+  {{"fopen"}, ::evalFopen},
+  {{"tmpfile"}, ::evalFopen},
+  };
+
   void evalFopen(const CallEvent , CheckerContext ) const;
   void evalFclose(const CallEvent , CheckerContext ) const;
   void evalFseek(const CallEvent , CheckerContext ) const;
 
+  void checkCall(const CallEvent , CheckerContext ,
+ const CallDescriptionMap ) const;
   void checkArgNullStream(const CallEvent , CheckerContext ,
   unsigned ArgI) const;
   bool checkNullStream(SVal SV, CheckerContext 

[PATCH] D69263: [clangd] Implement cross-file rename.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59816 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Thanks!


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

https://reviews.llvm.org/D69435



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

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Thanks. Would you mind committing this on my behalf? It seems I wasn't migrated 
from SVN access to git access. It may take some time to sort out.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69649



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> Did you see some significant perf improvements for Chromium?

I don't see major improvements in browser benchmarks like Speedometer, but in 
the Blink garbage collector, which queues destructors to be executed later, 
this change proved to reduce the number of them by 5%. That, in turn, reduces 
GC time on the main thread which is significant for latency.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 227278.
hokein marked 8 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer , PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer , PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer , PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer , PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,11 +96,11 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer ,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
-  Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
+llvm::Expected runRename(ClangdServer , PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
+  Server.rename(File, Pos, NewName, /*WantFormat=*/true,
+/*DirtyBuffer=*/nullptr, capture(Result));
   return std::move(*Result);
 }
 
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -9,8 +9,10 @@
 #include "Annotations.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,9 +20,68 @@
 namespace clangd {
 namespace {
 
+using testing::Eq;
+using testing::Matches;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
 MATCHER_P2(RenameRange, Code, Range, "") {
   return replacementToEdit(Code, arg).range == Range;
 }
+MATCHER_P2(EqualFileEdit, Code, Ranges, "") {
+  using ReplacementMatcher = testing::Matcher;
+  std::vector Expected;
+  for (const auto  : Ranges)
+Expected.push_back(RenameRange(Code, R));
+  auto Matcher =
+  testing::internal::UnorderedElementsAreArrayMatcher(
+  Expected.begin(), Expected.end());
+  return arg.InitialCode == Code && Matches(Matcher)(arg.Replacements);
+}
+
+std::unique_ptr buildRefSlab(const Annotations ,
+  llvm::StringRef SymbolName,
+  llvm::StringRef Path) {
+  RefSlab::Builder Builder;
+  TestTU TU;
+  TU.HeaderCode = Code.code();
+  auto Symbols = TU.headerSymbols();
+  const auto  = findSymbol(Symbols, SymbolName).ID;
+  for (const auto  : Code.ranges()) {
+Ref R;
+R.Kind = RefKind::Reference;
+R.Location.Start.setLine(Range.start.line);
+R.Location.Start.setColumn(Range.start.character);
+R.Location.End.setLine(Range.end.line);
+R.Location.End.setColumn(Range.end.character);
+auto U = URI::create(Path).toString();
+R.Location.FileURI = U.c_str();
+Builder.insert(SymbolID, R);
+  }
+
+  return std::make_unique(std::move(Builder).build());
+}
+
+RenameInputs renameInputs(const Annotations , llvm::StringRef NewName,
+  llvm::StringRef Path,
+  const SymbolIndex *Index = nullptr,
+  bool CrossFile = false) {
+  RenameInputs Inputs;
+  Inputs.Pos = Code.point();
+  Inputs.MainFileCode = Code.code();
+  Inputs.MainFilePath = Path;
+  Inputs.NewName = NewName;
+  Inputs.Index = Index;
+  Inputs.AllowCrossFile = CrossFile;
+  return Inputs;
+}
+
+std::vector> toVector(FileEdits FE) {
+  std::vector> Results;
+  for (auto  : FE)
+Results.emplace_back(It.first().str(), std::move(It.getValue()));
+  return Results;
+}
 
 TEST(RenameTest, SingleFile) {
   struct Test {
@@ -80,10 +141,13 @@
 

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:219
+  // us whether the ref results is completed.
+  RQuest.Limit = 100;
+  if (auto ID = getSymbolID(RenameDecl))

ilya-biryukov wrote:
> Why do we need to limit the number of references in the first place?
I was thinking this is for performance, if the index returns too many results 
(the number of affected files >> our limit), we will ends up doing many 
unnecessary work (like resolving URIs).  Removed it now, it seems a premature 
optimization, we can revisit this afterwards.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+llvm::Expected
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+  llvm::StringRef NewName, const SymbolIndex *Index,

ilya-biryukov wrote:
> NIT: Maybe call it `renameWithIndex` instead?
> It should capture what this function is doing better...
> 
> But up to you
I would keep the current name, as it is symmetrical to the `renameWithinFile`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Thanks James - won't this still leave problems for structs that need flattening?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69590



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

[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

What about codegen tests?




Comment at: clang/test/OpenMP/target_update_ast_print.cpp:25
+  U marr[10][10][10];
+#pragma omp target update to(marr[2][0:2][0:2])
+

It seems to me, the tests are not updated. All non-contiguous tests must pass 
only if OpenMP version is 50


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69316



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

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 227273.
SjoerdMeijer added a comment.

Thanks Michael!

- moved the handling of vectorize.enable to 1 place,
- that should have also sorted the relative ordering, and duplication of the 
metadata in some cases,
- created a separate file for the new tests, and added some more tests (also 
making sure there are no duplicates).


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

https://reviews.llvm.org/D69628

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp

Index: clang/test/CodeGenCXX/pragma-loop.cpp
===
--- clang/test/CodeGenCXX/pragma-loop.cpp
+++ clang/test/CodeGenCXX/pragma-loop.cpp
@@ -161,20 +161,20 @@
 // CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}
 // CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
 
-// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}
+// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE:.*]]}
 // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
 // CHECK: ![[DISTRIBUTE_DISABLE]] = !{!"llvm.loop.distribute.enable", i1 false}
 // CHECK: ![[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}
 // CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}
+// CHECK: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
 
-// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[INTENABLE_1:.*]], ![[FOLLOWUP_VECTOR_3:.*]]}
-// CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3:.*]]}
 // CHECK: ![[FOLLOWUP_VECTOR_3]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_3:.*]]}
 // CHECK: ![[AFTER_VECTOR_3]] = distinct !{![[AFTER_VECTOR_3]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 // CHECK: ![[ISVECTORIZED]] = !{!"llvm.loop.isvectorized"}
 // CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}
 
-// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}
+// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]], ![[VECTORIZE_ENABLE]]}
 // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
 
@@ -185,7 +185,7 @@
 // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_6:.*]]}
 // CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 
-// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]]}
+// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]], ![[VECTORIZE_ENABLE]]}
 // CHECK: ![[WIDTH_5]] = !{!"llvm.loop.vectorize.width", i32 5}
 
 // CHECK: ![[LOOP_8]] = distinct !{![[LOOP_8]], ![[WIDTH_5:.*]]}
@@ -207,11 +207,11 @@
 // CHECK: ![[AFTER_VECTOR_12]] = distinct !{![[AFTER_VECTOR_12]], ![[ISVECTORIZED:.*]], ![[UNROLL_24:.*]]}
 // CHECK: ![[UNROLL_24]] = !{!"llvm.loop.unroll.count", i32 24}
 
-// CHECK: ![[LOOP_13]] = distinct !{![[LOOP_13]], ![[WIDTH_8:.*]], ![[INTERLEAVE_16:.*]], ![[FOLLOWUP_VECTOR_13:.*]]}
+// CHECK: ![[LOOP_13]] = distinct !{![[LOOP_13]], ![[WIDTH_8:.*]], ![[INTERLEAVE_16:.*]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_13:.*]]}
 // CHECK: ![[INTERLEAVE_16]] = !{!"llvm.loop.interleave.count", i32 16}
 // CHECK: ![[FOLLOWUP_VECTOR_13]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_13:.*]]}
 // CHECK: ![[AFTER_VECTOR_13]] = distinct !{![[AFTER_VECTOR_13]], ![[ISVECTORIZED:.*]], ![[UNROLL_32:.*]]}
 // CHECK: ![[UNROLL_32]] = !{!"llvm.loop.unroll.count", i32 32}
 
-// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]]}
+// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]], ![[VECTORIZE_ENABLE]]}
 // CHECK: ![[WIDTH_10]] = !{!"llvm.loop.vectorize.width", i32 10}
Index: clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void loop1(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}loop1{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP1:.*]]
+
+  #pragma clang loop vectorize(enable) vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+// Here, vectorize.enable should be set, obviously, but also check that
+// metadata isn't added twice.
+void loop2(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}loop2{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP2:.*]]
+
+  #pragma clang loop vectorize(enable) vectorize_width(2)
+  for (int i 

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Tests are happy again after 1c66d09 
 , thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ilya-biryukov marked an inline comment as done.
Closed by commit rG733777a81662: [clangd] Fix namespace aliases in 
findExplicitReferences (authored by ilya-biryukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69624

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  DynTypedNode::create(*NNSL->getNestedNameSpecifier()),
+  DeclRelation::Alias)}};
+}
 if (const TypeLoc *TL = N.get())
   return refInTypeLoc(*TL);
 if (const CXXCtorInitializer *CCI = N.get()) {


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  

[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks, mostly LG!

The only important comment I have left is about limiting the number of 
references. Others are NITs.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:219
+  // us whether the ref results is completed.
+  RQuest.Limit = 100;
+  if (auto ID = getSymbolID(RenameDecl))

Why do we need to limit the number of references in the first place?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:223
+
+  // Helper.
+  auto ToRange = [](const SymbolLocation ) {

Maybe remove this comment?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:224
+  // Helper.
+  auto ToRange = [](const SymbolLocation ) {
+Range R;

Why not extract this as a helper function?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:233
+  llvm::StringMap>
+  AffectedFiles; // absolute file path => rename ocurrences in that file.
+  Index->refs(RQuest, [&](const Ref ) {

NIT: maybe put this comment on the line before the declaration?
Should lead to better formatting


Also, this comment would be most useful on the function declaration. Maybe move 
it there?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+llvm::Expected
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+  llvm::StringRef NewName, const SymbolIndex *Index,

NIT: Maybe call it `renameWithIndex` instead?
It should capture what this function is doing better...

But up to you



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:289
+
+  // The cross-file rename is purely based on the index, as we don't want to
+  // build all ASTs for affected files, which may cause a performance hit.

Maybe move this comment to the function itself?
It definitely does a great job of capturing what this function is doing and 
what the trade-offs are.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:277
+  //   - file on VFS for bar.cc;
+  Annotations MainCode("class  [[Fo^o]] {};");
+  auto MainFilePath = testPath("main.cc");

hokein wrote:
> ilya-biryukov wrote:
> > Thinking whether we can encode these transformations in a nicer way?
> > If we could convert the contents dirty buffer and replacements to something 
> > like:
> > ```
> > class [[Foo |-> newName]] {};
> > ```
> > Just a proposal, maybe there's a better syntax here.
> > 
> > We can easily match strings instead of matching ranges in the tests. This 
> > has the advantage of having textual diffs in case something goes wrong - 
> > much more pleasant than looking at the offsets in the ranges.
> > 
> > WDYT?
> I agree textual diff would give a more friendly results when there are 
> failures in the tests.
> 
> I don't have a better way to encode the transformation in the annotation 
> code, I think a better way maybe to use a hard-coded new name, and applying 
> the actual replacements on the testing code, and verify the the text diffs.
> 
> If we want to do this change, I'd prefer to do it in a followup, since it 
> would change the existing testcases as well. What do you think?
> 
Maybe I'm overthinking it... For rename having just the range is probably 
enough.



Comment at: clang-tools-extra/clangd/unittests/SyncAPI.cpp:103
+  Server.rename(File, Pos, NewName, /*WantFormat=*/true,
+/*DirtyBuffaer*/ nullptr, capture(Result));
   return std::move(*Result);

s/DirtyBuffaer/DirtyBuffer

also use `/*DirtryBuffer=*/` to be consistent with `/*WantFormat=*/` from the 
previous line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[clang-tools-extra] 733777a - [clangd] Fix namespace aliases in findExplicitReferences

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2019-10-31T13:35:25+01:00
New Revision: 733777a81662c40960e9298bb59da8c39a14f8d5

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

LOG: [clangd] Fix namespace aliases in findExplicitReferences

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: merge_guards_bot, MaskRay, jkorous, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 1ab80b72a90b..4e61d22dd7fb 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@ class ExplicitReferenceColletor
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  DynTypedNode::create(*NNSL->getNestedNameSpecifier()),
+  DeclRelation::Alias)}};
+}
 if (const TypeLoc *TL = N.get())
   return refInTypeLoc(*TL);
 if (const CXXCtorInitializer *CCI = N.get()) {

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 6b0e51b228b8..8f7c7aaeaefe 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@ TEST_F(FindExplicitReferencesTest, All) {
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto  : Cases) {



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

[clang-tools-extra] 1c66d09 - [clangd] Add fno-delayed-parsing to new define inline tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-31T13:22:52+01:00
New Revision: 1c66d09b739a8d9717ba4e9507649bc45ddf7f0d

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

LOG: [clangd] Add fno-delayed-parsing to new define inline tests

To unbreak windows buildbots.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 126aab61ada5..5a6df2e03e67 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1529,6 +1529,7 @@ est);
 void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){}
 )cpp"},
   };
+  ExtraArgs.push_back("-fno-delayed-template-parsing");
   for (const auto  : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
@@ -1559,6 +1560,7 @@ TEST_F(DefineInlineTest, TransformTemplParamNames) {
 };
 
 )cpp";
+  ExtraArgs.push_back("-fno-delayed-template-parsing");
   EXPECT_EQ(apply(Test), Expected);
 }
 



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

Re: [PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Kadir Çetinkaya via cfe-commits]

done. sorry for forgetting about this so soon.

On Thu, Oct 31, 2019 at 12:58 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added inline comments.
>
>
> 
> Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1388
>// results in arbitrary failures as function body becomes NULL.
>ExtraArgs.push_back("-fno-delayed-template-parsing");
> +  for (const auto  : Cases)
> 
> You might need something like this in some of the new tests.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68937/new/
>
> https://reviews.llvm.org/D68937
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits