Re: [PATCH] D14560: [Clang] Fix Clang-tidy modernize-use-auto in some files in lib/AST; other minor cleanups.
hans added a comment. Like the other patch, I'm not sure that using auto in all these places help readability. Repository: rL LLVM http://reviews.llvm.org/D14560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252711 - [X86] Use __builtin_ia32_paddq and __builtin_ia32_psubq to implement a couple intrinsics that were supposed to operate on MMX registers. Otherwise we end up operating on GPRs. Throw in a tes
Author: ctopper Date: Wed Nov 11 02:00:41 2015 New Revision: 252711 URL: http://llvm.org/viewvc/llvm-project?rev=252711&view=rev Log: [X86] Use __builtin_ia32_paddq and __builtin_ia32_psubq to implement a couple intrinsics that were supposed to operate on MMX registers. Otherwise we end up operating on GPRs. Throw in a test for _mm_mul_su32 while I was there. Modified: cfe/trunk/lib/Headers/emmintrin.h cfe/trunk/test/CodeGen/sse-builtins.c Modified: cfe/trunk/lib/Headers/emmintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/emmintrin.h?rev=252711&r1=252710&r2=252711&view=diff == --- cfe/trunk/lib/Headers/emmintrin.h (original) +++ cfe/trunk/lib/Headers/emmintrin.h Wed Nov 11 02:00:41 2015 @@ -647,7 +647,7 @@ _mm_add_epi32(__m128i __a, __m128i __b) static __inline__ __m64 __DEFAULT_FN_ATTRS _mm_add_si64(__m64 __a, __m64 __b) { - return __a + __b; + return (__m64)__builtin_ia32_paddq(__a, __b); } static __inline__ __m128i __DEFAULT_FN_ATTRS @@ -779,7 +779,7 @@ _mm_sub_epi32(__m128i __a, __m128i __b) static __inline__ __m64 __DEFAULT_FN_ATTRS _mm_sub_si64(__m64 __a, __m64 __b) { - return __a - __b; + return (__m64)__builtin_ia32_psubq(__a, __b); } static __inline__ __m128i __DEFAULT_FN_ATTRS Modified: cfe/trunk/test/CodeGen/sse-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sse-builtins.c?rev=252711&r1=252710&r2=252711&view=diff == --- cfe/trunk/test/CodeGen/sse-builtins.c (original) +++ cfe/trunk/test/CodeGen/sse-builtins.c Wed Nov 11 02:00:41 2015 @@ -495,3 +495,21 @@ __m128i test_mm_undefined_si128() { // CHECK: ret <2 x i64> undef return _mm_undefined_si128(); } + +__m64 test_mm_add_si64(__m64 __a, __m64 __b) { + // CHECK-LABEL: @test_mm_add_si64 + // CHECK @llvm.x86.mmx.padd.q(x86_mmx %{{.*}}, x86_mmx %{{.*}}) + return _mm_add_si64(__a, __b); +} + +__m64 test_mm_sub_si64(__m64 __a, __m64 __b) { + // CHECK-LABEL: @test_mm_sub_si64 + // CHECK @llvm.x86.mmx.psub.q(x86_mmx %{{.*}}, x86_mmx %{{.*}}) + return _mm_sub_si64(__a, __b); +} + +__m64 test_mm_mul_su32(__m64 __a, __m64 __b) { + // CHECK-LABEL: @test_mm_mul_su32 + // CHECK @llvm.x86.mmx.pmulu.dq(x86_mmx %{{.*}}, x86_mmx %{{.*}}) + return _mm_mul_su32(__a, __b); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252710 - [X86] Header formatting fixes. NFC
Author: ctopper Date: Wed Nov 11 02:00:39 2015 New Revision: 252710 URL: http://llvm.org/viewvc/llvm-project?rev=252710&view=rev Log: [X86] Header formatting fixes. NFC Modified: cfe/trunk/lib/Headers/avx512vlbwintrin.h cfe/trunk/lib/Headers/avx512vldqintrin.h Modified: cfe/trunk/lib/Headers/avx512vlbwintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vlbwintrin.h?rev=252710&r1=252709&r2=252710&view=diff == --- cfe/trunk/lib/Headers/avx512vlbwintrin.h (original) +++ cfe/trunk/lib/Headers/avx512vlbwintrin.h Wed Nov 11 02:00:39 2015 @@ -1,4 +1,4 @@ -/*=== avx512vlbwintrin.h - AVX512VL and AVX512BW intrinsics --=== +/*=== avx512vlbwintrin.h - AVX512VL and AVX512BW intrinsics === * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal Modified: cfe/trunk/lib/Headers/avx512vldqintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vldqintrin.h?rev=252710&r1=252709&r2=252710&view=diff == --- cfe/trunk/lib/Headers/avx512vldqintrin.h (original) +++ cfe/trunk/lib/Headers/avx512vldqintrin.h Wed Nov 11 02:00:39 2015 @@ -1,4 +1,4 @@ -/*=== avx512vldqintrin.h - AVX512VL and AVX512DQ intrinsics ---=== +/*=== avx512vldqintrin.h - AVX512VL and AVX512DQ intrinsics === * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252712 - [X86] Add 'pause' builtin that's already in llvm and use it instead of inline assembly to implement _mm_pause.
Author: ctopper Date: Wed Nov 11 02:13:33 2015 New Revision: 252712 URL: http://llvm.org/viewvc/llvm-project?rev=252712&view=rev Log: [X86] Add 'pause' builtin that's already in llvm and use it instead of inline assembly to implement _mm_pause. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/lib/Headers/emmintrin.h cfe/trunk/test/CodeGen/sse-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=252712&r1=252711&r2=252712&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Nov 11 02:13:33 2015 @@ -336,6 +336,7 @@ TARGET_BUILTIN(__builtin_ia32_cvttps2dq, TARGET_BUILTIN(__builtin_ia32_clflush, "vvC*", "", "sse2") TARGET_BUILTIN(__builtin_ia32_lfence, "v", "", "sse2") TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2") +TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2") TARGET_BUILTIN(__builtin_ia32_storedqu, "vc*V16c", "", "sse2") TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2") TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2") Modified: cfe/trunk/lib/Headers/emmintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/emmintrin.h?rev=252712&r1=252711&r2=252712&view=diff == --- cfe/trunk/lib/Headers/emmintrin.h (original) +++ cfe/trunk/lib/Headers/emmintrin.h Wed Nov 11 02:13:33 2015 @@ -1481,7 +1481,7 @@ _mm_castsi128_pd(__m128i __a) static __inline__ void __DEFAULT_FN_ATTRS _mm_pause(void) { - __asm__ volatile ("pause"); + __builtin_ia32_pause(); } #undef __DEFAULT_FN_ATTRS Modified: cfe/trunk/test/CodeGen/sse-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sse-builtins.c?rev=252712&r1=252711&r2=252712&view=diff == --- cfe/trunk/test/CodeGen/sse-builtins.c (original) +++ cfe/trunk/test/CodeGen/sse-builtins.c Wed Nov 11 02:13:33 2015 @@ -513,3 +513,9 @@ __m64 test_mm_mul_su32(__m64 __a, __m64 // CHECK @llvm.x86.mmx.pmulu.dq(x86_mmx %{{.*}}, x86_mmx %{{.*}}) return _mm_mul_su32(__a, __b); } + +void test_mm_pause() { + // CHECK-LABEL: @test_mm_pause + // CHECK @llvm.x86.sse2.pause() + return _mm_pause(); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252720 - Make test/Driver/biarch.c use FileCheck instead of grep
Author: askrobov Date: Wed Nov 11 04:45:44 2015 New Revision: 252720 URL: http://llvm.org/viewvc/llvm-project?rev=252720&view=rev Log: Make test/Driver/biarch.c use FileCheck instead of grep Summary: For clarity and ease of maintenance, I suggest porting this test to use the same tooling as the rest of the tests. Reviewers: joerg, rengolin, dougk, yaron.keren Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D14548 Modified: cfe/trunk/test/Driver/biarch.c Modified: cfe/trunk/test/Driver/biarch.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/biarch.c?rev=252720&r1=252719&r2=252720&view=diff == --- cfe/trunk/test/Driver/biarch.c (original) +++ cfe/trunk/test/Driver/biarch.c Wed Nov 11 04:45:44 2015 @@ -1,44 +1,32 @@ -// RUN: %clang -target i386--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "i386--netbsd"' %t - -// RUN: %clang -target i386--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "x86_64--netbsd"' %t - -// RUN: %clang -target x86_64--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "i386--netbsd"' %t - -// RUN: %clang -target x86_64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "x86_64--netbsd"' %t - -// RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "armv6k--netbsd-eabihf"' %t - -// RUN: %clang -target sparcv9--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc--netbsd"' %t - -// RUN: %clang -target sparcv9--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparcv9--netbsd"' %t - -// RUN: %clang -target sparc64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc64--netbsd"' %t - -// RUN: %clang -target sparc--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc--netbsd"' %t - -// RUN: %clang -target sparc--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparcv9--netbsd"' %t - -// RUN: %clang -target sparcel -o foo %s -### 2> %t -// RUN: grep 'gcc\(\.exe\)\?" "-EL" "-o" "foo"' %t - -// RUN: %clang -target mips64--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips--netbsd"' %t - -// RUN: %clang -target mips64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips64--netbsd"' %t - -// RUN: %clang -target mips--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips--netbsd"' %t - -// RUN: %clang -target mips--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips64--netbsd"' %t +// RUN: %clang -target i386--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=I386 %s +// RUN: %clang -target x86_64--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=I386 %s +// I386: "-cc1" "-triple" "i386--netbsd" + +// RUN: %clang -target i386--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s +// RUN: %clang -target x86_64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s +// X86_64: "-cc1" "-triple" "x86_64--netbsd" + +// RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2>&1 | FileCheck -check-prefix=ARMV6 %s +// ARMV6: "-cc1" "-triple" "armv6k--netbsd-eabihf" + +// RUN: %clang -target sparcv9--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s +// RUN: %clang -target sparc--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s +// SPARC: "-cc1" "-triple" "sparc--netbsd" + +// RUN: %clang -target sparcv9--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=SPARCV9 %s +// RUN: %clang -target sparc--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=SPARCV9 %s +// SPARCV9: "-cc1" "-triple" "sparcv9--netbsd" + +// RUN: %clang -target sparc64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=SPARC64 %s +// SPARC64: "-cc1" "-triple" "sparc64--netbsd" + +// RUN: %clang -target sparcel -o foo %s -### 2>&1 | FileCheck -check-prefix=SPARCEL %s +// SPARCEL: gcc{{(\.exe)?}}" "-EL" "-o" "foo" + +// RUN: %clang -target mips64--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=MIPS %s +// RUN: %clang -target mips--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=MIPS %s +// MIPS: "-cc1" "-triple" "mips--netbsd" + +// RUN: %clang -target mips64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=MIPS64 %s +// RUN: %clang -target mips--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=MIPS64 %s +// MIPS64: "-cc1" "-triple" "mips64--netbsd" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252721 - [ASan] Allow -fsanitize-recover=address.
Author: ygribov Date: Wed Nov 11 04:45:48 2015 New Revision: 252721 URL: http://llvm.org/viewvc/llvm-project?rev=252721&view=rev Log: [ASan] Allow -fsanitize-recover=address. Differential Revision: http://reviews.llvm.org/D14243 Modified: cfe/trunk/docs/UsersManual.rst cfe/trunk/lib/CodeGen/BackendUtil.cpp cfe/trunk/lib/Driver/SanitizerArgs.cpp cfe/trunk/test/Driver/fsanitize.c Modified: cfe/trunk/docs/UsersManual.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=252721&r1=252720&r2=252721&view=diff == --- cfe/trunk/docs/UsersManual.rst (original) +++ cfe/trunk/docs/UsersManual.rst Wed Nov 11 04:45:48 2015 @@ -1096,8 +1096,9 @@ are listed below. By default, non-fatal checks are those enabled by UndefinedBehaviorSanitizer, except for ``-fsanitize=return`` and ``-fsanitize=unreachable``. Some - sanitizers (e.g. :doc:`AddressSanitizer`) may not support recovery, - and always crash the program after the issue is detected. + sanitizers may not support recovery (or not support it by default + e.g. :doc:`AddressSanitizer`), and always crash the program after the issue + is detected. Note that the ``-fsanitize-trap`` flag has precedence over this flag. This means that if a check has been configured to trap elsewhere on the Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=252721&r1=252720&r2=252721&view=diff == --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original) +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Nov 11 04:45:48 2015 @@ -194,14 +194,20 @@ static void addSanitizerCoveragePass(con static void addAddressSanitizerPasses(const PassManagerBuilder &Builder, legacy::PassManagerBase &PM) { - PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/false)); - PM.add(createAddressSanitizerModulePass(/*CompileKernel*/false)); + const PassManagerBuilderWrapper &BuilderWrapper = + static_cast(Builder); + const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts(); + bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Address); + PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/false, Recover)); + PM.add(createAddressSanitizerModulePass(/*CompileKernel*/false, Recover)); } static void addKernelAddressSanitizerPasses(const PassManagerBuilder &Builder, legacy::PassManagerBase &PM) { - PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/true)); - PM.add(createAddressSanitizerModulePass(/*CompileKernel*/true)); + PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/true, +/*Recover*/true)); + PM.add(createAddressSanitizerModulePass(/*CompileKernel*/true, + /*Recover*/true)); } static void addMemorySanitizerPass(const PassManagerBuilder &Builder, Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=252721&r1=252720&r2=252721&view=diff == --- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original) +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Nov 11 04:45:48 2015 @@ -33,7 +33,7 @@ enum : SanitizerMask { NeedsUnwindTables = Address | Thread | Memory | DataFlow, SupportsCoverage = Address | Memory | Leak | Undefined | Integer | DataFlow, RecoverableByDefault = Undefined | Integer, - Unrecoverable = Address | Unreachable | Return, + Unrecoverable = Unreachable | Return, LegacyFsanitizeRecoverMask = Undefined | Integer, NeedsLTO = CFI, TrappingSupported = Modified: cfe/trunk/test/Driver/fsanitize.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=252721&r1=252720&r2=252721&view=diff == --- cfe/trunk/test/Driver/fsanitize.c (original) +++ cfe/trunk/test/Driver/fsanitize.c Wed Nov 11 04:45:48 2015 @@ -160,22 +160,25 @@ // RUN: %clang -target arm-linux-androideabi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ANDROID-NO-ASAN // CHECK-ANDROID-NO-ASAN: "-mrelocation-model" "pic" -// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER -// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER -// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover=all -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER -// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=undefined -### 2>&1 | FileCheck %s --check-prefix=CHEC
Re: [PATCH] D14548: Make test/Driver/biarch.c use FileCheck instead of grep
This revision was automatically updated to reflect the committed changes. Closed by commit rL252720: Make test/Driver/biarch.c use FileCheck instead of grep (authored by askrobov). Changed prior to commit: http://reviews.llvm.org/D14548?vs=39840&id=39889#toc Repository: rL LLVM http://reviews.llvm.org/D14548 Files: cfe/trunk/test/Driver/biarch.c Index: cfe/trunk/test/Driver/biarch.c === --- cfe/trunk/test/Driver/biarch.c +++ cfe/trunk/test/Driver/biarch.c @@ -1,44 +1,32 @@ -// RUN: %clang -target i386--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "i386--netbsd"' %t - -// RUN: %clang -target i386--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "x86_64--netbsd"' %t - -// RUN: %clang -target x86_64--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "i386--netbsd"' %t - -// RUN: %clang -target x86_64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "x86_64--netbsd"' %t - -// RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "armv6k--netbsd-eabihf"' %t - -// RUN: %clang -target sparcv9--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc--netbsd"' %t - -// RUN: %clang -target sparcv9--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparcv9--netbsd"' %t - -// RUN: %clang -target sparc64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc64--netbsd"' %t - -// RUN: %clang -target sparc--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc--netbsd"' %t - -// RUN: %clang -target sparc--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparcv9--netbsd"' %t - -// RUN: %clang -target sparcel -o foo %s -### 2> %t -// RUN: grep 'gcc\(\.exe\)\?" "-EL" "-o" "foo"' %t - -// RUN: %clang -target mips64--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips--netbsd"' %t - -// RUN: %clang -target mips64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips64--netbsd"' %t - -// RUN: %clang -target mips--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips--netbsd"' %t - -// RUN: %clang -target mips--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "mips64--netbsd"' %t +// RUN: %clang -target i386--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=I386 %s +// RUN: %clang -target x86_64--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=I386 %s +// I386: "-cc1" "-triple" "i386--netbsd" + +// RUN: %clang -target i386--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s +// RUN: %clang -target x86_64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s +// X86_64: "-cc1" "-triple" "x86_64--netbsd" + +// RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2>&1 | FileCheck -check-prefix=ARMV6 %s +// ARMV6: "-cc1" "-triple" "armv6k--netbsd-eabihf" + +// RUN: %clang -target sparcv9--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s +// RUN: %clang -target sparc--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s +// SPARC: "-cc1" "-triple" "sparc--netbsd" + +// RUN: %clang -target sparcv9--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=SPARCV9 %s +// RUN: %clang -target sparc--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=SPARCV9 %s +// SPARCV9: "-cc1" "-triple" "sparcv9--netbsd" + +// RUN: %clang -target sparc64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=SPARC64 %s +// SPARC64: "-cc1" "-triple" "sparc64--netbsd" + +// RUN: %clang -target sparcel -o foo %s -### 2>&1 | FileCheck -check-prefix=SPARCEL %s +// SPARCEL: gcc{{(\.exe)?}}" "-EL" "-o" "foo" + +// RUN: %clang -target mips64--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=MIPS %s +// RUN: %clang -target mips--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=MIPS %s +// MIPS: "-cc1" "-triple" "mips--netbsd" + +// RUN: %clang -target mips64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=MIPS64 %s +// RUN: %clang -target mips--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=MIPS64 %s +// MIPS64: "-cc1" "-triple" "mips64--netbsd" Index: cfe/trunk/test/Driver/biarch.c === --- cfe/trunk/test/Driver/biarch.c +++ cfe/trunk/test/Driver/biarch.c @@ -1,44 +1,32 @@ -// RUN: %clang -target i386--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "i386--netbsd"' %t - -// RUN: %clang -target i386--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "x86_64--netbsd"' %t - -// RUN: %clang -target x86_64--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "i386--netbsd"' %t - -// RUN: %clang -target x86_64--netbsd -m64 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "x86_64--netbsd"' %t - -// RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "armv6k--netbsd-eabihf"' %t - -// RUN: %clang -target sparcv9--netbsd -m32 %s -### 2> %t -// RUN: grep '"-cc1" "-triple" "sparc--netbsd"' %t - -// RUN: %clang -target sparcv9--netbsd -m64 %s -###
[PATCH] D14570: Handle ARMv6KZ naming
tyomitch created this revision. tyomitch added reviewers: rengolin, joerg, bogden. tyomitch added a subscriber: cfe-commits. Herald added subscribers: rengolin, aemerson. Update for clang tests for D14568 http://reviews.llvm.org/D14570 Files: test/Driver/arm-cortex-cpus.c test/Driver/biarch.c Index: test/Driver/biarch.c === --- test/Driver/biarch.c +++ test/Driver/biarch.c @@ -6,8 +6,9 @@ // RUN: %clang -target x86_64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s // X86_64: "-cc1" "-triple" "x86_64--netbsd" +// r196538 set arm1176jzf-s as default CPU for ARMv6 on NetBSD // RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2>&1 | FileCheck -check-prefix=ARMV6 %s -// ARMV6: "-cc1" "-triple" "armv6k--netbsd-eabihf" +// ARMV6: "-cc1" "-triple" "armv6kz--netbsd-eabihf" // RUN: %clang -target sparcv9--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s // RUN: %clang -target sparc--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s Index: test/Driver/arm-cortex-cpus.c === --- test/Driver/arm-cortex-cpus.c +++ test/Driver/arm-cortex-cpus.c @@ -73,11 +73,11 @@ // RUN: %clang -target armv6k -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K %s // RUN: %clang -target arm -march=armv6k -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K %s -// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176jzf-s" +// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176j-s" // RUN: %clang -target armv6k -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K-THUMB %s // RUN: %clang -target arm -march=armv6k -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K-THUMB %s -// CHECK-V6K-THUMB: "-cc1"{{.*}} "-triple" "thumbv6k-{{.*}} "-target-cpu" "arm1176jzf-s" +// CHECK-V6K-THUMB: "-cc1"{{.*}} "-triple" "thumbv6k-{{.*}} "-target-cpu" "arm1176j-s" // RUN: %clang -target armv6t2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6T2 %s // RUN: %clang -target arm -march=armv6t2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6T2 %s @@ -249,12 +249,14 @@ // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136jf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6 %s // CHECK-CPUV6: "-cc1"{{.*}} "-triple" "armv6-{{.*}} -// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s -// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcore -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcorenovfp -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s // CHECK-CPUV6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} +// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6KZ %s +// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6KZ %s +// CHECK-CPUV6KZ: "-cc1"{{.*}} "-triple" "armv6kz-{{.*}} + // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1156t2-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6T2 %s // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1156t2f-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6T2 %s // CHECK-CPUV6T2: "-cc1"{{.*}} "-triple" "armv6t2-{{.*}} Index: test/Driver/biarch.c === --- test/Driver/biarch.c +++ test/Driver/biarch.c @@ -6,8 +6,9 @@ // RUN: %clang -target x86_64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s // X86_64: "-cc1" "-triple" "x86_64--netbsd" +// r196538 set arm1176jzf-s as default CPU for ARMv6 on NetBSD // RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2>&1 | FileCheck -check-prefix=ARMV6 %s -// ARMV6: "-cc1" "-triple" "armv6k--netbsd-eabihf" +// ARMV6: "-cc1" "-triple" "armv6kz--netbsd-eabihf" // RUN: %clang -target sparcv9--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s // RUN: %clang -target sparc--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s Index: test/Driver/arm-cortex-cpus.c === --- test/Driver/arm-cortex-cpus.c +++ test/Driver/arm-cortex-cpus.c @@ -73,11 +73,11 @@ // RUN: %clang -target armv6k -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K %s // RUN: %clang -target arm -march=armv6k -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K %s -// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176jzf-s" +// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176j-s" // RUN: %clang -target armv6k -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K-THUMB %s // RUN: %clang -target arm -march=armv6k -mthumb -### -c %s 2>&1 | FileCheck -che
Re: [PATCH] D14570: Handle ARMv6KZ naming
Hi Artyom, Why isn't this just part of D14568? Cheers, James On Wed, 11 Nov 2015 at 12:08 A. Skrobov via cfe-commits < cfe-commits@lists.llvm.org> wrote: > tyomitch created this revision. > tyomitch added reviewers: rengolin, joerg, bogden. > tyomitch added a subscriber: cfe-commits. > Herald added subscribers: rengolin, aemerson. > > Update for clang tests for D14568 > > http://reviews.llvm.org/D14570 > > Files: > test/Driver/arm-cortex-cpus.c > test/Driver/biarch.c > > Index: test/Driver/biarch.c > === > --- test/Driver/biarch.c > +++ test/Driver/biarch.c > @@ -6,8 +6,9 @@ > // RUN: %clang -target x86_64--netbsd -m64 %s -### 2>&1 | FileCheck > -check-prefix=X86_64 %s > // X86_64: "-cc1" "-triple" "x86_64--netbsd" > > +// r196538 set arm1176jzf-s as default CPU for ARMv6 on NetBSD > // RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2>&1 | FileCheck > -check-prefix=ARMV6 %s > -// ARMV6: "-cc1" "-triple" "armv6k--netbsd-eabihf" > +// ARMV6: "-cc1" "-triple" "armv6kz--netbsd-eabihf" > > // RUN: %clang -target sparcv9--netbsd -m32 %s -### 2>&1 | FileCheck > -check-prefix=SPARC %s > // RUN: %clang -target sparc--netbsd -m32 %s -### 2>&1 | FileCheck > -check-prefix=SPARC %s > Index: test/Driver/arm-cortex-cpus.c > === > --- test/Driver/arm-cortex-cpus.c > +++ test/Driver/arm-cortex-cpus.c > @@ -73,11 +73,11 @@ > > // RUN: %clang -target armv6k -### -c %s 2>&1 | FileCheck > -check-prefix=CHECK-V6K %s > // RUN: %clang -target arm -march=armv6k -### -c %s 2>&1 | FileCheck > -check-prefix=CHECK-V6K %s > -// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" > "arm1176jzf-s" > +// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" > "arm1176j-s" > > // RUN: %clang -target armv6k -mthumb -### -c %s 2>&1 | FileCheck > -check-prefix=CHECK-V6K-THUMB %s > // RUN: %clang -target arm -march=armv6k -mthumb -### -c %s 2>&1 | > FileCheck -check-prefix=CHECK-V6K-THUMB %s > -// CHECK-V6K-THUMB: "-cc1"{{.*}} "-triple" "thumbv6k-{{.*}} "-target-cpu" > "arm1176jzf-s" > +// CHECK-V6K-THUMB: "-cc1"{{.*}} "-triple" "thumbv6k-{{.*}} "-target-cpu" > "arm1176j-s" > > // RUN: %clang -target armv6t2 -### -c %s 2>&1 | FileCheck > -check-prefix=CHECK-V6T2 %s > // RUN: %clang -target arm -march=armv6t2 -### -c %s 2>&1 | FileCheck > -check-prefix=CHECK-V6T2 %s > @@ -249,12 +249,14 @@ > // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136jf-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6 %s > // CHECK-CPUV6: "-cc1"{{.*}} "-triple" "armv6-{{.*}} > > -// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s > -// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s > // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcore -### -c %s 2>&1 | > FileCheck -check-prefix=CHECK-CPUV6K %s > // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcorenovfp -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s > // CHECK-CPUV6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} > > +// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6KZ %s > +// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6KZ %s > +// CHECK-CPUV6KZ: "-cc1"{{.*}} "-triple" "armv6kz-{{.*}} > + > // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1156t2-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6T2 %s > // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1156t2f-s -### -c %s > 2>&1 | FileCheck -check-prefix=CHECK-CPUV6T2 %s > // CHECK-CPUV6T2: "-cc1"{{.*}} "-triple" "armv6t2-{{.*}} > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14441: [OpenCL] Pipe types support.
pxli168 added a subscriber: pxli168. Comment at: test/CodeGenOpenCL/pipe_types.cl:5 @@ +4,3 @@ + +void test1(read_only pipe int p) { +// CHECK: define void @test1(%opencl.pipe_t* %p) Great work!! But I have tried your patch and find it does not support opencl gentype like int4 with the usual used typedef in other test cases. Maybe there is something wrong? http://reviews.llvm.org/D14441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252727 - Silencing a -Wreturn-type warning for control reaching the end of a non-void function.
Author: aaronballman Date: Wed Nov 11 07:42:02 2015 New Revision: 252727 URL: http://llvm.org/viewvc/llvm-project?rev=252727&view=rev Log: Silencing a -Wreturn-type warning for control reaching the end of a non-void function. Modified: cfe/trunk/lib/Basic/Targets.cpp Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=252727&r1=252726&r2=252727&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Wed Nov 11 07:42:02 2015 @@ -5839,6 +5839,7 @@ public: case CK_NIAGARA4: return CG_V9; } +llvm_unreachable("Unexpected CPU kind"); } CPUKind getCPUKind(StringRef Name) const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13746: [clang-tidy] add check cppcoreguidelines-pro-bounds-constant-array-index
alexfh added a comment. Sorry for the delay. A few more comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:86 @@ +85,3 @@ + if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr, +true)) { +SourceRange BaseRange; nit: What is `true` here? Please add an argument comment. Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:102 @@ +101,3 @@ + << FixItHint::CreateReplacement( + {BaseRange.getEnd().getLocWithOffset(1), + IndexRange.getBegin().getLocWithOffset(-1)}, I'm not sure initializer lists are supported by all toolchains this code needs to compile on. Please explicitly specify `SourceRange` here. Comment at: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:124 @@ +123,3 @@ + // Get uint64_t values, because different bitwidths would lead to an assertion + // in APInt::uge + if (Index.getZExtValue() >= ArraySize.getZExtValue()) { nit: please add trailing period. Comment at: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst:12 @@ +11,2 @@ +This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see +https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions nit: please add trailing period. Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp:51 @@ +50,3 @@ + + a[-1] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array So what's the difference between this warning and -Warray-bounds? http://reviews.llvm.org/D13746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252474 - Create install targets for scan-build and scan-view
This change causes MSVC to have two new projects sitting at the root of the solution: scan-build and scan-view, neither of which appear to do anything. Are these projects required for IDEs? If they're installation-only projects, I think the CMake should be guarded with `if(NOT CMAKE_CONFIGURATION_TYPES)`. ~Aaron On Mon, Nov 9, 2015 at 11:12 AM, Jonathan Roelofs via cfe-commits wrote: > Author: jroelofs > Date: Mon Nov 9 10:12:56 2015 > New Revision: 252474 > > URL: http://llvm.org/viewvc/llvm-project?rev=252474&view=rev > Log: > Create install targets for scan-build and scan-view > > http://reviews.llvm.org/D14403 > > Added: > cfe/trunk/tools/scan-build/CMakeLists.txt > cfe/trunk/tools/scan-build/Makefile > cfe/trunk/tools/scan-view/CMakeLists.txt > cfe/trunk/tools/scan-view/Makefile > Modified: > cfe/trunk/tools/CMakeLists.txt > cfe/trunk/tools/Makefile > cfe/trunk/tools/scan-build/scan-build > cfe/trunk/tools/scan-view/Reporter.py > cfe/trunk/tools/scan-view/ScanView.py > cfe/trunk/www/analyzer/installation.html > > Modified: cfe/trunk/tools/CMakeLists.txt > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/CMakeLists.txt?rev=252474&r1=252473&r2=252474&view=diff > == > --- cfe/trunk/tools/CMakeLists.txt (original) > +++ cfe/trunk/tools/CMakeLists.txt Mon Nov 9 10:12:56 2015 > @@ -5,6 +5,8 @@ add_clang_subdirectory(driver) > add_clang_subdirectory(clang-format) > add_clang_subdirectory(clang-format-vs) > add_clang_subdirectory(clang-fuzzer) > +add_clang_subdirectory(scan-build) > +add_clang_subdirectory(scan-view) > > add_clang_subdirectory(c-index-test) > add_clang_subdirectory(libclang) > > Modified: cfe/trunk/tools/Makefile > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/Makefile?rev=252474&r1=252473&r2=252474&view=diff > == > --- cfe/trunk/tools/Makefile (original) > +++ cfe/trunk/tools/Makefile Mon Nov 9 10:12:56 2015 > @@ -15,7 +15,7 @@ DIRS := > PARALLEL_DIRS := clang-format driver diagtool > > ifeq ($(ENABLE_CLANG_STATIC_ANALYZER), 1) > - PARALLEL_DIRS += clang-check > + PARALLEL_DIRS += clang-check scan-build scan-view > endif > > ifeq ($(ENABLE_CLANG_ARCMT), 1) > > Added: cfe/trunk/tools/scan-build/CMakeLists.txt > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/CMakeLists.txt?rev=252474&view=auto > == > --- cfe/trunk/tools/scan-build/CMakeLists.txt (added) > +++ cfe/trunk/tools/scan-build/CMakeLists.txt Mon Nov 9 10:12:56 2015 > @@ -0,0 +1,71 @@ > +add_custom_target(scan-build ALL) > + > +option(CLANG_INSTALL_SCANBUILD "Install the scan-build tool" ON) > + > +if (WIN32 AND NOT CYGWIN) > + set(BinFiles > +scan-build.bat) > + set(LibexecFiles > +ccc-analyzer.bat > +c++-analyzer.bat) > +else() > + set(BinFiles > +scan-build) > + set(LibexecFiles > +ccc-analyzer > +c++-analyzer) > + if (APPLE) > +set(BinFiles ${BinFiles} > + set-xcode-analyzer) > + endif() > +endif() > + > +set(ManPages > + scan-build.1) > + > +set(ResourceFiles > + scanview.css > + sorttable.js) > + > + > +if(CLANG_INSTALL_SCANBUILD) > + foreach(BinFile ${BinFiles}) > +add_custom_command(TARGET scan-build PRE_BUILD > + COMMAND ${CMAKE_COMMAND} -E make_directory > + ${CMAKE_BINARY_DIR}/bin > + COMMAND ${CMAKE_COMMAND} -E copy > + ${CMAKE_CURRENT_SOURCE_DIR}/${BinFile} > + ${CMAKE_BINARY_DIR}/bin/) > +install(PROGRAMS ${BinFile} DESTINATION bin) > + endforeach() > + > + foreach(LibexecFile ${LibexecFiles}) > +add_custom_command(TARGET scan-build PRE_BUILD > + COMMAND ${CMAKE_COMMAND} -E make_directory > + ${CMAKE_BINARY_DIR}/libexec > + COMMAND ${CMAKE_COMMAND} -E copy > + ${CMAKE_CURRENT_SOURCE_DIR}/${LibexecFile} > + ${CMAKE_BINARY_DIR}/libexec/) > +install(PROGRAMS ${LibexecFile} DESTINATION libexec) > + endforeach() > + > + foreach(ManPage ${ManPages}) > +add_custom_command(TARGET scan-build PRE_BUILD > + COMMAND ${CMAKE_COMMAND} -E make_directory > + ${CMAKE_BINARY_DIR}/share/man/man1 > + COMMAND ${CMAKE_COMMAND} -E copy > + ${CMAKE_CURRENT_SOURCE_DIR}/${ManPage} > + ${CMAKE_BINARY_DIR}/share/man/man1/) > +install(PROGRAMS scan-build.1 DESTINATION share/man/man1) > + endforeach() > + > + foreach(ResourceFile ${ResourceFiles}) > +add_custom_command(TARGET scan-build PRE_BUILD > + COMMAND ${CMAKE_COMMAND} -E make_directory > +
Re: [PATCH] D13899: Fix bug in suggested fix that truncated variable names to 1 character.
sbenza added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:25 @@ +24,3 @@ +template +static CharSourceRange removeNode(const MatchFinder::MatchResult &Result, + const T *PrevNode, const T *Node, aaron.ballman wrote: > Are we preferring anonymous namespaces over static functions these days? I didn't know there was a preference. I usually do anon namespaces, but these functions were already here as static. http://reviews.llvm.org/D13899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13899: Fix bug in suggested fix that truncated variable names to 1 character.
aaron.ballman added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:25 @@ +24,3 @@ +template +static CharSourceRange removeNode(const MatchFinder::MatchResult &Result, + const T *PrevNode, const T *Node, sbenza wrote: > aaron.ballman wrote: > > Are we preferring anonymous namespaces over static functions these days? > I didn't know there was a preference. > I usually do anon namespaces, but these functions were already here as static. There is, and I just went to find it instead of asking you to do it for me. ;-) http://llvm.org/docs/CodingStandards.html#anonymous-namespaces Since this code doesn't involve any class declarations, static is totally fine. http://reviews.llvm.org/D13899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13759: [Clang] Fix Clang-tidy modernize-use-auto warnings in headers and generated files; other minor cleanups.
aaron.ballman added inline comments. Comment at: include/clang/AST/ASTVector.h:385 @@ -383,3 +384,3 @@ // Allocate the memory from the ASTContext. - T *NewElts = new (C, llvm::alignOf()) T[NewCapacity]; + auto *NewElts = new (C, llvm::alignOf()) T[NewCapacity]; Eugene.Zelenko wrote: > hans wrote: > > I'm not sure this one is an improvement. > At least handling of new will be consistent. This will also silent Clang-tidy > warning. I'm not too concerned about the clang-tidy warning. It suggests that perhaps this checker could use an option to control this behavior, however. I agree with Hans that a lot of the treatments of new expressions don't really seem to be improvements. Comment at: include/clang/AST/DeclTemplate.h:1726 @@ -1725,4 +1725,3 @@ "Already set to a class template partial specialization!"); -SpecializedPartialSpecialization *PS - = new (getASTContext()) SpecializedPartialSpecialization(); +auto *PS = new (getASTContext()) SpecializedPartialSpecialization(); PS->PartialSpecialization = PartialSpec; hans wrote: > But here it does make it nicer :-) I wonder if the heuristic is: when the new expression spans multiple lines, only replace the declaration type with auto if it causes the new expression to become a single-line expression. Comment at: include/clang/Analysis/Analyses/Consumed.h:266 @@ -266,2 +265,3 @@ }; -}} // end namespace clang::consumed +} // end namespace consumed +} // end namespace clang Eugene.Zelenko wrote: > aaron.ballman wrote: > > I don't know whether this is an improvement or not. > This was changed for consistency with other code. I also think that it's good > idea to place one statement per line. This pattern is used in many places in the code base, it reduces vertical clutter, and it could be argued that it is easier to read because you don't have to interpret the namespace hierarchy. Comment at: include/clang/Rewrite/Core/Rewriter.h:171 @@ -170,4 +170,3 @@ const RewriteBuffer *getRewriteBufferFor(FileID FID) const { -std::map::const_iterator I = - RewriteBuffers.find(FID); +const auto I = RewriteBuffers.find(FID); return I == RewriteBuffers.end() ? nullptr : &I->second; Eugene.Zelenko wrote: > aaron.ballman wrote: > > Not that this matters in the cases in this code, but const iterator is not > > the same as const_iterator. I think these changes are a step forward in > > terms of readability, but a step backwards in terms of the type system. > I'm still learning C++11, so advices how to handle such situations properly > are welcome! In this case, I suppose it doesn't matter at all -- RewriteBuffers is a member variable being accessed from a const method, and so it will return a const_iterator. So technically, you don't even need the const here, just auto I. The issue is more with code like: ``` struct S { SomeContainer C; void f() { SomeContainer::const_iterator I = C.begin(); } }; ``` Replacing with auto here will deduce SomeContainer::iterator. The only way around that is with ugly casting, at which point I think leaving the declaration alone is the better choice. For this code, I would remove the const and just leave it as auto I. Repository: rL LLVM http://reviews.llvm.org/D13759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14560: [Clang] Fix Clang-tidy modernize-use-auto in some files in lib/AST; other minor cleanups.
aaron.ballman added a comment. In http://reviews.llvm.org/D14560#286819, @hans wrote: > Like the other patch, I'm not sure that using auto in all these places help > readability. I share these concerns. Repository: rL LLVM http://reviews.llvm.org/D14560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14560: [Clang] Fix Clang-tidy modernize-use-auto in some files in lib/AST; other minor cleanups.
aaron.ballman added inline comments. Comment at: lib/AST/ASTContext.cpp:3857 @@ -3856,4 +3856,3 @@ void *Mem = Allocate(sizeof(ObjCObjectPointerType), TypeAlignment); - ObjCObjectPointerType *QType = -new (Mem) ObjCObjectPointerType(Canonical, ObjectT); + auto *QType = new (Mem) ObjCObjectPointerType(Canonical, ObjectT); This one may be an improvement. Comment at: lib/AST/ASTContext.cpp:4366 @@ -4366,4 +4365,3 @@ - TemplateArgument *CanonArgs -= new (*this) TemplateArgument[Arg.pack_size()]; + auto *CanonArgs = new (*this) TemplateArgument[Arg.pack_size()]; unsigned Idx = 0; Same with this one. Comment at: lib/AST/ASTContext.cpp:7930 @@ -7931,3 +7929,3 @@ -ASTMutationListener::~ASTMutationListener() { } +ASTMutationListener::~ASTMutationListener() = default; This is... interesting. Explicitly defaulting an out-of-line function definition is not something I've ever encountered before. What benefit does this provide? Comment at: lib/AST/ASTContext.cpp:8519 @@ -8520,3 +8518,3 @@ -CXXABI::~CXXABI() {} +CXXABI::~CXXABI() = default; Same question applies here. Comment at: lib/AST/ASTImporter.cpp:5326 @@ -5324,4 +5325,3 @@ - Expr **ToArgs_Copied = new (Importer.getToContext()) -Expr*[NumArgs]; + auto **ToArgs_Copied = new (Importer.getToContext()) Expr*[NumArgs]; This may be an improvement. Comment at: lib/AST/ASTImporter.cpp:5348 @@ -5347,3 +5347,3 @@ -ASTImporter::~ASTImporter() { } +ASTImporter::~ASTImporter() = default; Similar out-of-line question here. Comment at: lib/AST/CommentSema.cpp:268 @@ -268,5 +267,3 @@ typedef BlockCommandComment::Argument Argument; - Argument *A = new (Allocator) Argument(SourceRange(ArgLocBegin, - ArgLocEnd), - Arg); + auto *A = new (Allocator) Argument(SourceRange(ArgLocBegin, ArgLocEnd), Arg); Command->setArgs(llvm::makeArrayRef(A, 1)); This is an improvement. Comment at: lib/AST/CommentSema.cpp:304 @@ -306,5 +303,3 @@ typedef BlockCommandComment::Argument Argument; - Argument *A = new (Allocator) Argument(SourceRange(ArgLocBegin, - ArgLocEnd), - Arg); + auto *A = new (Allocator) Argument(SourceRange(ArgLocBegin, ArgLocEnd), Arg); Command->setArgs(llvm::makeArrayRef(A, 1)); As is this one. Comment at: lib/AST/CommentSema.cpp:357 @@ -356,3 +351,1 @@ - - return; } Good catch on removing this one! Repository: rL LLVM http://reviews.llvm.org/D14560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12031: Const std::move() argument ClangTidy check
aaron.ballman added inline comments. Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:11 @@ +10,3 @@ +#include "MoveConstantArgumentCheck.h" + +namespace clang { > I didn't find how it can be done, could you please advice? This is the usual way we do it (in the registerMatchers() function): ``` if (!getLangOpts().CPlusPlus) return; ``` Comment at: test/clang-tidy/move-const-arg.cpp:1 @@ +1,2 @@ +// RUN: %check_clang_tidy %s misc-move-const-arg %t -- -- -std=c++11 + Please run clang-format over the test files as well. http://reviews.llvm.org/D12031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification
seaneveson added a comment. In http://reviews.llvm.org/D10305#286385, @zaks.anna wrote: > The reason I like names more than the numbers is that we may use different > solutions for issue hash generation and some users might prefer one over the > other. It is not necessarily clear which one is the best. Numbers would > obfuscate the heuristic used to produce the hash and the quality of the hash > and would be mainly based on the time when the hash was introduced. Thanks, I understand where you are coming from now. > > This makes forwards compatibility difficult, since there is no way to > > predict the names of future hashes > > > (As far as I understand). > > > Can you describe what you are trying to achieve? Just for the sake of explaining, lets say in 3 subsequent Analyzer releases the hashes are called “hash_1”, “hash_2” and “hash_3”. In the first release the suppression tool will record hash_1 to suppress a warning. Some developers will upgrade to the second and third release, where others may jump straight to the third release. Additionally some developers may temporarily downgrade to the second after upgrading to the third release. The suppression tool (which may not be upgraded during this period) needs to maintain suppressions between these version upgrades/downgrades. Also, some developers may upgrade before others, where they all share one repository of suppressions. Say there is an issue with hash_1 and two warnings collide; this is fixed by hash_2. When the user upgrades to the 2nd release, the suppression tool will record hash_2 on top of hash_1. The tool will then suppress using hash_2 if it is present in the plist file, ignoring hash_1. If the user downgrades and hash_2 is not in the plist file the tool will suppress using hash_1. For this to work the tool needs to know the order in which to look at the hashes. > We can agree that all issue hashes start with "issue_hash" prefix. If you > find an entry with "issue_hash" prefix and unknown suffix, you would know > that it's new. It would be the same as a number you have not seen so far. The tool would not be able to establish the order of multiple produced hashes when it was first run. > > A third alternative would be to have both semantic names (containing hash) > > and a number suffix > > > which indicates the ordering. > > > If there is a minor enhancement to the existing issue hash method, appending > the version number to it is fine by me. Though, this might be confusing in > it's own right.. This would work for us, although I agree it might be a little confusing. Would issue_hash__ be better? e.g. "issue_hash_1_content_of _line_in_context". http://reviews.llvm.org/D10305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"
aaron.ballman added a comment. Missing tests demonstrating use of the C++ spelling of the attribute. Perhaps a test showing it on a member function would be useful. Out of curiosity, what would be the expected behavior of the following: struct B { int g(int); [[clang::disable_tail_calls]] virtual int f(int a); // is this okay? }; struct D : B { int f(int a) override { // is this okay? return g(a); // Does this get TCO? } }; void f() { B *b = new D; b->f(12); } Comment at: include/clang/Basic/AttrDocs.td:1683 @@ +1682,3 @@ + int foo(int a) __attribute__((disable_tail_calls)) { +return callee(a); // This call is not tail-call optimized. + } It would be useful to have a declaration for callee() to make it obvious that it *could* have been TCOed otherwise. Comment at: lib/CodeGen/CGCall.cpp:1486 @@ -1484,1 +1485,3 @@ +(TargetDecl && TargetDecl->hasAttr()) || +CodeGenOpts.DisableTailCalls; FuncAttrs.addAttribute("disable-tail-calls", I would swap the order of the checks. It's less expensive when CodeGenOpts.DisableTailCalls is true than it is to call hasAttr(). http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"
ahatanak added a comment. Marking virtual functions as disable_tail_calls is fine since disable_tail_calls affects the call sites inside the body of the marked function. In your example, it prevents tail call optimization on call sites inside B::g, but doesn't affect call sites in D::g. http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D9600: Add scan-build python implementation
jroelofs added a subscriber: jroelofs. jroelofs added a comment. Would you mind re-uploading this patch as a diff against upstream trunk with full context? http://reviews.llvm.org/D9600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"
aaron.ballman added a comment. In http://reviews.llvm.org/D12547#287153, @ahatanak wrote: > Marking virtual functions as disable_tail_calls is fine since > disable_tail_calls affects the call sites inside the body of the marked > function. In your example, it prevents tail call optimization on call sites > inside B::g, but doesn't affect call sites in D::g. Okay, that's reasonable, but you should definitely update the documentation to reflect that and have at least one codegen test that ensures the behavior is implemented as-expected. http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14293: [libcxx] Add -fno-exceptions libcxx builders to zorg
rmaprath added a comment. Would it be OK if I commit the x86 buildbot changes (Dmitri approved these earlier) while the ARM buildbot changes are being reviewed? I suppose the changes would only take effect once @gkistanova restarts/reconfigs the build-master ? Cheers, - Asiri http://reviews.llvm.org/D14293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14293: [libcxx] Add -fno-exceptions libcxx builders to zorg
jroelofs added a comment. In http://reviews.llvm.org/D14293#287174, @rmaprath wrote: > Would it be OK if I commit the x86 buildbot changes (Dmitri approved these > earlier) while the ARM buildbot changes are being reviewed? I suppose the > changes would only take effect once @gkistanova restarts/reconfigs the > build-master ? > > Cheers, > > - Asiri That sounds reasonable to me. Be sure to notify Galina once you've made the change. http://reviews.llvm.org/D14293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [PATCH] D14570: Handle ARMv6KZ naming
> Why isn't this just part of D14568? It's in the other repository. From: James Molloy [mailto:ja...@jamesmolloy.co.uk] Sent: 11 November 2015 12:42 To: reviews+d14570+public+1de1f7f06e3d3...@reviews.llvm.org; Artyom Skrobov; renato.go...@linaro.org; jo...@netbsd.org; Bernard Ogden Cc: kanh...@a-bix.com; cfe-commits@lists.llvm.org Subject: Re: [PATCH] D14570: Handle ARMv6KZ naming Hi Artyom, Why isn't this just part of D14568? Cheers, James On Wed, 11 Nov 2015 at 12:08 A. Skrobov via cfe-commits wrote: tyomitch created this revision. tyomitch added reviewers: rengolin, joerg, bogden. tyomitch added a subscriber: cfe-commits. Herald added subscribers: rengolin, aemerson. Update for clang tests for D14568 http://reviews.llvm.org/D14570 Files: test/Driver/arm-cortex-cpus.c test/Driver/biarch.c Index: test/Driver/biarch.c === --- test/Driver/biarch.c +++ test/Driver/biarch.c @@ -6,8 +6,9 @@ // RUN: %clang -target x86_64--netbsd -m64 %s -### 2>&1 | FileCheck -check-prefix=X86_64 %s // X86_64: "-cc1" "-triple" "x86_64--netbsd" +// r196538 set arm1176jzf-s as default CPU for ARMv6 on NetBSD // RUN: %clang -target armv6--netbsd-eabihf -m32 %s -### 2>&1 | FileCheck -check-prefix=ARMV6 %s -// ARMV6: "-cc1" "-triple" "armv6k--netbsd-eabihf" +// ARMV6: "-cc1" "-triple" "armv6kz--netbsd-eabihf" // RUN: %clang -target sparcv9--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s // RUN: %clang -target sparc--netbsd -m32 %s -### 2>&1 | FileCheck -check-prefix=SPARC %s Index: test/Driver/arm-cortex-cpus.c === --- test/Driver/arm-cortex-cpus.c +++ test/Driver/arm-cortex-cpus.c @@ -73,11 +73,11 @@ // RUN: %clang -target armv6k -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K %s // RUN: %clang -target arm -march=armv6k -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K %s -// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176jzf-s" +// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176j-s" // RUN: %clang -target armv6k -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K-THUMB %s // RUN: %clang -target arm -march=armv6k -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6K-THUMB %s -// CHECK-V6K-THUMB: "-cc1"{{.*}} "-triple" "thumbv6k-{{.*}} "-target-cpu" "arm1176jzf-s" +// CHECK-V6K-THUMB: "-cc1"{{.*}} "-triple" "thumbv6k-{{.*}} "-target-cpu" "arm1176j-s" // RUN: %clang -target armv6t2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6T2 %s // RUN: %clang -target arm -march=armv6t2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V6T2 %s @@ -249,12 +249,14 @@ // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1136jf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6 %s // CHECK-CPUV6: "-cc1"{{.*}} "-triple" "armv6-{{.*}} -// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s -// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcore -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s // RUN: %clang -target arm-linux-gnueabi -mcpu=mpcorenovfp -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6K %s // CHECK-CPUV6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} +// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jz-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6KZ %s +// RUN: %clang -target arm-linux-gnueabi -mcpu=arm1176jzf-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6KZ %s +// CHECK-CPUV6KZ: "-cc1"{{.*}} "-triple" "armv6kz-{{.*}} + // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1156t2-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6T2 %s // RUN: %clang -target arm-linux-gnueabi -mcpu=arm1156t2f-s -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV6T2 %s // CHECK-CPUV6T2: "-cc1"{{.*}} "-triple" "armv6t2-{{.*}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252771 - Hiding the scan-build and scan-view projects under the Misc folder in IDEs instead of having them at the root view.
Author: aaronballman Date: Wed Nov 11 12:13:42 2015 New Revision: 252771 URL: http://llvm.org/viewvc/llvm-project?rev=252771&view=rev Log: Hiding the scan-build and scan-view projects under the Misc folder in IDEs instead of having them at the root view. Modified: cfe/trunk/tools/scan-build/CMakeLists.txt cfe/trunk/tools/scan-view/CMakeLists.txt Modified: cfe/trunk/tools/scan-build/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/CMakeLists.txt?rev=252771&r1=252770&r2=252771&view=diff == --- cfe/trunk/tools/scan-build/CMakeLists.txt (original) +++ cfe/trunk/tools/scan-build/CMakeLists.txt Wed Nov 11 12:13:42 2015 @@ -76,5 +76,6 @@ if(CLANG_INSTALL_SCANBUILD) endforeach() add_custom_target(scan-build ALL DEPENDS ${Depends}) + set_target_properties(scan-build PROPERTIES FOLDER "Misc") endif() Modified: cfe/trunk/tools/scan-view/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-view/CMakeLists.txt?rev=252771&r1=252770&r2=252771&view=diff == --- cfe/trunk/tools/scan-view/CMakeLists.txt (original) +++ cfe/trunk/tools/scan-view/CMakeLists.txt Wed Nov 11 12:13:42 2015 @@ -37,4 +37,5 @@ if(CLANG_INSTALL_SCANVIEW) endforeach() add_custom_target(scan-view ALL DEPENDS ${Depends}) + set_target_properties(scan-view PROPERTIES FOLDER "Misc") endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D14578: Cull non-standard variants of ARM architectures (NFC)
tyomitch created this revision. tyomitch added a reviewer: rengolin. tyomitch added a subscriber: cfe-commits. Herald added subscribers: rengolin, aemerson. Clang-side update, corresponding to D14577 http://reviews.llvm.org/D14578 Files: lib/Basic/Targets.cpp Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -4366,15 +4366,10 @@ default: return llvm::ARM::getCPUAttr(ArchKind); case llvm::ARM::AK_ARMV6M: -case llvm::ARM::AK_ARMV6SM: -case llvm::ARM::AK_ARMV6HL: return "6M"; case llvm::ARM::AK_ARMV7S: return "7S"; -case llvm::ARM::AK_ARMV7: case llvm::ARM::AK_ARMV7A: -case llvm::ARM::AK_ARMV7L: -case llvm::ARM::AK_ARMV7HL: return "7A"; case llvm::ARM::AK_ARMV7R: return "7R"; Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -4366,15 +4366,10 @@ default: return llvm::ARM::getCPUAttr(ArchKind); case llvm::ARM::AK_ARMV6M: -case llvm::ARM::AK_ARMV6SM: -case llvm::ARM::AK_ARMV6HL: return "6M"; case llvm::ARM::AK_ARMV7S: return "7S"; -case llvm::ARM::AK_ARMV7: case llvm::ARM::AK_ARMV7A: -case llvm::ARM::AK_ARMV7L: -case llvm::ARM::AK_ARMV7HL: return "7A"; case llvm::ARM::AK_ARMV7R: return "7R"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification
zaks.anna added a comment. > Just for the sake of explaining, lets say in 3 subsequent Analyzer releases > the hashes are called “hash_1”, “hash_2” and “hash_3”. > In the first release the suppression tool will record hash_1 to suppress a > warning. Some developers will upgrade to the second and third release, where > others may jump straight to the third release. Additionally some developers > may temporarily downgrade to the second after upgrading to the third > release. The suppression tool (which may not be upgraded during this period) > needs to maintain suppressions between these version > upgrades/downgrades. Also, some developers may upgrade before others, where > they all share one repository of suppressions. > Say there is an issue with hash_1 and two warnings collide; this is fixed by > hash_2. When the user upgrades to the 2nd release, the suppression tool will > record hash_2 on top of hash_1. The tool will then suppress using hash_2 if > it is present in the plist file, ignoring hash_1. If the user downgrades and > hash_2 is not in the plist file the tool will suppress using hash_1. For > this to work the tool needs to know the order in which to look at the hashes. I still do not understand how your system would work. If you have multiple users using a bug suppression system, I would design such system using only a single hash version across all users; using a mix seems error prone.. Once all of your users upgrade to a version of the analyzer where a new hash version is available, you upgrade the hash in the database to reflect that. Let's talk about your example. You have user1 using hash_version_1 and user2 using hash_version_2. If the second user suppresses an issue and hash_version_2 is used to record that, how will user1 be able to identity that issue? Also, as I've mentioned before, the newest issue hash would not necessarily be the best. > This would work for us, although I agree it might be a little confusing. > Would issue_hash__ be better? e.g. > "issue_hash_1_content_of _line_in_context". We could have a list of issue hashes and each item in the list containing type_of_hash, hash_version, hash. However, I am not yet convinced that this complexity is justified. http://reviews.llvm.org/D10305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r252773 - Fix bug in suggested fix that truncated variable names to 1 character.
Author: sbenza Date: Wed Nov 11 12:40:36 2015 New Revision: 252773 URL: http://llvm.org/viewvc/llvm-project?rev=252773&view=rev Log: Fix bug in suggested fix that truncated variable names to 1 character. Summary: Fix bug in suggested fix that truncated variable names to 1 character. Also, rework the suggested fix to try to remove unnecessary whitespace. Reviewers: alexfh, aaron.ballman Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D13899 Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=252773&r1=252772&r2=252773&view=diff == --- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Wed Nov 11 12:40:36 2015 @@ -10,6 +10,7 @@ #include "UnusedParametersCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; @@ -20,36 +21,39 @@ void UnusedParametersCheck::registerMatc Finder->addMatcher(functionDecl().bind("function"), this); } -static FixItHint removeParameter(const FunctionDecl *Function, unsigned Index) { - const ParmVarDecl *Param = Function->getParamDecl(Index); - unsigned ParamCount = Function->getNumParams(); - SourceRange RemovalRange = Param->getSourceRange(); - if (ParamCount == 1) -return FixItHint::CreateRemoval(RemovalRange); - - if (Index == 0) -RemovalRange.setEnd( -Function->getParamDecl(Index + 1)->getLocStart().getLocWithOffset(-1)); - else -RemovalRange.setBegin( -Function->getParamDecl(Index - 1)->getLocEnd().getLocWithOffset(1)); +template +static CharSourceRange removeNode(const MatchFinder::MatchResult &Result, + const T *PrevNode, const T *Node, + const T *NextNode) { + if (NextNode) +return CharSourceRange::getCharRange(Node->getLocStart(), + NextNode->getLocStart()); + + if (PrevNode) +return CharSourceRange::getTokenRange( +Lexer::getLocForEndOfToken(PrevNode->getLocEnd(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), +Node->getLocEnd()); - return FixItHint::CreateRemoval(RemovalRange); + return CharSourceRange::getTokenRange(Node->getSourceRange()); } -static FixItHint removeArgument(const CallExpr *Call, unsigned Index) { - unsigned ArgCount = Call->getNumArgs(); - const Expr *Arg = Call->getArg(Index); - SourceRange RemovalRange = Arg->getSourceRange(); - if (ArgCount == 1) -return FixItHint::CreateRemoval(RemovalRange); - if (Index == 0) -RemovalRange.setEnd( -Call->getArg(Index + 1)->getLocStart().getLocWithOffset(-1)); - else -RemovalRange.setBegin( -Call->getArg(Index - 1)->getLocEnd().getLocWithOffset(1)); - return FixItHint::CreateRemoval(RemovalRange); +static FixItHint removeParameter(const MatchFinder::MatchResult &Result, + const FunctionDecl *Function, unsigned Index) { + return FixItHint::CreateRemoval(removeNode( + Result, Index > 0 ? Function->getParamDecl(Index - 1) : nullptr, + Function->getParamDecl(Index), + Index + 1 < Function->getNumParams() ? Function->getParamDecl(Index + 1) + : nullptr)); +} + +static FixItHint removeArgument(const MatchFinder::MatchResult &Result, +const CallExpr *Call, unsigned Index) { + return FixItHint::CreateRemoval(removeNode( + Result, Index > 0 ? Call->getArg(Index - 1) : nullptr, + Call->getArg(Index), + Index + 1 < Call->getNumArgs() ? Call->getArg(Index + 1) : nullptr)); } void UnusedParametersCheck::warnOnUnusedParameter( @@ -85,7 +89,7 @@ void UnusedParametersCheck::warnOnUnused // Fix all redeclarations. for (const FunctionDecl *FD : Function->redecls()) if (FD->param_size()) - MyDiag << removeParameter(FD, ParamIndex); + MyDiag << removeParameter(Result, FD, ParamIndex); // Fix all call sites. auto CallMatches = ast_matchers::match( @@ -93,7 +97,8 @@ void UnusedParametersCheck::warnOnUnused callExpr(callee(functionDecl(equalsNode(Function.bind("x"))), *Result.Context->getTranslationUnitDecl(), *Result.Context); for (const auto &Match : CallMatches) -MyDiag << removeArgument(Match.getNodeAs("x"), ParamIndex); +MyDiag << removeArgument(Result, Match.getNodeAs("x"), + ParamIndex); } void UnusedParametersCheck::c
Re: [PATCH] D13899: Fix bug in suggested fix that truncated variable names to 1 character.
This revision was automatically updated to reflect the committed changes. Closed by commit rL252773: Fix bug in suggested fix that truncated variable names to 1 character. (authored by sbenza). Changed prior to commit: http://reviews.llvm.org/D13899?vs=37885&id=39944#toc Repository: rL LLVM http://reviews.llvm.org/D13899 Files: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp === --- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp @@ -10,6 +10,7 @@ #include "UnusedParametersCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; @@ -20,36 +21,39 @@ Finder->addMatcher(functionDecl().bind("function"), this); } -static FixItHint removeParameter(const FunctionDecl *Function, unsigned Index) { - const ParmVarDecl *Param = Function->getParamDecl(Index); - unsigned ParamCount = Function->getNumParams(); - SourceRange RemovalRange = Param->getSourceRange(); - if (ParamCount == 1) -return FixItHint::CreateRemoval(RemovalRange); - - if (Index == 0) -RemovalRange.setEnd( -Function->getParamDecl(Index + 1)->getLocStart().getLocWithOffset(-1)); - else -RemovalRange.setBegin( -Function->getParamDecl(Index - 1)->getLocEnd().getLocWithOffset(1)); - - return FixItHint::CreateRemoval(RemovalRange); -} - -static FixItHint removeArgument(const CallExpr *Call, unsigned Index) { - unsigned ArgCount = Call->getNumArgs(); - const Expr *Arg = Call->getArg(Index); - SourceRange RemovalRange = Arg->getSourceRange(); - if (ArgCount == 1) -return FixItHint::CreateRemoval(RemovalRange); - if (Index == 0) -RemovalRange.setEnd( -Call->getArg(Index + 1)->getLocStart().getLocWithOffset(-1)); - else -RemovalRange.setBegin( -Call->getArg(Index - 1)->getLocEnd().getLocWithOffset(1)); - return FixItHint::CreateRemoval(RemovalRange); +template +static CharSourceRange removeNode(const MatchFinder::MatchResult &Result, + const T *PrevNode, const T *Node, + const T *NextNode) { + if (NextNode) +return CharSourceRange::getCharRange(Node->getLocStart(), + NextNode->getLocStart()); + + if (PrevNode) +return CharSourceRange::getTokenRange( +Lexer::getLocForEndOfToken(PrevNode->getLocEnd(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), +Node->getLocEnd()); + + return CharSourceRange::getTokenRange(Node->getSourceRange()); +} + +static FixItHint removeParameter(const MatchFinder::MatchResult &Result, + const FunctionDecl *Function, unsigned Index) { + return FixItHint::CreateRemoval(removeNode( + Result, Index > 0 ? Function->getParamDecl(Index - 1) : nullptr, + Function->getParamDecl(Index), + Index + 1 < Function->getNumParams() ? Function->getParamDecl(Index + 1) + : nullptr)); +} + +static FixItHint removeArgument(const MatchFinder::MatchResult &Result, +const CallExpr *Call, unsigned Index) { + return FixItHint::CreateRemoval(removeNode( + Result, Index > 0 ? Call->getArg(Index - 1) : nullptr, + Call->getArg(Index), + Index + 1 < Call->getNumArgs() ? Call->getArg(Index + 1) : nullptr)); } void UnusedParametersCheck::warnOnUnusedParameter( @@ -85,15 +89,16 @@ // Fix all redeclarations. for (const FunctionDecl *FD : Function->redecls()) if (FD->param_size()) - MyDiag << removeParameter(FD, ParamIndex); + MyDiag << removeParameter(Result, FD, ParamIndex); // Fix all call sites. auto CallMatches = ast_matchers::match( decl(forEachDescendant( callExpr(callee(functionDecl(equalsNode(Function.bind("x"))), *Result.Context->getTranslationUnitDecl(), *Result.Context); for (const auto &Match : CallMatches) -MyDiag << removeArgument(Match.getNodeAs("x"), ParamIndex); +MyDiag << removeArgument(Result, Match.getNodeAs("x"), + ParamIndex); } void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) { Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp === --- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp @@ -42,7 +42,7 @@ static void staticFunctionC(int i, int j) { (void)j; } // CHECK-MESSAGES: :[[@LINE-1]]:33: warning
Re: [PATCH] D14203: [analyzer] Improve pointer arithmetic checker.
dcoughlin added a comment. Gabor, This is an alpha checker. Do you anticipate turning it on by default? Comments inline. Comment at: lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp:28 @@ -24,1 +27,3 @@ namespace { +enum class AllocKind { + SingletonNew, Is it necessary to distinguish so many cases here? For example, why do we need to distinguish between PlacementNew and OverloadedNew? Another thought: given the expense of tracking stuff in the GDM could we instead track whether pointer arithmetic is explicitly disallowed for a given region? Then we wouldn't have to track any data for the "good" pointers. Also, how does AllocKind relate to the AllocationFamily from MallocChecker? Could that checker's state be used so we don't have to track any additional information here? If you do keep AllocKind, I think it would be good to add a comment describing how this enum is used and the intended meaning of each element. Comment at: lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp:29 @@ +28,3 @@ +enum class AllocKind { + SingletonNew, + ArrayNew, I'm not sure "singleton" is the right term here. I associate "singleton" with the design pattern of only having a single instance of a class allocated at a given time (a form of global shared state). Also, SingletonMalloc doesn't appear to be used anywhere. Comment at: lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp:88 @@ -28,1 +87,3 @@ + mutable std::unique_ptr BT_polyArray; + mutable llvm::SmallVector AllocFunctions; I think it would be better to use llvm::SmallSet here. Comment at: lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp:137 @@ -43,2 +136,3 @@ - const MemRegion *LR = LV.getAsRegion(); +const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region, + bool &Polymorphic, I think it would be good to add a doc comment for this function describing what the function does and its parameters as well as whether they are input or output parameters. I also wonder if this logic is better expressed as a loop rather than recursion. What do you think? Comment at: lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp:150 @@ +149,3 @@ +return getArrayRegion(Region, Polymorphic, AKind, C); + default: +break; In general, I think it is better to avoid default in cases like these so that when an enum case is added the compiler issues a warning and thus forces the person adding the change to think about what the behavior of the new case should be. Comment at: lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp:198 @@ +197,3 @@ +this, "Dangerous pointer arithmetic", +"Pointer arithmetic does not account for polymorphic object sizes " +"and attempting to perform pointer arithmetic on a polymorphic " I think "polymorphic" is a bit jargony. Can this diagnostic be explained in terms of base and derived classes? Comment at: test/Analysis/ptr-arith.cpp:47 @@ +46,3 @@ + p = p + 2; // expected-warning{{}} + p = 2 + p; // expected-warning{{}} + p += 2; // expected-warning{{}} I think it would be good to add tests showing `p = i*0 + p' and `p = p + i*0' don't alarm. Also `p += i*0'. http://reviews.llvm.org/D14203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [Diffusion] rL244063: Add missing atomic libcall support.
tstellarAMD resigned from this audit. Users: jyknight (Author, Auditor) 3.7-release (Auditor) cfe-commits (Auditor) tstellarAMD (Auditor) compnerd (Auditor) majnemer (Auditor) rsmith (Auditor) http://reviews.llvm.org/rL244063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [Diffusion] rL244063: Add missing atomic libcall support.
tstellarAMD raised a concern with this commit. Users: jyknight (Author, Auditor) 3.7-release (Auditor) cfe-commits (Auditor) tstellarAMD (Auditor) compnerd (Auditor) majnemer (Auditor) rsmith (Auditor) http://reviews.llvm.org/rL244063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14200: Make FP_CONTRACT ON default.
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM, thanks! http://reviews.llvm.org/D14200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14517: Fix bug 25362 "cppcoreguidelines-pro-bounds-array-to-pointer-decay does not consider const"
mgehre updated this revision to Diff 39948. mgehre added a comment. Update for review comments http://reviews.llvm.org/D14517 Files: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp @@ -39,3 +39,9 @@ const char *g() { return "clang"; // OK, decay string literal to pointer } + +void f2(void *const *); +void bug25362() { + void *a[2]; + f2(static_cast(a)); // OK, explicit cast +} Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h === --- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h +++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h @@ -31,4 +31,3 @@ } // namespace clang #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_ARRAY_TO_POINTER_DECAY_H - Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp === --- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp +++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp @@ -28,6 +28,22 @@ .matches(Node, Finder, Builder); } +AST_MATCHER_P(Expr, hasParentIgnoringImpCasts, + ast_matchers::internal::Matcher, InnerMatcher) { + const Expr *E = &Node; + do { +ASTContext::DynTypedNodeList Parents = +Finder->getASTContext().getParents(*E); +if (Parents.size() != 1) + return false; +E = Parents[0].get(); +if (!E) + return false; + } while (isa(E)); + + return InnerMatcher.matches(*E, Finder, Builder); +} + void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) return; @@ -38,7 +54,7 @@ // 3) if it converts a string literal to a pointer Finder->addMatcher( implicitCastExpr(unless(hasParent(arraySubscriptExpr())), - unless(hasParent(explicitCastExpr())), + unless(hasParentIgnoringImpCasts(explicitCastExpr())), unless(isInsideOfRangeBeginEndStmt()), unless(hasSourceExpression(stringLiteral( .bind("cast"), Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp @@ -39,3 +39,9 @@ const char *g() { return "clang"; // OK, decay string literal to pointer } + +void f2(void *const *); +void bug25362() { + void *a[2]; + f2(static_cast(a)); // OK, explicit cast +} Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h === --- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h +++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h @@ -31,4 +31,3 @@ } // namespace clang #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_ARRAY_TO_POINTER_DECAY_H - Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp === --- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp +++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp @@ -28,6 +28,22 @@ .matches(Node, Finder, Builder); } +AST_MATCHER_P(Expr, hasParentIgnoringImpCasts, + ast_matchers::internal::Matcher, InnerMatcher) { + const Expr *E = &Node; + do { +ASTContext::DynTypedNodeList Parents = +Finder->getASTContext().getParents(*E); +if (Parents.size() != 1) + return false; +E = Parents[0].get(); +if (!E) + return false; + } while (isa(E)); + + return InnerMatcher.matches(*E, Finder, Builder); +} + void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) return; @@ -38,7 +54,7 @@ // 3) if it converts a string literal to a pointer Finder->addMatcher( implicitCastExpr(unless(hasParent(arraySubscriptExpr())), - unless(hasParent(explicitCastExpr())), + unless(hasParentIgnoringImpCasts(explicitCastExpr())), unless(isInsideOfRangeBeginEndStmt()), unless(hasSourceExpression(stringLiteral( .bind("cast"), ___ cfe-commits
Re: [PATCH] Change memcpy/memmove/memset to have dest and source alignment
- Original Message - > From: "Pete Cooper" > To: "Hal Finkel" > Cc: "Lang Hames" , "LLVM Commits" > , cfe-commits@lists.llvm.org > Sent: Monday, September 28, 2015 12:46:36 PM > Subject: Re: [PATCH] Change memcpy/memmove/memset to have dest and > source alignment > Hey Hal > Thanks for the review. I really appreciate it given the scale of > this. > > On Sep 25, 2015, at 1:13 PM, Hal Finkel < hfin...@anl.gov > wrote: > > > Hi Pete, > > > Thanks for working on this. > > > + class IntegerAlignment { > > > + private: > > > + uint64_t Align; > > > You explain in the patch summary why this is here, but please add a > > comment with the explanation as well. > > Will do. Good catch. > > Regarding the auto-upgrade, are we going to run into problems if we > > separate our the 'volatile' tag for the source and the destination > > as Lang had suggested? If we're going to do that, should we do it > > all at the same time? Does it change the need for the > > IntegerAlignment class? > > Luckily I think this will be easier as its just an addition, unlike > this patch which is moving things around to attributes. I’m also > pretty confident about using sed to auto-upgrade all the tests with > the volatile flag. Its much easier to sed for the ‘i1 [true|false])’ > at the end of the call and just duplicate that piece, than it is to > extract the alignment out from the middle and print it back out as > an attribute. > Saying that, there will be similar churn on things like the > IRBuilder, and we may want some way to (temporarily at the least) > make sure that everyone is forced to consider whether they want to > set both isVolatile flags given that they set one of them. A few > possibilities are: > > // Bad, because we don’t know whether current users who set > > isDestVolatile to true also want source volatility too > > > Create(..., bool isDestVolatile = false, bool isSrcVolatile = > > false) > > > // Better, all current users will have to change anyway, and then > > we > > know they all care about src/dest volatility > > > Create(…, std::pair isDestSrcVolatile = std::pair > bool>(false, false)) > > > // Another option, and we would use an assert perhaps to make sure > > that if dest is set, then so is src > > > Create(…, Optional isDestVolatile = None, Optional > > isSrcVolatile = None) { > > > assert(isDestVolatile.hasValue() == isSrcVolatile.hasValue()); > > > … > > Anyway, I think changes can come later, but just a few ideas there. > > Everything else looks good, and I like the cleanup in > > AlignmentFromAssumptions :-) > > Thanks :) Can I take that as a LGTM, or... It seems like I dropped the ball on this. Yes, I recall being fine with them otherwise. Thanks again, Hal > > Thanks again, > > > Hal > > > P.S. I find full-context patches in Phabricator much easier than > > diffs; this does not matter for the automated regression-test > > updates, but for the code changes, I appreciate patches there. > > … would you prefer to see the patch in phab first? I’m happy either > way. I will leave the test case changes here though, and just put > the code in phab if needed and if thats ok. > Cheers, > Pete > > - Original Message - > > > > From: "Pete Cooper" < peter_coo...@apple.com > > > > > > > To: "Hal J. Finkel" < hfin...@anl.gov > > > > > > > Cc: "Lang Hames" < lha...@apple.com >, "LLVM Commits" < > > > llvm-comm...@lists.llvm.org >, cfe-commits@lists.llvm.org > > > > > > Sent: Friday, August 28, 2015 5:59:18 PM > > > > > > Subject: [PATCH] Change memcpy/memmove/memset to have dest and > > > source > > > alignment > > > > > > Hi Hal/Lang > > > > > > This came out of a discussion here: > > > > > > http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html > > > > > > We want to be able to provide alignment for both the source and > > > dest > > > > > > of mem intrinsics, instead of the alignment argument which only > > > > > > provides the minimum of dest/src. > > > > > > Instead of adding another argument, I removed the alignment > > > argument > > > > > > and added the align attributes to the src/dest pointer arguments. > > > > > > I’ve updated the MemIntrinsic definition to handle this, and all > > > of > > > > > > the code to now call getDestAlignment/getSrcAlignment instead of > > > > > > getAlignment. For the few places where it wasn’t clear whether > > > > > > dest/src was the right choice, i’ve left a FIXME and I take the > > > > > > minimum of dest/src so as to match the current behavior. > > > > > > I’ve also updated the create methods in the IR builder. There is > > > a > > > > > > (temporary) class there to handle the new source alignment > > > > > > parameter, as otherwise existing callers of this code could end > > > up > > > > > > having the isVolatile bool implicitly converted to the source > > > > > > alignment. I’ll remove this once out-of-tree users have had a > > > chance > > >
r252777 - [TLS] move setting tls_guard in tls_init.
Author: mren Date: Wed Nov 11 13:19:26 2015 New Revision: 252777 URL: http://llvm.org/viewvc/llvm-project?rev=252777&view=rev Log: [TLS] move setting tls_guard in tls_init. We used to emit the store prior to branch in the entry block. To make it more efficient, this commit moves it to the init block. We still mark as initialized before initializing anything else. Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp cfe/trunk/test/OpenMP/threadprivate_codegen.cpp Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=252777&r1=252776&r2=252777&view=diff == --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Wed Nov 11 13:19:26 2015 @@ -518,14 +518,14 @@ CodeGenFunction::GenerateCXXGlobalInitFu llvm::Value *GuardVal = Builder.CreateLoad(Guard); llvm::Value *Uninit = Builder.CreateIsNull(GuardVal, "guard.uninitialized"); - // Mark as initialized before initializing anything else. If the - // initializers use previously-initialized thread_local vars, that's - // probably supposed to be OK, but the standard doesn't say. - Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); llvm::BasicBlock *InitBlock = createBasicBlock("init"); ExitBlock = createBasicBlock("exit"); Builder.CreateCondBr(Uninit, InitBlock, ExitBlock); EmitBlock(InitBlock); + // Mark as initialized before initializing anything else. If the + // initializers use previously-initialized thread_local vars, that's + // probably supposed to be OK, but the standard doesn't say. + Builder.CreateStore(llvm::ConstantInt::get(GuardVal->getType(),1), Guard); } RunCleanupsScope Scope(*this); Modified: cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp?rev=252777&r1=252776&r2=252777&view=diff == --- cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp (original) +++ cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp Wed Nov 11 13:19:26 2015 @@ -173,9 +173,9 @@ void set_anon_i() { // CHECK: define {{.*}}@__tls_init() // CHECK: load i8, i8* @__tls_guard // CHECK: %[[NEED_TLS_INIT:.*]] = icmp eq i8 %{{.*}}, 0 -// CHECK: store i8 1, i8* @__tls_guard // CHECK: br i1 %[[NEED_TLS_INIT]], // init: +// CHECK: store i8 1, i8* @__tls_guard // CHECK: call void @[[A_INIT]]() // CHECK: call void @[[D_INIT]]() // CHECK: call void @[[U_M_INIT]]() Modified: cfe/trunk/test/OpenMP/threadprivate_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/threadprivate_codegen.cpp?rev=252777&r1=252776&r2=252777&view=diff == --- cfe/trunk/test/OpenMP/threadprivate_codegen.cpp (original) +++ cfe/trunk/test/OpenMP/threadprivate_codegen.cpp Wed Nov 11 13:19:26 2015 @@ -939,9 +939,9 @@ int foobar() { // CHECK-TLS: define internal void @__tls_init() // CHECK-TLS: [[GRD:%.*]] = load i8, i8* @__tls_guard // CHECK-TLS-NEXT: [[IS_INIT:%.*]] = icmp eq i8 [[GRD]], 0 -// CHECK-TLS-NEXT: store i8 1, i8* @__tls_guard // CHECK-TLS-NEXT: br i1 [[IS_INIT]], label %[[INIT_LABEL:[^,]+]], label %[[DONE_LABEL:[^,]+]]{{.*}} // CHECK-TLS: [[INIT_LABEL]] +// CHECK-TLS-NEXT: store i8 1, i8* @__tls_guard // CHECK-TLS: call void [[GS1_CXX_INIT]] // CHECK-TLS-NOT: call void [[GS2_CXX_INIT]] // CHECK-TLS: call void [[ARR_X_CXX_INIT]] ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] Change memcpy/memmove/memset to have dest and source alignment
> On Nov 11, 2015, at 11:16 AM, Hal Finkel wrote: > > It seems like I dropped the ball on this. Yes, I recall being fine with them > otherwise. Thanks Hal! I’ll rebase the patches and land them over the next day or two. Thanks, Pete > > Thanks again, > Hal ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252785 - [Lit Test] Updated 26 Lit tests to be C++11 compatible.
Author: lcharles Date: Wed Nov 11 13:34:47 2015 New Revision: 252785 URL: http://llvm.org/viewvc/llvm-project?rev=252785&view=rev Log: [Lit Test] Updated 26 Lit tests to be C++11 compatible. Expected diagnostics have been expanded to vary by C++ dialect. RUN line has also been expanded to: default, C++98/03 and C++11. Modified: cfe/trunk/test/CXX/class.access/class.friend/p2-cxx03.cpp cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.fct.spec/p6.cpp cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p3.cpp cfe/trunk/test/CXX/temp/temp.fct.spec/temp.arg.explicit/p3.cpp cfe/trunk/test/CodeGen/ubsan-type-blacklist.cpp cfe/trunk/test/FixIt/fixit-vexing-parse.cpp cfe/trunk/test/SemaCXX/addr-of-overloaded-function.cpp cfe/trunk/test/SemaCXX/const-cast.cpp cfe/trunk/test/SemaCXX/convert-to-bool.cpp cfe/trunk/test/SemaCXX/copy-initialization.cpp cfe/trunk/test/SemaCXX/cxx0x-return-init-list.cpp cfe/trunk/test/SemaCXX/decltype-crash.cpp cfe/trunk/test/SemaCXX/gnu-flags.cpp cfe/trunk/test/SemaCXX/invalid-member-expr.cpp cfe/trunk/test/SemaCXX/member-expr.cpp cfe/trunk/test/SemaCXX/member-pointer.cpp cfe/trunk/test/SemaCXX/new-array-size-conv.cpp cfe/trunk/test/SemaCXX/offsetof.cpp cfe/trunk/test/SemaCXX/printf-block.cpp cfe/trunk/test/SemaCXX/undefined-internal.cpp cfe/trunk/test/SemaObjCXX/crash.mm cfe/trunk/test/SemaObjCXX/vararg-non-pod.mm cfe/trunk/test/SemaTemplate/default-arguments.cpp cfe/trunk/test/SemaTemplate/temp_class_spec_neg.cpp cfe/trunk/test/SemaTemplate/typename-specifier-4.cpp cfe/trunk/test/SemaTemplate/typename-specifier.cpp Modified: cfe/trunk/test/CXX/class.access/class.friend/p2-cxx03.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/class.friend/p2-cxx03.cpp?rev=252785&r1=252784&r2=252785&view=diff == --- cfe/trunk/test/CXX/class.access/class.friend/p2-cxx03.cpp (original) +++ cfe/trunk/test/CXX/class.access/class.friend/p2-cxx03.cpp Wed Nov 11 13:34:47 2015 @@ -1,7 +1,14 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s template class X0 { - friend T; // expected-warning{{non-class friend type 'T' is a C++11 extension}} + friend T; +#if __cplusplus <= 199711L // C++03 or earlier modes + // expected-warning@-2{{non-class friend type 'T' is a C++11 extension}} +#else + // expected-no-diagnostics +#endif }; class X1 { }; Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.fct.spec/p6.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.fct.spec/p6.cpp?rev=252785&r1=252784&r2=252785&view=diff == --- cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.fct.spec/p6.cpp (original) +++ cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.fct.spec/p6.cpp Wed Nov 11 13:34:47 2015 @@ -1,10 +1,15 @@ // RUN: %clang_cc1 -verify %s +// RUN: %clang_cc1 -verify -std=c++98 %s +// RUN: %clang_cc1 -verify -std=c++11 %s class A { public: explicit A(); - explicit operator int(); // expected-warning {{explicit conversion functions are a C++11 extension}} + explicit operator int(); +#if __cplusplus <= 199711L // C++03 or earlier modes + // expected-warning@-2 {{explicit conversion functions are a C++11 extension}} +#endif explicit void f0(); // expected-error {{'explicit' can only be applied to a constructor or conversion function}} @@ -12,8 +17,11 @@ public: }; explicit A::A() { } // expected-error {{'explicit' can only be specified inside the class definition}} -explicit A::operator bool() { return false; } // expected-warning {{explicit conversion functions are a C++11 extension}}\ - // expected-error {{'explicit' can only be specified inside the class definition}} +explicit A::operator bool() { return false; } +#if __cplusplus <= 199711L // C++03 or earlier modes +// expected-warning@-2 {{explicit conversion functions are a C++11 extension}} +#endif +// expected-error@-4 {{'explicit' can only be specified inside the class definition}} class B { friend explicit A::A(); // expected-error {{'explicit' is invalid in friend declarations}} Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p3.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p3.cpp?rev=252785&r1=252784&r2=252785&view=diff == --- cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p3.cpp (original) +++ cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p3.cpp Wed Nov 11 13:34:47 2015 @@ -1,4 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++9
Re: [PATCH] D13746: [clang-tidy] add check cppcoreguidelines-pro-bounds-constant-array-index
mgehre updated this revision to Diff 39952. mgehre added a comment. update for review comments; removed bounds check on static arrays in favor of clang-diagnostic-array-bounds http://reviews.llvm.org/D13746 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t -- -config='{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader, value: "dir1/gslheader.h"}]}' -- -std=c++11 +#include +// CHECK-FIXES: #include "dir1/gslheader.h" + +namespace gsl { + template + T& at( T(&a)[N], size_t index ); + + template + T& at( std::array &a, size_t index ); +} + +constexpr int const_index(int base) { + return base + 3; +} + +void f(std::array a, int pos) { + a [ pos / 2 /*comment*/] = 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] + // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1; + int j = a[pos - 1]; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead + // CHECK-FIXES: int j = gsl::at(a, pos - 1); + + a.at(pos-1) = 2; // OK, at() instead of [] + gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] + + a[-1] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array [cppcoreguidelines-pro-bounds-constant-array-index] + a[10] = 4; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index] + + a[const_index(7)] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) + + a[0] = 3; // OK, constant index and inside bounds + a[1] = 3; // OK, constant index and inside bounds + a[9] = 3; // OK, constant index and inside bounds + a[const_index(6)] = 3; // OK, constant index and inside bounds +} + +void g() { + int a[10]; + for (int i = 0; i < 10; ++i) { +a[i] = i; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead +// CHECK-FIXES: gsl::at(a, i) = i; +gsl::at(a, i) = i; // OK, gsl::at() instead of [] + } + + a[-1] = 3; // flagged by clang-diagnostic-array-bounds + a[10] = 4; // flagged by clang-diagnostic-array-bounds + a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds + + a[0] = 3; // OK, constant index and inside bounds + a[1] = 3; // OK, constant index and inside bounds + a[9] = 3; // OK, constant index and inside bounds + a[const_index(6)] = 3; // OK, constant index and inside bounds +} + +struct S { + int& operator[](int i); +}; + +void customOperator() { + S s; + int i = 0; + s[i] = 3; // OK, custom operator +} Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -5,6 +5,7 @@ cert-setlongjmp cert-variadic-function-def cppcoreguidelines-pro-bounds-array-to-pointer-decay + cppcoreguidelines-pro-bounds-constant-array-index cppcoreguidelines-pro-bounds-pointer-arithmetic cppcoreguidelines-pro-type-const-cast cppcoreguidelines-pro-type-reinterpret-cast Index: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst === --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst @@ -0,0 +1,13 @@ +cppcoreguidelines-pro-bounds-constant-array-index += + +This check flags all array subscriptions on static arrays and std::arrays that either have a non-compile-time constant index or are out of bounds (for std::array). +For out-of-bounds checking of static arrays, see the clang-diagnostic-array-bounds check. + +Dynamic accesses into arrays are difficult for both tools and humans to validate as safe. gsl::span is a bounds-checked, safe type for accessing arrays of data. gsl::at() is another alternative that ensures single accesses are bounds-checked. If iterators are neede
Re: [PATCH] D13746: [clang-tidy] add check cppcoreguidelines-pro-bounds-constant-array-index
mgehre updated this revision to Diff 39953. mgehre added a comment. simplify code http://reviews.llvm.org/D13746 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t -- -config='{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader, value: "dir1/gslheader.h"}]}' -- -std=c++11 +#include +// CHECK-FIXES: #include "dir1/gslheader.h" + +namespace gsl { + template + T& at( T(&a)[N], size_t index ); + + template + T& at( std::array &a, size_t index ); +} + +constexpr int const_index(int base) { + return base + 3; +} + +void f(std::array a, int pos) { + a [ pos / 2 /*comment*/] = 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] + // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1; + int j = a[pos - 1]; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead + // CHECK-FIXES: int j = gsl::at(a, pos - 1); + + a.at(pos-1) = 2; // OK, at() instead of [] + gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] + + a[-1] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array [cppcoreguidelines-pro-bounds-constant-array-index] + a[10] = 4; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index] + + a[const_index(7)] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) + + a[0] = 3; // OK, constant index and inside bounds + a[1] = 3; // OK, constant index and inside bounds + a[9] = 3; // OK, constant index and inside bounds + a[const_index(6)] = 3; // OK, constant index and inside bounds +} + +void g() { + int a[10]; + for (int i = 0; i < 10; ++i) { +a[i] = i; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead +// CHECK-FIXES: gsl::at(a, i) = i; +gsl::at(a, i) = i; // OK, gsl::at() instead of [] + } + + a[-1] = 3; // flagged by clang-diagnostic-array-bounds + a[10] = 4; // flagged by clang-diagnostic-array-bounds + a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds + + a[0] = 3; // OK, constant index and inside bounds + a[1] = 3; // OK, constant index and inside bounds + a[9] = 3; // OK, constant index and inside bounds + a[const_index(6)] = 3; // OK, constant index and inside bounds +} + +struct S { + int& operator[](int i); +}; + +void customOperator() { + S s; + int i = 0; + s[i] = 3; // OK, custom operator +} Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -5,6 +5,7 @@ cert-setlongjmp cert-variadic-function-def cppcoreguidelines-pro-bounds-array-to-pointer-decay + cppcoreguidelines-pro-bounds-constant-array-index cppcoreguidelines-pro-bounds-pointer-arithmetic cppcoreguidelines-pro-type-const-cast cppcoreguidelines-pro-type-reinterpret-cast Index: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst === --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst @@ -0,0 +1,13 @@ +cppcoreguidelines-pro-bounds-constant-array-index += + +This check flags all array subscriptions on static arrays and std::arrays that either have a non-compile-time constant index or are out of bounds (for std::array). +For out-of-bounds checking of static arrays, see the clang-diagnostic-array-bounds check. + +Dynamic accesses into arrays are difficult for both tools and humans to validate as safe. gsl::span is a bounds-checked, safe type for accessing arrays of data. gsl::at() is another alternative that ensures single accesses are bounds-checked. If iterators are needed to access an array, use the iterators from an gsl::span constructed over the array. + +The ch
[PATCH] D14582: [clang-tidy] cppcoreguidelines-pro-bounds-pointer-arithmetic: ignore generated pointer arithmetic
mgehre created this revision. mgehre added reviewers: alexfh, sbenza, bkramer, aaron.ballman. mgehre added a subscriber: cfe-commits. Inside a range-based for-loop over an array, the compiler generates pointer arithmetic (end = array + size). Don't flag this. http://reviews.llvm.org/D14582 Files: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp @@ -84,4 +84,6 @@ i = j - 1; auto diff = p - q; // OK, result is arithmetic + + for(int ii : a) ; // OK, pointer arithmetic generated by compiler } Index: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp === --- clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp +++ clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp @@ -16,15 +16,28 @@ namespace clang { namespace tidy { +AST_MATCHER_P(CXXForRangeStmt, hasRangeBeginEndStmt, + ast_matchers::internal::Matcher, InnerMatcher) { + const DeclStmt *const Stmt = Node.getBeginEndStmt(); + return (Stmt != nullptr && InnerMatcher.matches(*Stmt, Finder, Builder)); +} + +AST_MATCHER(Stmt, isInsideOfRangeBeginEndStmt) { + return stmt(hasAncestor(cxxForRangeStmt( + hasRangeBeginEndStmt(hasDescendant(equalsNode(&Node)) + .matches(Node, Finder, Builder); +} + void ProBoundsPointerArithmeticCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) return; // Flag all operators +, -, +=, -=, ++, -- that result in a pointer Finder->addMatcher( binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-"), hasOperatorName("+="), hasOperatorName("-=")), - hasType(pointerType())) + hasType(pointerType()), + unless(isInsideOfRangeBeginEndStmt())) .bind("expr"), this); Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp @@ -84,4 +84,6 @@ i = j - 1; auto diff = p - q; // OK, result is arithmetic + + for(int ii : a) ; // OK, pointer arithmetic generated by compiler } Index: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp === --- clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp +++ clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp @@ -16,15 +16,28 @@ namespace clang { namespace tidy { +AST_MATCHER_P(CXXForRangeStmt, hasRangeBeginEndStmt, + ast_matchers::internal::Matcher, InnerMatcher) { + const DeclStmt *const Stmt = Node.getBeginEndStmt(); + return (Stmt != nullptr && InnerMatcher.matches(*Stmt, Finder, Builder)); +} + +AST_MATCHER(Stmt, isInsideOfRangeBeginEndStmt) { + return stmt(hasAncestor(cxxForRangeStmt( + hasRangeBeginEndStmt(hasDescendant(equalsNode(&Node)) + .matches(Node, Finder, Builder); +} + void ProBoundsPointerArithmeticCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus) return; // Flag all operators +, -, +=, -=, ++, -- that result in a pointer Finder->addMatcher( binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-"), hasOperatorName("+="), hasOperatorName("-=")), - hasType(pointerType())) + hasType(pointerType()), + unless(isInsideOfRangeBeginEndStmt())) .bind("expr"), this); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252793 - [SemaDeclCXX] Use isTemplateParamScope() rather than accessing raw bits.
Author: davide Date: Wed Nov 11 14:06:35 2015 New Revision: 252793 URL: http://llvm.org/viewvc/llvm-project?rev=252793&view=rev Log: [SemaDeclCXX] Use isTemplateParamScope() rather than accessing raw bits. Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=252793&r1=252792&r2=252793&view=diff == --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Nov 11 14:06:35 2015 @@ -7572,7 +7572,7 @@ Decl *Sema::ActOnUsingDirective(Scope *S assert(IdentLoc.isValid() && "Invalid NamespceName location."); // This can only happen along a recovery path. - while (S->getFlags() & Scope::TemplateParamScope) + while (S->isTemplateParamScope()) S = S->getParent(); assert(S->getFlags() & Scope::DeclScope && "Invalid Scope."); @@ -8531,7 +8531,7 @@ Decl *Sema::ActOnAliasDeclaration(Scope TypeResult Type, Decl *DeclFromDeclSpec) { // Skip up to the relevant declaration scope. - while (S->getFlags() & Scope::TemplateParamScope) + while (S->isTemplateParamScope()) S = S->getParent(); assert((S->getFlags() & Scope::DeclScope) && "got alias-declaration outside of declaration scope"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14582: [clang-tidy] cppcoreguidelines-pro-bounds-pointer-arithmetic: ignore generated pointer arithmetic
aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp:19 @@ -18,1 +18,3 @@ +AST_MATCHER_P(CXXForRangeStmt, hasRangeBeginEndStmt, + ast_matchers::internal::Matcher, InnerMatcher) { This same code exists in ProBoundsArrayToPointerDecayCheck.cpp and should not be duplicated. I kind of wonder whether this belongs in clangTidyUtils, or in the AST matchers. I think for right now, clangTidyUtils because they still seem a little niche. http://reviews.llvm.org/D14582 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13973: CFG: Delay creating Dtors for CompoundStmts which end in ReturnStmt
mgehre added a comment. I won't be able to do more about this issue than what the fix currently does (+ corrections/maybe a loop). If this is not the preferred solution, and something more general should be done, I will abandon this patch for now. Currently, I cannot work on a more general solution. I don't have the experience with the clang static analyzer that a more general solution would require. http://reviews.llvm.org/D13973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D14583: Report Windows error code in a fatal error after a system call
probinson created this revision. probinson added a reviewer: aaron.ballman. probinson added a subscriber: cfe-commits. We've had two reports in the past year that CryptAcquireContextW failed; but without the Windows error code it's hard to know what's going on. http://reviews.llvm.org/D14583 Files: lib/Support/Windows/Process.inc Index: lib/Support/Windows/Process.inc === --- lib/Support/Windows/Process.inc +++ lib/Support/Windows/Process.inc @@ -417,16 +417,23 @@ return 0; } +// Include GetLastError() in a fatal error message. +static void ReportLastErrorFatal(const char *msg) { + std::string ErrMsg; + MakeErrMsg(&ErrMsg, msg); + report_fatal_error(ErrMsg); +} + unsigned Process::GetRandomNumber() { HCRYPTPROV HCPC; if (!::CryptAcquireContextW(&HCPC, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) -report_fatal_error("Could not acquire a cryptographic context"); +ReportLastErrorFatal("Could not acquire a cryptographic context"); ScopedCryptContext CryptoProvider(HCPC); unsigned Ret; if (!::CryptGenRandom(CryptoProvider, sizeof(Ret), reinterpret_cast(&Ret))) -report_fatal_error("Could not generate a random number"); +ReportLastErrorFatal("Could not generate a random number"); return Ret; } Index: lib/Support/Windows/Process.inc === --- lib/Support/Windows/Process.inc +++ lib/Support/Windows/Process.inc @@ -417,16 +417,23 @@ return 0; } +// Include GetLastError() in a fatal error message. +static void ReportLastErrorFatal(const char *msg) { + std::string ErrMsg; + MakeErrMsg(&ErrMsg, msg); + report_fatal_error(ErrMsg); +} + unsigned Process::GetRandomNumber() { HCRYPTPROV HCPC; if (!::CryptAcquireContextW(&HCPC, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) -report_fatal_error("Could not acquire a cryptographic context"); +ReportLastErrorFatal("Could not acquire a cryptographic context"); ScopedCryptContext CryptoProvider(HCPC); unsigned Ret; if (!::CryptGenRandom(CryptoProvider, sizeof(Ret), reinterpret_cast(&Ret))) -report_fatal_error("Could not generate a random number"); +ReportLastErrorFatal("Could not generate a random number"); return Ret; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13973: CFG: Delay creating Dtors for CompoundStmts which end in ReturnStmt
klimek added a comment. Jordan, your call :) http://reviews.llvm.org/D13973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14583: Report Windows error code in a fatal error after a system call
bcraig added a subscriber: bcraig. bcraig added a comment. Looks good to me, but since this is in the LLVMSupport library, you should probably add llvm-commits to the subscriber list. http://reviews.llvm.org/D14583 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252771 - Hiding the scan-build and scan-view projects under the Misc folder in IDEs instead of having them at the root view.
On 11/11/15 11:13 AM, Aaron Ballman via cfe-commits wrote: Author: aaronballman Date: Wed Nov 11 12:13:42 2015 New Revision: 252771 URL: http://llvm.org/viewvc/llvm-project?rev=252771&view=rev Log: Hiding the scan-build and scan-view projects under the Misc folder in IDEs instead of having them at the root view. Thanks! Jon -- Jon Roelofs jonat...@codesourcery.com CodeSourcery / Mentor Embedded ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14583: Report Windows error code in a fatal error after a system call
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a nit. Also, I am really curious to know why CryptAcquireContext fails in those cases! Comment at: lib/Support/Windows/Process.inc:421 @@ +420,3 @@ +// Include GetLastError() in a fatal error message. +static void ReportLastErrorFatal(const char *msg) { + std::string ErrMsg; msg should be Msg for style conventions. http://reviews.llvm.org/D14583 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252474 - Create install targets for scan-build and scan-view
On 11/11/15 8:07 AM, Aaron Ballman wrote: This change causes MSVC to have two new projects sitting at the root of the solution: scan-build and scan-view, neither of which appear to do anything. Are these projects required for IDEs? If they're installation-only projects, I think the CMake should be guarded with `if(NOT CMAKE_CONFIGURATION_TYPES)`. For the record, we discussed this on IRC, and you fixed it in r252771. Thanks again, Jon ~Aaron -- Jon Roelofs jonat...@codesourcery.com CodeSourcery / Mentor Embedded ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14471: [AArch64] Fix a crash in driver
rengolin added a comment. In http://reviews.llvm.org/D14471#286380, @ahatanak wrote: > I think I can use macro __aarch64__ to have getAArch64TargetCPU return > "native" when the compiler is not run on an AArch64 platform, but it doesn't > sound like that was what you had in mind? Not at all. :) It seems like a simple thing really, but it piles up pretty quickly. The command line: $ clang -arch arm64 -mtune=native -c hello.c doesn't make sense on an x86 machine at all. "native" means it should understand the host as an ARM64 hardware, which it's not. This should bail on error. You haven't provided any tests (you should really, what should work and what should return errors), so I can't tell just by looking at the code what you expect the new behaviour to be. I'm assuming you still want things to break, but you want your error message to have a "native" in it instead of crashing the compiler. Back to the code... Passing the pointer to a boolean is leaking logic from getAArch64TargetCPU out. If you know inside that the CPU is "native", why not just return that instead? Would that break other things? If so, these other things should be fixed. Fudging the parameters and keeping every other caller in the dark (because you're overriding the bool pointer) is certainly not the way we want to go, because that would mean two different behaviours depending on how you call the function. If the function returns a string with the CPU name, and the CPU name is "native", than logic dictates that the return value should be std::string("native"); If the function wants to add additional logic to grab the native CPU name and return that, it's extra. If it can't find the CPU name, it should return "native", not "generic" or "cyclone". Makes sense? cheers, --renato http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252797 - [analyzer] Fix scan-build to handle missing output directories.
Author: dcoughlin Date: Wed Nov 11 14:39:03 2015 New Revision: 252797 URL: http://llvm.org/viewvc/llvm-project?rev=252797&view=rev Log: [analyzer] Fix scan-build to handle missing output directories. Cwd::abs_path has a somewhat tricky semantics: if it's operand directory does not exist, it'll return undefined (see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=257568). This may cause scan-build to silently ignore output directory (specified with -o) and use /tmp instead of trying to create directory. This tiny patch fixes the problem. A patch by Yury Gribov! Differential Revision: http://reviews.llvm.org/D14535 Modified: cfe/trunk/tools/scan-build/scan-build Modified: cfe/trunk/tools/scan-build/scan-build URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/scan-build?rev=252797&r1=252796&r2=252797&view=diff == --- cfe/trunk/tools/scan-build/scan-build (original) +++ cfe/trunk/tools/scan-build/scan-build Wed Nov 11 14:39:03 2015 @@ -1478,7 +1478,9 @@ sub ProcessArgs { # Construct an absolute path. Uses the current working directory # as a base if the original path was not absolute. - $Options{OutputDir} = abs_path(shift @$Args); + my $OutDir = shift @$Args; + mkpath($OutDir) unless (-e $OutDir); # abs_path wants existing dir + $Options{OutputDir} = abs_path($OutDir); next; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14583: Report Windows error code in a fatal error after a system call
probinson marked an inline comment as done. probinson added a comment. In http://reviews.llvm.org/D14583#287406, @bcraig wrote: > Looks good to me, but since this is in the LLVMSupport library, you should > probably add llvm-commits to the subscriber list. Ah, right, I should have. But I'm not seeing how to add subscribers post-facto. In http://reviews.llvm.org/D14583#287407, @aaron.ballman wrote: > LGTM with a nit. > > Also, I am really curious to know why CryptAcquireContext fails in those > cases! Me too. Once is a fluke, twice is a trend. Probably some obscure system-resource problem but it would be nice to be sure. Comment at: lib/Support/Windows/Process.inc:421 @@ +420,3 @@ +// Include GetLastError() in a fatal error message. +static void ReportLastErrorFatal(const char *msg) { + std::string ErrMsg; aaron.ballman wrote: > msg should be Msg for style conventions. The prevailing style in this file is for lowercase parameter names, but I guess we gotta start somewhere! http://reviews.llvm.org/D14583 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14583: Report Windows error code in a fatal error after a system call
probinson closed this revision. probinson marked an inline comment as done. probinson added a comment. r252800, thanks! http://reviews.llvm.org/D14583 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13330: Implement __attribute__((unique_instantiation))
aaron.ballman added a comment. Modulo the question you and David are discussing about variable templates (for which I don't have an answer handy), I just have a few small testing nits. Comment at: test/SemaCXX/unique-instantiations.cpp:24 @@ +23,3 @@ +template struct foo5; // expected-error{{'unique_instantiation' attribute must be specified for all declarations and definitions of this explicit template instantiation}} + +template > Isn't the correct usage checked for in the CodeGen tests above? It is, but those are just tests for code generation, which are all expected to succeed semantically. We generally expect tests in Sema (and related folders) that demonstrate successful cases as well as failure cases for features. Comment at: test/SemaCXX/unique-instantiations.cpp:28 @@ +27,2 @@ +extern template struct __attribute__((unique_instantiation(16))) foo6; // expected-error{{'unique_instantiation' attribute takes no arguments}} +template struct __attribute__((unique_instantiation("Hello World"))) foo6; // expected-error{{'unique_instantiation' attribute takes no arguments}} The test with unique_instantiation(16) is sufficient, no need for the "Hello World" test as well. http://reviews.llvm.org/D13330 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14583: Report Windows error code in a fatal error after a system call
gbedwell added a subscriber: gbedwell. gbedwell added a comment. > > Also, I am really curious to know why CryptAcquireContext fails in those > > cases! > > > Me too. Once is a fluke, twice is a trend. Probably some obscure > system-resource problem but it would be nice to be sure. I'm out of the office this week, so can't test, but I'm pretty sure you can reproduce this by spawning clang in an environment where the SYSTEMROOT environment variable has not been propagated (or by running it from a cmd window after typing "set SYSTEMROOT=". http://reviews.llvm.org/D14583 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [Diffusion] rL246882: Don't crash on a self-alias declaration
tstellarAMD added a subscriber: tstellarAMD. tstellarAMD accepted this commit. tstellarAMD added a comment. r252808 Users: hfinkel (Author, Auditor) 3.7-release (Auditor) cfe-commits (Auditor) tstellarAMD (Auditor) rjmccall (Auditor) http://reviews.llvm.org/rL246882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [Diffusion] rL246882: Don't crash on a self-alias declaration
tstellarAMD added a project: 3.7.1-merged. Users: hfinkel (Author, Auditor) 3.7-release (Auditor) cfe-commits (Auditor) tstellarAMD (Auditor) rjmccall (Auditor) http://reviews.llvm.org/rL246882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13263: Addition of __attribute__((pass_object_size)) to Clang
aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2068 @@ -2064,1 +2067,3 @@ def err_attribute_pointers_only : Error; +def err_attribute_constant_pointers_only : Error< + "%0 attribute only applies to constant pointer arguments">; Instead of making a new diagnostic, I think it better to modify warn_attribute_pointers_only to use a %select statement (then err_attribute_pointers_only also gets updated). I would separate that into a prequel patch since it's likely to introduce a fair amount of unrelated changes to this patch. Comment at: lib/Sema/SemaDeclAttr.cpp:820 @@ +819,3 @@ + Expr *E = Attr.getArgAsExpr(0); + IntegerLiteral *Int = dyn_cast(E->IgnoreParenCasts()); + if (Int == nullptr) { We don't usually ignore parens and casts for attributes like this for other attributes. Is that important for you uses for some reason? Comment at: lib/Sema/SemaDeclAttr.cpp:828 @@ +827,3 @@ + auto APType = Int->getValue(); + if (APType.ugt(3) || APType.slt(0)) { +S.Diag(E->getLocStart(), diag::err_attribute_argument_out_of_bounds) rsmith wrote: > The second check here is redundant if `APType` is more than 2 bits wide (and > looks wrong when `Int` is of type `bool`). I'm wondering why the magic value 3 at all? It seems like this (and the above code) should be replaced by a call to checkFunctionOrMethodParameterIndex(). Comment at: lib/Sema/SemaDeclAttr.cpp:828 @@ +827,3 @@ + auto APType = Int->getValue(); + if (APType.ugt(3) || APType.slt(0)) { +S.Diag(E->getLocStart(), diag::err_attribute_argument_out_of_bounds) aaron.ballman wrote: > rsmith wrote: > > The second check here is redundant if `APType` is more than 2 bits wide > > (and looks wrong when `Int` is of type `bool`). > I'm wondering why the magic value 3 at all? It seems like this (and the above > code) should be replaced by a call to checkFunctionOrMethodParameterIndex(). There should be comments explaining the magic number 3. Comment at: lib/Sema/SemaDeclAttr.cpp:829 @@ +828,3 @@ + if (APType.ugt(3) || APType.slt(0)) { +S.Diag(E->getLocStart(), diag::err_attribute_argument_out_of_bounds) + << Attr.getName() << 1 << E->getSourceRange(); That isn't the correct diagnostic to emit. That one is used when a function attribute that refers to a parameter (like format_arg) uses an index that does not refer to a parameter. For this one, I would probably modify err_attribute_argument_outof_range to not be specific to 'init_priority'. http://reviews.llvm.org/D13263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14170: Fix false positive warning about memory leak for QApplication::postEvent
Dushistov marked an inline comment as done. Dushistov added a comment. http://reviews.llvm.org/D14170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252809 - [CMake] Fixing passthrough for variables starting with COMPILER_RT
Author: cbieneman Date: Wed Nov 11 15:53:08 2015 New Revision: 252809 URL: http://llvm.org/viewvc/llvm-project?rev=252809&view=rev Log: [CMake] Fixing passthrough for variables starting with COMPILER_RT This allows COMPILER_RT_* variables to be passed from the top-level CMake into the external project when LLVM_BUILD_EXTERNAL_COMPILER_RT=On. Modified: cfe/trunk/runtime/CMakeLists.txt Modified: cfe/trunk/runtime/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/runtime/CMakeLists.txt?rev=252809&r1=252808&r2=252809&view=diff == --- cfe/trunk/runtime/CMakeLists.txt (original) +++ cfe/trunk/runtime/CMakeLists.txt Wed Nov 11 15:53:08 2015 @@ -48,7 +48,7 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND E # them. get_cmake_property(variableNames VARIABLES) foreach(varaibleName ${variableNames}) -if(${varaibleName} MATCHES "^COMPILER_RT") +if(varaibleName MATCHES "^COMPILER_RT") list(APPEND COMPILER_RT_PASSTHROUGH_VARIABLES -D${varaibleName}=${${varaibleName}}) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const
rsmith added a comment. Why does this construct justify the compiler emitting a warning? It seems to be reporting a fact about the code rather than a bug, and as there are many coding styles where variables are not routinely marked as const whenever possible, this appears to be checking that the code conforms to a particular coding style. As such, this seems like a better fit as a clang-tidy check than as a compiler warning. The choice to only apply this check to pointer parameters to functions seems arbitrary. What is the motivation for that? Comment at: include/clang/AST/Decl.h:854 @@ +853,3 @@ + /// \brief Whether this variable has non-const use so it can't be const. + unsigned NonConstUse:1; + This is not OK; it'll make all `VarDecl`s 8 bytes larger on 64-bit systems. http://reviews.llvm.org/D12359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14560: [Clang] Fix Clang-tidy modernize-use-auto in some files in lib/AST; other minor cleanups.
Eugene.Zelenko added a comment. I'm adept of consistency :-) It's also easier to fix all similar patterns in code then do such cleanups selectively. Actually, similar fixes were made recently in Decl.cpp when casts were involved, but not new. Comment at: lib/AST/ASTContext.cpp:7930 @@ -7931,3 +7929,3 @@ -ASTMutationListener::~ASTMutationListener() { } +ASTMutationListener::~ASTMutationListener() = default; aaron.ballman wrote: > This is... interesting. Explicitly defaulting an out-of-line function > definition is not something I've ever encountered before. What benefit does > this provide? It's explicitly tells that destructor has default implementation. Repository: rL LLVM http://reviews.llvm.org/D14560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14134: [OpenMP] Parsing and sema support for map clause
kkwli0 marked 13 inline comments as done. kkwli0 added a comment. http://reviews.llvm.org/D14134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252814 - [TLS on Darwin] change how we handle globals with linkonce or weak linkage.
Author: mren Date: Wed Nov 11 16:42:31 2015 New Revision: 252814 URL: http://llvm.org/viewvc/llvm-project?rev=252814&view=rev Log: [TLS on Darwin] change how we handle globals with linkonce or weak linkage. This is about how we handle static member of a template. Before this commit, we use internal linkage for the IR thread-local variable, which is inefficient. With this commit, we will start to follow Itanium C++ ABI. rdar://problem/23415206 Reviewed by John McCall. Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/test/CodeGenCXX/cxx11-thread-local-reference.cpp cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp cfe/trunk/test/CodeGenCXX/tls-init-funcs.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=252814&r1=252813&r2=252814&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Nov 11 16:42:31 2015 @@ -2315,12 +2315,17 @@ void CodeGenModule::EmitGlobalVarDefinit llvm::GlobalValue::LinkageTypes Linkage = getLLVMLinkageVarDefinition(D, GV->isConstant()); - // On Darwin, the backing variable for a C++11 thread_local variable always - // has internal linkage; all accesses should just be calls to the + // On Darwin, if the normal linkage of a C++ thread_local variable is + // LinkOnce or Weak, we keep the normal linkage to prevent multiple + // copies within a linkage unit; otherwise, the backing variable has + // internal linkage and all accesses should just be calls to the // Itanium-specified entry point, which has the normal linkage of the - // variable. + // variable. This is to preserve the ability to change the implementation + // behind the scenes. if (!D->isStaticLocal() && D->getTLSKind() == VarDecl::TLS_Dynamic && - Context.getTargetInfo().getTriple().isMacOSX()) + Context.getTargetInfo().getTriple().isMacOSX() && + !llvm::GlobalVariable::isLinkOnceLinkage(Linkage) && + !llvm::GlobalVariable::isWeakLinkage(Linkage)) Linkage = llvm::GlobalValue::InternalLinkage; GV->setLinkage(Linkage); Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=252814&r1=252813&r2=252814&view=diff == --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed Nov 11 16:42:31 2015 @@ -2163,12 +2163,10 @@ getThreadLocalWrapperLinkage(const VarDe return VarLinkage; // If the thread wrapper is replaceable, give it appropriate linkage. - if (isThreadWrapperReplaceable(VD, CGM)) { -if (llvm::GlobalVariable::isLinkOnceLinkage(VarLinkage) || -llvm::GlobalVariable::isWeakODRLinkage(VarLinkage)) - return llvm::GlobalVariable::WeakAnyLinkage; -return VarLinkage; - } + if (isThreadWrapperReplaceable(VD, CGM)) +if (!llvm::GlobalVariable::isLinkOnceLinkage(VarLinkage) && +!llvm::GlobalVariable::isWeakODRLinkage(VarLinkage)) + return VarLinkage; return llvm::GlobalValue::WeakODRLinkage; } @@ -2194,7 +2192,9 @@ ItaniumCXXABI::getOrCreateThreadLocalWra llvm::Function::Create(FnTy, getThreadLocalWrapperLinkage(VD, CGM), WrapperName.str(), &CGM.getModule()); // Always resolve references to the wrapper at link time. - if (!Wrapper->hasLocalLinkage() && !isThreadWrapperReplaceable(VD, CGM)) + if (!Wrapper->hasLocalLinkage() && !(isThreadWrapperReplaceable(VD, CGM) && + !llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) && + !llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage( Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility); return Wrapper; } Modified: cfe/trunk/test/CodeGenCXX/cxx11-thread-local-reference.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-thread-local-reference.cpp?rev=252814&r1=252813&r2=252814&view=diff == --- cfe/trunk/test/CodeGenCXX/cxx11-thread-local-reference.cpp (original) +++ cfe/trunk/test/CodeGenCXX/cxx11-thread-local-reference.cpp Wed Nov 11 16:42:31 2015 @@ -1,11 +1,14 @@ -// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s +// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s +// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s int &f(); -// CHECK: @r = thread_local global i32* null +// LINUX: @r = thread_local global i32* null +// DARWIN: @r = internal thread_local global i32* null thread_local in
LLVM buildmaster will be restarted tonight
Hello everyone, LLVM buildmaster will be updated restarted after 7 PM Pacific time today. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252819 - Extract out a function onto CodeGenModule for getting the map of
Author: echristo Date: Wed Nov 11 17:05:08 2015 New Revision: 252819 URL: http://llvm.org/viewvc/llvm-project?rev=252819&view=rev Log: Extract out a function onto CodeGenModule for getting the map of features for a particular function, then use it to clean up some code. Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=252819&r1=252818&r2=252819&view=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Nov 11 17:05:08 2015 @@ -353,7 +353,8 @@ bool CodeGenFunction::checkBuiltinTarget // Get the current enclosing function if it exists. If it doesn't // we can't check the target features anyhow. const FunctionDecl *FD = dyn_cast_or_null(CurFuncDecl); - if (!FD) return true; + if (!FD) +return true; unsigned BuiltinID = TargetDecl->getBuiltinID(); const char *FeatureList = @@ -362,32 +363,8 @@ bool CodeGenFunction::checkBuiltinTarget if (!FeatureList || StringRef(FeatureList) == "") return true; - StringRef TargetCPU = Target.getTargetOpts().CPU; llvm::StringMap FeatureMap; - - if (const auto *TD = FD->getAttr()) { -// If we have a TargetAttr build up the feature map based on that. -TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse(); - -// Make a copy of the features as passed on the command line into the -// beginning of the additional features from the function to override. -ParsedAttr.first.insert(ParsedAttr.first.begin(), -Target.getTargetOpts().FeaturesAsWritten.begin(), -Target.getTargetOpts().FeaturesAsWritten.end()); - -if (ParsedAttr.second != "") - TargetCPU = ParsedAttr.second; - -// Now populate the feature map, first with the TargetCPU which is either -// the default or a new one from the target attribute string. Then we'll use -// the passed in features (FeaturesAsWritten) along with the new ones from -// the attribute. -Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU, - ParsedAttr.first); - } else { -Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU, - Target.getTargetOpts().Features); - } + CGM.getFunctionFeatureMap(FeatureMap, FD); // If we have at least one of the features in the feature list return // true, otherwise return false. Modified: cfe/trunk/lib/CodeGen/CGCall.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=252819&r1=252818&r2=252819&view=diff == --- cfe/trunk/lib/CodeGen/CGCall.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Nov 11 17:05:08 2015 @@ -1506,24 +1506,7 @@ void CodeGenModule::ConstructAttributeLi const FunctionDecl *FD = dyn_cast_or_null(TargetDecl); if (FD && FD->hasAttr()) { llvm::StringMap FeatureMap; - const auto *TD = FD->getAttr(); - TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse(); - - // Make a copy of the features as passed on the command line into the - // beginning of the additional features from the function to override. - ParsedAttr.first.insert( - ParsedAttr.first.begin(), - getTarget().getTargetOpts().FeaturesAsWritten.begin(), - getTarget().getTargetOpts().FeaturesAsWritten.end()); - - if (ParsedAttr.second != "") -TargetCPU = ParsedAttr.second; - - // Now populate the feature map, first with the TargetCPU which is either - // the default or a new one from the target attribute string. Then we'll - // use the passed in features (FeaturesAsWritten) along with the new ones - // from the attribute. - getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, ParsedAttr.first); + getFunctionFeatureMap(FeatureMap, FD); // Produce the canonical string for this set of features. std::vector Features; @@ -1533,6 +1516,13 @@ void CodeGenModule::ConstructAttributeLi Features.push_back((it->second ? "+" : "-") + it->first().str()); // Now add the target-cpu and target-features to the function. + // While we populated the feature map above, we still need to + // get and parse the target attribute so we can get the cpu for + // the function. + const auto *TD = FD->getAttr(); + TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse(); + if (ParsedAttr.second != "") +TargetCPU = ParsedAttr.second; if (TargetCPU != "") FuncAttrs.addAttribute("target-cpu", TargetCPU); if (!Features.empty()) { Modified: cf
r252820 - [TLS on Darwin] treat all Darwin platforms in the same way.
Author: mren Date: Wed Nov 11 17:08:18 2015 New Revision: 252820 URL: http://llvm.org/viewvc/llvm-project?rev=252820&view=rev Log: [TLS on Darwin] treat all Darwin platforms in the same way. rdar://problem/9001553 Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=252820&r1=252819&r2=252820&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Nov 11 17:08:18 2015 @@ -2323,7 +2323,7 @@ void CodeGenModule::EmitGlobalVarDefinit // variable. This is to preserve the ability to change the implementation // behind the scenes. if (!D->isStaticLocal() && D->getTLSKind() == VarDecl::TLS_Dynamic && - Context.getTargetInfo().getTriple().isMacOSX() && + Context.getTargetInfo().getTriple().isOSDarwin() && !llvm::GlobalVariable::isLinkOnceLinkage(Linkage) && !llvm::GlobalVariable::isWeakLinkage(Linkage)) Linkage = llvm::GlobalValue::InternalLinkage; Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=252820&r1=252819&r2=252820&view=diff == --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed Nov 11 17:08:18 2015 @@ -2088,7 +2088,7 @@ static void emitGlobalDtorWithCXAAtExit( const char *Name = "__cxa_atexit"; if (TLS) { const llvm::Triple &T = CGF.getTarget().getTriple(); -Name = T.isMacOSX() ? "_tlv_atexit" : "__cxa_thread_atexit"; +Name = T.isOSDarwin() ? "_tlv_atexit" : "__cxa_thread_atexit"; } // We're assuming that the destructor function is something we can @@ -2144,10 +2144,10 @@ void ItaniumCXXABI::registerGlobalDtor(C static bool isThreadWrapperReplaceable(const VarDecl *VD, CodeGen::CodeGenModule &CGM) { assert(!VD->isStaticLocal() && "static local VarDecls don't need wrappers!"); - // OS X prefers to have references to thread local variables to go through + // Darwin prefers to have references to thread local variables to go through // the thread wrapper instead of directly referencing the backing variable. return VD->getTLSKind() == VarDecl::TLS_Dynamic && - CGM.getTarget().getTriple().isMacOSX(); + CGM.getTarget().getTriple().isOSDarwin(); } /// Get the appropriate linkage for the wrapper function. This is essentially ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D14592: Qt (version 4 or 5) signal/method checker
Dushistov created this revision. Dushistov added reviewers: dcoughlin, Ayal, xazax.hun, zaks.anna. Dushistov added a subscriber: cfe-commits. In Qt 4/5 it is possible connect classes methods in such way: connect(ObjectPointer1, SIGNAL(methodOfObject1()), ObjectPointer2, SLOT(methodOfObject2()); and when you call this method -> `ObjectPointer1->methodOfObject1()` method of `ObjectPointer2->methodOfObject2()` will be called automatically. The only problem that you can check of correctness of method name and its argument only at runtime (more details can be found here http://doc.qt.io/qt-4.8/signalsandslots.html). So I implement this checker to help re factoring large Qt based projects. Note: Qt 4 have only such method to connect signals and slots. Qt 5 uses two methods, described above and based on function pointers, but because of SIGNAL/SLOT syntax with disadvantages have also advantages (see http://doc.qt.io/qt-5/signalsandslots-syntaxes.html) sometime Qt5 also have not compile time checked signal/slot connection. What this checker do exactly: 1)It checks that such methods with such signatures exists in both classes 2)It checks that slot subset of parameters match the first parameters of signal 3)It checks that signature of method written in proper way (normalized), this improve performance, see https://marcmutz.wordpress.com/effective-qt/prefer-to-use-normalised-signalslot-signatures/ and http://doc.qt.io/qt-5/qmetaobject.html#normalizedSignature http://reviews.llvm.org/D14592 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/QtSignalSlotChecker.cpp test/Analysis/qt_connect.cpp unittests/StaticAnalyzer/CMakeLists.txt unittests/StaticAnalyzer/QtSignalSlotCheckerTest.cpp Index: unittests/StaticAnalyzer/QtSignalSlotCheckerTest.cpp === --- /dev/null +++ unittests/StaticAnalyzer/QtSignalSlotCheckerTest.cpp @@ -0,0 +1,37 @@ +#include "llvm/ADT/StringRef.h" +#include "gtest/gtest.h" + +namespace clang { +namespace ento { + +extern std::string qtNormalizeSignature(const llvm::StringRef &Method); + +TEST(QtSignalSlotChecker, QMetaObjectNormalizedSignature) { + const std::pair NormConv[] = { +{ + "a(const int&, int const&, const int *, int const *, int * const, const int * const, int &, int *)", + "a(int,int,const int*,const int*,int*const,int*const,int&,int*)" +}, +{ + "b(const unsigned&, const unsigned long&, unsigned char, std::vector&, std::vector const&, unsigned short, short)", + "b(uint,ulong,unsigned char,std::vector&,std::vector,unsigned short,short)" +}, +{ + "c(const int * const * const)", + "c(int*const*const)" +}, +{ + "d(class QString&, enum QScriptEngine::QObjectWrapOption, struct QString*)", + "d(QString&,QScriptEngine::QObjectWrapOption,QString*)" +}, +{ + "a(int const a, const int& b)", + "a(int a,int&b)" +} + }; + for (auto&& InOut : NormConv) { +EXPECT_EQ(InOut.second, qtNormalizeSignature(InOut.first)); + } +} +} // end namespace ento +} // end namespace clang Index: unittests/StaticAnalyzer/CMakeLists.txt === --- unittests/StaticAnalyzer/CMakeLists.txt +++ unittests/StaticAnalyzer/CMakeLists.txt @@ -4,10 +4,12 @@ add_clang_unittest(StaticAnalysisTests AnalyzerOptionsTest.cpp + QtSignalSlotCheckerTest.cpp ) target_link_libraries(StaticAnalysisTests clangBasic clangAnalysis - clangStaticAnalyzerCore + clangStaticAnalyzerCore + clangStaticAnalyzerCheckers ) Index: test/Analysis/qt_connect.cpp === --- /dev/null +++ test/Analysis/qt_connect.cpp @@ -0,0 +1,82 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,cplusplus -analyzer-store=region -verify %s + +const char *qFlagLocation(const char *method); +#define Q_OBJECT +#define QTOSTRING_HELPER(s) #s +#define QTOSTRING(s) QTOSTRING_HELPER(s) +# define QLOCATION "\0" __FILE__ ":" QTOSTRING(__LINE__) +# define SLOT(a) qFlagLocation("1"#a QLOCATION) +# define SIGNAL(a) qFlagLocation("2"#a QLOCATION) + +# define slots +# define signals protected + +namespace Qt { +enum ConnectionType { +AutoConnection, +DirectConnection, +QueuedConnection, +AutoCompatConnection, +BlockingQueuedConnection, +UniqueConnection = 0x80 +}; +} +struct QString {}; +class QObject { +public: + QObject() {} + static bool connect(const QObject *sender, const char *signal, + const QObject *receiver, const char *member, Qt::ConnectionType = Qt::AutoConnection); +}; + +class QWidget : public QObject { +public: + QWidget() {} +}; + +class Sender : public QWidget { + Q_OBJECT +signals: + void empty(); + void looksEmpty(bool = false); + void stringRef(const QString&); + void stringMu
Re: [PATCH] D14170: Fix false positive warning about memory leak for QApplication::postEvent
Dushistov updated this revision to Diff 39985. Dushistov added a comment. mistype, actually I want to use '&&' here, not '||' to not create std::string, if match failed. http://reviews.llvm.org/D14170 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/Inputs/qt-simulator.h test/Analysis/qt_malloc.cpp Index: test/Analysis/qt_malloc.cpp === --- /dev/null +++ test/Analysis/qt_malloc.cpp @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s +// expected-no-diagnostics +#include "Inputs/qt-simulator.h" + +void send(QObject *obj) +{ + QEvent *event = new QEvent(QEvent::None); + static_cast(QCoreApplication::instance())->postEvent(obj, event); +} Index: test/Analysis/Inputs/qt-simulator.h === --- /dev/null +++ test/Analysis/Inputs/qt-simulator.h @@ -0,0 +1,16 @@ +#pragma clang system_header + +struct QObject { +}; + +struct QEvent { + enum Type { None }; + QEvent(Type) {} +}; + +struct QCoreApplication : public QObject { + static void postEvent(QObject *receiver, QEvent *event); + static QCoreApplication *instance(); +}; + +struct QApplication : public QCoreApplication {}; Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp === --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -2511,6 +2511,11 @@ return true; } + if (FName.endswith("postEvent") && + FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") { +return true; + } + // Handle cases where we know a buffer's /address/ can escape. // Note that the above checks handle some special cases where we know that // even though the address escapes, it's still our responsibility to free the Index: test/Analysis/qt_malloc.cpp === --- /dev/null +++ test/Analysis/qt_malloc.cpp @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s +// expected-no-diagnostics +#include "Inputs/qt-simulator.h" + +void send(QObject *obj) +{ + QEvent *event = new QEvent(QEvent::None); + static_cast(QCoreApplication::instance())->postEvent(obj, event); +} Index: test/Analysis/Inputs/qt-simulator.h === --- /dev/null +++ test/Analysis/Inputs/qt-simulator.h @@ -0,0 +1,16 @@ +#pragma clang system_header + +struct QObject { +}; + +struct QEvent { + enum Type { None }; + QEvent(Type) {} +}; + +struct QCoreApplication : public QObject { + static void postEvent(QObject *receiver, QEvent *event); + static QCoreApplication *instance(); +}; + +struct QApplication : public QCoreApplication {}; Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp === --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -2511,6 +2511,11 @@ return true; } + if (FName.endswith("postEvent") && + FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") { +return true; + } + // Handle cases where we know a buffer's /address/ can escape. // Note that the above checks handle some special cases where we know that // even though the address escapes, it's still our responsibility to free the ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252827 - Add diagnostics which fall under [dcl.spec.concept]p5
Author: nwilson Date: Wed Nov 11 17:53:35 2015 New Revision: 252827 URL: http://llvm.org/viewvc/llvm-project?rev=252827&view=rev Log: Add diagnostics which fall under [dcl.spec.concept]p5 Summary: Diagnose when a function concept declaration has parameter(s) Reviewers: rsmith, faisalv, aaron.ballman, hubert.reinterpretcast Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D14352 Added: cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaDecl.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252827&r1=252826&r2=252827&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 17:53:35 2015 @@ -1994,6 +1994,8 @@ def err_function_concept_exception_spec def err_concept_decl_invalid_specifiers : Error< "%select{variable|function}0 concept cannot be declared " "'%select{thread_local|inline|friend|constexpr}1'">; +def err_function_concept_with_params : Error< + "function concept cannot have any parameters">; // C++11 char16_t/char32_t def warn_cxx98_compat_unicode_type : Warning< Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=252827&r1=252826&r2=252827&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Nov 11 17:53:35 2015 @@ -7607,6 +7607,13 @@ Sema::ActOnFunctionDeclarator(Scope *S, } else { Context.adjustExceptionSpec(NewFD, EST_BasicNoexcept); } + +// C++ Concepts TS [dcl.spec.concept]p5: A function concept has the +// following restrictions: +// - The declaration's parameter list shall be equivalent to an empty +// parameter list. +if ((FPT->getNumParams() > 0) || (FPT->isVariadic())) + Diag(NewFD->getLocation(), diag::err_function_concept_with_params); } // C++ Concepts TS [dcl.spec.concept]p2: Every concept definition is Added: cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp?rev=252827&view=auto == --- cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp (added) +++ cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp Wed Nov 11 17:53:35 2015 @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s + +template +concept bool fcpv(void) { return true; } + +template +concept bool fcpi(int i = 0) { return true; } // expected-error {{function concept cannot have any parameters}} + +template +concept bool fcpp(Ts... ts) { return true; } // expected-error {{function concept cannot have any parameters}} + +template +concept bool fcpva(...) { return true; } // expected-error {{function concept cannot have any parameters}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14352: Add diagnostics which fall under [dcl.spec.concept]p5
This revision was automatically updated to reflect the committed changes. Closed by commit rL252827: Add diagnostics which fall under [dcl.spec.concept]p5 (authored by nwilson). Changed prior to commit: http://reviews.llvm.org/D14352?vs=39386&id=39986#toc Repository: rL LLVM http://reviews.llvm.org/D14352 Files: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -1994,6 +1994,8 @@ def err_concept_decl_invalid_specifiers : Error< "%select{variable|function}0 concept cannot be declared " "'%select{thread_local|inline|friend|constexpr}1'">; +def err_function_concept_with_params : Error< + "function concept cannot have any parameters">; // C++11 char16_t/char32_t def warn_cxx98_compat_unicode_type : Warning< Index: cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp === --- cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp +++ cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s + +template +concept bool fcpv(void) { return true; } + +template +concept bool fcpi(int i = 0) { return true; } // expected-error {{function concept cannot have any parameters}} + +template +concept bool fcpp(Ts... ts) { return true; } // expected-error {{function concept cannot have any parameters}} + +template +concept bool fcpva(...) { return true; } // expected-error {{function concept cannot have any parameters}} Index: cfe/trunk/lib/Sema/SemaDecl.cpp === --- cfe/trunk/lib/Sema/SemaDecl.cpp +++ cfe/trunk/lib/Sema/SemaDecl.cpp @@ -7607,6 +7607,13 @@ } else { Context.adjustExceptionSpec(NewFD, EST_BasicNoexcept); } + +// C++ Concepts TS [dcl.spec.concept]p5: A function concept has the +// following restrictions: +// - The declaration's parameter list shall be equivalent to an empty +// parameter list. +if ((FPT->getNumParams() > 0) || (FPT->isVariadic())) + Diag(NewFD->getLocation(), diag::err_function_concept_with_params); } // C++ Concepts TS [dcl.spec.concept]p2: Every concept definition is Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -1994,6 +1994,8 @@ def err_concept_decl_invalid_specifiers : Error< "%select{variable|function}0 concept cannot be declared " "'%select{thread_local|inline|friend|constexpr}1'">; +def err_function_concept_with_params : Error< + "function concept cannot have any parameters">; // C++11 char16_t/char32_t def warn_cxx98_compat_unicode_type : Warning< Index: cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp === --- cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp +++ cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s + +template +concept bool fcpv(void) { return true; } + +template +concept bool fcpi(int i = 0) { return true; } // expected-error {{function concept cannot have any parameters}} + +template +concept bool fcpp(Ts... ts) { return true; } // expected-error {{function concept cannot have any parameters}} + +template +concept bool fcpva(...) { return true; } // expected-error {{function concept cannot have any parameters}} Index: cfe/trunk/lib/Sema/SemaDecl.cpp === --- cfe/trunk/lib/Sema/SemaDecl.cpp +++ cfe/trunk/lib/Sema/SemaDecl.cpp @@ -7607,6 +7607,13 @@ } else { Context.adjustExceptionSpec(NewFD, EST_BasicNoexcept); } + +// C++ Concepts TS [dcl.spec.concept]p5: A function concept has the +// following restrictions: +// - The declaration's parameter list shall be equivalent to an empty +// parameter list. +if ((FPT->getNumParams() > 0) || (FPT->isVariadic())) + Diag(NewFD->getLocation(), diag::err_function_concept_with_params); } // C++ Concepts TS [dcl.spec.concept]p2: Every concept definition is ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14592: Qt (version 4 or 5) signal/method checker
jroelofs added a subscriber: jroelofs. Comment at: lib/StaticAnalyzer/Checkers/QtSignalSlotChecker.cpp:114 @@ +113,3 @@ + printMethodNameWithPramaTypes(Out, C, FName, M, false); + const std::string NS = qtNormalizeSignature(Out.str()); + if (NS == MethodWithParamsNorm) Why are you throwing away all the work that the parser did to build up the type signature as a QualType, pretty-printing it, and the re-parsing it by hand? ISTM it would be much better if qtNormalizeSignature had this type signature: `QualType qtNormalizeSignature(QualType)`. http://reviews.llvm.org/D14592 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14471: [AArch64] Fix a crash in driver
ahatanak added a comment. In http://reviews.llvm.org/D14471#287412, @rengolin wrote: > In http://reviews.llvm.org/D14471#286380, @ahatanak wrote: > > > I think I can use macro __aarch64__ to have getAArch64TargetCPU return > > "native" when the compiler is not run on an AArch64 platform, but it > > doesn't sound like that was what you had in mind? > > > Not at all. :) > > It seems like a simple thing really, but it piles up pretty quickly. > > The command line: > > $ clang -arch arm64 -mtune=native -c hello.c > > > doesn't make sense on an x86 machine at all. "native" means it should > understand the host as an ARM64 hardware, which it's not. This should bail on > error. I agree and that is what I'm trying to do. It should error out with something like "native is not supported". > You haven't provided any tests (you should really, what should work and what > should return errors), so I can't tell just by looking at the code what you > expect the new behaviour to be. I'm assuming you still want things to break, > but you want your error message to have a "native" in it instead of crashing > the compiler. That's correct. The error message I'm looking for is something like the one I wrote in the summary: error: the clang compiler does not support 'native' I didn't include a test case because I didn't know how to write a test that passes on an aarch64 host and fails on anything else. Do you know of any test cases in trunk that pass or fail depending on which host it is running on? > Back to the code... > > Passing the pointer to a boolean is leaking logic from getAArch64TargetCPU > out. If you know inside that the CPU is "native", why not just return that > instead? Would that break other things? If so, these other things should be > fixed. Fudging the parameters and keeping every other caller in the dark > (because you're overriding the bool pointer) is certainly not the way we want > to go, because that would mean two different behaviours depending on how you > call the function. Wouldn't it break on an aarch64 host? With "-mtune=native", the current code in trunk will get the host cpu name (which I believe is currently always "generic" for aarch64). But if I apply this patch, it will error out because "native" is not a valid cpu name. http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252828 - [Basic] Fix DRY violation, just call getLineTable() (NFC)
Author: vedantk Date: Wed Nov 11 18:11:19 2015 New Revision: 252828 URL: http://llvm.org/viewvc/llvm-project?rev=252828&view=rev Log: [Basic] Fix DRY violation, just call getLineTable() (NFC) Modified: cfe/trunk/lib/Basic/SourceManager.cpp Modified: cfe/trunk/lib/Basic/SourceManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=252828&r1=252827&r2=252828&view=diff == --- cfe/trunk/lib/Basic/SourceManager.cpp (original) +++ cfe/trunk/lib/Basic/SourceManager.cpp Wed Nov 11 18:11:19 2015 @@ -279,9 +279,7 @@ void LineTableInfo::AddEntry(FileID FID, /// getLineTableFilenameID - Return the uniqued ID for the specified filename. /// unsigned SourceManager::getLineTableFilenameID(StringRef Name) { - if (!LineTable) -LineTable = new LineTableInfo(); - return LineTable->getLineTableFilenameID(Name); + return getLineTable().getLineTableFilenameID(Name); } @@ -302,9 +300,7 @@ void SourceManager::AddLineNote(SourceLo // Remember that this file has #line directives now if it doesn't already. const_cast(FileInfo).setHasLineDirectives(); - if (!LineTable) -LineTable = new LineTableInfo(); - LineTable->AddLineNote(LocInfo.first, LocInfo.second, LineNo, FilenameID); + getLineTable().AddLineNote(LocInfo.first, LocInfo.second, LineNo, FilenameID); } /// AddLineNote - Add a GNU line marker to the line table. @@ -332,8 +328,7 @@ void SourceManager::AddLineNote(SourceLo // Remember that this file has #line directives now if it doesn't already. const_cast(FileInfo).setHasLineDirectives(); - if (!LineTable) -LineTable = new LineTableInfo(); + (void) getLineTable(); SrcMgr::CharacteristicKind FileKind; if (IsExternCHeader) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14471: [AArch64] Fix a crash in driver
ahatanak added a comment. Sorry, there were mistakes in my comments. What I meant to say is that changing getAArch64TargetCPU to return "native" would break the case where clang is being run on an aarch64 host. The current code in trunk will get the host cpu name (which I believe is currently always "generic" for aarch64) if "-mtune=native" is on the command line. If we change getAArch64TargetCPU to return "native", clang will error out because "native" is not a valid cpu name. http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14471: [AArch64] Fix a crash in driver
On 11/11/15 5:01 PM, Akira Hatanaka via cfe-commits wrote: ahatanak added a comment. In http://reviews.llvm.org/D14471#287412, @rengolin wrote: In http://reviews.llvm.org/D14471#286380, @ahatanak wrote: I think I can use macro __aarch64__ to have getAArch64TargetCPU return "native" when the compiler is not run on an AArch64 platform, but it doesn't sound like that was what you had in mind? Not at all. :) It seems like a simple thing really, but it piles up pretty quickly. The command line: $ clang -arch arm64 -mtune=native -c hello.c doesn't make sense on an x86 machine at all. "native" means it should understand the host as an ARM64 hardware, which it's not. This should bail on error. I agree and that is what I'm trying to do. It should error out with something like "native is not supported". You haven't provided any tests (you should really, what should work and what should return errors), so I can't tell just by looking at the code what you expect the new behaviour to be. I'm assuming you still want things to break, but you want your error message to have a "native" in it instead of crashing the compiler. That's correct. The error message I'm looking for is something like the one I wrote in the summary: error: the clang compiler does not support 'native' I think diag::err_drv_unsupported_opt_for_target is the most appropriate one for this. I didn't include a test case because I didn't know how to write a test that passes on an aarch64 host and fails on anything else. Do you know of any test cases in trunk that pass or fail depending on which host it is running on? Back to the code... Passing the pointer to a boolean is leaking logic from getAArch64TargetCPU out. If you know inside that the CPU is "native", why not just return that instead? Would that break other things? If so, these other things should be fixed. Fudging the parameters and keeping every other caller in the dark (because you're overriding the bool pointer) is certainly not the way we want to go, because that would mean two different behaviours depending on how you call the function. Wouldn't it break on an aarch64 host? With "-mtune=native", the current code in trunk will get the host cpu name (which I believe is currently always "generic" for aarch64). But if I apply this patch, it will error out because "native" is not a valid cpu name. http://reviews.llvm.org/D14471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits -- Jon Roelofs jonat...@codesourcery.com CodeSourcery / Mentor Embedded ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14592: Qt (version 4 or 5) signal/method checker
On Wed, Nov 11, 2015 at 11:58:34PM +, Jonathan Roelofs wrote: > jroelofs added a subscriber: jroelofs. > > > Comment at: lib/StaticAnalyzer/Checkers/QtSignalSlotChecker.cpp:114 > @@ +113,3 @@ > + printMethodNameWithPramaTypes(Out, C, FName, M, > false); > + const std::string NS = > qtNormalizeSignature(Out.str()); > + if (NS == MethodWithParamsNorm) > > Why are you throwing away all the work that the parser did to build up the > type signature as a QualType, pretty-printing it, and the re-parsing it by > hand? ISTM it would be much better if qtNormalizeSignature had this type > signature: `QualType qtNormalizeSignature(QualType)`. > > First of all this code compare two things: 1)QualType from clang, 2)string that I get from connect(obj1, "1f(unsgined int)", obj2, "2g(uint)"); ^ ^ here and here so to simplify things I need compare QualType and data from StringLiteral. The second thing, that should be menntioned,that "Qt normalization" and converting output to string (this normalization used by Qt when it compare at runtime that signal match slot) not match exactly to already existing in clang print type functionality: - it remove spaces every where, where possible - it print "uint" instead of "unsigned int" so I glad to have inteface like this `QualType qtNormalizeSignature(QualType)` but to use such interface I need convert StringLiteral conent to function signature, in other words run parser inside parser, plus I have to hach PrintType from AST to make possible print function signature in `Qt` way. -- /Evgeniy ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14592: Qt (version 4 or 5) signal/method checker
Dushistov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/QtSignalSlotChecker.cpp:114 @@ +113,3 @@ + printMethodNameWithPramaTypes(Out, C, FName, M, false); + const std::string NS = qtNormalizeSignature(Out.str()); + if (NS == MethodWithParamsNorm) jroelofs wrote: > Why are you throwing away all the work that the parser did to build up the > type signature as a QualType, pretty-printing it, and the re-parsing it by > hand? ISTM it would be much better if qtNormalizeSignature had this type > signature: `QualType qtNormalizeSignature(QualType)`. Looks like `reviews` have no SMTP interface, so duplicate answer here: First of all this code compare two things: 1)QualType from clang, 2)string that I get from connect(obj1, "1f(unsgined int)", obj2, "2g(uint)"); ^ ^ here and here so to simplify things I need compare QualType and data from StringLiteral. The second thing, that should be menntioned,that "Qt normalization" and converting output to string (this normalization used by Qt when it compare at runtime that signal match slot) not match exactly to already existing in clang "print type functionality": - it remove spaces every where, where possible - it print "uint" instead of "unsigned int" so I glad to have inteface like this `QualType qtNormalizeSignature(QualType)` but to use such interface I need convert StringLiteral conent to function signature, in other words run parser inside parser, plus I have to hack PrintType from AST to make possible print function signature in `Qt` way. http://reviews.llvm.org/D14592 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252834 - Provide a frontend based error for always_inline functions that require
Author: echristo Date: Wed Nov 11 18:44:12 2015 New Revision: 252834 URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev Log: Provide a frontend based error for always_inline functions that require target features that the caller function doesn't provide. This matches the existing backend failure to inline functions that don't have matching target features - and diagnoses earlier in the case of always_inline. Fix up a few test cases that were, in fact, invalid if you tried to generate code from the backend with the specified target features and add a couple of tests to illustrate what's going on. This should fix PR25246. Added: cfe/trunk/test/CodeGen/target-features-error-2.c cfe/trunk/test/CodeGen/target-features-error.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/CodeGen/CGExpr.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/test/CodeGen/3dnow-builtins.c cfe/trunk/test/CodeGen/avx512vl-builtins.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 18:44:12 2015 @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi def err_arm_invalid_specialreg : Error<"invalid special register for builtin">; def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">; def err_builtin_needs_feature : Error<"%0 needs target feature %1">; +def err_function_needs_feature +: Error<"function %0 and always_inline callee function %1 are required to " +"have matching target features">; def warn_builtin_unknown : Warning<"use of unknown builtin %0">, InGroup, DefaultError; def warn_dyn_class_memaccess : Warning< Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp assert(CalleeType->isFunctionPointerType() && "Call must have function pointer type!"); + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) +// If this isn't an always_inline function we can't guarantee that any +// function isn't being used correctly so only check if we have the +// attribute and a set of target attributes that might be different from +// our default. +if (TargetDecl->hasAttr() && +TargetDecl->hasAttr()) + checkTargetFeatures(E, FD); + CalleeType = getContext().getCanonicalType(CalleeType); const auto *FnType = Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 @@ -1843,7 +1843,8 @@ template void CGBuilderInsertergetBuiltinID(); - const char *FeatureList = - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); - - if (!FeatureList || StringRef(FeatureList) == "") -return; - - llvm::StringMap FeatureMap; - CGM.getFunctionFeatureMap(FeatureMap, FD); - - // If we have at least one of the features in the feature list return - // true, otherwise return false. - SmallVector AttrFeatures; - StringRef(FeatureList).split(AttrFeatures, ","); - if (!std::all_of(AttrFeatures.begin(), AttrFeatures.end(), - [&](StringRef &Feature) { - SmallVector OrFeatures; - Feature.split(OrFeatures, "|"); - return std::any_of(OrFeatures.begin(), OrFeatures.end(), - [&](StringRef &Feature) { -return FeatureMap[Feature]; - }); - })) -CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature) -<< TargetDecl->getDeclName() -<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); + if (BuiltinID) { +SmallVector ReqFeatures; +const char *FeatureList = +CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); +// Return if the builtin doesn't have any required features. +if (!FeatureList || StringRef(FeatureList) == "") + return; +StringRef(FeatureList).split(ReqFeatures, ","); + +// If there aren't any required features l
r252832 - In preparation to use it in more places rename
Author: echristo Date: Wed Nov 11 18:44:04 2015 New Revision: 252832 URL: http://llvm.org/viewvc/llvm-project?rev=252832&view=rev Log: In preparation to use it in more places rename checkBuiltinTargetFeatures to checkTargetFeatures and sink the error handling into the function. Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=252832&r1=252831&r2=252832&view=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Nov 11 18:44:04 2015 @@ -344,24 +344,24 @@ Value *CodeGenFunction::EmitVAStartEnd(V } // Returns true if we have a valid set of target features. -bool CodeGenFunction::checkBuiltinTargetFeatures( -const FunctionDecl *TargetDecl) { +void CodeGenFunction::checkTargetFeatures(const CallExpr *E, + const FunctionDecl *TargetDecl) { // Early exit if this is an indirect call. if (!TargetDecl) -return true; +return; // Get the current enclosing function if it exists. If it doesn't // we can't check the target features anyhow. const FunctionDecl *FD = dyn_cast_or_null(CurFuncDecl); if (!FD) -return true; +return; unsigned BuiltinID = TargetDecl->getBuiltinID(); const char *FeatureList = CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); if (!FeatureList || StringRef(FeatureList) == "") -return true; +return; llvm::StringMap FeatureMap; CGM.getFunctionFeatureMap(FeatureMap, FD); @@ -370,7 +370,7 @@ bool CodeGenFunction::checkBuiltinTarget // true, otherwise return false. SmallVector AttrFeatures; StringRef(FeatureList).split(AttrFeatures, ","); - return std::all_of(AttrFeatures.begin(), AttrFeatures.end(), + if (!std::all_of(AttrFeatures.begin(), AttrFeatures.end(), [&](StringRef &Feature) { SmallVector OrFeatures; Feature.split(OrFeatures, "|"); @@ -378,7 +378,10 @@ bool CodeGenFunction::checkBuiltinTarget [&](StringRef &Feature) { return FeatureMap[Feature]; }); - }); + })) +CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature) +<< TargetDecl->getDeclName() +<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); } RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD, @@ -1969,10 +1972,7 @@ RValue CodeGenFunction::EmitBuiltinExpr( // This is down here to avoid non-target specific builtins, however, if // generic builtins start to require generic target features then we // can move this up to the beginning of the function. - if (!checkBuiltinTargetFeatures(FD)) -CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature) -<< FD->getDeclName() -<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); + checkTargetFeatures(E, FD); // See if we have a target specific intrinsic. const char *Name = getContext().BuiltinInfo.getName(BuiltinID); Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=252832&r1=252831&r2=252832&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Nov 11 18:44:04 2015 @@ -2638,9 +2638,7 @@ public: RValue EmitCallExpr(const CallExpr *E, ReturnValueSlot ReturnValue = ReturnValueSlot()); - void getFunctionFeatureMap(llvm::StringMap &FeatureMap, - const FunctionDecl *FD); - bool checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl); + void checkTargetFeatures(const CallExpr *E, const FunctionDecl *TargetDecl); llvm::CallInst *EmitRuntimeCall(llvm::Value *callee, const Twine &name = ""); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r252833 - Move checkTargetFeatures to CodeGenFunction.cpp to make it
Author: echristo Date: Wed Nov 11 18:44:07 2015 New Revision: 252833 URL: http://llvm.org/viewvc/llvm-project?rev=252833&view=rev Log: Move checkTargetFeatures to CodeGenFunction.cpp to make it more obvious that it's generic. Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=252833&r1=252832&r2=252833&view=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Nov 11 18:44:07 2015 @@ -21,7 +21,6 @@ #include "clang/Basic/TargetBuiltins.h" #include "clang/Basic/TargetInfo.h" #include "clang/CodeGen/CGFunctionInfo.h" -#include "clang/Sema/SemaDiagnostic.h" #include "llvm/ADT/StringExtras.h" #include "llvm/IR/CallSite.h" #include "llvm/IR/DataLayout.h" @@ -343,47 +342,6 @@ Value *CodeGenFunction::EmitVAStartEnd(V return Builder.CreateCall(CGM.getIntrinsic(inst), ArgValue); } -// Returns true if we have a valid set of target features. -void CodeGenFunction::checkTargetFeatures(const CallExpr *E, - const FunctionDecl *TargetDecl) { - // Early exit if this is an indirect call. - if (!TargetDecl) -return; - - // Get the current enclosing function if it exists. If it doesn't - // we can't check the target features anyhow. - const FunctionDecl *FD = dyn_cast_or_null(CurFuncDecl); - if (!FD) -return; - - unsigned BuiltinID = TargetDecl->getBuiltinID(); - const char *FeatureList = - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); - - if (!FeatureList || StringRef(FeatureList) == "") -return; - - llvm::StringMap FeatureMap; - CGM.getFunctionFeatureMap(FeatureMap, FD); - - // If we have at least one of the features in the feature list return - // true, otherwise return false. - SmallVector AttrFeatures; - StringRef(FeatureList).split(AttrFeatures, ","); - if (!std::all_of(AttrFeatures.begin(), AttrFeatures.end(), - [&](StringRef &Feature) { - SmallVector OrFeatures; - Feature.split(OrFeatures, "|"); - return std::any_of(OrFeatures.begin(), OrFeatures.end(), - [&](StringRef &Feature) { -return FeatureMap[Feature]; - }); - })) -CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature) -<< TargetDecl->getDeclName() -<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); -} - RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD, unsigned BuiltinID, const CallExpr *E, ReturnValueSlot ReturnValue) { Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252833&r1=252832&r2=252833&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:07 2015 @@ -29,6 +29,7 @@ #include "clang/Basic/TargetInfo.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "clang/Frontend/CodeGenOptions.h" +#include "clang/Sema/SemaDiagnostic.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/MDBuilder.h" @@ -1841,3 +1842,45 @@ template void CGBuilderInserter(CurFuncDecl); + if (!FD) +return; + + unsigned BuiltinID = TargetDecl->getBuiltinID(); + const char *FeatureList = + CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); + + if (!FeatureList || StringRef(FeatureList) == "") +return; + + llvm::StringMap FeatureMap; + CGM.getFunctionFeatureMap(FeatureMap, FD); + + // If we have at least one of the features in the feature list return + // true, otherwise return false. + SmallVector AttrFeatures; + StringRef(FeatureList).split(AttrFeatures, ","); + if (!std::all_of(AttrFeatures.begin(), AttrFeatures.end(), + [&](StringRef &Feature) { + SmallVector OrFeatures; + Feature.split(OrFeatures, "|"); + return std::any_of(OrFeatures.begin(), OrFeatures.end(), + [&](StringRef &Feature) { +return FeatureMap[Feature]; + }); + })) +CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature) +<< TargetDecl->getDeclName() +<< CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); +} + ___ cfe-commits ma
Re: r252834 - Provide a frontend based error for always_inline functions that require
FWIW we should also have the backend avoid inlining and perhaps produce a diagnostic if something makes it there as well. -eric On Wed, Nov 11, 2015 at 4:46 PM Eric Christopher via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: echristo > Date: Wed Nov 11 18:44:12 2015 > New Revision: 252834 > > URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev > Log: > Provide a frontend based error for always_inline functions that require > target features that the caller function doesn't provide. This matches > the existing backend failure to inline functions that don't have > matching target features - and diagnoses earlier in the case of > always_inline. > > Fix up a few test cases that were, in fact, invalid if you tried > to generate code from the backend with the specified target features > and add a couple of tests to illustrate what's going on. > > This should fix PR25246. > > Added: > cfe/trunk/test/CodeGen/target-features-error-2.c > cfe/trunk/test/CodeGen/target-features-error.c > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/CodeGen/CGExpr.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > cfe/trunk/test/CodeGen/3dnow-builtins.c > cfe/trunk/test/CodeGen/avx512vl-builtins.c > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 > 18:44:12 2015 > @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi > def err_arm_invalid_specialreg : Error<"invalid special register for > builtin">; > def err_invalid_cpu_supports : Error<"invalid cpu feature string for > builtin">; > def err_builtin_needs_feature : Error<"%0 needs target feature %1">; > +def err_function_needs_feature > +: Error<"function %0 and always_inline callee function %1 are > required to " > +"have matching target features">; > def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >InGroup, DefaultError; > def warn_dyn_class_memaccess : Warning< > > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 > @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >assert(CalleeType->isFunctionPointerType() && > "Call must have function pointer type!"); > > + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) > +// If this isn't an always_inline function we can't guarantee that any > +// function isn't being used correctly so only check if we have the > +// attribute and a set of target attributes that might be different > from > +// our default. > +if (TargetDecl->hasAttr() && > +TargetDecl->hasAttr()) > + checkTargetFeatures(E, FD); > + >CalleeType = getContext().getCanonicalType(CalleeType); > >const auto *FnType = > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 > @@ -1843,7 +1843,8 @@ template void CGBuilderInserter llvm::BasicBlock::iterator InsertPt) const; > #undef PreserveNames > > -// Returns true if we have a valid set of target features. > +// Emits an error if we don't have a valid set of target features for the > +// called function. > void CodeGenFunction::checkTargetFeatures(const CallExpr *E, >const FunctionDecl *TargetDecl) > { >// Early exit if this is an indirect call. > @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature >if (!FD) > return; > > + // Grab the required features for the call. For a builtin this is > listed in > + // the td file with the default cpu, for an always_inline function this > is any > + // listed cpu and any listed features. >unsigned BuiltinID = TargetDecl->getBuiltinID(); > - const char *FeatureList = > - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > - > - if (!FeatureList || StringRef(FeatureList) == "") > -return; > - > - llvm::StringMap FeatureMap; > - CGM.getFunctionFeatureMap(FeatureMap, FD); > - > - // If we have at least one of the features in the feature list return > - // true, othe
r252836 - [CMake] Setup an install component for libclang and c-index-test.
Author: akirtzidis Date: Wed Nov 11 18:46:57 2015 New Revision: 252836 URL: http://llvm.org/viewvc/llvm-project?rev=252836&view=rev Log: [CMake] Setup an install component for libclang and c-index-test. Also don't create libclang dylib symlinks on darwin. Modified: cfe/trunk/CMakeLists.txt cfe/trunk/tools/c-index-test/CMakeLists.txt cfe/trunk/tools/libclang/CMakeLists.txt Modified: cfe/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=252836&r1=252835&r2=252836&view=diff == --- cfe/trunk/CMakeLists.txt (original) +++ cfe/trunk/CMakeLists.txt Wed Nov 11 18:46:57 2015 @@ -354,7 +354,7 @@ endmacro() macro(add_clang_library name) cmake_parse_arguments(ARG -"" +"SHARED" "" "ADDITIONAL_HEADERS" ${ARGN}) @@ -390,17 +390,29 @@ macro(add_clang_library name) ${ARG_ADDITIONAL_HEADERS} # It may contain unparsed unknown args. ) endif() - llvm_add_library(${name} ${ARG_UNPARSED_ARGUMENTS} ${srcs}) + if(ARG_SHARED) +set(ARG_ENABLE_SHARED SHARED) + endif() + llvm_add_library(${name} ${ARG_ENABLE_SHARED} ${ARG_UNPARSED_ARGUMENTS} ${srcs}) if(TARGET ${name}) target_link_libraries(${name} ${cmake_2_8_12_INTERFACE} ${LLVM_COMMON_LIBS}) if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "libclang") install(TARGETS ${name} +COMPONENT ${name} EXPORT ClangTargets LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX} ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX} RUNTIME DESTINATION bin) + + if (${ARG_SHARED} AND NOT CMAKE_CONFIGURATION_TYPES) +add_custom_target(install-${name} + DEPENDS ${name} + COMMAND "${CMAKE_COMMAND}" + -DCMAKE_INSTALL_COMPONENT=${name} + -P "${CMAKE_BINARY_DIR}/cmake_install.cmake") + endif() endif() set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS ${name}) else() @@ -451,6 +463,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY) endif() install(DIRECTORY include/clang-c + COMPONENT libclang DESTINATION include FILES_MATCHING PATTERN "*.h" Modified: cfe/trunk/tools/c-index-test/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/CMakeLists.txt?rev=252836&r1=252835&r2=252836&view=diff == --- cfe/trunk/tools/c-index-test/CMakeLists.txt (original) +++ cfe/trunk/tools/c-index-test/CMakeLists.txt Wed Nov 11 18:46:57 2015 @@ -28,3 +28,12 @@ if (CLANG_HAVE_LIBXML) include_directories(SYSTEM ${LIBXML2_INCLUDE_DIR}) target_link_libraries(c-index-test ${LIBXML2_LIBRARIES}) endif() + +install(TARGETS c-index-test + RUNTIME DESTINATION local/bin + COMPONENT c-index-test) +add_custom_target(install-c-index-test + DEPENDS c-index-test + COMMAND "${CMAKE_COMMAND}" + -DCMAKE_INSTALL_COMPONENT=c-index-test + -P "${CMAKE_BINARY_DIR}/cmake_install.cmake") Modified: cfe/trunk/tools/libclang/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CMakeLists.txt?rev=252836&r1=252835&r2=252836&view=diff == --- cfe/trunk/tools/libclang/CMakeLists.txt (original) +++ cfe/trunk/tools/libclang/CMakeLists.txt Wed Nov 11 18:46:57 2015 @@ -102,18 +102,16 @@ if(ENABLE_SHARED) PROPERTIES VERSION ${LIBCLANG_LIBRARY_VERSION} DEFINE_SYMBOL _CINDEX_LIB_) - else() -set_target_properties(libclang - PROPERTIES - VERSION ${LIBCLANG_LIBRARY_VERSION} - DEFINE_SYMBOL _CINDEX_LIB_) - endif() - - if(APPLE) + elseif(APPLE) set(LIBCLANG_LINK_FLAGS " -Wl,-compatibility_version -Wl,1") set(LIBCLANG_LINK_FLAGS "${LIBCLANG_LINK_FLAGS} -Wl,-current_version -Wl,${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}") set_property(TARGET libclang APPEND_STRING PROPERTY LINK_FLAGS ${LIBCLANG_LINK_FLAGS}) + else() +set_target_properties(libclang + PROPERTIES + VERSION ${LIBCLANG_LIBRARY_VERSION} + DEFINE_SYMBOL _CINDEX_LIB_) endif() endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: echristo > Date: Wed Nov 11 18:44:12 2015 > New Revision: 252834 > > URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev > Log: > Provide a frontend based error for always_inline functions that require > target features that the caller function doesn't provide. This matches > the existing backend failure to inline functions that don't have > matching target features - and diagnoses earlier in the case of > always_inline. > > Fix up a few test cases that were, in fact, invalid if you tried > to generate code from the backend with the specified target features > and add a couple of tests to illustrate what's going on. > > This should fix PR25246. > > Added: > cfe/trunk/test/CodeGen/target-features-error-2.c > cfe/trunk/test/CodeGen/target-features-error.c > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/CodeGen/CGExpr.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > cfe/trunk/test/CodeGen/3dnow-builtins.c > cfe/trunk/test/CodeGen/avx512vl-builtins.c > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 > 18:44:12 2015 > @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi > def err_arm_invalid_specialreg : Error<"invalid special register for > builtin">; > def err_invalid_cpu_supports : Error<"invalid cpu feature string for > builtin">; > def err_builtin_needs_feature : Error<"%0 needs target feature %1">; > +def err_function_needs_feature > +: Error<"function %0 and always_inline callee function %1 are > required to " > +"have matching target features">; > def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >InGroup, DefaultError; > def warn_dyn_class_memaccess : Warning< > > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 > @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >assert(CalleeType->isFunctionPointerType() && > "Call must have function pointer type!"); > > + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) > +// If this isn't an always_inline function we can't guarantee that any > +// function isn't being used correctly so only check if we have the > +// attribute and a set of target attributes that might be different > from > +// our default. > +if (TargetDecl->hasAttr() && > +TargetDecl->hasAttr()) > + checkTargetFeatures(E, FD); > + >CalleeType = getContext().getCanonicalType(CalleeType); > >const auto *FnType = > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 > @@ -1843,7 +1843,8 @@ template void CGBuilderInserter llvm::BasicBlock::iterator InsertPt) const; > #undef PreserveNames > > -// Returns true if we have a valid set of target features. > +// Emits an error if we don't have a valid set of target features for the > +// called function. > void CodeGenFunction::checkTargetFeatures(const CallExpr *E, >const FunctionDecl *TargetDecl) > { >// Early exit if this is an indirect call. > @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature >if (!FD) > return; > > + // Grab the required features for the call. For a builtin this is > listed in > + // the td file with the default cpu, for an always_inline function this > is any > + // listed cpu and any listed features. >unsigned BuiltinID = TargetDecl->getBuiltinID(); > - const char *FeatureList = > - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > - > - if (!FeatureList || StringRef(FeatureList) == "") > -return; > - > - llvm::StringMap FeatureMap; > - CGM.getFunctionFeatureMap(FeatureMap, FD); > - > - // If we have at least one of the features in the feature list return > - // true, otherwise return false. > - SmallVector AttrFeatures; > - StringRef(FeatureList).split(AttrFeatures, ","); > - if (!std::all_of(At
Re: r252827 - Add diagnostics which fall under [dcl.spec.concept]p5
On Wed, Nov 11, 2015 at 3:53 PM, Nathan Wilson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: nwilson > Date: Wed Nov 11 17:53:35 2015 > New Revision: 252827 > > URL: http://llvm.org/viewvc/llvm-project?rev=252827&view=rev > Log: > Add diagnostics which fall under [dcl.spec.concept]p5 > > Summary: Diagnose when a function concept declaration has parameter(s) > > Reviewers: rsmith, faisalv, aaron.ballman, hubert.reinterpretcast > > Subscribers: cfe-commits > > Differential Revision: http://reviews.llvm.org/D14352 > > Added: > cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/SemaDecl.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252827&r1=252826&r2=252827&view=diff > > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 > 17:53:35 2015 > @@ -1994,6 +1994,8 @@ def err_function_concept_exception_spec > def err_concept_decl_invalid_specifiers : Error< >"%select{variable|function}0 concept cannot be declared " >"'%select{thread_local|inline|friend|constexpr}1'">; > +def err_function_concept_with_params : Error< > + "function concept cannot have any parameters">; > > // C++11 char16_t/char32_t > def warn_cxx98_compat_unicode_type : Warning< > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=252827&r1=252826&r2=252827&view=diff > > == > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Nov 11 17:53:35 2015 > @@ -7607,6 +7607,13 @@ Sema::ActOnFunctionDeclarator(Scope *S, > } else { >Context.adjustExceptionSpec(NewFD, EST_BasicNoexcept); > } > + > +// C++ Concepts TS [dcl.spec.concept]p5: A function concept has > the > +// following restrictions: > +// - The declaration's parameter list shall be equivalent to an > empty > +// parameter list. > +if ((FPT->getNumParams() > 0) || (FPT->isVariadic())) > You don't need parens around either of these conditions. > + Diag(NewFD->getLocation(), > diag::err_function_concept_with_params); >} > >// C++ Concepts TS [dcl.spec.concept]p2: Every concept definition is > > Added: > cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp?rev=252827&view=auto > > == > --- > cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp > (added) > +++ > cfe/trunk/test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p5.cpp Wed > Nov 11 17:53:35 2015 > @@ -0,0 +1,13 @@ > +// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s > + > +template > +concept bool fcpv(void) { return true; } > + > +template > +concept bool fcpi(int i = 0) { return true; } // expected-error > {{function concept cannot have any parameters}} > + > +template > +concept bool fcpp(Ts... ts) { return true; } // expected-error {{function > concept cannot have any parameters}} > + > +template > +concept bool fcpva(...) { return true; } // expected-error {{function > concept cannot have any parameters}} > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: echristo > Date: Wed Nov 11 18:44:12 2015 > New Revision: 252834 > > URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev > Log: > Provide a frontend based error for always_inline functions that require > target features that the caller function doesn't provide. This matches > the existing backend failure to inline functions that don't have > matching target features - and diagnoses earlier in the case of > always_inline. > > Fix up a few test cases that were, in fact, invalid if you tried > to generate code from the backend with the specified target features > and add a couple of tests to illustrate what's going on. > > This should fix PR25246. > > Added: > cfe/trunk/test/CodeGen/target-features-error-2.c > cfe/trunk/test/CodeGen/target-features-error.c > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/CodeGen/CGExpr.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > cfe/trunk/test/CodeGen/3dnow-builtins.c > cfe/trunk/test/CodeGen/avx512vl-builtins.c > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 > 18:44:12 2015 > @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi > def err_arm_invalid_specialreg : Error<"invalid special register for > builtin">; > def err_invalid_cpu_supports : Error<"invalid cpu feature string for > builtin">; > def err_builtin_needs_feature : Error<"%0 needs target feature %1">; > +def err_function_needs_feature > +: Error<"function %0 and always_inline callee function %1 are > required to " > +"have matching target features">; > It would be useful to say which feature is missing here. Also, "matching" implies to me that the sets must be the same, whereas I think the rule is that the caller must at least all the features that the callee uses. Something like: "always_inline callee function %0 uses target feature %1 that caller %2 does not require" might be more useful? > def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >InGroup, DefaultError; > def warn_dyn_class_memaccess : Warning< > > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 > @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >assert(CalleeType->isFunctionPointerType() && > "Call must have function pointer type!"); > > + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) > +// If this isn't an always_inline function we can't guarantee that any > +// function isn't being used correctly so only check if we have the > +// attribute and a set of target attributes that might be different > from > +// our default. > +if (TargetDecl->hasAttr() && > +TargetDecl->hasAttr()) > + checkTargetFeatures(E, FD); > + >CalleeType = getContext().getCanonicalType(CalleeType); > >const auto *FnType = > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff > > == > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 > @@ -1843,7 +1843,8 @@ template void CGBuilderInserter llvm::BasicBlock::iterator InsertPt) const; > #undef PreserveNames > > -// Returns true if we have a valid set of target features. > +// Emits an error if we don't have a valid set of target features for the > +// called function. > void CodeGenFunction::checkTargetFeatures(const CallExpr *E, >const FunctionDecl *TargetDecl) > { >// Early exit if this is an indirect call. > @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature >if (!FD) > return; > > + // Grab the required features for the call. For a builtin this is > listed in > + // the td file with the default cpu, for an always_inline function this > is any > + // listed cpu and any listed features. >unsigned BuiltinID = TargetDecl->getBuiltinID(); > - const char *FeatureList = > - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); > - > - if (!FeatureList || String
r252840 - Refactor out some common code from r252834
Author: dblaikie Date: Wed Nov 11 19:09:58 2015 New Revision: 252840 URL: http://llvm.org/viewvc/llvm-project?rev=252840&view=rev Log: Refactor out some common code from r252834 Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252840&r1=252839&r2=252840&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 19:09:58 2015 @@ -1843,6 +1843,30 @@ template void CGBuilderInserter &ReqFeatures, +CodeGenModule &CGM, const FunctionDecl *FD) { + // If there aren't any required features listed then go ahead and return. + if (ReqFeatures.empty()) +return false; + + // Now build up the set of caller features and verify that all the required + // features are there. + llvm::StringMap CallerFeatureMap; + CGM.getFunctionFeatureMap(CallerFeatureMap, FD); + + // If we have at least one of the features in the feature list return + // true, otherwise return false. + return std::all_of( + ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef Feature) { +SmallVector OrFeatures; +Feature.split(OrFeatures, "|"); +return std::any_of(OrFeatures.begin(), OrFeatures.end(), + [&](StringRef Feature) { + return CallerFeatureMap.lookup(Feature); + }); + }); +} + // Emits an error if we don't have a valid set of target features for the // called function. void CodeGenFunction::checkTargetFeatures(const CallExpr *E, @@ -1870,26 +1894,7 @@ void CodeGenFunction::checkTargetFeature return; StringRef(FeatureList).split(ReqFeatures, ","); -// If there aren't any required features listed then go ahead and return. -if (ReqFeatures.empty()) - return; - -// Now build up the set of caller features and verify that all the required -// features are there. -llvm::StringMap CallerFeatureMap; -CGM.getFunctionFeatureMap(CallerFeatureMap, FD); - -// If we have at least one of the features in the feature list return -// true, otherwise return false. -if (!std::all_of( -ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef &Feature) { - SmallVector OrFeatures; - Feature.split(OrFeatures, "|"); - return std::any_of(OrFeatures.begin(), OrFeatures.end(), - [&](StringRef &Feature) { - return CallerFeatureMap.lookup(Feature); - }); -})) +if (!hasRequiredFeatures(ReqFeatures, CGM, FD)) CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature) << TargetDecl->getDeclName() << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); @@ -1901,25 +1906,7 @@ void CodeGenFunction::checkTargetFeature CGM.getFunctionFeatureMap(CalleeFeatureMap, TargetDecl); for (const auto &F : CalleeFeatureMap) ReqFeatures.push_back(F.getKey()); -// If there aren't any required features listed then go ahead and return. -if (ReqFeatures.empty()) - return; - -// Now get the features that the caller provides. -llvm::StringMap CallerFeatureMap; -CGM.getFunctionFeatureMap(CallerFeatureMap, FD); - -// If we have at least one of the features in the feature list return -// true, otherwise return false. -if (!std::all_of( -ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef &Feature) { - SmallVector OrFeatures; - Feature.split(OrFeatures, "|"); - return std::any_of(OrFeatures.begin(), OrFeatures.end(), - [&](StringRef &Feature) { - return CallerFeatureMap.lookup(Feature); - }); -})) +if (!hasRequiredFeatures(ReqFeatures, CGM, FD)) CGM.getDiags().Report(E->getLocStart(), diag::err_function_needs_feature) << FD->getDeclName() << TargetDecl->getDeclName(); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 4:54 PM, David Blaikie wrote: > > > On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: echristo >> Date: Wed Nov 11 18:44:12 2015 >> New Revision: 252834 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev >> Log: >> Provide a frontend based error for always_inline functions that require >> target features that the caller function doesn't provide. This matches >> the existing backend failure to inline functions that don't have >> matching target features - and diagnoses earlier in the case of >> always_inline. >> >> Fix up a few test cases that were, in fact, invalid if you tried >> to generate code from the backend with the specified target features >> and add a couple of tests to illustrate what's going on. >> >> This should fix PR25246. >> >> Added: >> cfe/trunk/test/CodeGen/target-features-error-2.c >> cfe/trunk/test/CodeGen/target-features-error.c >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/CodeGen/CGExpr.cpp >> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >> cfe/trunk/test/CodeGen/3dnow-builtins.c >> cfe/trunk/test/CodeGen/avx512vl-builtins.c >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 >> 18:44:12 2015 >> @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi >> def err_arm_invalid_specialreg : Error<"invalid special register for >> builtin">; >> def err_invalid_cpu_supports : Error<"invalid cpu feature string for >> builtin">; >> def err_builtin_needs_feature : Error<"%0 needs target feature %1">; >> +def err_function_needs_feature >> +: Error<"function %0 and always_inline callee function %1 are >> required to " >> +"have matching target features">; >> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >>InGroup, DefaultError; >> def warn_dyn_class_memaccess : Warning< >> >> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 >> @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >>assert(CalleeType->isFunctionPointerType() && >> "Call must have function pointer type!"); >> >> + if (const FunctionDecl *FD = >> dyn_cast_or_null(TargetDecl)) >> +// If this isn't an always_inline function we can't guarantee that >> any >> +// function isn't being used correctly so only check if we have the >> +// attribute and a set of target attributes that might be different >> from >> +// our default. >> +if (TargetDecl->hasAttr() && >> +TargetDecl->hasAttr()) >> + checkTargetFeatures(E, FD); >> + >>CalleeType = getContext().getCanonicalType(CalleeType); >> >>const auto *FnType = >> >> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 >> @@ -1843,7 +1843,8 @@ template void CGBuilderInserter> llvm::BasicBlock::iterator InsertPt) const; >> #undef PreserveNames >> >> -// Returns true if we have a valid set of target features. >> +// Emits an error if we don't have a valid set of target features for the >> +// called function. >> void CodeGenFunction::checkTargetFeatures(const CallExpr *E, >>const FunctionDecl >> *TargetDecl) { >>// Early exit if this is an indirect call. >> @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature >>if (!FD) >> return; >> >> + // Grab the required features for the call. For a builtin this is >> listed in >> + // the td file with the default cpu, for an always_inline function >> this is any >> + // listed cpu and any listed features. >>unsigned BuiltinID = TargetDecl->getBuiltinID(); >> - const char *FeatureList = >> - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); >> - >> - if (!FeatureList || StringRef(FeatureList) == "") >> -return; >> - >> - llvm::StringMap FeatureMap; >> - CGM.getFunctionFeatureMap(FeatureMap, FD); >> - >> - // If we have at least one of the
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 5:03 PM Richard Smith wrote: > On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: echristo >> Date: Wed Nov 11 18:44:12 2015 >> New Revision: 252834 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev >> Log: >> Provide a frontend based error for always_inline functions that require >> target features that the caller function doesn't provide. This matches >> the existing backend failure to inline functions that don't have >> matching target features - and diagnoses earlier in the case of >> always_inline. >> >> Fix up a few test cases that were, in fact, invalid if you tried >> to generate code from the backend with the specified target features >> and add a couple of tests to illustrate what's going on. >> >> This should fix PR25246. >> >> Added: >> cfe/trunk/test/CodeGen/target-features-error-2.c >> cfe/trunk/test/CodeGen/target-features-error.c >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/CodeGen/CGExpr.cpp >> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >> cfe/trunk/test/CodeGen/3dnow-builtins.c >> cfe/trunk/test/CodeGen/avx512vl-builtins.c >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 >> 18:44:12 2015 >> @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi >> def err_arm_invalid_specialreg : Error<"invalid special register for >> builtin">; >> def err_invalid_cpu_supports : Error<"invalid cpu feature string for >> builtin">; >> def err_builtin_needs_feature : Error<"%0 needs target feature %1">; >> +def err_function_needs_feature >> +: Error<"function %0 and always_inline callee function %1 are >> required to " >> +"have matching target features">; >> > > It would be useful to say which feature is missing here. Also, "matching" > implies to > Right. > me that the sets must be the same, whereas I think the rule is that the > caller must at least all the features that the callee uses. Something like: > "always_inline callee function %0 uses target feature %1 that caller %2 > does not require" might be more useful? > So, I was waiting on this I admit :) What we really need is a "string set" way of doing set difference and then optionally translating that to command line options. I'm really leery of doing the "full set of options missing" though due to just the sheer size of the error messages out the other end. i.e. "default" compiles and "function that requires everything a skylake processor has". An intermediate result could be taking a look at the target attribute on the callee and posting that as a "set" of features that are required by the caller without bothering to do the set difference? I'll look into doing that, though it might not be immediately. Thoughts? -eric > > >> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >>InGroup, DefaultError; >> def warn_dyn_class_memaccess : Warning< >> >> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 >> @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >>assert(CalleeType->isFunctionPointerType() && >> "Call must have function pointer type!"); >> >> + if (const FunctionDecl *FD = >> dyn_cast_or_null(TargetDecl)) >> +// If this isn't an always_inline function we can't guarantee that >> any >> +// function isn't being used correctly so only check if we have the >> +// attribute and a set of target attributes that might be different >> from >> +// our default. >> +if (TargetDecl->hasAttr() && >> +TargetDecl->hasAttr()) >> + checkTargetFeatures(E, FD); >> + >>CalleeType = getContext().getCanonicalType(CalleeType); >> >>const auto *FnType = >> >> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 >> @@ -1843,7 +1843,8 @@ template void CGBuilderInserter> llvm::BasicBlock::iterator InsertPt) const; >> #undef PreserveNames >
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 5:13 PM David Blaikie wrote: > On Wed, Nov 11, 2015 at 4:54 PM, David Blaikie wrote: > >> >> >> On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: echristo >>> Date: Wed Nov 11 18:44:12 2015 >>> New Revision: 252834 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev >>> Log: >>> Provide a frontend based error for always_inline functions that require >>> target features that the caller function doesn't provide. This matches >>> the existing backend failure to inline functions that don't have >>> matching target features - and diagnoses earlier in the case of >>> always_inline. >>> >>> Fix up a few test cases that were, in fact, invalid if you tried >>> to generate code from the backend with the specified target features >>> and add a couple of tests to illustrate what's going on. >>> >>> This should fix PR25246. >>> >>> Added: >>> cfe/trunk/test/CodeGen/target-features-error-2.c >>> cfe/trunk/test/CodeGen/target-features-error.c >>> Modified: >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> cfe/trunk/lib/CodeGen/CGExpr.cpp >>> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >>> cfe/trunk/test/CodeGen/3dnow-builtins.c >>> cfe/trunk/test/CodeGen/avx512vl-builtins.c >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 >>> 18:44:12 2015 >>> @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi >>> def err_arm_invalid_specialreg : Error<"invalid special register for >>> builtin">; >>> def err_invalid_cpu_supports : Error<"invalid cpu feature string for >>> builtin">; >>> def err_builtin_needs_feature : Error<"%0 needs target feature %1">; >>> +def err_function_needs_feature >>> +: Error<"function %0 and always_inline callee function %1 are >>> required to " >>> +"have matching target features">; >>> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >>>InGroup, DefaultError; >>> def warn_dyn_class_memaccess : Warning< >>> >>> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 >>> @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >>>assert(CalleeType->isFunctionPointerType() && >>> "Call must have function pointer type!"); >>> >>> + if (const FunctionDecl *FD = >>> dyn_cast_or_null(TargetDecl)) >>> +// If this isn't an always_inline function we can't guarantee that >>> any >>> +// function isn't being used correctly so only check if we have the >>> +// attribute and a set of target attributes that might be different >>> from >>> +// our default. >>> +if (TargetDecl->hasAttr() && >>> +TargetDecl->hasAttr()) >>> + checkTargetFeatures(E, FD); >>> + >>>CalleeType = getContext().getCanonicalType(CalleeType); >>> >>>const auto *FnType = >>> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 >>> @@ -1843,7 +1843,8 @@ template void CGBuilderInserter>> llvm::BasicBlock::iterator InsertPt) const; >>> #undef PreserveNames >>> >>> -// Returns true if we have a valid set of target features. >>> +// Emits an error if we don't have a valid set of target features for >>> the >>> +// called function. >>> void CodeGenFunction::checkTargetFeatures(const CallExpr *E, >>>const FunctionDecl >>> *TargetDecl) { >>>// Early exit if this is an indirect call. >>> @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature >>>if (!FD) >>> return; >>> >>> + // Grab the required features for the call. For a builtin this is >>> listed in >>> + // the td file with the default cpu, for an always_inline function >>> this is any >>> + // listed cpu and any listed features. >>>unsigned BuiltinID = TargetDecl->getBuiltinID(); >>> - const char *FeatureList = >>> - CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID); >>> - >>> - if (!FeatureList || St
r252841 - Change code owner for Clang Static Analyzer to Anna Zaks.
Author: kremenek Date: Wed Nov 11 19:31:27 2015 New Revision: 252841 URL: http://llvm.org/viewvc/llvm-project?rev=252841&view=rev Log: Change code owner for Clang Static Analyzer to Anna Zaks. Modified: cfe/trunk/CODE_OWNERS.TXT Modified: cfe/trunk/CODE_OWNERS.TXT URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/CODE_OWNERS.TXT?rev=252841&r1=252840&r2=252841&view=diff == --- cfe/trunk/CODE_OWNERS.TXT (original) +++ cfe/trunk/CODE_OWNERS.TXT Wed Nov 11 19:31:27 2015 @@ -41,8 +41,8 @@ N: Anton Korobeynikov E: an...@korobeynikov.info D: Exception handling, Windows codegen, ARM EABI -N: Ted Kremenek -E: kreme...@apple.com +N: Anna Zaks +E: ga...@apple.com D: Clang Static Analyzer N: John McCall ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 5:25 PM, Eric Christopher wrote: > On Wed, Nov 11, 2015 at 5:03 PM Richard Smith > wrote: > >> On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: echristo >>> Date: Wed Nov 11 18:44:12 2015 >>> New Revision: 252834 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev >>> Log: >>> Provide a frontend based error for always_inline functions that require >>> target features that the caller function doesn't provide. This matches >>> the existing backend failure to inline functions that don't have >>> matching target features - and diagnoses earlier in the case of >>> always_inline. >>> >>> Fix up a few test cases that were, in fact, invalid if you tried >>> to generate code from the backend with the specified target features >>> and add a couple of tests to illustrate what's going on. >>> >>> This should fix PR25246. >>> >>> Added: >>> cfe/trunk/test/CodeGen/target-features-error-2.c >>> cfe/trunk/test/CodeGen/target-features-error.c >>> Modified: >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> cfe/trunk/lib/CodeGen/CGExpr.cpp >>> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >>> cfe/trunk/test/CodeGen/3dnow-builtins.c >>> cfe/trunk/test/CodeGen/avx512vl-builtins.c >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 >>> 18:44:12 2015 >>> @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi >>> def err_arm_invalid_specialreg : Error<"invalid special register for >>> builtin">; >>> def err_invalid_cpu_supports : Error<"invalid cpu feature string for >>> builtin">; >>> def err_builtin_needs_feature : Error<"%0 needs target feature %1">; >>> +def err_function_needs_feature >>> +: Error<"function %0 and always_inline callee function %1 are >>> required to " >>> +"have matching target features">; >>> >> >> It would be useful to say which feature is missing here. Also, "matching" >> implies to >> > > Right. > > >> me that the sets must be the same, whereas I think the rule is that the >> caller must at least all the features that the callee uses. Something like: >> "always_inline callee function %0 uses target feature %1 that caller %2 >> does not require" might be more useful? >> > > So, I was waiting on this I admit :) > > What we really need is a "string set" way of doing set difference and then > optionally translating that to command line options. I'm really leery of > doing the "full set of options missing" though due to just the sheer size > of the error messages out the other end. i.e. "default" compiles and > "function that requires everything a skylake processor has". An > intermediate result could be taking a look at the target attribute on the > callee and posting that as a "set" of features that are required by the > caller without bothering to do the set difference? > > I'll look into doing that, though it might not be immediately. > > Thoughts? > In the short term, just listing the first feature you find that's missing would help (the feature that causes you to fall out of the all_of check below). > -eric > > >> >> >>> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >>>InGroup, DefaultError; >>> def warn_dyn_class_memaccess : Warning< >>> >>> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 >>> @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >>>assert(CalleeType->isFunctionPointerType() && >>> "Call must have function pointer type!"); >>> >>> + if (const FunctionDecl *FD = >>> dyn_cast_or_null(TargetDecl)) >>> +// If this isn't an always_inline function we can't guarantee that >>> any >>> +// function isn't being used correctly so only check if we have the >>> +// attribute and a set of target attributes that might be different >>> from >>> +// our default. >>> +if (TargetDecl->hasAttr() && >>> +TargetDecl->hasAttr()) >>> + checkTargetFeatures(E, FD); >>> + >>>CalleeType = getContext().getCanonicalType(CalleeType); >>> >>>const auto *FnType = >>> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 5:38 PM Richard Smith wrote: > On Wed, Nov 11, 2015 at 5:25 PM, Eric Christopher > wrote: > >> On Wed, Nov 11, 2015 at 5:03 PM Richard Smith >> wrote: >> >>> On Wed, Nov 11, 2015 at 4:44 PM, Eric Christopher via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> Author: echristo Date: Wed Nov 11 18:44:12 2015 New Revision: 252834 URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev Log: Provide a frontend based error for always_inline functions that require target features that the caller function doesn't provide. This matches the existing backend failure to inline functions that don't have matching target features - and diagnoses earlier in the case of always_inline. Fix up a few test cases that were, in fact, invalid if you tried to generate code from the backend with the specified target features and add a couple of tests to illustrate what's going on. This should fix PR25246. Added: cfe/trunk/test/CodeGen/target-features-error-2.c cfe/trunk/test/CodeGen/target-features-error.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/CodeGen/CGExpr.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/test/CodeGen/3dnow-builtins.c cfe/trunk/test/CodeGen/avx512vl-builtins.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 18:44:12 2015 @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi def err_arm_invalid_specialreg : Error<"invalid special register for builtin">; def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">; def err_builtin_needs_feature : Error<"%0 needs target feature %1">; +def err_function_needs_feature +: Error<"function %0 and always_inline callee function %1 are required to " +"have matching target features">; >>> >>> It would be useful to say which feature is missing here. Also, >>> "matching" implies to >>> >> >> Right. >> >> >>> me that the sets must be the same, whereas I think the rule is that the >>> caller must at least all the features that the callee uses. Something like: >>> "always_inline callee function %0 uses target feature %1 that caller %2 >>> does not require" might be more useful? >>> >> >> So, I was waiting on this I admit :) >> >> What we really need is a "string set" way of doing set difference and >> then optionally translating that to command line options. I'm really leery >> of doing the "full set of options missing" though due to just the sheer >> size of the error messages out the other end. i.e. "default" compiles and >> "function that requires everything a skylake processor has". An >> intermediate result could be taking a look at the target attribute on the >> callee and posting that as a "set" of features that are required by the >> caller without bothering to do the set difference? >> >> I'll look into doing that, though it might not be immediately. >> >> Thoughts? >> > > In the short term, just listing the first feature you find that's missing > would help (the feature that causes you to fall out of the all_of check > below). > > Sure, that's reasonable. -eric > -eric >> >> >>> >>> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, InGroup, DefaultError; def warn_dyn_class_memaccess : Warning< Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp assert(CalleeType->isFunctionPointerType() && "Call must have function pointer type!"); + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) +// If this isn't an always_inline function we can't guarantee that any +// function isn't being used correctly so only check if we have the +// attribute and a set of target attributes that might be different from +// our default. +if (TargetDecl->hasAttr() && +TargetDecl->hasAttr()) + checkTargetFeatures(E, FD); + CalleeType = getContext().getCanonicalType(CalleeType); >
Re: r252834 - Provide a frontend based error for always_inline functions that require
I think you are suggesting we change the inliner to produce a diagnostic (error or warning?) when the callee is marked always_inline and its function attributes are not compatible with the caller's (functionsHaveCompatibleAttributes returns false). Is that correct? On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher wrote: > FWIW we should also have the backend avoid inlining and perhaps produce a > diagnostic if something makes it there as well. > > -eric > > On Wed, Nov 11, 2015 at 4:46 PM Eric Christopher via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: echristo >> Date: Wed Nov 11 18:44:12 2015 >> New Revision: 252834 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev >> Log: >> Provide a frontend based error for always_inline functions that require >> target features that the caller function doesn't provide. This matches >> the existing backend failure to inline functions that don't have >> matching target features - and diagnoses earlier in the case of >> always_inline. >> >> Fix up a few test cases that were, in fact, invalid if you tried >> to generate code from the backend with the specified target features >> and add a couple of tests to illustrate what's going on. >> >> This should fix PR25246. >> >> Added: >> cfe/trunk/test/CodeGen/target-features-error-2.c >> cfe/trunk/test/CodeGen/target-features-error.c >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/CodeGen/CGExpr.cpp >> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >> cfe/trunk/test/CodeGen/3dnow-builtins.c >> cfe/trunk/test/CodeGen/avx512vl-builtins.c >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 >> 18:44:12 2015 >> @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi >> def err_arm_invalid_specialreg : Error<"invalid special register for >> builtin">; >> def err_invalid_cpu_supports : Error<"invalid cpu feature string for >> builtin">; >> def err_builtin_needs_feature : Error<"%0 needs target feature %1">; >> +def err_function_needs_feature >> +: Error<"function %0 and always_inline callee function %1 are >> required to " >> +"have matching target features">; >> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >>InGroup, DefaultError; >> def warn_dyn_class_memaccess : Warning< >> >> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 >> @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >>assert(CalleeType->isFunctionPointerType() && >> "Call must have function pointer type!"); >> >> + if (const FunctionDecl *FD = >> dyn_cast_or_null(TargetDecl)) >> +// If this isn't an always_inline function we can't guarantee that >> any >> +// function isn't being used correctly so only check if we have the >> +// attribute and a set of target attributes that might be different >> from >> +// our default. >> +if (TargetDecl->hasAttr() && >> +TargetDecl->hasAttr()) >> + checkTargetFeatures(E, FD); >> + >>CalleeType = getContext().getCanonicalType(CalleeType); >> >>const auto *FnType = >> >> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 >> @@ -1843,7 +1843,8 @@ template void CGBuilderInserter> llvm::BasicBlock::iterator InsertPt) const; >> #undef PreserveNames >> >> -// Returns true if we have a valid set of target features. >> +// Emits an error if we don't have a valid set of target features for the >> +// called function. >> void CodeGenFunction::checkTargetFeatures(const CallExpr *E, >>const FunctionDecl >> *TargetDecl) { >>// Early exit if this is an indirect call. >> @@ -1856,31 +1857,70 @@ void CodeGenFunction::checkTargetFeature >>if (!FD) >> return; >> >> + // Grab the required features for the call. For a builtin this is >> listed in >> + // the td file with the default cpu, for an always_inline function >> this is any >> + //
Re: r252834 - Provide a frontend based error for always_inline functions that require
On Wed, Nov 11, 2015 at 6:08 PM Akira Hatanaka wrote: > I think you are suggesting we change the inliner to produce a diagnostic > (error or warning?) when the callee is marked always_inline and its > function attributes are not compatible with the caller's > (functionsHaveCompatibleAttributes returns false). Is that correct? > > Yep. And I think an error is appropriate here. The programmer said that the function must always inline and it's not always being inlined. I think it's a larger change because it'll require some refactoring and the ability to give reasons out of the always inline pass when faced with cost analysis. -eric > On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher > wrote: > >> FWIW we should also have the backend avoid inlining and perhaps produce a >> diagnostic if something makes it there as well. >> >> -eric >> >> On Wed, Nov 11, 2015 at 4:46 PM Eric Christopher via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: echristo >>> Date: Wed Nov 11 18:44:12 2015 >>> New Revision: 252834 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev >>> Log: >>> Provide a frontend based error for always_inline functions that require >>> target features that the caller function doesn't provide. This matches >>> the existing backend failure to inline functions that don't have >>> matching target features - and diagnoses earlier in the case of >>> always_inline. >>> >>> Fix up a few test cases that were, in fact, invalid if you tried >>> to generate code from the backend with the specified target features >>> and add a couple of tests to illustrate what's going on. >>> >>> This should fix PR25246. >>> >>> Added: >>> cfe/trunk/test/CodeGen/target-features-error-2.c >>> cfe/trunk/test/CodeGen/target-features-error.c >>> Modified: >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> cfe/trunk/lib/CodeGen/CGExpr.cpp >>> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >>> cfe/trunk/test/CodeGen/3dnow-builtins.c >>> cfe/trunk/test/CodeGen/avx512vl-builtins.c >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 >>> 18:44:12 2015 >>> @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi >>> def err_arm_invalid_specialreg : Error<"invalid special register for >>> builtin">; >>> def err_invalid_cpu_supports : Error<"invalid cpu feature string for >>> builtin">; >>> def err_builtin_needs_feature : Error<"%0 needs target feature %1">; >>> +def err_function_needs_feature >>> +: Error<"function %0 and always_inline callee function %1 are >>> required to " >>> +"have matching target features">; >>> def warn_builtin_unknown : Warning<"use of unknown builtin %0">, >>>InGroup, DefaultError; >>> def warn_dyn_class_memaccess : Warning< >>> >>> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 >>> @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp >>>assert(CalleeType->isFunctionPointerType() && >>> "Call must have function pointer type!"); >>> >>> + if (const FunctionDecl *FD = >>> dyn_cast_or_null(TargetDecl)) >>> +// If this isn't an always_inline function we can't guarantee that >>> any >>> +// function isn't being used correctly so only check if we have the >>> +// attribute and a set of target attributes that might be different >>> from >>> +// our default. >>> +if (TargetDecl->hasAttr() && >>> +TargetDecl->hasAttr()) >>> + checkTargetFeatures(E, FD); >>> + >>>CalleeType = getContext().getCanonicalType(CalleeType); >>> >>>const auto *FnType = >>> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1=252833&r2=252834&view=diff >>> >>> == >>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Nov 11 18:44:12 2015 >>> @@ -1843,7 +1843,8 @@ template void CGBuilderInserter>> llvm::BasicBlock::iterator InsertPt) const; >>> #undef PreserveNames >>> >>> -// Returns true if we have a valid set of target features. >>> +// Emits an error if we don't have a valid set of target features for >>> the >>> +// called functi
Re: r252834 - Provide a frontend based error for always_inline functions that require
Currently, we inline a function only if the call to isInlineViable returns true, which means there are cases where we don't inline functions marked always_inline. Is there a reason we haven't made changes to produce any diagnostic in those cases? The comment also says "should be inlined whenever possible", which gives the impression that this is only a best effort attribute. On Wed, Nov 11, 2015 at 6:09 PM, Eric Christopher wrote: > > > On Wed, Nov 11, 2015 at 6:08 PM Akira Hatanaka wrote: > >> I think you are suggesting we change the inliner to produce a diagnostic >> (error or warning?) when the callee is marked always_inline and its >> function attributes are not compatible with the caller's >> (functionsHaveCompatibleAttributes returns false). Is that correct? >> >> > Yep. And I think an error is appropriate here. The programmer said that > the function must always inline and it's not always being inlined. > > I think it's a larger change because it'll require some refactoring and > the ability to give reasons out of the always inline pass when faced with > cost analysis. > > -eric > > >> On Wed, Nov 11, 2015 at 4:48 PM, Eric Christopher >> wrote: >> >>> FWIW we should also have the backend avoid inlining and perhaps produce >>> a diagnostic if something makes it there as well. >>> >>> -eric >>> >>> On Wed, Nov 11, 2015 at 4:46 PM Eric Christopher via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> Author: echristo Date: Wed Nov 11 18:44:12 2015 New Revision: 252834 URL: http://llvm.org/viewvc/llvm-project?rev=252834&view=rev Log: Provide a frontend based error for always_inline functions that require target features that the caller function doesn't provide. This matches the existing backend failure to inline functions that don't have matching target features - and diagnoses earlier in the case of always_inline. Fix up a few test cases that were, in fact, invalid if you tried to generate code from the backend with the specified target features and add a couple of tests to illustrate what's going on. This should fix PR25246. Added: cfe/trunk/test/CodeGen/target-features-error-2.c cfe/trunk/test/CodeGen/target-features-error.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/CodeGen/CGExpr.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/test/CodeGen/3dnow-builtins.c cfe/trunk/test/CodeGen/avx512vl-builtins.c Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov 11 18:44:12 2015 @@ -431,6 +431,9 @@ def err_builtin_definition : Error<"defi def err_arm_invalid_specialreg : Error<"invalid special register for builtin">; def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">; def err_builtin_needs_feature : Error<"%0 needs target feature %1">; +def err_function_needs_feature +: Error<"function %0 and always_inline callee function %1 are required to " +"have matching target features">; def warn_builtin_unknown : Warning<"use of unknown builtin %0">, InGroup, DefaultError; def warn_dyn_class_memaccess : Warning< Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=252834&r1=252833&r2=252834&view=diff == --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Nov 11 18:44:12 2015 @@ -3747,6 +3747,15 @@ RValue CodeGenFunction::EmitCall(QualTyp assert(CalleeType->isFunctionPointerType() && "Call must have function pointer type!"); + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) +// If this isn't an always_inline function we can't guarantee that any +// function isn't being used correctly so only check if we have the +// attribute and a set of target attributes that might be different from +// our default. +if (TargetDecl->hasAttr() && +TargetDecl->hasAttr()) + checkTargetFeatures(E, FD); + CalleeType = getContext().getCanonicalType(CalleeType); const auto *FnType = Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=252834&r1