[PATCH] D47111: : Implement monotonic_buffer_resource.

2018-06-10 Thread Arthur O'Dwyer via Phabricator via cfe-commits 
Quuxplusone added inline comments.



Comment at: include/experimental/memory_resource:489
+memory_resource* __res_;
+size_t __next_buffer_size_;
+};

I've discovered that Boost.Container does not bother to preserve this state 
across calls to `release()`. If that's legal, then we can save 8 bytes here. 
I've asked for an LWG issue to be opened on the subject of "what the heck is 
`release()` supposed to do anyway."


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



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

[PATCH] D47693: [X86] Use target independent masked expandload and compressstore intrinsics to implement expandload/compressstore builtins.

2018-06-10 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334366: [X86] Use target independent masked expandload and 
compressstore intrinsics to… (authored by ctopper, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47693?vs=149657=150654#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47693

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGen/avx512f-builtins.c
  cfe/trunk/test/CodeGen/avx512vbmi2-builtins.c
  cfe/trunk/test/CodeGen/avx512vl-builtins.c
  cfe/trunk/test/CodeGen/avx512vlvbmi2-builtins.c

Index: cfe/trunk/test/CodeGen/avx512f-builtins.c
===
--- cfe/trunk/test/CodeGen/avx512f-builtins.c
+++ cfe/trunk/test/CodeGen/avx512f-builtins.c
@@ -7293,40 +7293,52 @@
 }
 __m512i test_mm512_mask_expandloadu_epi64(__m512i __W, __mmask8 __U, void const *__P) {
   // CHECK-LABEL: @test_mm512_mask_expandloadu_epi64
-  // CHECK: @llvm.x86.avx512.mask.expand.load.q.512
+  // CHECK: @llvm.masked.expandload.v8i64(i64* %{{.*}}, <8 x i1> %{{.*}}, <8 x i64> %{{.*}})
   return _mm512_mask_expandloadu_epi64(__W, __U, __P); 
 }
 
 __m512i test_mm512_maskz_expandloadu_epi64(__mmask8 __U, void const *__P) {
   // CHECK-LABEL: @test_mm512_maskz_expandloadu_epi64
-  // CHECK: @llvm.x86.avx512.mask.expand.load.q.512
+  // CHECK: @llvm.masked.expandload.v8i64(i64* %{{.*}}, <8 x i1> %{{.*}}, <8 x i64> %{{.*}})
   return _mm512_maskz_expandloadu_epi64(__U, __P); 
 }
 
 __m512d test_mm512_mask_expandloadu_pd(__m512d __W, __mmask8 __U, void const *__P) {
   // CHECK-LABEL: @test_mm512_mask_expandloadu_pd
-  // CHECK: @llvm.x86.avx512.mask.expand.load.pd.512
+  // CHECK: @llvm.masked.expandload.v8f64(double* %{{.*}}, <8 x i1> %{{.*}}, <8 x double> %{{.*}})
   return _mm512_mask_expandloadu_pd(__W, __U, __P); 
 }
 
 __m512d test_mm512_maskz_expandloadu_pd(__mmask8 __U, void const *__P) {
   // CHECK-LABEL: @test_mm512_maskz_expandloadu_pd
-  // CHECK: @llvm.x86.avx512.mask.expand.load.pd.512
+  // CHECK: @llvm.masked.expandload.v8f64(double* %{{.*}}, <8 x i1> %{{.*}}, <8 x double> %{{.*}})
   return _mm512_maskz_expandloadu_pd(__U, __P); 
 }
 
 __m512i test_mm512_mask_expandloadu_epi32(__m512i __W, __mmask16 __U, void const *__P) {
   // CHECK-LABEL: @test_mm512_mask_expandloadu_epi32
-  // CHECK: @llvm.x86.avx512.mask.expand.load.d.512
+  // CHECK: @llvm.masked.expandload.v16i32(i32* %{{.*}}, <16 x i1> %{{.*}}, <16 x i32> %{{.*}})
   return _mm512_mask_expandloadu_epi32(__W, __U, __P); 
 }
 
 __m512i test_mm512_maskz_expandloadu_epi32(__mmask16 __U, void const *__P) {
   // CHECK-LABEL: @test_mm512_maskz_expandloadu_epi32
-  // CHECK: @llvm.x86.avx512.mask.expand.load.d.512
+  // CHECK: @llvm.masked.expandload.v16i32(i32* %{{.*}}, <16 x i1> %{{.*}}, <16 x i32> %{{.*}})
   return _mm512_maskz_expandloadu_epi32(__U, __P); 
 }
 
+__m512 test_mm512_mask_expandloadu_ps(__m512 __W, __mmask16 __U, void const *__P) {
+  // CHECK-LABEL: @test_mm512_mask_expandloadu_ps
+  // CHECK: @llvm.masked.expandload.v16f32(float* %{{.*}}, <16 x i1> %{{.*}}, <16 x float> %{{.*}})
+  return _mm512_mask_expandloadu_ps(__W, __U, __P); 
+}
+
+__m512 test_mm512_maskz_expandloadu_ps(__mmask16 __U, void const *__P) {
+  // CHECK-LABEL: @test_mm512_maskz_expandloadu_ps
+  // CHECK: @llvm.masked.expandload.v16f32(float* %{{.*}}, <16 x i1> %{{.*}}, <16 x float> %{{.*}})
+  return _mm512_maskz_expandloadu_ps(__U, __P); 
+}
+
 __m512 test_mm512_mask_expand_ps(__m512 __W, __mmask16 __U, __m512 __A) {
   // CHECK-LABEL: @test_mm512_mask_expand_ps
   // CHECK: @llvm.x86.avx512.mask.expand.ps.512
@@ -7428,25 +7440,25 @@
 
 void test_mm512_mask_compressstoreu_pd(void *__P, __mmask8 __U, __m512d __A) {
   // CHECK-LABEL: @test_mm512_mask_compressstoreu_pd
-  // CHECK: @llvm.x86.avx512.mask.compress.store.pd.512
+  // CHECK: @llvm.masked.compressstore.v8f64(<8 x double> %{{.*}}, double* %{{.*}}, <8 x i1> %{{.*}})
   return _mm512_mask_compressstoreu_pd(__P, __U, __A); 
 }
 
 void test_mm512_mask_compressstoreu_epi64(void *__P, __mmask8 __U, __m512i __A) {
   // CHECK-LABEL: @test_mm512_mask_compressstoreu_epi64
-  // CHECK: @llvm.x86.avx512.mask.compress.store.q.512
+  // CHECK: @llvm.masked.compressstore.v8i64(<8 x i64> %{{.*}}, i64* %{{.*}}, <8 x i1> %{{.*}})
   return _mm512_mask_compressstoreu_epi64(__P, __U, __A); 
 }
 
 void test_mm512_mask_compressstoreu_ps(void *__P, __mmask16 __U, __m512 __A) {
   // CHECK-LABEL: @test_mm512_mask_compressstoreu_ps
-  // CHECK: @llvm.x86.avx512.mask.compress.store.ps.512
+  // CHECK: @llvm.masked.compressstore.v16f32(<16 x float> %{{.*}}, float* %{{.*}}, <16 x i1> %{{.*}})
   return _mm512_mask_compressstoreu_ps(__P, __U, __A); 
 }
 
 void test_mm512_mask_compressstoreu_epi32(void *__P, __mmask16 __U, __m512i __A) {
   // CHECK-LABEL: @test_mm512_mask_compressstoreu_epi32
-  // CHECK: @llvm.x86.avx512.mask.compress.store.d.512
+  // CHECK: @llvm.masked.compressstore.v16i32(<16 x i32>

r334366 - [X86] Use target independent masked expandload and compressstore intrinsics to implement expandload/compressstore builtins.

2018-06-10 Thread Craig Topper via cfe-commits 
Author: ctopper
Date: Sun Jun 10 10:27:05 2018
New Revision: 334366

URL: http://llvm.org/viewvc/llvm-project?rev=334366=rev
Log:
[X86] Use target independent masked expandload and compressstore intrinsics to 
implement expandload/compressstore builtins.

Summary: We've had these target independent intrinsics for at least a year and 
a half. Looks like they do exactly what we need here and the backend already 
supports them.

Reviewers: RKSimon, delena, spatel, GBuella

Reviewed By: RKSimon

Subscribers: cfe-commits, llvm-commits

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

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/avx512f-builtins.c
cfe/trunk/test/CodeGen/avx512vbmi2-builtins.c
cfe/trunk/test/CodeGen/avx512vl-builtins.c
cfe/trunk/test/CodeGen/avx512vlvbmi2-builtins.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=334366=334365=334366=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Sun Jun 10 10:27:05 2018
@@ -8496,6 +8496,40 @@ static Value *EmitX86MaskedLoad(CodeGenF
   return CGF.Builder.CreateMaskedLoad(Ptr, Align, MaskVec, Ops[1]);
 }
 
+static Value *EmitX86ExpandLoad(CodeGenFunction ,
+ArrayRef Ops) {
+  llvm::Type *ResultTy = Ops[1]->getType();
+  llvm::Type *PtrTy = ResultTy->getVectorElementType();
+
+  // Cast the pointer to element type.
+  Value *Ptr = CGF.Builder.CreateBitCast(Ops[0],
+ llvm::PointerType::getUnqual(PtrTy));
+
+  Value *MaskVec = getMaskVecValue(CGF, Ops[2],
+   ResultTy->getVectorNumElements());
+
+  llvm::Function *F = CGF.CGM.getIntrinsic(Intrinsic::masked_expandload,
+   ResultTy);
+  return CGF.Builder.CreateCall(F, { Ptr, MaskVec, Ops[1] });
+}
+
+static Value *EmitX86CompressStore(CodeGenFunction ,
+   ArrayRef Ops) {
+  llvm::Type *ResultTy = Ops[1]->getType();
+  llvm::Type *PtrTy = ResultTy->getVectorElementType();
+
+  // Cast the pointer to element type.
+  Value *Ptr = CGF.Builder.CreateBitCast(Ops[0],
+ llvm::PointerType::getUnqual(PtrTy));
+
+  Value *MaskVec = getMaskVecValue(CGF, Ops[2],
+   ResultTy->getVectorNumElements());
+
+  llvm::Function *F = CGF.CGM.getIntrinsic(Intrinsic::masked_compressstore,
+   ResultTy);
+  return CGF.Builder.CreateCall(F, { Ops[1], Ptr, MaskVec });
+}
+
 static Value *EmitX86MaskLogic(CodeGenFunction , Instruction::BinaryOps 
Opc,
   unsigned NumElts, ArrayRef Ops,
   bool InvertLHS = false) {
@@ -9219,6 +9253,46 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 return EmitX86MaskedLoad(*this, Ops, Align);
   }
 
+  case X86::BI__builtin_ia32_expandloaddf128_mask:
+  case X86::BI__builtin_ia32_expandloaddf256_mask:
+  case X86::BI__builtin_ia32_expandloaddf512_mask:
+  case X86::BI__builtin_ia32_expandloadsf128_mask:
+  case X86::BI__builtin_ia32_expandloadsf256_mask:
+  case X86::BI__builtin_ia32_expandloadsf512_mask:
+  case X86::BI__builtin_ia32_expandloaddi128_mask:
+  case X86::BI__builtin_ia32_expandloaddi256_mask:
+  case X86::BI__builtin_ia32_expandloaddi512_mask:
+  case X86::BI__builtin_ia32_expandloadsi128_mask:
+  case X86::BI__builtin_ia32_expandloadsi256_mask:
+  case X86::BI__builtin_ia32_expandloadsi512_mask:
+  case X86::BI__builtin_ia32_expandloadhi128_mask:
+  case X86::BI__builtin_ia32_expandloadhi256_mask:
+  case X86::BI__builtin_ia32_expandloadhi512_mask:
+  case X86::BI__builtin_ia32_expandloadqi128_mask:
+  case X86::BI__builtin_ia32_expandloadqi256_mask:
+  case X86::BI__builtin_ia32_expandloadqi512_mask:
+return EmitX86ExpandLoad(*this, Ops);
+
+  case X86::BI__builtin_ia32_compressstoredf128_mask:
+  case X86::BI__builtin_ia32_compressstoredf256_mask:
+  case X86::BI__builtin_ia32_compressstoredf512_mask:
+  case X86::BI__builtin_ia32_compressstoresf128_mask:
+  case X86::BI__builtin_ia32_compressstoresf256_mask:
+  case X86::BI__builtin_ia32_compressstoresf512_mask:
+  case X86::BI__builtin_ia32_compressstoredi128_mask:
+  case X86::BI__builtin_ia32_compressstoredi256_mask:
+  case X86::BI__builtin_ia32_compressstoredi512_mask:
+  case X86::BI__builtin_ia32_compressstoresi128_mask:
+  case X86::BI__builtin_ia32_compressstoresi256_mask:
+  case X86::BI__builtin_ia32_compressstoresi512_mask:
+  case X86::BI__builtin_ia32_compressstorehi128_mask:
+  case X86::BI__builtin_ia32_compressstorehi256_mask:
+  case X86::BI__builtin_ia32_compressstorehi512_mask:
+  case X86::BI__builtin_ia32_compressstoreqi128_mask:
+  case X86::BI__builtin_ia32_compressstoreqi256_mask:
+  case

[PATCH] D47979: [X86] Lowering Mask Scalar add/sub/mul/div intrinsics to native IR (Clang part)

2018-06-10 Thread Craig Topper via Phabricator via cfe-commits 
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D47979



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

[PATCH] D47979: [X86] Lowering Mask Scalar add/sub/mul/div intrinsics to native IR (Clang part)

2018-06-10 Thread Tomasz Krupa via Phabricator via cfe-commits 
tkrupa added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:9926
+Value *Div = Builder.CreateFDiv(A, B);
+llvm::VectorType *MaskTy = llvm::VectorType::get(Builder.getInt1Ty(),
+ 
cast(Mask->getType())->getBitWidth());

craig.topper wrote:
> Can we just emit the and+icmp that the other operations end up with?
We can't - if select condition is a CmpInst, CodeGenPrepare::optimizeSelectInst 
replaces it with a branch condition in case of expensive operations such as 
div. That's the reason I'm handling it in CGBuiltin in the first place.


Repository:
  rC Clang

https://reviews.llvm.org/D47979



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

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

2018-06-10 Thread Hamza Sood via Phabricator via cfe-commits 
hamzasood marked 2 inline comments as done.
hamzasood added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:16185-16194
+  if (getLangOpts().ModulesTS) {
+Module *CurrentModule = getCurrentModule();
+assert(CurrentModule && "Expected to be in a module scope");
+
+// If the current module has been loaded from disk, then this is an
+// implementation unit and hence we shouldn't modify the module.
+// FIXME: Is that a hacky assumption? We can't just check

rsmith wrote:
> This is not appropriate; generally whether we serialize to an AST file should 
> be treated as orthogonal to whether we're in / importing a module.
> 
> The right check here is probably `getLangOpts().getCompilingModule() == 
> CMK_ModuleInterface`.
Thanks! I completely missed that lang opt.


https://reviews.llvm.org/D40443



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

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

2018-06-10 Thread Hamza Sood via Phabricator via cfe-commits 
hamzasood updated this revision to Diff 150649.
hamzasood added a comment.

Addressed Richard's comments.


https://reviews.llvm.org/D40443

Files:
  include/clang/Basic/Module.h
  lib/Basic/Module.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
  test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
  
test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp

Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1.cpp
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -fmodules-ts %s -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -verify -emit-module-interface -o %t
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -verify -fmodule-file=%t -o /dev/null
-//
-// RUN: %clang_cc1 -fmodules-ts %s -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DINTERFACE -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-// RUN: %clang_cc1 -fmodules-ts %s -DIMPLEMENTATION -DBUILT_AS_INTERFACE -emit-module-interface -verify -o /dev/null
-
-#if INTERFACE
-// expected-no-diagnostics
-export module A;
-#elif IMPLEMENTATION
-module A;
- #ifdef BUILT_AS_INTERFACE
-  // expected-error@-2 {{missing 'export' specifier in module declaration while building module interface}}
-  #define INTERFACE
- #endif
-#else
- #ifdef BUILT_AS_INTERFACE
-  // expected-error@1 {{missing 'export module' declaration in module interface unit}}
- #endif
-#endif
-
-#ifndef INTERFACE
-export int b; // expected-error {{export declaration can only be used within a module interface unit}}
-#else
-export int a;
-#endif
Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
===
--- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
+++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p1/interface-imports.cpp
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo 'export module a; export class A { };' > %t/a.cppm
+// RUN: echo 'export module b; export class B { };' > %t/b.cppm
+//
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules-ts -emit-module-interface %t/b.cppm -o %t/b.pcm
+// RUN: %clang_cc1 -fmodules-ts -fprebuilt-module-path=%t -emit-module-interface %s -o %t/test.pcm -DTEST_INTERFACE
+//
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify -DTEST_IMPLEMENTATION
+// RUN: %clang_cc1 -fmodules-ts -I%t -fmodule-file=%t/test.pcm %s -verify -DOTHER_TU
+
+
+#ifdef TEST_INTERFACE
+import a;
+export module test;
+import b;
+#endif
+
+#ifdef TEST_IMPLEMENTATION
+module test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+// Module b is imported within the purview of this module's interface unit.
+// So its exported definitions should be visible here.
+B b;
+#endif
+
+
+#ifdef OTHER_TU
+import test;
+
+A a; // expected-error {{must be imported from module 'a'}}
+ // expected-n...@a.cppm:1 {{here}}
+
+B b; // expected-error {{must be imported from module 'b'}}
+ // expected-n...@b.cppm:1 {{here}}
+#endif
Index: test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
===
--- test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
+++ test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp
@@ -10,9 +10,7 @@
 //
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N.pcm -DMODULE_INTERFACE -DNO_ERRORS
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -verify
-// FIXME: Once we start importing "import" declarations properly, this should
-// be rejected (-verify should be added to the following line).
-// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT
+// RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N.pcm -DNO_IMPORT -verify
 //
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/M.pcm -emit-module-interface -o %t/N-no-M.pcm -DMODULE_INTERFACE -DNO_ERRORS -DNO_IMPORT
 // RUN: %clang_cc1 -fmodules-ts -std=c++17 %s -fmodule-file=%t/N-no-M.pcm -verify
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -16570,6 +16570,17 @@
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(Mod);
 
+  // Modules TS + p0731r0 [dcl.module.interface]p1:
+  //   Every name of an entity with linkage other than internal linkage made
+  //   visible in the

[PATCH] D47671: [analyzer] Implement copy elision.

2018-06-10 Thread Gábor Horváth via Phabricator via cfe-commits 
xazax.hun added a comment.
Herald added a subscriber: mikhail.ramalho.

Just for the record, there is a common example where relying on copy elision 
might bite and google do not recommend relying on it for correctness: 
https://abseil.io/tips/120

The main purpose of sharing is to add some more context to the discussion, I do 
not consider this to be an argument, because I can still see that this practice 
as opinionated.


Repository:
  rC Clang

https://reviews.llvm.org/D47671



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

2018-06-10 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In https://reviews.llvm.org/D45050#1127449, @Charusso wrote:

> In https://reviews.llvm.org/D45050#1119974, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45050#1119973, @Charusso wrote:
> >
> > > In https://reviews.llvm.org/D45050#1116446, @lebedev.ri wrote:
> > >
> > > > I would like to see more negative tests.
> > > >  What does it do if the len/size is a constant?
> > > >  Variable that wasn't just assigned with `strlen()` return?
> > >
> > >
> > > Thanks for the comment! What do you mean by negative test?
> >
> >
> > The tests where the check matches and issues a warning is a positive test.
> >  The tests where the check does not match and/or does not issue a warning 
> > is a negative test.
>
>
> @lebedev.ri there is all the false positive results from the last publicated 
> result-dump:
>
> 1. F6334659: curl-lib-curl_path-c.html 
> 2. second result: F6334660: ffmpeg-libavformat-sdp.c.html 
> 
> 3. all result: F6334663: openssl-apps-ca-c.html 
> 
> 4. F6334665: postgresql-src-interfaces-ecpg-preproc-pgc-c.html 
> 
> 5. F6334667: redis-src-redis-benchmark-c.html 
> 
> 6. may false positive: F6334693: ffmpeg-libavformat-rdt-c.html 
> 
>
>   (Note: the two `memchr()` result were false positive in that post and there 
> is no new result with the new matcher.)
>
>   Does the new test cases cover your advice?


Not exactly.
I want to see **tests**.
I.e. they should be in `test/clang-tidy/bugprone-not-null-terminated-result-*`.
E.g. i did not find the following tests:

  void test1(char* dst, char* src) {
strcpy(dst, src, 10);
  }
  void test2(char* dst, char* src, int len) {
strcpy(dst, src, len);
  }
  void test3(char* dst, char* src) {
memcpy(dst, src, 10);
  }
  void test4(char* dst, char* src, int len) {
memcpy(dst, src, len);
  }
  ...




Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:377-381
+FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), ") + 1");
+Diag << InsertFirstParenFix << InsertPlusOneAndSecondParenFix;
+  } else {
+const auto InsertPlusOne =
+FixItHint::CreateInsertion(exprLocEnd(LengthExpr, Result), " + 1");

The fixits are incorrect.
Increment by `int(1)` can result in overflow, which is then extended to `size_t`
It should be `+ 1UL`.
https://godbolt.org/g/4nQiek



Comment at: 
test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c:158-180
+void bad_memset_1(char *dest, const char *src) {
+  int length = getLengthWithInc(src);
+  memset(dest, '-', length);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'memset' 
is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: memset(dest, '-', length - 1);
+}
+

??
These look like false-negatives.
How do you make an assumption about `dest` buffer from `src` string length?


https://reviews.llvm.org/D45050



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

r334359 - [X86] Remove masking from the 512-bit packed floating point add/sub/mul/div builtins. Use select in IR instead.

2018-06-10 Thread Craig Topper via cfe-commits 
Author: ctopper
Date: Sat Jun  9 23:01:42 2018
New Revision: 334359

URL: http://llvm.org/viewvc/llvm-project?rev=334359=rev
Log:
[X86] Remove masking from the 512-bit packed floating point add/sub/mul/div 
builtins. Use select in IR instead.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGen/avx512f-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=334359=334358=334359=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Sat Jun  9 23:01:42 2018
@@ -1087,14 +1087,14 @@ TARGET_BUILTIN(__builtin_ia32_pmulhrsw51
 TARGET_BUILTIN(__builtin_ia32_pmulhuw512, "V32sV32sV32s", "nc", "avx512bw")
 TARGET_BUILTIN(__builtin_ia32_pmulhw512, "V32sV32sV32s", "nc", "avx512bw")
 
-TARGET_BUILTIN(__builtin_ia32_addpd512_mask, "V8dV8dV8dV8dUcIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_addps512_mask, "V16fV16fV16fV16fUsIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_divpd512_mask, "V8dV8dV8dV8dUcIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_divps512_mask, "V16fV16fV16fV16fUsIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_mulpd512_mask, "V8dV8dV8dV8dUcIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_mulps512_mask, "V16fV16fV16fV16fUsIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_subpd512_mask, "V8dV8dV8dV8dUcIi", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_subps512_mask, "V16fV16fV16fV16fUsIi", "nc", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_addpd512, "V8dV8dV8dIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_addps512, "V16fV16fV16fIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_divpd512, "V8dV8dV8dIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_divps512, "V16fV16fV16fIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_mulpd512, "V8dV8dV8dIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_mulps512, "V16fV16fV16fIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_subpd512, "V8dV8dV8dIi", "nc", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_subps512, "V16fV16fV16fIi", "nc", "avx512f")
 
 TARGET_BUILTIN(__builtin_ia32_pmaddubsw512, "V32sV64cV64c", "nc", "avx512bw")
 TARGET_BUILTIN(__builtin_ia32_pmaddwd512, "V16iV32sV32s", "nc", "avx512bw")

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=334359=334358=334359=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Sat Jun  9 23:01:42 2018
@@ -2060,40 +2060,32 @@ _mm512_maskz_add_ps(__mmask16 __U, __m51
 }
 
 #define _mm512_add_round_pd(A, B, R) \
-  (__m512d)__builtin_ia32_addpd512_mask((__v8df)(__m512d)(A), \
-(__v8df)(__m512d)(B), \
-(__v8df)_mm512_setzero_pd(), \
-(__mmask8)-1, (int)(R))
+  (__m512d)__builtin_ia32_addpd512((__v8df)(__m512d)(A), \
+   (__v8df)(__m512d)(B), (int)(R))
 
 #define _mm512_mask_add_round_pd(W, U, A, B, R) \
-  (__m512d)__builtin_ia32_addpd512_mask((__v8df)(__m512d)(A), \
-(__v8df)(__m512d)(B), \
-(__v8df)(__m512d)(W), (__mmask8)(U), \
-(int)(R))
+  (__m512d)__builtin_ia32_selectpd_512((__mmask8)(U), \
+   (__v8df)_mm512_add_round_pd((A), (B), (R)), 
\
+   (__v8df)(__m512d)(W));
 
 #define _mm512_maskz_add_round_pd(U, A, B, R) \
-  (__m512d)__builtin_ia32_addpd512_mask((__v8df)(__m512d)(A), \
-(__v8df)(__m512d)(B), \
-(__v8df)_mm512_setzero_pd(), \
-(__mmask8)(U), (int)(R))
+  (__m512d)__builtin_ia32_selectpd_512((__mmask8)(U), \
+   (__v8df)_mm512_add_round_pd((A), (B), (R)), 
\
+   (__v8df)_mm512_setzero_pd());
 
 #define _mm512_add_round_ps(A, B, R) \
-  (__m512)__builtin_ia32_addps512_mask((__v16sf)(__m512)(A), \
-   (__v16sf)(__m512)(B), \
-   (__v16sf)_mm512_setzero_ps(), \
-   (__mmask16)-1, (int)(R))
+  (__m512)__builtin_ia32_addps512((__v16sf)(__m512)(A), \
+  (__v16sf)(__m512)(B), (int)(R))
 
 #define _mm512_mask_add_round_ps(W, U, A, B, R) \
-  (__m512)__builtin_ia32_addps512_mask((__v16sf)(__m512)(A), \
-   (__v16sf)(__m512)(B), \