[clang] Fix printing of templated records. (PR #86339)
mikaelholmen wrote: > @zahiraam I still get > > ``` > ../../clang/unittests/AST/DeclPrinterTest.cpp:1394:3: error: expression > result unused [-Werror,-Wunused-value] > [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; }; > ^ > ../../clang/unittests/AST/DeclPrinterTest.cpp:1405:3: error: expression > result unused [-Werror,-Wunused-value] > [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; }; > ^ > ../../clang/unittests/AST/DeclPrinterTest.cpp:1416:3: error: expression > result unused [-Werror,-Wunused-value] > [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; }; > ^~~~ > ../../clang/unittests/AST/DeclPrinterTest.cpp:1435:3: error: expression > result unused [-Werror,-Wunused-value] > [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; }; > ^ > ../../clang/unittests/AST/DeclPrinterTest.cpp:1455:3: error: expression > result unused [-Werror,-Wunused-value] > [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; }; > ^~~~ > 5 errors generated. > ``` > > This has been broken for quite a while now, can you please fix or revert? Thank you @alinas for fixing the problem in 577e0ef94fb0 ! https://github.com/llvm/llvm-project/pull/86339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] defc485 - [AArch64] Remove Automatic Enablement of FEAT_F32MM (#85203)
Author: Jack Styles Date: 2024-03-27T07:49:38Z New Revision: defc4859b032ccaec69f24b6cfd9882fece5f093 URL: https://github.com/llvm/llvm-project/commit/defc4859b032ccaec69f24b6cfd9882fece5f093 DIFF: https://github.com/llvm/llvm-project/commit/defc4859b032ccaec69f24b6cfd9882fece5f093.diff LOG: [AArch64] Remove Automatic Enablement of FEAT_F32MM (#85203) When `+sve` is passed in the command line, if the Architecture being targeted is V8.6A/V9.1A or later, `+f32mm` is also added. This enables FEAT_32MM, however at the time of writing no CPU's support this. This leads to the FEAT_32MM instructions being compiled for CPU's that do not support them. This commit removes the automatic enablement, however the option is still able to be used by passing `+f32mm`. Added: Modified: clang/test/Driver/aarch64-sve.c clang/test/Preprocessor/aarch64-target-features.c llvm/docs/ReleaseNotes.rst llvm/lib/TargetParser/AArch64TargetParser.cpp llvm/unittests/TargetParser/TargetParserTest.cpp Removed: diff --git a/clang/test/Driver/aarch64-sve.c b/clang/test/Driver/aarch64-sve.c index f34b2700deb91c..4a33c2e3c8d367 100644 --- a/clang/test/Driver/aarch64-sve.c +++ b/clang/test/Driver/aarch64-sve.c @@ -6,12 +6,11 @@ // RUN: %clang --target=aarch64 -march=armv8.6a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV8A-NOSVE %s // GENERICV8A-NOSVE-NOT: "-target-feature" "+sve" -// The 32-bit floating point matrix multiply extension is enabled by default -// for armv8.6-a targets (or later) with SVE, and can optionally be enabled for -// any target from armv8.2a onwards (we don't enforce not using it with earlier -// targets). +// The 32-bit floating point matrix multiply extension is an optional feature +// that can be used for any target from armv8.2a and onwards. This can be +// enabled using the `+f32mm` option.`. // RUN: %clang --target=aarch64 -march=armv8.6a -### -c %s 2>&1 | FileCheck -check-prefix=NO-F32MM %s -// RUN: %clang --target=aarch64 -march=armv8.6a+sve -### -c %s 2>&1 | FileCheck -check-prefix=F32MM %s +// RUN: %clang --target=aarch64 -march=armv8.6a+sve+f32mm -### -c %s 2>&1 | FileCheck -check-prefix=F32MM %s // RUN: %clang --target=aarch64 -march=armv8.5a+f32mm -### -c %s 2>&1 | FileCheck -check-prefix=F32MM %s // NO-F32MM-NOT: "-target-feature" "+f32mm" // F32MM: "-target-feature" "+f32mm" diff --git a/clang/test/Preprocessor/aarch64-target-features.c b/clang/test/Preprocessor/aarch64-target-features.c index 9f8a8bdeeb9cb0..85762b7fed4d71 100644 --- a/clang/test/Preprocessor/aarch64-target-features.c +++ b/clang/test/Preprocessor/aarch64-target-features.c @@ -196,7 +196,7 @@ // CHECK-8_6-NOT: __ARM_FEATURE_SHA3 1 // CHECK-8_6-NOT: __ARM_FEATURE_SM4 1 -// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.6-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE-8_6 %s +// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.6-a+sve+f32mm -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE-8_6 %s // CHECK-SVE-8_6: __ARM_FEATURE_SVE 1 // CHECK-SVE-8_6: __ARM_FEATURE_SVE_BF16 1 // CHECK-SVE-8_6: __ARM_FEATURE_SVE_MATMUL_FP32 1 diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index c2b1a9d3d73835..7588048334d792 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -76,6 +76,7 @@ Changes to the AMDGPU Backend Changes to the ARM Backend -- +* FEAT_F32MM is no longer activated by default when using `+sve` on v8.6-A or greater. The feature is still available and can be used by adding `+f32mm` to the command line options. Changes to the AVR Backend -- diff --git a/llvm/lib/TargetParser/AArch64TargetParser.cpp b/llvm/lib/TargetParser/AArch64TargetParser.cpp index e36832f563eed8..71099462d5ecff 100644 --- a/llvm/lib/TargetParser/AArch64TargetParser.cpp +++ b/llvm/lib/TargetParser/AArch64TargetParser.cpp @@ -186,11 +186,6 @@ void AArch64::ExtensionSet::enable(ArchExtKind E) { // Special cases for dependencies which vary depending on the base // architecture version. if (BaseArch) { -// +sve implies +f32mm if the base architecture is v8.6A+ or v9.1A+ -// It isn't the case in general that sve implies both f64mm and f32mm -if (E == AEK_SVE && BaseArch->is_superset(ARMV8_6A)) - enable(AEK_F32MM); - // +fp16 implies +fp16fml for v8.4A+, but not v9.0-A+ if (E == AEK_FP16 && BaseArch->is_superset(ARMV8_4A) && !BaseArch->is_superset(ARMV9A)) diff --git a/llvm/unittests/TargetParser/TargetParserTest.cpp b/llvm/unittests/TargetParser/TargetParserTest.cpp index a7d0b1687a7f91..2c72a7229b5274 100644 --- a/llvm/unittests/TargetParser/TargetParserTest.cpp +++ b/llvm/unittests/TargetParser/TargetParserTest.cpp @@ -2347,13 +2347,6 @@ AArch64ExtensionDependenciesBaseArchTestParams {}, {"aes", "sh
[clang] [llvm] [AArch64] Remove Automatic Enablement of FEAT_F32MM (PR #85203)
https://github.com/Stylie777 closed https://github.com/llvm/llvm-project/pull/85203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/komalverma04 edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
@@ -1399,13 +1401,22 @@ RValue AtomicInfo::convertAtomicTempToRValue(Address addr, LVal.getBaseInfo(), TBAAAccessInfo())); } +static bool shouldCastToInt(llvm::Type *ValTy, bool CmpXchg) { arsenm wrote: The answer should just be false. I see no reason to maintain any of this logic in clang https://github.com/llvm/llvm-project/pull/86691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
arsenm wrote: > The non-ieee FP types left out as it seems easier if someone working with > that target does this part including test updates, which should be simple > enough by now. Just add the tests https://github.com/llvm/llvm-project/pull/86691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang][UBSan] Add implicit conversion check for bitfields (PR #75481)
Zonotora wrote: > Also, I don't have commit access (first PR here), someone has to commit on my > behalf! @AaronBallman https://github.com/llvm/llvm-project/pull/75481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [CodeGen][arm64e] Add methods and data members to Address, which are needed to authenticate signed pointers (PR #86721)
https://github.com/asl approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/86721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PowerPC] Implement 32-bit expansion for rldimi (PR #86783)
https://github.com/ecnelises created https://github.com/llvm/llvm-project/pull/86783 rldimi is 64-bit instruction, due to backward compatibility, it needs to be expanded into series of rlwimi in 32-bit environment. In the future, we may improve bit permutation selector and remove such direct codegen. >From 3362a81ca64e5dec6e64e4ed544c30078025db15 Mon Sep 17 00:00:00 2001 From: Qiu Chaofan Date: Wed, 27 Mar 2024 17:11:04 +0800 Subject: [PATCH] [PowerPC] Implement 32-bit expansion for rldimi rldimi is 64-bit instruction, due to backward compatibility, it needs to be expanded into series of rlwimi in 32-bit environment. In the future, we may improve bit permutation selector and remove such direct codegen. --- clang/lib/Sema/SemaChecking.cpp | 1 - llvm/lib/Target/PowerPC/PPCISelLowering.cpp | 109 -- llvm/test/CodeGen/PowerPC/rldimi.ll | 366 3 files changed, 454 insertions(+), 22 deletions(-) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 08449581330934..5e8228ed998978 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5236,7 +5236,6 @@ static bool isPPC_64Builtin(unsigned BuiltinID) { case PPC::BI__builtin_ppc_fetch_and_andlp: case PPC::BI__builtin_ppc_fetch_and_orlp: case PPC::BI__builtin_ppc_fetch_and_swaplp: - case PPC::BI__builtin_ppc_rldimi: return true; } return false; diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp index cce0efad39c75b..7e42773f3aa1cd 100644 --- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp @@ -643,6 +643,7 @@ PPCTargetLowering::PPCTargetLowering(const PPCTargetMachine &TM, // We want to custom lower some of our intrinsics. setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::Other, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::f64, Custom); + setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::i64, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::ppcf128, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::v4f32, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::v2f64, Custom); @@ -10748,6 +10749,88 @@ static bool getVectorCompareInfo(SDValue Intrin, int &CompareOpc, return true; } +static SDValue getRotateInsert32(SelectionDAG &DAG, SDLoc Loc, SDValue Dst, + SDValue Src, unsigned SH, unsigned MB, + unsigned ME) { + assert(SH < 32 && MB < 32 && ME < 32 && + "Invalid argument for rotate insert!"); + return SDValue( + DAG.getMachineNode(PPC::RLWIMI, Loc, MVT::i32, + {Dst, Src, DAG.getTargetConstant(SH, Loc, MVT::i32), + DAG.getTargetConstant(MB, Loc, MVT::i32), + DAG.getTargetConstant(ME, Loc, MVT::i32)}), + 0); +} + +static SDValue getRotateInsert64(SelectionDAG &DAG, SDLoc Loc, SDValue Dst, + SDValue Src, unsigned SH, unsigned MB, + unsigned ME, bool IsPPC64) { + assert(SH < 64 && MB < 64 && ME < 64 && + "Invalid argument for rotate insert!"); + if (IsPPC64) { +// rldimi requires ME=63-SH, otherwise rotation is needed before rldimi. +if (ME < 63 - SH) { + Src = DAG.getNode(ISD::ROTL, Loc, MVT::i64, Src, +DAG.getConstant(ME + SH + 1, Loc, MVT::i32)); +} else if (ME > 63 - SH) { + Src = DAG.getNode(ISD::ROTL, Loc, MVT::i64, Src, +DAG.getConstant(ME + SH - 63, Loc, MVT::i32)); +} +return SDValue(DAG.getMachineNode( + PPC::RLDIMI, Loc, MVT::i64, + {Dst, Src, DAG.getTargetConstant(63 - ME, Loc, MVT::i32), +DAG.getTargetConstant(MB, Loc, MVT::i32)}), + 0); + } + + // To implement rldimi(Dst, Src) on 32-bit target, four parts are needed. SH + // is adjusted to simplify cases. Invalid ranges will be skipped. + // - SrcHi inserted into DstHi with [0, 32-SH) + // - SrcLo inserted into DstHi with [32-SH, 32) + // - SrcHi inserted into DstLo with [32, 64-SH) + // - SrcLo inserted into DstLo with [64-SH, 64) + auto [SrcLo, SrcHi] = DAG.SplitScalar(Src, Loc, MVT::i32, MVT::i32); + auto [DstLo, DstHi] = DAG.SplitScalar(Dst, Loc, MVT::i32, MVT::i32); + if (SH >= 32) { +SH -= 32; +std::swap(SrcLo, SrcHi); + } + auto GetSubInsert = [&DAG, &Loc, SH](unsigned Left, unsigned Right, + SDValue Src, SDValue Dst, unsigned MB, + unsigned ME) { +if (Left > Right) + return Dst; + +if (MB <= ME) { + if (MB <= Right && ME >= Left) +return getRotateInsert32(DAG, Loc, Dst, Src, SH, + std::max(MB, Left) % 32, + std::min(ME, Right) % 32)
[clang] [llvm] [PowerPC] Implement 32-bit expansion for rldimi (PR #86783)
llvmbot wrote: @llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-powerpc Author: Qiu Chaofan (ecnelises) Changes rldimi is 64-bit instruction, due to backward compatibility, it needs to be expanded into series of rlwimi in 32-bit environment. In the future, we may improve bit permutation selector and remove such direct codegen. --- Patch is 20.74 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86783.diff 3 Files Affected: - (modified) clang/lib/Sema/SemaChecking.cpp (-1) - (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+88-21) - (modified) llvm/test/CodeGen/PowerPC/rldimi.ll (+366) ``diff diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 08449581330934..5e8228ed998978 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5236,7 +5236,6 @@ static bool isPPC_64Builtin(unsigned BuiltinID) { case PPC::BI__builtin_ppc_fetch_and_andlp: case PPC::BI__builtin_ppc_fetch_and_orlp: case PPC::BI__builtin_ppc_fetch_and_swaplp: - case PPC::BI__builtin_ppc_rldimi: return true; } return false; diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp index cce0efad39c75b..7e42773f3aa1cd 100644 --- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp @@ -643,6 +643,7 @@ PPCTargetLowering::PPCTargetLowering(const PPCTargetMachine &TM, // We want to custom lower some of our intrinsics. setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::Other, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::f64, Custom); + setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::i64, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::ppcf128, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::v4f32, Custom); setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::v2f64, Custom); @@ -10748,6 +10749,88 @@ static bool getVectorCompareInfo(SDValue Intrin, int &CompareOpc, return true; } +static SDValue getRotateInsert32(SelectionDAG &DAG, SDLoc Loc, SDValue Dst, + SDValue Src, unsigned SH, unsigned MB, + unsigned ME) { + assert(SH < 32 && MB < 32 && ME < 32 && + "Invalid argument for rotate insert!"); + return SDValue( + DAG.getMachineNode(PPC::RLWIMI, Loc, MVT::i32, + {Dst, Src, DAG.getTargetConstant(SH, Loc, MVT::i32), + DAG.getTargetConstant(MB, Loc, MVT::i32), + DAG.getTargetConstant(ME, Loc, MVT::i32)}), + 0); +} + +static SDValue getRotateInsert64(SelectionDAG &DAG, SDLoc Loc, SDValue Dst, + SDValue Src, unsigned SH, unsigned MB, + unsigned ME, bool IsPPC64) { + assert(SH < 64 && MB < 64 && ME < 64 && + "Invalid argument for rotate insert!"); + if (IsPPC64) { +// rldimi requires ME=63-SH, otherwise rotation is needed before rldimi. +if (ME < 63 - SH) { + Src = DAG.getNode(ISD::ROTL, Loc, MVT::i64, Src, +DAG.getConstant(ME + SH + 1, Loc, MVT::i32)); +} else if (ME > 63 - SH) { + Src = DAG.getNode(ISD::ROTL, Loc, MVT::i64, Src, +DAG.getConstant(ME + SH - 63, Loc, MVT::i32)); +} +return SDValue(DAG.getMachineNode( + PPC::RLDIMI, Loc, MVT::i64, + {Dst, Src, DAG.getTargetConstant(63 - ME, Loc, MVT::i32), +DAG.getTargetConstant(MB, Loc, MVT::i32)}), + 0); + } + + // To implement rldimi(Dst, Src) on 32-bit target, four parts are needed. SH + // is adjusted to simplify cases. Invalid ranges will be skipped. + // - SrcHi inserted into DstHi with [0, 32-SH) + // - SrcLo inserted into DstHi with [32-SH, 32) + // - SrcHi inserted into DstLo with [32, 64-SH) + // - SrcLo inserted into DstLo with [64-SH, 64) + auto [SrcLo, SrcHi] = DAG.SplitScalar(Src, Loc, MVT::i32, MVT::i32); + auto [DstLo, DstHi] = DAG.SplitScalar(Dst, Loc, MVT::i32, MVT::i32); + if (SH >= 32) { +SH -= 32; +std::swap(SrcLo, SrcHi); + } + auto GetSubInsert = [&DAG, &Loc, SH](unsigned Left, unsigned Right, + SDValue Src, SDValue Dst, unsigned MB, + unsigned ME) { +if (Left > Right) + return Dst; + +if (MB <= ME) { + if (MB <= Right && ME >= Left) +return getRotateInsert32(DAG, Loc, Dst, Src, SH, + std::max(MB, Left) % 32, + std::min(ME, Right) % 32); +} else { + if (MB < Left || ME > Right) +return getRotateInsert32(DAG, Loc, Dst, Src, SH, Left % 32, Right % 32); + + if (MB <= Right && ME < Left) +return getRotateInsert32(DAG, Loc, Dst, Src, SH, MB % 32, Right % 32); + + if (MB <= Righ
[clang] [clang] CTAD: Track template template type parameters that referenced in (PR #85405)
https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/85405 >From 2d3aee2e763e3e4970d97406e59df66e5861d5ba Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Fri, 15 Mar 2024 15:09:10 +0100 Subject: [PATCH 1/2] [clang] CTAD: Track template template type parameters that referenced in the template arguments of the RHS. Fixes https://github.com/llvm/llvm-project/issues/85385. The Finder was missing for this case, for the crash test, the template parameter TTP was incorrectly considered as not referenced/appeared in the template arguments of the right hand side of the alias template decl, thus the synthesized deduction decl doesn't contain this TTP in the template parameter list, but we have references in the declaration, thus it caused crashes. --- clang/lib/Sema/SemaTemplate.cpp | 6 ++ clang/test/SemaCXX/cxx20-ctad-type-alias.cpp | 16 2 files changed, 22 insertions(+) diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 51e8db2dfbaac8..27714066d2748b 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -2696,6 +2696,12 @@ SmallVector TemplateParamsReferencedInTemplateArgumentList( return true; } +bool TraverseTemplateName(TemplateName Template) { + if (auto *TD = Template.getAsTemplateDecl()) +MarkAppeared(TD); + return RecursiveASTVisitor::TraverseTemplateName(Template); +} + void MarkAppeared(NamedDecl *ND) { if (TemplateParams.contains(ND)) ReferencedTemplateParams.insert(ND); diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp index 3ce26c8fcd984e..7ada48b7e58f01 100644 --- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp +++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp @@ -247,3 +247,19 @@ using Bar = Foo; // expected-note {{could not match 'Foo' Bar s = {1}; // expected-error {{no viable constructor or deduction guide for deduction of template arguments}} } // namespace test18 + +// GH85385 +namespace test19 { +template typename T> +struct K {}; + +template +class Foo {}; + +// Verify that template template type parameter TTP is referenced/used in the +// template arguments of the RHS. +template typename TTP> +using Bar = Foo>; // expected-note {{candidate template ignored: could not match 'Foo>' against 'int'}} + +Bar s = 1; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of}} +} >From 35153db8d8160b9fb7f821b95c7191d1045d5fbf Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Wed, 27 Mar 2024 10:50:24 +0100 Subject: [PATCH 2/2] Add a testcase to cover the valid code. --- clang/test/SemaCXX/cxx20-ctad-type-alias.cpp | 4 1 file changed, 4 insertions(+) diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp index 7ada48b7e58f01..1c596f3255 100644 --- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp +++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp @@ -261,5 +261,9 @@ class Foo {}; template typename TTP> using Bar = Foo>; // expected-note {{candidate template ignored: could not match 'Foo>' against 'int'}} +template +class Container {}; +Bar t = Foo>(); + Bar s = 1; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of}} } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: Track template template type parameters that referenced in (PR #85405)
hokein wrote: > But my question is: how does the missing call to `MarkAppeared()` leads to > the out-of-bounds access? Do we allocate a vector that has the size of > `max(Index)` and expect that we have previously marked all template > parameters mentioned in the type? Yes, almost. The template parameter list of the synthesized deduction guide consists of two parts: 1) the template parameters of the alias template that appeared in the deductions; 2) the template parameters of the underlying template (RHS of the alias) that were not deduced; MarkAppeared() performs the step 1, the crash is that we don't track the template template type parameters, thus we miss this kind of template parameter in the list, and we have a reference to it in the synthesized deduction guide declaration which could lead to an out-of-bounds access (for this particular case, the template parameter list is empty). > I have tried to turn your example into valid code and it [also > crashed](https://gcc.godbolt.org/z/38c3dKG7b). Could we add it to the tests? Great, thanks! Added. https://github.com/llvm/llvm-project/pull/85405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86_64] fix arg pass error in struct. (PR #85394)
https://github.com/CoTinker updated https://github.com/llvm/llvm-project/pull/85394 >From ad805988f682030cd3ed6ff6b063488ed6f5707c Mon Sep 17 00:00:00 2001 From: Longsheng Mou Date: Fri, 15 Mar 2024 20:50:54 +0800 Subject: [PATCH] [X86_64] fix arg pass error in struct. In some struct like s67, only one i64 register is used when the structure parameter is transferred, which is obviously incorrect.So we need to treat the split case specially, using memory like gcc. --- clang/lib/CodeGen/Targets/X86.cpp | 6 +- clang/test/CodeGen/X86/x86_64-arguments.c | 18 ++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index 2291c991fb1107..de0dfe32a54d3a 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -2100,8 +2100,12 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, postMerge(Size, Lo, Hi); return; } + + bool InMemory = Offset % getContext().getTypeAlign(i->getType()) || + (i->getType()->getAs() && + Offset % getContext().getTypeSize(i->getType())); // Note, skip this test for bit-fields, see below. - if (!BitField && Offset % getContext().getTypeAlign(i->getType())) { + if (!BitField && InMemory) { Lo = Memory; postMerge(Size, Lo, Hi); return; diff --git a/clang/test/CodeGen/X86/x86_64-arguments.c b/clang/test/CodeGen/X86/x86_64-arguments.c index cf5636cfd518b6..82845f0a2b31fd 100644 --- a/clang/test/CodeGen/X86/x86_64-arguments.c +++ b/clang/test/CodeGen/X86/x86_64-arguments.c @@ -533,6 +533,24 @@ typedef float t66 __attribute__((__vector_size__(128), __aligned__(128))); void f66(t66 a0) { } +typedef long long t67 __attribute__((aligned (4))); +struct s67 { + int a; + t67 b; +}; +// CHECK-LABEL: define{{.*}} void @f67(ptr noundef byval(%struct.s67) align 8 %x) +void f67(struct s67 x) { +} + +typedef double t68 __attribute__((aligned (4))); +struct s68 { + int a; + t68 b; +}; +// CHECK-LABEL: define{{.*}} void @f68(ptr noundef byval(%struct.s68) align 8 %x) +void f68(struct s68 x) { +} + /// The synthesized __va_list_tag does not have file/line fields. // CHECK: = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "__va_list_tag", // CHECK-NOT: file: ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/7] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/7] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/7] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor b
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
steakhal wrote: I figured, it might be easier to directly propose and apply some of my thoughts. Let me know if you like it. My goal was: - be specific what is the impact of not having the optimal layout: memory, cachemisses - have a single block of examples, demonstrating the bad, the good and an alternative good (bitpacking) case, while have no header dependencies - rework the example to work with the default configuration of the checker (this makes it more similar to the average user's diagnostics) - move all the advanced explanations to the end, as most users won't consider setting a custom threshold - make the definition of "when it warns" more accurate. I've checked [the code](https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp#L103-L109) and it seems that the difference counts compared to the optimal layout. - discourage the use of `pragma pack` - add guide for fixing the issue, along with a practical hint. Given that it looks now as I would like it, I'll invite @NagyDonat for a round of review. Let me know if you like it too, or have suggestions for making it better. Lastly, now the generated html would look like this:  https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
@@ -75,6 +75,25 @@ class TextTokenRetokenizer { return *Pos.BufferPtr; } + char peekNext(unsigned offset) const { +assert(!isEnd()); +assert(Pos.BufferPtr != Pos.BufferEnd); +if (Pos.BufferPtr + offset <= Pos.BufferEnd) { sdkrystian wrote: Shouldn't this be `if (Pos.BufferPtr + offset < Pos.BufferEnd)` (unless this depends of the buffer being null terminated)? Otherwise this allows for an `offset` which results in `Pos.BufferEnd` being read. https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
@@ -149,6 +276,93 @@ class TextTokenRetokenizer { addToken(); } + /// Extract a type argument + bool lexType(Token &Tok) { +if (isEnd()) + return false; +Position SavedPos = Pos; +consumeWhitespace(); +SmallString<32> NextToken; +SmallString<32> WordText; +const char *WordBegin = Pos.BufferPtr; +SourceLocation Loc = getSourceLocation(); +StringRef ConstVal = StringRef("const"); +StringRef PointerVal = StringRef("*"); +StringRef ReferenceVal = StringRef("&"); +bool ConstPointer = false; + +while (!isEnd()) { + const char C = peek(); + if (!isWhitespace(C)) { +if (C == '<') { + if (!lexTemplate(WordText)) +return false; +} else { + WordText.push_back(C); sdkrystian wrote: Does this treat any non-whitespace character (other than `<`) as being part of a name? https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
@@ -149,6 +276,93 @@ class TextTokenRetokenizer { addToken(); } + /// Extract a type argument + bool lexType(Token &Tok) { +if (isEnd()) + return false; +Position SavedPos = Pos; +consumeWhitespace(); +SmallString<32> NextToken; +SmallString<32> WordText; +const char *WordBegin = Pos.BufferPtr; +SourceLocation Loc = getSourceLocation(); +StringRef ConstVal = StringRef("const"); +StringRef PointerVal = StringRef("*"); +StringRef ReferenceVal = StringRef("&"); +bool ConstPointer = false; + +while (!isEnd()) { sdkrystian wrote: Some comments wouldn't hurt... https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
@@ -89,6 +108,114 @@ class TextTokenRetokenizer { } } + bool continueInt(SmallString<32> &NextToken) { +return NextToken.ends_with(StringRef("char")) || sdkrystian wrote: Don't really understand what this is for... https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Comments] Add argument parsing for @throw @throws @exception (PR #84726)
@@ -149,6 +276,93 @@ class TextTokenRetokenizer { addToken(); } + /// Extract a type argument + bool lexType(Token &Tok) { +if (isEnd()) + return false; +Position SavedPos = Pos; +consumeWhitespace(); +SmallString<32> NextToken; +SmallString<32> WordText; +const char *WordBegin = Pos.BufferPtr; +SourceLocation Loc = getSourceLocation(); +StringRef ConstVal = StringRef("const"); +StringRef PointerVal = StringRef("*"); sdkrystian wrote: What about pointers to members? Arrays? Function? More complex declarators like `void(*)(int, int)`? https://github.com/llvm/llvm-project/pull/84726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
JonPsson1 wrote: > > The non-ieee FP types left out as it seems easier if someone working with > > that target does this part including test updates, which should be simple > > enough by now. > > Just add the tests I think this case isn't that simple as it is an 80 bit value. Currently that is loaded atomically first with i128, then stored as a temporary and then loaded as an fp80. If I remove that casting, the verifier complains "atomic memory access' operand must have a power-of-two size". https://github.com/llvm/llvm-project/pull/86691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix buildbot failure. (PR #86741)
https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/86741 >From 8d113344d0742f80bc967019131c9111a0db78ff Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat Date: Tue, 26 Mar 2024 14:21:16 -0700 Subject: [PATCH] Fix buildbot https://lab.llvm.org/buildbot/#/builders/36/builds/43998/steps/12/logs/stdio --- clang/unittests/AST/DeclPrinterTest.cpp | 54 + 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp index 07fa02bd96e25d..f2b027a25621ce 100644 --- a/clang/unittests/AST/DeclPrinterTest.cpp +++ b/clang/unittests/AST/DeclPrinterTest.cpp @@ -1387,34 +1387,38 @@ TEST(DeclPrinter, TestTemplateArgumentList16) { } TEST(DeclPrinter, TestCXXRecordDecl17) { - ASSERT_TRUE(PrintedDeclCXX98Matches("template struct Z {};" - "struct X {};" - "Z A;", - "A", "Z A")); - [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; }; + ASSERT_TRUE(PrintedDeclCXX98Matches( + "template struct Z {};" + "struct X {};" + "Z A;", + "A", "Z A", + [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; })); } TEST(DeclPrinter, TestCXXRecordDecl18) { - ASSERT_TRUE(PrintedDeclCXX98Matches("template struct Z {};" - "struct X {};" - "Z A;" - "template " - "struct Y{};" - "Y, 2> B;", - "B", "Y, 2> B")); - [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; }; + ASSERT_TRUE(PrintedDeclCXX98Matches( + "template struct Z {};" + "struct X {};" + "Z A;" + "template " + "struct Y{};" + "Y, 2> B;", + "B", "Y, 2> B", + [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; })); } TEST(DeclPrinter, TestCXXRecordDecl19) { - ASSERT_TRUE(PrintedDeclCXX98Matches("template struct Z {};" - "struct X {};" - "Z A;" - "template " - "struct Y{};" - "Y, 2> B;", - "B", "Y, 2> B")); - [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; }; + ASSERT_TRUE(PrintedDeclCXX98Matches( + "template struct Z {};" + "struct X {};" + "Z A;" + "template " + "struct Y{};" + "Y, 2> B;", + "B", "Y, 2> B", + [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; })); } + TEST(DeclPrinter, TestCXXRecordDecl20) { ASSERT_TRUE(PrintedDeclCXX98Matches( "template class Inner;" @@ -1431,8 +1435,8 @@ TEST(DeclPrinter, TestCXXRecordDecl20) { "};" "Outer, 5>::NestedStruct nestedInstance(100);", "nestedInstance", - "Outer, 5>::NestedStruct nestedInstance(100)")); - [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; }; + "Outer, 5>::NestedStruct nestedInstance(100)", + [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; })); } TEST(DeclPrinter, TestCXXRecordDecl21) { @@ -1451,8 +1455,8 @@ TEST(DeclPrinter, TestCXXRecordDecl21) { "};" "Outer, 5>::NestedStruct nestedInstance(100);", "nestedInstance", - "Outer, 5>::NestedStruct nestedInstance(100)")); - [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; }; + "Outer, 5>::NestedStruct nestedInstance(100)", + [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; })); } TEST(DeclPrinter, TestFunctionParamUglified) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
@@ -124,34 +124,45 @@ class CStringChecker : public Checker< eval::Call, const CallEvent &)>; CallDescriptionMap Callbacks = { - {{CDM::CLibrary, {"memcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"memcpy"}, 3}, std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Regular)}, - {{CDM::CLibrary, {"wmemcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmemcpy"}, 3}, std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"mempcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"mempcpy"}, 3}, std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Regular)}, - {{CDM::Unspecified, {"wmempcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmempcpy"}, 3}, std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Wide)}, {{CDM::CLibrary, {"memcmp"}, 3}, std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)}, {{CDM::CLibrary, {"wmemcmp"}, 3}, std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"memmove"}, 3}, + {{CDM::CLibraryMaybeHardened, {"memmove"}, 3}, std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Regular)}, - {{CDM::CLibrary, {"wmemmove"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmemmove"}, 3}, std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"memset"}, 3}, &CStringChecker::evalMemset}, + {{CDM::CLibraryMaybeHardened, {"memset"}, 3}, + &CStringChecker::evalMemset}, {{CDM::CLibrary, {"explicit_memset"}, 3}, &CStringChecker::evalMemset}, - {{CDM::CLibrary, {"strcpy"}, 2}, &CStringChecker::evalStrcpy}, - {{CDM::CLibrary, {"strncpy"}, 3}, &CStringChecker::evalStrncpy}, - {{CDM::CLibrary, {"stpcpy"}, 2}, &CStringChecker::evalStpcpy}, - {{CDM::CLibrary, {"strlcpy"}, 3}, &CStringChecker::evalStrlcpy}, - {{CDM::CLibrary, {"strcat"}, 2}, &CStringChecker::evalStrcat}, - {{CDM::CLibrary, {"strncat"}, 3}, &CStringChecker::evalStrncat}, - {{CDM::CLibrary, {"strlcat"}, 3}, &CStringChecker::evalStrlcat}, - {{CDM::CLibrary, {"strlen"}, 1}, &CStringChecker::evalstrLength}, + /* FIXME: C23 introduces 'memset_explicit', maybe also model that */ + {{CDM::CLibraryMaybeHardened, {"strcpy"}, 2}, + &CStringChecker::evalStrcpy}, + {{CDM::CLibraryMaybeHardened, {"strncpy"}, 3}, + &CStringChecker::evalStrncpy}, + {{CDM::CLibraryMaybeHardened, {"stpcpy"}, 2}, + &CStringChecker::evalStpcpy}, steakhal wrote: These function descriptions also encode the expected argument count. Consequently, it will also set the `RequiredParams` and `RequiredArgs` of the `CallDescription` to the specified value. This should defeat the matching of the hardened variants of these function - as they can't accept the extra harden argument. If this is the case, why do we use the `Hardened` matching, or why we specify the expected argument count in the description? Or does it mean that the hardened variants of these functions have accept the same number of arguments in the same order? https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
@@ -110,13 +110,24 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD, if (FName.starts_with("__inline") && FName.contains(Name)) return true; - if (FName.starts_with("__") && FName.ends_with("_chk") && - FName.contains(Name)) -return true; - return false; } +bool CheckerContext::isHardenedVariantOf(const FunctionDecl *FD, + StringRef Name) { + const IdentifierInfo *II = FD->getIdentifier(); + if (!II) +return false; + + StringRef FName = II->getName(); + std::string ChkName = "__" + std::string(Name) + "_chk"; + + // This is using `equals()` instead of more lenient prefix/suffix/substring + // checks because we don't want to say that e.g. `__builtin___vsprintf_chk()` + // is a hardened variant of `sprintf()`. + return FName.equals(ChkName) || FName.equals("__builtin_" + ChkName); +} steakhal wrote: ```suggestion bool CheckerContext::isHardenedVariantOf(const FunctionDecl *FD, StringRef Name) { const IdentifierInfo *II = FD->getIdentifier(); if (!II) return false; auto CompletelyMatchesParts = [II](auto... Parts) -> bool { StringRef FName = II->getName(); return (FName.consume_front(Parts) && ...) && FName.empty(); }; return CompletelyMatchesParts("__", Name, "_chk") || CompletelyMatchesParts("__builtin_", "__", Name, "_chk"); } ``` I disliked the transient allocation, but here is my alternative achieving the same operating on StringRefs. You had a comment about `__builtin___vsprintf_chk`. I wonder if we could have a test demonstrating that. https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
https://github.com/steakhal commented: I like your patch. This is an important fix. I only had a couple nits, but overall looks good. https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
@@ -124,34 +124,45 @@ class CStringChecker : public Checker< eval::Call, const CallEvent &)>; CallDescriptionMap Callbacks = { - {{CDM::CLibrary, {"memcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"memcpy"}, 3}, std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Regular)}, - {{CDM::CLibrary, {"wmemcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmemcpy"}, 3}, std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"mempcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"mempcpy"}, 3}, std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Regular)}, - {{CDM::Unspecified, {"wmempcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmempcpy"}, 3}, std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Wide)}, {{CDM::CLibrary, {"memcmp"}, 3}, std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)}, {{CDM::CLibrary, {"wmemcmp"}, 3}, std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"memmove"}, 3}, + {{CDM::CLibraryMaybeHardened, {"memmove"}, 3}, std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Regular)}, - {{CDM::CLibrary, {"wmemmove"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmemmove"}, 3}, std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"memset"}, 3}, &CStringChecker::evalMemset}, + {{CDM::CLibraryMaybeHardened, {"memset"}, 3}, + &CStringChecker::evalMemset}, {{CDM::CLibrary, {"explicit_memset"}, 3}, &CStringChecker::evalMemset}, - {{CDM::CLibrary, {"strcpy"}, 2}, &CStringChecker::evalStrcpy}, - {{CDM::CLibrary, {"strncpy"}, 3}, &CStringChecker::evalStrncpy}, - {{CDM::CLibrary, {"stpcpy"}, 2}, &CStringChecker::evalStpcpy}, - {{CDM::CLibrary, {"strlcpy"}, 3}, &CStringChecker::evalStrlcpy}, - {{CDM::CLibrary, {"strcat"}, 2}, &CStringChecker::evalStrcat}, - {{CDM::CLibrary, {"strncat"}, 3}, &CStringChecker::evalStrncat}, - {{CDM::CLibrary, {"strlcat"}, 3}, &CStringChecker::evalStrlcat}, - {{CDM::CLibrary, {"strlen"}, 1}, &CStringChecker::evalstrLength}, + /* FIXME: C23 introduces 'memset_explicit', maybe also model that */ steakhal wrote: ```suggestion // FIXME: C23 introduces 'memset_explicit', maybe also model that ``` I'd prefer single-line comments to be able comment out sections of this CDM when debugging. https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ExtractAPI] Add ability to create multiple symbol graphs (PR #86676)
https://github.com/daniel-grumberg deleted https://github.com/llvm/llvm-project/pull/86676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
@@ -1399,13 +1401,22 @@ RValue AtomicInfo::convertAtomicTempToRValue(Address addr, LVal.getBaseInfo(), TBAAAccessInfo())); } +static bool shouldCastToInt(llvm::Type *ValTy, bool CmpXchg) { JonPsson1 wrote: This is a bit messy... - For one thing, there can't currently be an atomic CmpXchg of float type - per the spec - so that must be casted to int. That would be a later patch on its own, including as well changes in the spec document and AtomicExpand. - An atomic struct needs to be casted to integer in EmitAtomicLoadOp(). - In ConvertToValueOrAtomic() and convertRValueToInt(), scalars are treated specially. But still FP80 and float/CmpXchg scalars currrently need casting. - FP80 also seems like a separate patch if there's a need. I guess maybe in that case it does make sense to cast it to i128 here, as there is only one target using it, and it's a special non-power-of-2 case..? So it still looks to me that there are these two changes that would first need handling, and then this could probably be improved further. https://github.com/llvm/llvm-project/pull/86691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang][UBSan] Add implicit conversion check for bitfields (PR #75481)
@@ -147,6 +147,7 @@ struct ImplicitConversionData { const TypeDescriptor &FromType; const TypeDescriptor &ToType; /* ImplicitConversionCheckKind */ unsigned char Kind; + unsigned int BitfieldBits; AaronBallman wrote: Ping @zygoloid -- I'm not certain if we have an ABI stability policy for sanitizers or not. https://github.com/llvm/llvm-project/pull/75481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang][UBSan] Add implicit conversion check for bitfields (PR #75481)
AaronBallman wrote: > > Also, I don't have commit access (first PR here), someone has to commit on > > my behalf! > > @AaronBallman Thank you for mentioning this! I'll commit on your behalf, but I'd like to hear from @zygoloid on the ABI question. It would also be nice if @efriedma-quic signed off on the testing changes (I think they're sufficient, but he may have other cases in mind). https://github.com/llvm/llvm-project/pull/75481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (PR #84050)
sdkrystian wrote: Ping @erichkeane https://github.com/llvm/llvm-project/pull/84050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [CodeGen][LLVM] Make the `va_list` related intrinsics generic. (PR #85460)
https://github.com/AlexVlx closed https://github.com/llvm/llvm-project/pull/85460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [CodeGen][LLVM] Make the `va_list` related intrinsics generic. (PR #85460)
AlexVlx wrote: Thank you everyone for the reviews. https://github.com/llvm/llvm-project/pull/85460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/NagyDonat approved this pull request. Overall LGTM, I have minor inline suggestions that clarify the description of bitpacking (but the current state is also acceptable). https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,88 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted +memory thus decreased performance by reducing the effectiveness of the +processor cache. Padding bytes are added by compilers to align data accesses +as some processors require data to be aligned to certain boundaries. On others, +unaligned data access are possible, but impose significantly larger latencies. + +To avoid padding bytes, the fields of a struct should be ordered by decreasing +by alignment. Usually, its easier to think of the ``sizeof`` of the fields, and +ordering the fields by ``sizeof`` would usually also lead to the same optimal +layout. + +In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed +layout too, but it is discouraged and the reordering of fields should be +preferred. NagyDonat wrote: ```suggestion In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed layout too, but it can significantly increase the access times, so reordering the fields is usually a better solution. ``` https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
@@ -804,10 +804,88 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted +memory thus decreased performance by reducing the effectiveness of the +processor cache. Padding bytes are added by compilers to align data accesses +as some processors require data to be aligned to certain boundaries. On others, +unaligned data access are possible, but impose significantly larger latencies. + +To avoid padding bytes, the fields of a struct should be ordered by decreasing +by alignment. Usually, its easier to think of the ``sizeof`` of the fields, and +ordering the fields by ``sizeof`` would usually also lead to the same optimal +layout. + +In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed +layout too, but it is discouraged and the reordering of fields should be +preferred. + +.. code-block:: cpp + + // warn: Excessive padding in 'struct NonOptimal' (35 padding bytes, where 3 is optimal) + struct NonOptimal { + char c1; + // 7 bytes of padding + std::int64_t big1; // 8 bytes + char c2; + // 7 bytes of padding + std::int64_t big2; // 8 bytes + char c3; + // 7 bytes of padding + std::int64_t big3; // 8 bytes + char c4; + // 7 bytes of padding + std::int64_t big4; // 8 bytes + char c5; + // 7 bytes of padding + }; + static_assert(sizeof(NonOptimal) == 4*8+5+5*7); + + // no-warning: The fields are nicely aligned to have the minimal amount of padding bytes. + struct Optimal { + std::int64_t big1; // 8 bytes + std::int64_t big2; // 8 bytes + std::int64_t big3; // 8 bytes + std::int64_t big4; // 8 bytes + char c1; + char c2; + char c3; + char c4; + char c5; + // 3 bytes of padding + }; + static_assert(sizeof(Optimal) == 4*8+5+3); + + // no-warning: Enforcing bitpacked representation. + // Access times will have signifficant overhead. Prefer reordering the fields instead. NagyDonat wrote: ```suggestion // no-warning: Bit packing representation is also accepted by this checker, but // it can significantly increase access times, so prefer reordering the fields. ``` https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
@@ -124,34 +124,45 @@ class CStringChecker : public Checker< eval::Call, const CallEvent &)>; CallDescriptionMap Callbacks = { - {{CDM::CLibrary, {"memcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"memcpy"}, 3}, std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Regular)}, - {{CDM::CLibrary, {"wmemcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmemcpy"}, 3}, std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"mempcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"mempcpy"}, 3}, std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Regular)}, - {{CDM::Unspecified, {"wmempcpy"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmempcpy"}, 3}, std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Wide)}, {{CDM::CLibrary, {"memcmp"}, 3}, std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)}, {{CDM::CLibrary, {"wmemcmp"}, 3}, std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"memmove"}, 3}, + {{CDM::CLibraryMaybeHardened, {"memmove"}, 3}, std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Regular)}, - {{CDM::CLibrary, {"wmemmove"}, 3}, + {{CDM::CLibraryMaybeHardened, {"wmemmove"}, 3}, std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Wide)}, - {{CDM::CLibrary, {"memset"}, 3}, &CStringChecker::evalMemset}, + {{CDM::CLibraryMaybeHardened, {"memset"}, 3}, + &CStringChecker::evalMemset}, {{CDM::CLibrary, {"explicit_memset"}, 3}, &CStringChecker::evalMemset}, - {{CDM::CLibrary, {"strcpy"}, 2}, &CStringChecker::evalStrcpy}, - {{CDM::CLibrary, {"strncpy"}, 3}, &CStringChecker::evalStrncpy}, - {{CDM::CLibrary, {"stpcpy"}, 2}, &CStringChecker::evalStpcpy}, - {{CDM::CLibrary, {"strlcpy"}, 3}, &CStringChecker::evalStrlcpy}, - {{CDM::CLibrary, {"strcat"}, 2}, &CStringChecker::evalStrcat}, - {{CDM::CLibrary, {"strncat"}, 3}, &CStringChecker::evalStrncat}, - {{CDM::CLibrary, {"strlcat"}, 3}, &CStringChecker::evalStrlcat}, - {{CDM::CLibrary, {"strlen"}, 1}, &CStringChecker::evalstrLength}, + /* FIXME: C23 introduces 'memset_explicit', maybe also model that */ + {{CDM::CLibraryMaybeHardened, {"strcpy"}, 2}, + &CStringChecker::evalStrcpy}, + {{CDM::CLibraryMaybeHardened, {"strncpy"}, 3}, + &CStringChecker::evalStrncpy}, + {{CDM::CLibraryMaybeHardened, {"stpcpy"}, 2}, + &CStringChecker::evalStpcpy}, NagyDonat wrote: The mode `CDM::CLibraryMaybeHardened` understands that the hardened variants of the functions may have more arguments/parameters than the value that's encoded in the `CallDescription` (and it can also match the "usual" variants and for them it checks that the arg/param count exactly matches). For example ``` {CDM::CLibraryMaybeHardened, {"strcpy"}, 2} ``` can match `strcpy(x, y)` (a normal strcpy call, two arguments), `__builtin_strcpy(x, y)` (a non-hardened builtin variant, two arguments), `__strcpy_chk(x, y, z)` (a hardened strcpy call, three arguments); but it doesn't match `strcpy(x, y, z)` (a non-hardened call with more than two arguments) or anything that has less than two arguments. Note that the hardened variants usually have one additional argument (compared to the "normal" variant), but there are some (IIRC "sprintf" and "snprintf") that get two additional arguments, so I don't check their exact arg/param count and accept any call where the number of args/params is not less than the number of args/params for the original function. (Additional arguments don't disturb the checker, but an unexpectedly missing argument could lead to a checker crash.) This means that the current code would accept `__strcpy_chk(x, y)` and `__strcpy_chk(x, y, z, w)`, which are invalid calls (but I don't think that they would ever appear in real code and they don't cause checker crashes). https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/86536 >From fdde1056e8a34ad642f50eef120dbc8ee08f8825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Don=C3=A1t=20Nagy?= Date: Mon, 18 Mar 2024 14:47:57 +0100 Subject: [PATCH 1/2] [analyzer] Make recognition of hardened __FOO_chk functions explicit In builds that use source hardening (-D_FORTIFY_SOURCE), many standard functions are implemented as macros that expand to calls of hardened functions that take one additional argument compared to the "usual" variant and perform additional input validation. For example, a `memcpy` call may expand to `__memcpy_chk()` or `__builtin___memcpy_chk()`. Before this commit, `CallDescription`s created with the matching mode `CDM::CLibrary` automatically matched these hardened variants (in a addition to the "usual" function) with a fairly lenient heuristic. Unfortunately this heuristic meant that the `CLibrary` matching mode was only usable by checkers that were prepared to handle matches with an unusual number of arguments. This commit limits the recognition of the hardened functions to a separate matching mode `CDM::CLibraryMaybeHardened` and applies this mode for functions that have hardened variants and were previously recognized with `CDM::CLibrary`. This way checkers that are prepared to handle the hardened variants will be able to detect them easily; while other checkers can simply use `CDM::CLibrary` for matching C library functions (and they won't encounter surprising argument counts). The initial motivation for refactoring this area was that previously `CDM::CLibrary` accepted calls with more arguments/parameters than the expected number, so I wasn't able to use it for `malloc` without accidentally matching calls to the 3-argument BSD kernel malloc. After this commit this "may have more args/params" logic will only activate when we're actually matching a hardened variant function (in `CDM::CLibraryMaybeHardened` mode). The recognition of "sprintf()" and "snprintf()" in CStringChecker was refactored, because previously it was abusing the behavior that extra arguments are accepted even if the matched function is not a hardened variant. This commit also fixes the oversight that the old code would've recognized e.g. `__wmemcpy_chk` as a hardened variant of `memcpy`. After this commit I'm planning to create several follow-up commits that ensure that checkers looking for C library functions use `CDM::CLibrary` as a "sane default" matching mode. This commit is not truly NFC (it eliminates some buggy corner cases), but it does not intentionally modify the behavior of CSA on real-world non-crazy code. As a minor unrelated change I'm eliminating the argument/variable "IsBuiltin" from the evalSprintf function family in CStringChecker, because it was completely unused. --- .../Core/PathSensitive/CallDescription.h | 26 ++-- .../Core/PathSensitive/CheckerContext.h | 24 +++- .../Checkers/CStringChecker.cpp | 76 +++ .../Checkers/GenericTaintChecker.cpp | 27 ++-- .../StaticAnalyzer/Core/CallDescription.cpp | 122 +- .../StaticAnalyzer/Core/CheckerContext.cpp| 19 ++- .../StaticAnalyzer/CallDescriptionTest.cpp| 54 ++-- 7 files changed, 217 insertions(+), 131 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h index b4e1636130ca7c..ccfe8d47c290bc 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h @@ -32,19 +32,22 @@ namespace ento { class CallDescription { public: enum class Mode { -/// Match calls to functions from the C standard library. On some platforms -/// some functions may be implemented as macros that expand to calls to -/// built-in variants of the given functions, so in this mode we use some -/// heuristics to recognize these implementation-defined variants: -/// - We also accept calls where the name is derived from the specified -///name by adding "__builtin" or similar prefixes/suffixes. -/// - We also accept calls where the number of arguments or parameters is -///greater than the specified value. +/// Match calls to functions from the C standard library. This also +/// recognizes builtin variants whose name is derived by adding +/// "__builtin", "__inline" or similar prefixes or suffixes; but only +/// matches functions than are externally visible and are declared either +/// directly within a TU or in the namespace 'std'. /// For the exact heuristics, see CheckerContext::isCLibraryFunction(). -/// (This mode only matches functions that are declared either directly -/// within a TU or in the namespace `std`.) CLibrary, +/// An extended version of the `CLibrary` mode that also ma
[clang] [Clang][Sema] Allow flexible arrays in unions and alone in structs (PR #84428)
AaronBallman wrote: > LGTM > > The one thing I'm worried about here is the possibility that a future C > standard version standardizes a similar extension, but with different > semantics (since we're not using any reserved keywords here). Like I > mentioned before, that's very unlikely to happen for non-zero-size unions, > but zero-size types are a completely unexplored space in the standard. > > But I guess if that happens, we can come with some sort of transition plan > using attributes. And I don't have any specific idea of what the standards > committee might choose to do differently, so maybe it's not that likely. WG14 tries pretty hard to standardize existing practice when possible (we don't like breaking existing user code), so if we find ourselves in this position. I think the committee would be receptive to feedback that helps obviate or ease such a transition. https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][CLANG] Fix null pointer dereferences (PR #86760)
HoBoIs wrote: Looks good to me. https://github.com/llvm/llvm-project/pull/86760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
NagyDonat wrote: [Replying to the following inline comment of @steakhal :] > You had a comment about __builtin___vsprintf_chk. ``` // This is using `equals()` instead of more lenient prefix/suffix/substring // checks because we don't want to say that e.g. `__builtin___vsprintf_chk()` // is a hardened variant of `sprintf()`. ``` > I wonder if we could have a test demonstrating that. Instead of demonstrating this with `sprintf` / `vsprintf`, I ended up creating testcases that use `memcpy` / `wmemcpy`, because in the non-hardened case the analogous problem was tested with `memcpy` / `wmemcpy`. I wrote this comment before creating those testcases; now that the testcases are there, I removed the comment (by merging your suggestion that tweaks the adjacent code). https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
komalverma04 wrote: Okay, i completely agree with you. Thank you so much for helping. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix an out-of-bound crash when checking template partial specializations. (PR #86794)
https://github.com/hokein created https://github.com/llvm/llvm-project/pull/86794 I found this issue (a separate one) during the investigation of #86757, the crash is similar in substituteParameterMappings, but at different inner places. This was an out-of-bound issue where we access front element in an empty written template argument list to get the instantiation source range. This patch fixes it by adding a proper guard. >From 9d3eaab5efce82024dfcd4338b4cf03feb3a5966 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Wed, 27 Mar 2024 13:04:13 +0100 Subject: [PATCH] [clang] Fix an out-of-bound crash when checking class template partial specialization. I found this issue (a separate one) during the investigation of #86757, the crash is similar in substituteParameterMappings, but at different inner places. This was an out-of-bound issue where we access front element in an empty written template argument list to get the instantiation source range. This patch fix it by adding a proper guard. --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/SemaConcept.cpp | 12 ++-- clang/test/SemaTemplate/concepts.cpp | 10 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0dd026a5de5c6f..0fdd9e3fb3eee2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -454,6 +454,7 @@ Bug Fixes to C++ Support - Fix a crash when instantiating a lambda that captures ``this`` outside of its context. Fixes (#GH85343). - Fix an issue where a namespace alias could be defined using a qualified name (all name components following the first `::` were ignored). +- Fix an out-of-bounds crash when checking the validity of template partial specializations. (part of #GH86757). Bug Fixes to AST Handling ^ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 1c546e9f5894f0..b6c4d3d540ef50 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -1269,10 +1269,18 @@ substituteParameterMappings(Sema &S, NormalizedConstraint &N, : SourceLocation())); Atomic.ParameterMapping.emplace(TempArgs, OccurringIndices.count()); } + SourceLocation InstLocBegin = + ArgsAsWritten->arguments().empty() + ? ArgsAsWritten->getLAngleLoc() + : ArgsAsWritten->arguments().front().getSourceRange().getBegin(); + SourceLocation InstLocEnd = + ArgsAsWritten->arguments().empty() + ? ArgsAsWritten->getRAngleLoc() + : ArgsAsWritten->arguments().front().getSourceRange().getEnd(); Sema::InstantiatingTemplate Inst( - S, ArgsAsWritten->arguments().front().getSourceRange().getBegin(), + S, InstLocBegin, Sema::InstantiatingTemplate::ParameterMappingSubstitution{}, Concept, - ArgsAsWritten->arguments().front().getSourceRange()); + {InstLocBegin, InstLocEnd}); if (S.SubstTemplateArguments(*Atomic.ParameterMapping, MLTAL, SubstArgs)) return true; diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index b7ea0d003a52d7..787cc809e25353 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++20 -verify %s +// RUN: %clang_cc1 -std=c++20 -ferror-limit 0 -verify %s namespace PR47043 { template concept True = true; @@ -1114,3 +1114,11 @@ void foo() { } } // namespace GH64808 + +namespace GH86757_1 { +template concept b = false; +template concept c = b<>; +template concept f = c< d >; +template struct e; // expected-note {{}} +template struct e; // expected-error {{class template partial specialization is not more specialized than the primary template}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix an out-of-bound crash when checking template partial specializations. (PR #86794)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Haojian Wu (hokein) Changes I found this issue (a separate one) during the investigation of #86757, the crash is similar in substituteParameterMappings, but at different inner places. This was an out-of-bound issue where we access front element in an empty written template argument list to get the instantiation source range. This patch fixes it by adding a proper guard. --- Full diff: https://github.com/llvm/llvm-project/pull/86794.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/lib/Sema/SemaConcept.cpp (+10-2) - (modified) clang/test/SemaTemplate/concepts.cpp (+9-1) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0dd026a5de5c6f..0fdd9e3fb3eee2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -454,6 +454,7 @@ Bug Fixes to C++ Support - Fix a crash when instantiating a lambda that captures ``this`` outside of its context. Fixes (#GH85343). - Fix an issue where a namespace alias could be defined using a qualified name (all name components following the first `::` were ignored). +- Fix an out-of-bounds crash when checking the validity of template partial specializations. (part of #GH86757). Bug Fixes to AST Handling ^ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 1c546e9f5894f0..b6c4d3d540ef50 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -1269,10 +1269,18 @@ substituteParameterMappings(Sema &S, NormalizedConstraint &N, : SourceLocation())); Atomic.ParameterMapping.emplace(TempArgs, OccurringIndices.count()); } + SourceLocation InstLocBegin = + ArgsAsWritten->arguments().empty() + ? ArgsAsWritten->getLAngleLoc() + : ArgsAsWritten->arguments().front().getSourceRange().getBegin(); + SourceLocation InstLocEnd = + ArgsAsWritten->arguments().empty() + ? ArgsAsWritten->getRAngleLoc() + : ArgsAsWritten->arguments().front().getSourceRange().getEnd(); Sema::InstantiatingTemplate Inst( - S, ArgsAsWritten->arguments().front().getSourceRange().getBegin(), + S, InstLocBegin, Sema::InstantiatingTemplate::ParameterMappingSubstitution{}, Concept, - ArgsAsWritten->arguments().front().getSourceRange()); + {InstLocBegin, InstLocEnd}); if (S.SubstTemplateArguments(*Atomic.ParameterMapping, MLTAL, SubstArgs)) return true; diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index b7ea0d003a52d7..787cc809e25353 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++20 -verify %s +// RUN: %clang_cc1 -std=c++20 -ferror-limit 0 -verify %s namespace PR47043 { template concept True = true; @@ -1114,3 +1114,11 @@ void foo() { } } // namespace GH64808 + +namespace GH86757_1 { +template concept b = false; +template concept c = b<>; +template concept f = c< d >; +template struct e; // expected-note {{}} +template struct e; // expected-error {{class template partial specialization is not more specialized than the primary template}} +} `` https://github.com/llvm/llvm-project/pull/86794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] add some missing Kinds to libclang python bindings (PR #85571)
schenker wrote: Thanks for the fix. Will these changes find their way into an 18.x release? https://github.com/llvm/llvm-project/pull/85571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Allow flexible arrays in unions and alone in structs (PR #84428)
https://github.com/AaronBallman approved this pull request. I think the changes LGTM, but I do think we've got some more work to do to make this a first-class extension. For example, we lack constant expression support: ``` struct S { int y[]; }; constexpr struct S a1 = { 8, 12 }; ``` gives: ``` C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:20: error: constexpr variable 'a1' must be initialized by a constant expression 2 | constexpr struct S a1 = { 8, 12 }; |^~ C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:27: note: flexible array initialization is not yet supported 2 | constexpr struct S a1 = { 8, 12 }; | ^ 1 error generated. ``` That said, I think this PR is incremental progress and I don't want to hold it up by expanding the scope. https://github.com/llvm/llvm-project/pull/84428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Make recognition of hardened __FOO_chk functions explicit (PR #86536)
https://github.com/steakhal approved this pull request. LGTM, but I'd recommed @Xazax-hun to also have a look. If nothing happens, merge this after one week. https://github.com/llvm/llvm-project/pull/86536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/8] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/8] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/8] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor b
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/86411 >From b6ca6f0ef83d03e299d6ee9a8ed9b8044477914e Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:14:44 -0700 Subject: [PATCH 1/9] Update checkers.rst Modification of documentation to demonstrate utilization of AllowedPad within PaddingChecker, along with its use and effects on code analysis. --- clang/docs/analyzer/checkers.rst | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index fe211514914272..64b09bc6ecd1d8 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,10 +804,28 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (PaddingChecker) +""" Check for excessively padded structs. +.. code-block:: objc + + struct TestStruct { + int data1; // 4 bytes + char data2; // 1 byte + char padding[27]; // 27 bytes of padding +}; // Total size: 32 bytes + + void TestFunction() { + struct TestStruct struct1; // Warning is generated due to excessive padding. +} + + // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. + + // AllowedPad: Default Value: 24 bytes + + Usage: `AllowedPad=` + .. _optin-portability-UnixAPI: optin.portability.UnixAPI >From 403115cd960653a3afe0491d2855d35d377d9312 Mon Sep 17 00:00:00 2001 From: komalverma04 <114138604+komalverm...@users.noreply.github.com> Date: Sat, 23 Mar 2024 11:20:46 -0700 Subject: [PATCH 2/9] Update Checkers.td Changed NotDocumented to HasDocumentation for Padding Checker under performance checker. --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 686e5e99f4a62c..c0e4e9a70c2ef3 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" >From cdef12fa6a6b6c4833bc2017dae9557ee7115c92 Mon Sep 17 00:00:00 2001 From: komalverma04 Date: Sun, 24 Mar 2024 20:39:56 +0530 Subject: [PATCH 3/9] Update checkers.rst Made Indentation consistent and added description on limitation of checker. --- clang/docs/analyzer/checkers.rst | 44 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 64b09bc6ecd1d8..85f3f33d68bc5d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -804,27 +804,37 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding (PaddingChecker) -""" +optin.performance.Padding (C, C++, objC) + Check for excessively padded structs. -.. code-block:: objc - - struct TestStruct { - int data1; // 4 bytes - char data2; // 1 byte - char padding[27]; // 27 bytes of padding -}; // Total size: 32 bytes - - void TestFunction() { - struct TestStruct struct1; // Warning is generated due to excessive padding. -} - - // Reports are only generated if the excessive padding exceeds 'AllowedPad' in bytes. +This checker detects structs with excessive padding, which can lead to wasted memory and decreased performance. Padding bytes are added by compilers to align data within the struct for performance optimization or memory alignment purposes. However, excessive padding can significantly increase the size of the struct without adding useful data, leading to inefficient memory usage, cache misses, and decreased performance. + +.. code-block:: C + + #include + // #pragma pack(1) // Uncomment it to disable structure padding + struct TestStruct { + char data1; // 1 byte + char data2; // 1 byte + int data3; // 4 bytes + }; // Total size: 6 bytes + + int main () { + struct TestStruct struct1; + print("Structure size: %d",sizeof(struct1)); // Structure size: 8 + return 0; + } + +Total memory used is 8 bytes due to structure padding. In this case, padding is of 2 bytes. Padding is done to decrease the number of CPU cycles needed to access data members of the structure; it increases the performance of the processor b
[clang] [Clang][analyzer] add documentation for optin performance padding (padding checker) #73675 (PR #86411)
https://github.com/steakhal approved this pull request. https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ExprConst] Fix second arg of __builtin_{clzg,ctzg} not always being evaluated (PR #86742)
https://github.com/overmighty updated https://github.com/llvm/llvm-project/pull/86742 >From c615f656153fcb1d4158548660f87d1d69960864 Mon Sep 17 00:00:00 2001 From: OverMighty Date: Tue, 26 Mar 2024 21:25:59 + Subject: [PATCH 1/3] [clang][ExprConst] Fix second arg of __builtin_{clzg,ctzg} not always being evaluated --- clang/lib/AST/ExprConstant.cpp| 30 +- .../constant-builtins-all-args-evaluated.cpp | 39 +++ 2 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 clang/test/Sema/constant-builtins-all-args-evaluated.cpp diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 5a36621dc5cce2..dae8f32fc02951 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -12361,12 +12361,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, if (!EvaluateInteger(E->getArg(0), Val, Info)) return false; +std::optional Fallback; +if (BuiltinOp == Builtin::BI__builtin_clzg && E->getNumArgs() > 1) { + APSInt FallbackTemp; + if (!EvaluateInteger(E->getArg(1), FallbackTemp, Info)) +return false; + Fallback = FallbackTemp; +} + if (!Val) { - if (BuiltinOp == Builtin::BI__builtin_clzg && E->getNumArgs() > 1) { -if (!EvaluateInteger(E->getArg(1), Val, Info)) - return false; -return Success(Val, E); - } + if (Fallback) +return Success(*Fallback, E); // When the argument is 0, the result of GCC builtins is undefined, // whereas for Microsoft intrinsics, the result is the bit-width of the @@ -12425,12 +12430,17 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, if (!EvaluateInteger(E->getArg(0), Val, Info)) return false; +std::optional Fallback; +if (BuiltinOp == Builtin::BI__builtin_ctzg && E->getNumArgs() > 1) { + APSInt FallbackTemp; + if (!EvaluateInteger(E->getArg(1), FallbackTemp, Info)) +return false; + Fallback = FallbackTemp; +} + if (!Val) { - if (BuiltinOp == Builtin::BI__builtin_ctzg && E->getNumArgs() > 1) { -if (!EvaluateInteger(E->getArg(1), Val, Info)) - return false; -return Success(Val, E); - } + if (Fallback) +return Success(*Fallback, E); return Error(E); } diff --git a/clang/test/Sema/constant-builtins-all-args-evaluated.cpp b/clang/test/Sema/constant-builtins-all-args-evaluated.cpp new file mode 100644 index 00..0dee74ddf690cf --- /dev/null +++ b/clang/test/Sema/constant-builtins-all-args-evaluated.cpp @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics + +constexpr int increment(int& x) { + x++; + return x; +} + +constexpr int test_clzg_0() { + int x = 0; + [[maybe_unused]] int unused = __builtin_clzg(0U, increment(x)); + return x; +} + +static_assert(test_clzg_0() == 1); + +constexpr int test_clzg_1() { + int x = 0; + [[maybe_unused]] int unused = __builtin_clzg(1U, increment(x)); + return x; +} + +static_assert(test_clzg_1() == 1); + +constexpr int test_ctzg_0() { + int x = 0; + [[maybe_unused]] int unused = __builtin_ctzg(0U, increment(x)); + return x; +} + +static_assert(test_ctzg_0() == 1); + +constexpr int test_ctzg_1() { + int x = 0; + [[maybe_unused]] int unused = __builtin_ctzg(1U, increment(x)); + return x; +} + +static_assert(test_ctzg_1() == 1); >From a0f90042bea48050e1e23aa95488e315fa3b40a2 Mon Sep 17 00:00:00 2001 From: OverMighty Date: Tue, 26 Mar 2024 23:09:56 + Subject: [PATCH 2/3] fixup! [clang][ExprConst] Fix second arg of __builtin_{clzg,ctzg} not always being evaluated Co-authored-by: Richard Smith --- clang/test/Sema/constant-builtins-all-args-evaluated.cpp | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/test/Sema/constant-builtins-all-args-evaluated.cpp b/clang/test/Sema/constant-builtins-all-args-evaluated.cpp index 0dee74ddf690cf..f4452b1478e3fd 100644 --- a/clang/test/Sema/constant-builtins-all-args-evaluated.cpp +++ b/clang/test/Sema/constant-builtins-all-args-evaluated.cpp @@ -8,7 +8,7 @@ constexpr int increment(int& x) { constexpr int test_clzg_0() { int x = 0; - [[maybe_unused]] int unused = __builtin_clzg(0U, increment(x)); + (void)__builtin_clzg(0U, increment(x)); return x; } @@ -16,7 +16,7 @@ static_assert(test_clzg_0() == 1); constexpr int test_clzg_1() { int x = 0; - [[maybe_unused]] int unused = __builtin_clzg(1U, increment(x)); + (void)__builtin_clzg(1U, increment(x)); return x; } @@ -24,7 +24,7 @@ static_assert(test_clzg_1() == 1); constexpr int test_ctzg_0() { int x = 0; - [[maybe_unused]] int unused = __builtin_ctzg(0U, increment(x)); + (void)__builtin_ctzg(0U, increment(x)); return x; } @@ -32,7 +32,7 @@ static_assert(test_ctzg_0() == 1); constexpr int test_ctzg_1() { int x = 0; - [[maybe_unused]] int unused = __builtin_ctzg(
[clang] [RFC][Clang] Enable custom type checking for printf (PR #86801)
https://github.com/vikramRH created https://github.com/llvm/llvm-project/pull/86801 The motivation for this change comes from an ongoing PR (#72556 ) , which enables hostcall based printf lowering for AMDGPU target and OpenCL inputs. The OpenCL printf has a different signature than the C printf. the difference being the explicit address space specifier for format string arg as follows int printf(__constant const char* st, ...) __attribute__((format(printf, 1, 2))); This is not considered a builtin because of the type mismatch. The patch #72556 tried to address this scenario by declaring OCL printf essentially as a target specific printf overload. However, the discussions there resulted in the decision that this should not be target-specific. The idea in this patch is that the changes are NFC for current framework (i.e the semantic checks for printf are preserved) however the printf declarations are considered builtins now regardless of LangOpt. This would allow me to hanlde the printf CodeGen without any target specific hacks. PS: feel free to add additional reviewers, I'm not aware of others who could comment here. >From a2731d056cee9c4e75d49f8d4fa3325dc532b207 Mon Sep 17 00:00:00 2001 From: Vikram Date: Wed, 27 Mar 2024 04:24:10 -0400 Subject: [PATCH] [Clang] Implement custom type checking for printf --- clang/include/clang/Basic/Builtins.td | 4 +-- clang/include/clang/Sema/Sema.h | 1 + clang/lib/AST/Decl.cpp| 6 +++- clang/lib/Basic/Builtins.cpp | 3 +- clang/lib/CodeGen/CGBuiltin.cpp | 2 +- clang/lib/Sema/SemaChecking.cpp | 51 +++ 6 files changed, 62 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index 52c0dd52c28b11..f795c7c42c7b25 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/clang/include/clang/Basic/Builtins.td @@ -2816,14 +2816,14 @@ def StrLen : LibBuiltin<"string.h"> { // FIXME: This list is incomplete. def Printf : LibBuiltin<"stdio.h"> { let Spellings = ["printf"]; - let Attributes = [PrintfFormat<0>]; + let Attributes = [PrintfFormat<0>, CustomTypeChecking]; let Prototype = "int(char const*, ...)"; } // FIXME: The builtin and library function should have the same signature. def BuiltinPrintf : Builtin { let Spellings = ["__builtin_printf"]; - let Attributes = [NoThrow, PrintfFormat<0>, FunctionWithBuiltinPrefix]; + let Attributes = [NoThrow, PrintfFormat<0>, FunctionWithBuiltinPrefix, CustomTypeChecking]; let Prototype = "int(char const* restrict, ...)"; } diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 5ecd2f9eb2881f..b18b208a75bdf4 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2245,6 +2245,7 @@ class Sema final { bool SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall); bool SemaBuiltinVAStartARMMicrosoft(CallExpr *Call); + bool SemaBuiltinPrintf(FunctionDecl *FDecl, CallExpr *TheCall); bool SemaBuiltinUnorderedCompare(CallExpr *TheCall, unsigned BuiltinID); bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs, unsigned BuiltinID); diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 131f82985e903b..298223f874cda3 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3629,8 +3629,12 @@ unsigned FunctionDecl::getBuiltinID(bool ConsiderWrapperFunctions) const { // OpenCL v1.2 s6.9.f - The library functions defined in // the C99 standard headers are not available. if (Context.getLangOpts().OpenCL && - Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) + Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) { +if (Context.getLangOpts().getOpenCLCompatibleVersion() >= 120 && +(BuiltinID == Builtin::BIprintf)) + return BuiltinID; return 0; + } // CUDA does not have device-side standard library. printf and malloc are the // only special cases that are supported by device-side runtime. diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp index 3467847ac1672e..25590ed9299e8b 100644 --- a/clang/lib/Basic/Builtins.cpp +++ b/clang/lib/Basic/Builtins.cpp @@ -235,7 +235,8 @@ bool Builtin::Context::performsCallback(unsigned ID, bool Builtin::Context::canBeRedeclared(unsigned ID) const { return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start || - ID == Builtin::BI__builtin_assume_aligned || + ID == Builtin::BI__builtin_assume_aligned || ID == Builtin::BIprintf || + ID == Builtin::BI__builtin_printf || (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) || isInStdNamespace(ID); } diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 3cfdb261a0eac0..baed36acb12437 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp
[clang] [analyzer] Document the `optin.performance.Padding` checker (PR #86411)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AMDGPU] Treat printf as builtin for OpenCL (PR #72554)
https://github.com/vikramRH closed https://github.com/llvm/llvm-project/pull/72554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AMDGPU] Treat printf as builtin for OpenCL (PR #72554)
vikramRH wrote: closing this in favour of https://github.com/llvm/llvm-project/pull/86801 https://github.com/llvm/llvm-project/pull/72554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b8cc838 - [analyzer][docs] Document the `optin.performance.Padding` checker (#86411)
Author: komalverma04 Date: 2024-03-27T13:51:27+01:00 New Revision: b8cc838427efa80eb5ca4ec7c8adb53e4adfc4c7 URL: https://github.com/llvm/llvm-project/commit/b8cc838427efa80eb5ca4ec7c8adb53e4adfc4c7 DIFF: https://github.com/llvm/llvm-project/commit/b8cc838427efa80eb5ca4ec7c8adb53e4adfc4c7.diff LOG: [analyzer][docs] Document the `optin.performance.Padding` checker (#86411) Closes #73675 Co-authored-by: Balazs Benics Co-authored-by: NagyDonat Added: Modified: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Checkers.td Removed: diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 66da1c7b35f28b..8af99a021ebdfd 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -849,10 +849,89 @@ Check for performance anti-patterns when using Grand Central Dispatch. .. _optin-performance-Padding: -optin.performance.Padding -" +optin.performance.Padding (C, C++, ObjC) + Check for excessively padded structs. +This checker detects structs with excessive padding, which can lead to wasted +memory thus decreased performance by reducing the effectiveness of the +processor cache. Padding bytes are added by compilers to align data accesses +as some processors require data to be aligned to certain boundaries. On others, +unaligned data access are possible, but impose significantly larger latencies. + +To avoid padding bytes, the fields of a struct should be ordered by decreasing +by alignment. Usually, its easier to think of the ``sizeof`` of the fields, and +ordering the fields by ``sizeof`` would usually also lead to the same optimal +layout. + +In rare cases, one can use the ``#pragma pack(1)`` directive to enforce a packed +layout too, but it can significantly increase the access times, so reordering the +fields is usually a better solution. + + +.. code-block:: cpp + + // warn: Excessive padding in 'struct NonOptimal' (35 padding bytes, where 3 is optimal) + struct NonOptimal { + char c1; + // 7 bytes of padding + std::int64_t big1; // 8 bytes + char c2; + // 7 bytes of padding + std::int64_t big2; // 8 bytes + char c3; + // 7 bytes of padding + std::int64_t big3; // 8 bytes + char c4; + // 7 bytes of padding + std::int64_t big4; // 8 bytes + char c5; + // 7 bytes of padding + }; + static_assert(sizeof(NonOptimal) == 4*8+5+5*7); + + // no-warning: The fields are nicely aligned to have the minimal amount of padding bytes. + struct Optimal { + std::int64_t big1; // 8 bytes + std::int64_t big2; // 8 bytes + std::int64_t big3; // 8 bytes + std::int64_t big4; // 8 bytes + char c1; + char c2; + char c3; + char c4; + char c5; + // 3 bytes of padding + }; + static_assert(sizeof(Optimal) == 4*8+5+3); + + // no-warning: Bit packing representation is also accepted by this checker, but + // it can significantly increase access times, so prefer reordering the fields. + #pragma pack(1) + struct BitPacked { + char c1; + std::int64_t big1; // 8 bytes + char c2; + std::int64_t big2; // 8 bytes + char c3; + std::int64_t big3; // 8 bytes + char c4; + std::int64_t big4; // 8 bytes + char c5; + }; + static_assert(sizeof(BitPacked) == 4*8+5); + +The ``AllowedPad`` option can be used to specify a threshold for the number +padding bytes raising the warning. If the number of padding bytes of the struct +and the optimal number of padding bytes diff er by more than the threshold value, +a warning will be raised. + +By default, the ``AllowedPad`` threshold is 24 bytes. + +To override this threshold to e.g. 4 bytes, use the +``-analyzer-config optin.performance.Padding:AllowedPad=4`` option. + + .. _optin-portability-UnixAPI: optin.portability.UnixAPI diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index bf46766d44b391..5fe5c9286dabb7 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -908,7 +908,7 @@ def PaddingChecker : Checker<"Padding">, "24", Released> ]>, - Documentation; + Documentation; } // end: "padding" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Document the `optin.performance.Padding` checker (PR #86411)
https://github.com/steakhal closed https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][docs] Document the `optin.performance.Padding` checker (PR #86411)
https://github.com/steakhal edited https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][docs] Document the `optin.performance.Padding` checker (PR #86411)
github-actions[bot] wrote: @komalverma04 Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may recieve a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/86411 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix an out-of-bound crash when checking template partial specializations. (PR #86794)
https://github.com/Sirraide approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/86794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
arsenm wrote: > I think this case isn't that simple as it is an 80 bit value. Currently that > is loaded atomically first with i128, then stored as a temporary and then > loaded as an fp80. If I remove that casting, the verifier complains "atomic > memory access' operand must have a power-of-two size". I think this should be more targeted based on the bitesize. PPCf128 should be simple https://github.com/llvm/llvm-project/pull/86691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [APINotes] Upstream the remaining API Notes fixes and tests (PR #84773)
egorzhdan wrote: I'm going to land this patch now to unblock other work on APINotes, but I'm happy to address more review feedback post-merge. https://github.com/llvm/llvm-project/pull/84773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [APINotes] Upstream the remaining API Notes fixes and tests (PR #84773)
https://github.com/egorzhdan closed https://github.com/llvm/llvm-project/pull/84773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Fix anonymous reference parameter with default value (PR #86254)
https://github.com/rayroudc updated https://github.com/llvm/llvm-project/pull/86254 >From bda7c429a4072e54b99d4fbd561fbd5d406692fd Mon Sep 17 00:00:00 2001 From: "C. Rayroud" Date: Mon, 18 Mar 2024 06:39:26 + Subject: [PATCH] [clang-format] Fix anonymous reference parameter with default value --- clang/lib/Format/WhitespaceManager.cpp | 5 +++-- clang/unittests/Format/FormatTest.cpp | 20 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 710bf8d8a8ec70..d06c42d5f4c5c5 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -464,10 +464,11 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End, if (i + 1 != Changes.size()) Changes[i + 1].PreviousEndOfTokenColumn += Shift; -// If PointerAlignment is PAS_Right, keep *s or &s next to the token +// If PointerAlignment is PAS_Right, keep *s or &s next to the token, +// except if the token is equal, then a space is needed. if ((Style.PointerAlignment == FormatStyle::PAS_Right || Style.ReferenceAlignment == FormatStyle::RAS_Right) && -CurrentChange.Spaces != 0) { +CurrentChange.Spaces != 0 && CurrentChange.Tok->isNot(tok::equal)) { const bool ReferenceNotRightAligned = Style.ReferenceAlignment != FormatStyle::RAS_Right && Style.ReferenceAlignment != FormatStyle::RAS_Pointer; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 03005384a6f667..d1e977dfa66af5 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -19066,6 +19066,11 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) { verifyFormat("inta(int x);\n" "double b();", Alignment); + verifyFormat("inta(const Test & = Test());\n" + "inta1(int &foo, const Test & = Test());\n" + "inta2(int &foo, const Test &name = Test());\n" + "double b();", + Alignment); verifyFormat("struct Test {\n" " Test(const Test &) = default;\n" " ~Test() = default;\n" @@ -19102,6 +19107,13 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) { "int x,\n" "bool y);", Alignment); + // Set ColumnLimit low so that we break the argument list in multiple lines. + Alignment.ColumnLimit = 35; + verifyFormat("inta3(SomeTypeName1 &x,\n" + " SomeTypeName2 &y,\n" + " const Test & = Test());\n" + "double b();", + Alignment); Alignment.ColumnLimit = OldColumnLimit; // Ensure function pointers don't screw up recursive alignment verifyFormat("inta(int x, void (*fp)(int y));\n" @@ -19287,6 +19299,10 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) { "intfoobar;", AlignmentLeft); + verifyFormat("inta(SomeType& foo, const Test& = Test());\n" + "double b();", + AlignmentLeft); + // PAS_Middle FormatStyle AlignmentMiddle = Alignment; AlignmentMiddle.PointerAlignment = FormatStyle::PAS_Middle; @@ -19347,6 +19363,10 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) { "int foobar;", AlignmentMiddle); + verifyFormat("inta(SomeType & foo, const Test & = Test());\n" + "double b();", + AlignmentMiddle); + Alignment.AlignConsecutiveAssignments.Enabled = false; Alignment.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; verifyFormat("#define A \\\n" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][PowerPC] Add flag to enable compatibility with GNU for complex arguments (PR #77732)
@@ -396,12 +405,85 @@ CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const { return CharUnits::fromQuantity(4); } +ABIArgInfo PPC32_SVR4_ABIInfo::handleComplex(uint64_t &TypeSize) const { + llvm::Type *ElemTy; + unsigned RegsNeeded; // Registers Needed for Complex. + + // Choice of using llvm::Type::getInt64Ty(getVMContext()) for complex + // single-precision floats is based on the ABI ATR-PASS-COMPLEX-IN-GPRS + // specification. According to the specification: + // - For complex single-precision floats: If the register (gr) is even, it's + // incremented by one, and the lower-addressed word of the argument is loaded + // into gr, while the higher-addressed word is loaded into gr + 1. Then, gr is + // incremented by 2. + // - For complex double-precision floats: The words of the argument are loaded + // in memory-address order into gr, gr + 1, gr + 2, and gr + 3, with gr being + // incremented by 4. Thus, to maintain even alignment and adhere to the ABI + // specification, llvm::Type::getInt64Ty(getVMContext()) is used when TypeSize + // is 64. Powerpc backend handles this alignment requirement. Specifically, + // you can refer to the CC_PPC32_SVR4_Custom_AlignArgRegs method from + // PPCCallingconvention.cpp. For more context, refer to the previous + // discussion: https://reviews.llvm.org/D146942 and the related LLVM pull + // request: #77732 + + if (TypeSize == 64) { +ElemTy = llvm::Type::getInt64Ty(getVMContext()); +RegsNeeded = 1; + } else { +ElemTy = llvm::Type::getInt32Ty(getVMContext()); +RegsNeeded = TypeSize >> 5; + } + return ABIArgInfo::getDirect(llvm::ArrayType::get(ElemTy, RegsNeeded)); +} + +ABIArgInfo PPC32_SVR4_ABIInfo::classifyArgumentType(QualType Ty, +int &ArgGPRsLeft) const { + Ty = useFirstFieldIfTransparentUnion(Ty); + + if (!(getCodeGenOpts().getComplexInRegABI() == CodeGenOptions::CMPLX_InGPR) || + !ArgGPRsLeft) +return DefaultABIInfo::classifyArgumentType(Ty); + + assert(ArgGPRsLeft >= 0 && "Arg GPR must be large or equal than zero"); + ASTContext &Context = getContext(); + uint64_t TypeSize = Context.getTypeSize(Ty); + + if (Ty->isAnyComplexType()) { +// If gr is even set gr = gr + 1 for TypeSize=64. +if (TypeSize == 64 && ArgGPRsLeft % 2 == 1) + --ArgGPRsLeft; + +if (TypeSize <= RegLen * ArgGPRsLeft) { + ArgGPRsLeft -= TypeSize / RegLen; + return handleComplex(TypeSize); +} + } + + // Records with non-trivial destructors/copy-constructors should not be + // passed by value. + if (isAggregateTypeForABI(Ty)) +--ArgGPRsLeft; + else if (!Ty->isFloatingType()) { +// For other primitive types. +if (TypeSize == 64) { + // If gr is even set gr = gr + 1 for TypeSize=64. + if (ArgGPRsLeft % 2 == 1) +--ArgGPRsLeft; + if (TypeSize <= ArgGPRsLeft * RegLen) { +ArgGPRsLeft -= TypeSize / RegLen; + } +} else + --ArgGPRsLeft; Long5hot wrote: I missed the part for soft-float, for soft-float we pass long-double(128) in GPRs, Below change works. ``` else if (!Ty->isFloatingType() || (Ty->isFloatingType() && IsSoftFloatABI)) { if (TypeSize == 64 && ArgGPRsLeft % 2 == 1) --ArgGPRsLeft; // If gr is even set gr = gr + 1 for TypeSize=64. if (TypeSize <= ArgGPRsLeft * RegLen) ArgGPRsLeft -= TypeSize / RegLen; } ``` I will update the review as your comments. Do i need to add the tests for soft-floats as well?? https://github.com/llvm/llvm-project/pull/77732 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][PowerPC] Add flag to enable compatibility with GNU for complex arguments (PR #77732)
https://github.com/Long5hot edited https://github.com/llvm/llvm-project/pull/77732 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
AaronBallman wrote: > > I think we need to consider the following things: > > ``` > > * Should `Ty[0]` and `Ty[]` be handled the same way? (Remember, we support > > `[0]` in structures as a sort of flexible array member and we support > > flexible array members in C++ as an extension.) > > ``` > > I'm not sure what you're referring to. AFAICT `T[]` at the end of a struct > denotes a flexible array member, but `T[0]` doesn't. Or does it? It doesn't per spec but it does as a common compiler extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html So my question is more that, if `T[0]` isn't considered an array because it has no size, should `T[]` be considered the same way? > > ``` > > * We should probably change the value of `__is_bounded_array` at the same > > time as `__is_array`. > > ``` > > Yes. I'll update the PR accordingly. > > > ``` > > * What should the behavior of `__is_aggregate` be? > > ``` > > I guess it should be true? I can't think of any case where a zero-sized array > would be different from a non-zero-sized one at least. I don't have a strong > opinion either way though. I'm not entirely certain of how this particular type trait is used in practice. Based on https://sourcegraph.com/search?q=context:global+lang:C%2B%2B+std::is_aggregate+-file:.*libcxx.*+-file:.*test.*+-file:.*clang.*+-file:.*gcc.*&patternType=keyword&sm=0 it seems that most uses for this trait are to decide whether you can use aggregate initialization or not, and `Ty[0]` can be "used" in empty aggregate initialization (e.g., `int x[0] = {};`) so perhaps returning `true` makes sense? But then again, an aggregate is a class or array type, so the fact that `Ty[0]` will be reported as `false` for both of those doesn't seem ideal. CC @ldionne @mordante @cor3ntin for more opinions > > ``` > > * What about other type traits (`__is_compound`, `__is_abstract`, etc)? > > Basically, what's the principle we're using for this type? > > ``` > > Maybe we could think of it as array-like? i.e. it has the same traits as an > array except that it's not one. I think the semantics only really break down > for array-specific features. Just to get a sense, I've gone through a few > traits: > > * `is_compound`: it seems pretty clear to me that it is a compound type, > since `T[0]` is a distinct type from `T`, but contains a type `T` in its > definition. > > * `is_class`: It seems pretty clear to me that it's not a class. I don't > think anybody would disagree here. > > * `is_abstract`: You can define a variable of it, so it's not. It's also > not a class type. > > * `is_object`: I think it is, but this could be argued either way. The > best thing I could come up with is that it's closer to an array than it is to > a function or reference type. > > > `is_empty` seems like an interesting case. It's the smallest type there is, > with `sizeof(int[0]) == 0`, but it's not considered empty by any > implementation. MSVC rejects the `sizeof` call though. I think it shouldn't > be considered empty though, since `is_empty` kind-of implies that the type is > a class type. My natural inclination is that it is array-like, but... that just makes me want `__is_array` to return `true` for it all the more. That's... what it is. I wonder if we should be considering making zero-length arrays into a non-conforming extension behind a flag? e.g., `-fzero-sized-arrays` and then it does report true for `__is_array`, `__is_bounded_array`, and handles template specializations, etc as though it were really an array. That solves two problems: 1) we can make the feature regular as opposed to having so many sharp edges, 2) it pushes a surprising and questionable extension behind an opt-in feature flag, and it creates at least one problem: 1) potentially would change behavior of existing code in C++. So maybe this isn't a tenable idea, but do we think it's worth exploring? Oh wow, and it gets even more surprising... adding a zero-length array to a structure *reduces the size of the structure* (in C++): https://godbolt.org/z/5E4YsT1Ye CC @nickdesaulniers @kees @bwendling for more opinions from people who live with this kind of extension... https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][OpenMP] Add const-qualified `getIteratorModifier` to OMPMapClause (PR #86666)
https://github.com/kparzysz closed https://github.com/llvm/llvm-project/pull/8 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][OpenMP] Add const-qualified `getIteratorModifier` to OMPMapClause (PR #86666)
kparzysz wrote: Will revisit this when the using code is ready for review/merge. https://github.com/llvm/llvm-project/pull/8 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/86806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
llvmbot wrote: @llvm/pr-subscribers-libcxx @llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-lldb @llvm/pr-subscribers-clang @llvm/pr-subscribers-mlir Author: None (AngryLoki) Changes This fixes "SyntaxWarning: invalid escape sequence" and "SyntaxWarning: "is" with 'int' literal", except for polly and summarizeStats.py, which won't run on Python 3. The fix is done in every file, because otherwise with OS-wide Python 3.12 these SyntaxWarnings appear here and there, as Python 3.12 [now produce a SyntaxWarning](https://docs.python.org/3/whatsnew/3.12.html#other-language-changes) instead of DeprecationWarning. There are no warnings anymore, which can be checked with `python -m compileall -x "polly|summarizeStats.py" -d . -f -q .` --- Patch is 128.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86806.diff 106 Files Affected: - (modified) .github/workflows/version-check.py (+1-1) - (modified) clang-tools-extra/clang-tidy/add_new_check.py (+6-6) - (modified) clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py (+2-2) - (modified) clang-tools-extra/docs/clang-tidy/checks/gen-static-analyzer-docs.py (+1-1) - (modified) clang/docs/tools/dump_ast_matchers.py (+2-2) - (modified) clang/test/Analysis/check-analyzer-fixit.py (+1-1) - (modified) clang/utils/ABITest/TypeGen.py (+1-1) - (modified) compiler-rt/lib/asan/scripts/asan_symbolize.py (+2-2) - (modified) cross-project-tests/debuginfo-tests/dexter/dex/command/ParseCommand.py (+2-2) - (modified) cross-project-tests/lit.cfg.py (+3-3) - (modified) libcxx/test/libcxx/transitive_includes.gen.py (+1-1) - (modified) libcxx/utils/generate_escaped_output_table.py (+1-1) - (modified) libcxx/utils/generate_width_estimation_table.py (+1-1) - (modified) lld/test/MachO/tools/validate-unwind-info.py (+1-1) - (modified) lld/utils/benchmark.py (+1-1) - (modified) lldb/examples/python/crashlog.py (+4-4) - (modified) lldb/examples/python/delta.py (+1-1) - (modified) lldb/examples/python/gdbremote.py (+5-5) - (modified) lldb/examples/python/jump.py (+3-3) - (modified) lldb/examples/python/performance.py (+1-1) - (modified) lldb/examples/python/symbolication.py (+4-4) - (modified) lldb/packages/Python/lldbsuite/test/decorators.py (+1-1) - (modified) lldb/packages/Python/lldbsuite/test/lldbpexpect.py (+1-1) - (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+1-1) - (modified) lldb/packages/Python/lldbsuite/test/test_runner/process_control.py (+1-1) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py (+2-2) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py (+1-1) - (modified) lldb/test/API/commands/command/backticks/TestBackticksInAlias.py (+2-2) - (modified) lldb/test/API/commands/expression/memory-allocation/TestMemoryAllocSettings.py (+1-1) - (modified) lldb/test/API/commands/expression/test/TestExprs.py (+1-1) - (modified) lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py (+1-1) - (modified) lldb/test/API/commands/help/TestHelp.py (+3-3) - (modified) lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py (+1-1) - (modified) lldb/test/API/commands/register/register/TestRegistersUnavailable.py (+2-2) - (modified) lldb/test/API/commands/settings/TestSettings.py (+6-6) - (modified) lldb/test/API/commands/target/basic/TestTargetCommand.py (+1-1) - (modified) lldb/test/API/commands/target/dump-separate-debug-info/dwo/TestDumpDwo.py (+10-10) - (modified) lldb/test/API/commands/target/dump-separate-debug-info/oso/TestDumpOso.py (+11-11) - (modified) lldb/test/API/commands/trace/TestTraceDumpInfo.py (+1-1) - (modified) lldb/test/API/commands/trace/TestTraceEvents.py (+2-2) - (modified) lldb/test/API/commands/trace/TestTraceStartStop.py (+6-6) - (modified) lldb/test/API/commands/trace/TestTraceTSC.py (+5-5) - (modified) lldb/test/API/driver/quit_speed/TestQuitWithProcess.py (+1-1) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py (+1-1) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py (+2-2) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py (+3-3) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py (+3-3) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py (+8-8) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py (+1-1) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py (+11-11) - (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibC
[clang] [clang] Fix an out-of-bound crash when checking template partial specializations. (PR #86794)
https://github.com/hokein closed https://github.com/llvm/llvm-project/pull/86794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 11b20d7 - [clang] Fix an out-of-bound crash when checking template partial specializations. (#86794)
Author: Haojian Wu Date: 2024-03-27T15:31:55+01:00 New Revision: 11b20d7ab09511d9e2bcd40606dfd3b31976efe0 URL: https://github.com/llvm/llvm-project/commit/11b20d7ab09511d9e2bcd40606dfd3b31976efe0 DIFF: https://github.com/llvm/llvm-project/commit/11b20d7ab09511d9e2bcd40606dfd3b31976efe0.diff LOG: [clang] Fix an out-of-bound crash when checking template partial specializations. (#86794) I found this issue (a separate one) during the investigation of #86757, the crash is similar in substituteParameterMappings, but at different inner places. This was an out-of-bound issue where we access front element in an empty written template argument list to get the instantiation source range. This patch fixes it by adding a proper guard. Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaConcept.cpp clang/test/SemaTemplate/concepts.cpp Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0dd026a5de5c6f..0fdd9e3fb3eee2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -454,6 +454,7 @@ Bug Fixes to C++ Support - Fix a crash when instantiating a lambda that captures ``this`` outside of its context. Fixes (#GH85343). - Fix an issue where a namespace alias could be defined using a qualified name (all name components following the first `::` were ignored). +- Fix an out-of-bounds crash when checking the validity of template partial specializations. (part of #GH86757). Bug Fixes to AST Handling ^ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 1c546e9f5894f0..b6c4d3d540ef50 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -1269,10 +1269,18 @@ substituteParameterMappings(Sema &S, NormalizedConstraint &N, : SourceLocation())); Atomic.ParameterMapping.emplace(TempArgs, OccurringIndices.count()); } + SourceLocation InstLocBegin = + ArgsAsWritten->arguments().empty() + ? ArgsAsWritten->getLAngleLoc() + : ArgsAsWritten->arguments().front().getSourceRange().getBegin(); + SourceLocation InstLocEnd = + ArgsAsWritten->arguments().empty() + ? ArgsAsWritten->getRAngleLoc() + : ArgsAsWritten->arguments().front().getSourceRange().getEnd(); Sema::InstantiatingTemplate Inst( - S, ArgsAsWritten->arguments().front().getSourceRange().getBegin(), + S, InstLocBegin, Sema::InstantiatingTemplate::ParameterMappingSubstitution{}, Concept, - ArgsAsWritten->arguments().front().getSourceRange()); + {InstLocBegin, InstLocEnd}); if (S.SubstTemplateArguments(*Atomic.ParameterMapping, MLTAL, SubstArgs)) return true; diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index b7ea0d003a52d7..787cc809e25353 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++20 -verify %s +// RUN: %clang_cc1 -std=c++20 -ferror-limit 0 -verify %s namespace PR47043 { template concept True = true; @@ -1114,3 +1114,11 @@ void foo() { } } // namespace GH64808 + +namespace GH86757_1 { +template concept b = false; +template concept c = b<>; +template concept f = c< d >; +template struct e; // expected-note {{}} +template struct e; // expected-error {{class template partial specialization is not more specialized than the primary template}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DRIVER] Resolve Argument Visibility and Duplication Issue for Cuda ToolChain (PR #86807)
https://github.com/jle-quel created https://github.com/llvm/llvm-project/pull/86807 # Description This PR makes the argument `-Xcuda-ptxas` visible to the driver in cl-mode. Furthermore, it has been noticed that the arguments are being passed twice to `ptxas`. This also has been fixed by filtering out the arguments before appending them to the new `DAL` created by `CudaToolChain::TranslateArgs`. >From e28fab463b54192b8e8440aa8c349c2d42bb949a Mon Sep 17 00:00:00 2001 From: Jefferson Le Quellec Date: Wed, 27 Mar 2024 15:25:16 +0100 Subject: [PATCH 1/3] Make the argument -Xcuda-ptxas visible to the driver in cl-mode --- clang/include/clang/Driver/Options.td | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 29066ea14280c2..a5e132dc48a3ef 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1001,7 +1001,8 @@ def : Joined<["-"], "Xclang=">, Group, def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">, HelpText<"Pass to fatbinary invocation">, MetaVarName<"">; def Xcuda_ptxas : Separate<["-"], "Xcuda-ptxas">, - HelpText<"Pass to the ptxas assembler">, MetaVarName<"">; + HelpText<"Pass to the ptxas assembler">, MetaVarName<"">, + Visibility<[ClangOption, CLOption]>; def Xopenmp_target : Separate<["-"], "Xopenmp-target">, Group, HelpText<"Pass to the target offloading toolchain.">, MetaVarName<"">; def Xopenmp_target_EQ : JoinedAndSeparate<["-"], "Xopenmp-target=">, Group, >From be1fd093d9c3f2553cefd061d476d052848b853a Mon Sep 17 00:00:00 2001 From: Jefferson Le Quellec Date: Wed, 27 Mar 2024 15:26:52 +0100 Subject: [PATCH 2/3] Make sure arguments are not duplicated in new DAL --- clang/lib/Driver/ToolChains/Cuda.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp index 5f0b516e1a1a08..6634e6d818b33e 100644 --- a/clang/lib/Driver/ToolChains/Cuda.cpp +++ b/clang/lib/Driver/ToolChains/Cuda.cpp @@ -990,7 +990,10 @@ CudaToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, } for (Arg *A : Args) { -DAL->append(A); +// Make sure flags are not duplicated. +if (!llvm::is_contained(*DAL, A)) { + DAL->append(A); +} } if (!BoundArch.empty()) { >From fe2ab25ac7efc10851ca3aaa51caecc80f9000d4 Mon Sep 17 00:00:00 2001 From: Jefferson Le Quellec Date: Wed, 27 Mar 2024 15:27:33 +0100 Subject: [PATCH 3/3] Add visibility and duplication tests --- clang/test/Driver/cuda-external-tools.cu | 8 1 file changed, 8 insertions(+) diff --git a/clang/test/Driver/cuda-external-tools.cu b/clang/test/Driver/cuda-external-tools.cu index 946e144fce38fb..d9564d026b4faa 100644 --- a/clang/test/Driver/cuda-external-tools.cu +++ b/clang/test/Driver/cuda-external-tools.cu @@ -86,6 +86,12 @@ // RUN: -Xcuda-fatbinary -bar1 -Xcuda-ptxas -foo2 -Xcuda-fatbinary -bar2 %s 2>&1 \ // RUN: | FileCheck -check-prefixes=CHECK,SM35,PTXAS-EXTRA,FATBINARY-EXTRA %s +// Check -Xcuda-ptxas with clang-cl +// RUN: %clang_cl -### -c -Xcuda-ptxas -foo1 \ +// RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \ +// RUN: -Xcuda-ptxas -foo2 %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHECK,SM35,PTXAS-EXTRA %s + // MacOS spot-checks // RUN: %clang -### --target=x86_64-apple-macosx -O0 -c %s 2>&1 \ // RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \ @@ -140,6 +146,8 @@ // CHECK-SAME: "[[PTXFILE]]" // PTXAS-EXTRA-SAME: "-foo1" // PTXAS-EXTRA-SAME: "-foo2" +// CHECK-NOT: "-foo1" +// CHECK-NOT: "-foo2" // RDC-SAME: "-c" // CHECK-NOT: "-c" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DRIVER] Resolve Argument Visibility and Duplication Issue for Cuda ToolChain (PR #86807)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/86807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DRIVER] Resolve Argument Visibility and Duplication Issue for Cuda ToolChain (PR #86807)
llvmbot wrote: @llvm/pr-subscribers-clang-driver Author: Jefferson Le Quellec (jle-quel) Changes # Description This PR makes the argument `-Xcuda-ptxas` visible to the driver in cl-mode. Furthermore, it has been noticed that the arguments are being passed twice to `ptxas`. This also has been fixed by filtering out the arguments before appending them to the new `DAL` created by `CudaToolChain::TranslateArgs`. --- Full diff: https://github.com/llvm/llvm-project/pull/86807.diff 3 Files Affected: - (modified) clang/include/clang/Driver/Options.td (+2-1) - (modified) clang/lib/Driver/ToolChains/Cuda.cpp (+4-1) - (modified) clang/test/Driver/cuda-external-tools.cu (+8) ``diff diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 29066ea14280c2..a5e132dc48a3ef 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1001,7 +1001,8 @@ def : Joined<["-"], "Xclang=">, Group, def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">, HelpText<"Pass to fatbinary invocation">, MetaVarName<"">; def Xcuda_ptxas : Separate<["-"], "Xcuda-ptxas">, - HelpText<"Pass to the ptxas assembler">, MetaVarName<"">; + HelpText<"Pass to the ptxas assembler">, MetaVarName<"">, + Visibility<[ClangOption, CLOption]>; def Xopenmp_target : Separate<["-"], "Xopenmp-target">, Group, HelpText<"Pass to the target offloading toolchain.">, MetaVarName<"">; def Xopenmp_target_EQ : JoinedAndSeparate<["-"], "Xopenmp-target=">, Group, diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp index 5f0b516e1a1a08..6634e6d818b33e 100644 --- a/clang/lib/Driver/ToolChains/Cuda.cpp +++ b/clang/lib/Driver/ToolChains/Cuda.cpp @@ -990,7 +990,10 @@ CudaToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, } for (Arg *A : Args) { -DAL->append(A); +// Make sure flags are not duplicated. +if (!llvm::is_contained(*DAL, A)) { + DAL->append(A); +} } if (!BoundArch.empty()) { diff --git a/clang/test/Driver/cuda-external-tools.cu b/clang/test/Driver/cuda-external-tools.cu index 946e144fce38fb..d9564d026b4faa 100644 --- a/clang/test/Driver/cuda-external-tools.cu +++ b/clang/test/Driver/cuda-external-tools.cu @@ -86,6 +86,12 @@ // RUN: -Xcuda-fatbinary -bar1 -Xcuda-ptxas -foo2 -Xcuda-fatbinary -bar2 %s 2>&1 \ // RUN: | FileCheck -check-prefixes=CHECK,SM35,PTXAS-EXTRA,FATBINARY-EXTRA %s +// Check -Xcuda-ptxas with clang-cl +// RUN: %clang_cl -### -c -Xcuda-ptxas -foo1 \ +// RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \ +// RUN: -Xcuda-ptxas -foo2 %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHECK,SM35,PTXAS-EXTRA %s + // MacOS spot-checks // RUN: %clang -### --target=x86_64-apple-macosx -O0 -c %s 2>&1 \ // RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \ @@ -140,6 +146,8 @@ // CHECK-SAME: "[[PTXFILE]]" // PTXAS-EXTRA-SAME: "-foo1" // PTXAS-EXTRA-SAME: "-foo2" +// CHECK-NOT: "-foo1" +// CHECK-NOT: "-foo2" // RDC-SAME: "-c" // CHECK-NOT: "-c" `` https://github.com/llvm/llvm-project/pull/86807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CLANG][DRIVER] Resolve Argument Visibility and Duplication Issue for Cuda ToolChain (PR #86807)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Jefferson Le Quellec (jle-quel) Changes # Description This PR makes the argument `-Xcuda-ptxas` visible to the driver in cl-mode. Furthermore, it has been noticed that the arguments are being passed twice to `ptxas`. This also has been fixed by filtering out the arguments before appending them to the new `DAL` created by `CudaToolChain::TranslateArgs`. --- Full diff: https://github.com/llvm/llvm-project/pull/86807.diff 3 Files Affected: - (modified) clang/include/clang/Driver/Options.td (+2-1) - (modified) clang/lib/Driver/ToolChains/Cuda.cpp (+4-1) - (modified) clang/test/Driver/cuda-external-tools.cu (+8) ``diff diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 29066ea14280c2..a5e132dc48a3ef 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1001,7 +1001,8 @@ def : Joined<["-"], "Xclang=">, Group, def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">, HelpText<"Pass to fatbinary invocation">, MetaVarName<"">; def Xcuda_ptxas : Separate<["-"], "Xcuda-ptxas">, - HelpText<"Pass to the ptxas assembler">, MetaVarName<"">; + HelpText<"Pass to the ptxas assembler">, MetaVarName<"">, + Visibility<[ClangOption, CLOption]>; def Xopenmp_target : Separate<["-"], "Xopenmp-target">, Group, HelpText<"Pass to the target offloading toolchain.">, MetaVarName<"">; def Xopenmp_target_EQ : JoinedAndSeparate<["-"], "Xopenmp-target=">, Group, diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp b/clang/lib/Driver/ToolChains/Cuda.cpp index 5f0b516e1a1a08..6634e6d818b33e 100644 --- a/clang/lib/Driver/ToolChains/Cuda.cpp +++ b/clang/lib/Driver/ToolChains/Cuda.cpp @@ -990,7 +990,10 @@ CudaToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, } for (Arg *A : Args) { -DAL->append(A); +// Make sure flags are not duplicated. +if (!llvm::is_contained(*DAL, A)) { + DAL->append(A); +} } if (!BoundArch.empty()) { diff --git a/clang/test/Driver/cuda-external-tools.cu b/clang/test/Driver/cuda-external-tools.cu index 946e144fce38fb..d9564d026b4faa 100644 --- a/clang/test/Driver/cuda-external-tools.cu +++ b/clang/test/Driver/cuda-external-tools.cu @@ -86,6 +86,12 @@ // RUN: -Xcuda-fatbinary -bar1 -Xcuda-ptxas -foo2 -Xcuda-fatbinary -bar2 %s 2>&1 \ // RUN: | FileCheck -check-prefixes=CHECK,SM35,PTXAS-EXTRA,FATBINARY-EXTRA %s +// Check -Xcuda-ptxas with clang-cl +// RUN: %clang_cl -### -c -Xcuda-ptxas -foo1 \ +// RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \ +// RUN: -Xcuda-ptxas -foo2 %s 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHECK,SM35,PTXAS-EXTRA %s + // MacOS spot-checks // RUN: %clang -### --target=x86_64-apple-macosx -O0 -c %s 2>&1 \ // RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \ @@ -140,6 +146,8 @@ // CHECK-SAME: "[[PTXFILE]]" // PTXAS-EXTRA-SAME: "-foo1" // PTXAS-EXTRA-SAME: "-foo2" +// CHECK-NOT: "-foo1" +// CHECK-NOT: "-foo2" // RDC-SAME: "-c" // CHECK-NOT: "-c" `` https://github.com/llvm/llvm-project/pull/86807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Print the "aggregate" for aggregate deduction guide decl. (PR #84018)
https://github.com/sam-mccall edited https://github.com/llvm/llvm-project/pull/84018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Print the "aggregate" for aggregate deduction guide decl. (PR #84018)
https://github.com/sam-mccall approved this pull request. https://github.com/llvm/llvm-project/pull/84018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Print the "aggregate" for aggregate deduction guide decl. (PR #84018)
@@ -1990,6 +1990,18 @@ void TextNodeDumper::VisitFunctionDecl(const FunctionDecl *D) { } } +void TextNodeDumper::VisitCXXDeductionGuideDecl(const CXXDeductionGuideDecl *D) { + VisitFunctionDecl(D); + switch (D->getDeductionCandidateKind()) { + case DeductionCandidate::Normal: + case DeductionCandidate::Copy: sam-mccall wrote: while here, I think adding "copy" might also make sense? (not sure, i'm not immersed in CTAD stuff, so up to you) https://github.com/llvm/llvm-project/pull/84018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][RISCV] Enable RVV with function attribute __attribute__((target("arch=+v"))) (PR #83674)
@@ -8927,8 +8927,13 @@ void Sema::CheckVariableDeclarationType(VarDecl *NewVD) { } } - if (T->isRVVSizelessBuiltinType()) -checkRVVTypeSupport(T, NewVD->getLocation(), cast(CurContext)); + if (T->isRVVSizelessBuiltinType() && isa(CurContext)) { +const FunctionDecl *FD = cast(CurContext); +llvm::StringMap CallerFeatureMap; +Context.getFunctionFeatureMap(CallerFeatureMap, FD); 4vtomat wrote: @lukel97 Got it, thanks! https://github.com/llvm/llvm-project/pull/83674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies… (PR #86808)
https://github.com/smanna12 created https://github.com/llvm/llvm-project/pull/86808 … with auto keyword Reported by Static Analyzer Tool: In clang::installapi::InstallAPIVisitor::VisitFunctionDecl(clang::FunctionDecl const *): Using the auto keyword without an & causes the copy of an object of type DynTypedNode. >From 64ee510df037daeccb98a3fd9fa7e2ac5c41d279 Mon Sep 17 00:00:00 2001 From: "Manna, Soumi" Date: Wed, 27 Mar 2024 07:41:09 -0700 Subject: [PATCH] [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies with auto keyword Reported by Static Analyzer Tool: In clang::installapi::InstallAPIVisitor::VisitFunctionDecl(clang::FunctionDecl const *): Using the auto keyword without an & causes the copy of an object of type DynTypedNode. --- clang/lib/InstallAPI/Visitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/InstallAPI/Visitor.cpp b/clang/lib/InstallAPI/Visitor.cpp index f8f5d8d53d5691..43878463caccbb 100644 --- a/clang/lib/InstallAPI/Visitor.cpp +++ b/clang/lib/InstallAPI/Visitor.cpp @@ -255,7 +255,7 @@ bool InstallAPIVisitor::VisitFunctionDecl(const FunctionDecl *D) { return true; // Skip methods in CXX RecordDecls. -for (auto P : D->getASTContext().getParents(*M)) { +for (const auto &P : D->getASTContext().getParents(*M)) { if (P.get()) return true; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies… (PR #86808)
llvmbot wrote: @llvm/pr-subscribers-clang Author: None (smanna12) Changes … with auto keyword Reported by Static Analyzer Tool: In clang::installapi::InstallAPIVisitor::VisitFunctionDecl(clang::FunctionDecl const *): Using the auto keyword without an & causes the copy of an object of type DynTypedNode. --- Full diff: https://github.com/llvm/llvm-project/pull/86808.diff 1 Files Affected: - (modified) clang/lib/InstallAPI/Visitor.cpp (+1-1) ``diff diff --git a/clang/lib/InstallAPI/Visitor.cpp b/clang/lib/InstallAPI/Visitor.cpp index f8f5d8d53d5691..43878463caccbb 100644 --- a/clang/lib/InstallAPI/Visitor.cpp +++ b/clang/lib/InstallAPI/Visitor.cpp @@ -255,7 +255,7 @@ bool InstallAPIVisitor::VisitFunctionDecl(const FunctionDecl *D) { return true; // Skip methods in CXX RecordDecls. -for (auto P : D->getASTContext().getParents(*M)) { +for (const auto &P : D->getASTContext().getParents(*M)) { if (P.get()) return true; } `` https://github.com/llvm/llvm-project/pull/86808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies… (PR #86808)
https://github.com/smanna12 edited https://github.com/llvm/llvm-project/pull/86808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies with auto keyword (PR #86808)
https://github.com/smanna12 edited https://github.com/llvm/llvm-project/pull/86808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Enable -fconvergent-functions by default (PR #86571)
aniplcc wrote: I was getting match errors with the RUN script. I went ahead and updated it with the checks in: `clang/test/CodeGen/convergent-functions.cpp` Hope that's correct/OK? ```hlsl // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck -check-prefixes=CHECK,CONVFUNC %s // CHECK: attributes // NOCONVFUNC-NOT: convergent // CONVFUNC-SAME: convergent // CHECK-SAME: } void fn() { }; ``` Now it just *checks* the presence of `convergent` in attributes, as it does in `convergent-functions.cpp`. Note: I also left out the `#0` in the attributes to generalize it better. Is that okay? Also for reference, the emitted llvmir from my forked clang build ```llvm ; ModuleID = 'CodeGenHLSL/convergent-funtions.hlsl' source_filename = "CodeGenHLSL/convergent-funtions.hlsl" target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64" target triple = "dxil-pc-shadermodel6.4-library" ; Function Attrs: convergent noinline nounwind optnone define void @"?fn@@YAXXZ"() #0 { entry: ret void } attributes #0 = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" } ...*some more lines* ``` https://github.com/llvm/llvm-project/pull/86571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
https://github.com/JonPsson1 updated https://github.com/llvm/llvm-project/pull/86691 >From ea3c2d02073ecd3761b53783ad78c132ba88486e Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Tue, 26 Mar 2024 01:16:25 +0100 Subject: [PATCH 1/2] Refactoring with shouldCastToInt(). Also include atomic pointers. --- clang/lib/CodeGen/CGAtomic.cpp | 80 +- clang/test/CodeGen/atomic.c| 9 ++-- 2 files changed, 44 insertions(+), 45 deletions(-) diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp index fb03d013e8afc7..f543529efe5b69 100644 --- a/clang/lib/CodeGen/CGAtomic.cpp +++ b/clang/lib/CodeGen/CGAtomic.cpp @@ -197,11 +197,11 @@ namespace { llvm::Value *getScalarRValValueOrNull(RValue RVal) const; /// Converts an rvalue to integer value if needed. -llvm::Value *convertRValueToInt(RValue RVal, bool CastFP = true) const; +llvm::Value *convertRValueToInt(RValue RVal, bool CmpXchg = false) const; RValue ConvertToValueOrAtomic(llvm::Value *IntVal, AggValueSlot ResultSlot, SourceLocation Loc, bool AsValue, - bool CastFP = true) const; + bool CmpXchg = false) const; /// Copy an atomic r-value into atomic-layout memory. void emitCopyIntoMemory(RValue rvalue) const; @@ -264,7 +264,7 @@ namespace { llvm::AtomicOrdering AO, bool IsVolatile); /// Emits atomic load as LLVM instruction. llvm::Value *EmitAtomicLoadOp(llvm::AtomicOrdering AO, bool IsVolatile, - bool CastFP = true); + bool CmpXchg = false); /// Emits atomic compare-and-exchange op as a libcall. llvm::Value *EmitAtomicCompareExchangeLibcall( llvm::Value *ExpectedAddr, llvm::Value *DesiredAddr, @@ -272,7 +272,9 @@ namespace { llvm::AtomicOrdering::SequentiallyConsistent, llvm::AtomicOrdering Failure = llvm::AtomicOrdering::SequentiallyConsistent); -/// Emits atomic compare-and-exchange op as LLVM instruction. +/// Emits atomic compare-and-exchange op as LLVM instruction. Operands +/// must be of integer or pointer type, so float must be casted. +/// TODO: this could change - see comment in AtomicExpandPass.cpp. std::pair EmitAtomicCompareExchangeOp( llvm::Value *ExpectedVal, llvm::Value *DesiredVal, llvm::AtomicOrdering Success = @@ -1399,13 +1401,22 @@ RValue AtomicInfo::convertAtomicTempToRValue(Address addr, LVal.getBaseInfo(), TBAAAccessInfo())); } +static bool shouldCastToInt(llvm::Type *ValTy, bool CmpXchg) { + // TODO: Also pass through non-ieee FP types. + bool KeepType = (ValTy->isIntegerTy() || ValTy->isPointerTy() || + (ValTy->isIEEELikeFPTy() && !CmpXchg)); + return !KeepType; +} + RValue AtomicInfo::ConvertToValueOrAtomic(llvm::Value *Val, AggValueSlot ResultSlot, SourceLocation Loc, bool AsValue, - bool CastFP) const { + bool CmpXchg) const { // Try not to in some easy cases. - assert((Val->getType()->isIntegerTy() || Val->getType()->isIEEELikeFPTy()) && - "Expected integer or floating point value"); + assert((Val->getType()->isIntegerTy() || Val->getType()->isPointerTy() || + Val->getType()->isIEEELikeFPTy()) && + "Expected integer, pointer or floating point value when converting " + "result."); if (getEvaluationKind() == TEK_Scalar && (((!LVal.isBitField() || LVal.getBitFieldInfo().Size == ValueSizeInBits) && @@ -1414,13 +1425,11 @@ RValue AtomicInfo::ConvertToValueOrAtomic(llvm::Value *Val, auto *ValTy = AsValue ? CGF.ConvertTypeForMem(ValueTy) : getAtomicAddress().getElementType(); -if (ValTy->isIntegerTy() || (!CastFP && ValTy->isIEEELikeFPTy())) { +if (!shouldCastToInt(ValTy, CmpXchg)) { assert((!ValTy->isIntegerTy() || Val->getType() == ValTy) && "Different integer types."); return RValue::get(CGF.EmitFromMemory(Val, ValueTy)); -} else if (ValTy->isPointerTy()) - return RValue::get(CGF.Builder.CreateIntToPtr(Val, ValTy)); -else if (llvm::CastInst::isBitCastable(Val->getType(), ValTy)) +} else if (llvm::CastInst::isBitCastable(Val->getType(), ValTy)) return RValue::get(CGF.Builder.CreateBitCast(Val, ValTy)); } @@ -1457,10 +1466,10 @@ void AtomicInfo::EmitAtomicLoadLibcall(llvm::Value *AddForLoaded, } llvm::Value *AtomicInfo::EmitAtomicLoadOp(llvm::AtomicOrdering AO, - bool IsVolatile, bool CastFP) { + bool IsVolatile, bool CmpXchg) { // Okay, we're doing this natively. Address Addr = g
[clang] [ClangFE] Improve handling of casting of atomic memory operations. (PR #86691)
JonPsson1 wrote: > > I think this case isn't that simple as it is an 80 bit value. Currently > > that is loaded atomically first with i128, then stored as a temporary and > > then loaded as an fp80. If I remove that casting, the verifier complains > > "atomic memory access' operand must have a power-of-two size". > > I think this should be more targeted based on the bitesize. PPCf128 should be > simple Patch updated to handle all FP types except FP80. https://github.com/llvm/llvm-project/pull/86691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Introduce a helper class for handling record initializer lists. (PR #86675)
https://github.com/ymand edited https://github.com/llvm/llvm-project/pull/86675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Introduce a helper class for handling record initializer lists. (PR #86675)
https://github.com/ymand approved this pull request. https://github.com/llvm/llvm-project/pull/86675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Introduce a helper class for handling record initializer lists. (PR #86675)
@@ -744,6 +744,35 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, std::vector getFieldsForInitListExpr(const InitListExpr *InitList); +/// Helper class for initialization of a record with an `InitListExpr`. +/// `InitListExpr::inits()` contains the initializers for both the base classes +/// and the fields of the record; this helper class separates these out into two +/// different lists. In addition, it deals with special cases associated with +/// unions. +class RecordInitListHelper { ymand wrote: Why an object vs something like: ``` struct RecordInits { SmallVector> BaseInits; SmallVector> FieldInits; }; RecordInits RecordInitListHelper(const InitListExpr *InitList); ``` Is it because of the optional storage needed for the union? https://github.com/llvm/llvm-project/pull/86675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
https://github.com/ldionne approved this pull request. Libc++ parts LGTM. https://github.com/llvm/llvm-project/pull/86806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] Enable -fconvergent-functions by default (PR #86571)
https://github.com/aniplcc updated https://github.com/llvm/llvm-project/pull/86571 >From c22f63359b7391e9f69d74e5d4f4bdf6cf585c2c Mon Sep 17 00:00:00 2001 From: aniplcc Date: Tue, 26 Mar 2024 01:35:36 +0530 Subject: [PATCH 1/2] [HLSL] Enable -fconvergent-functions by default --- clang/lib/Frontend/CompilerInvocation.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 7bd91d4791ecf0..30638aca1d 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -3516,7 +3516,8 @@ void CompilerInvocationBase::GenerateLangArgs(const LangOptions &Opts, GenerateArg(Consumer, OPT_fblocks); if (Opts.ConvergentFunctions && - !(Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) || Opts.SYCLIsDevice)) + !(Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) || Opts.SYCLIsDevice || +Opts.HLSL)) GenerateArg(Consumer, OPT_fconvergent_functions); if (Opts.NoBuiltin && !Opts.Freestanding) @@ -3914,7 +3915,7 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, Opts.ConvergentFunctions = Args.hasArg(OPT_fconvergent_functions) || Opts.OpenCL || (Opts.CUDA && Opts.CUDAIsDevice) || - Opts.SYCLIsDevice; + Opts.SYCLIsDevice || Opts.HLSL; Opts.NoBuiltin = Args.hasArg(OPT_fno_builtin) || Opts.Freestanding; if (!Opts.NoBuiltin) >From e4fdf29d71803afa87939726292feb9e90f97e04 Mon Sep 17 00:00:00 2001 From: aniplcc Date: Wed, 27 Mar 2024 20:23:45 +0530 Subject: [PATCH 2/2] add CodeGenHLSL/convergent-functions.hlsl for testing new defaults --- clang/test/CodeGenHLSL/convergent-functions.hlsl | 8 1 file changed, 8 insertions(+) create mode 100644 clang/test/CodeGenHLSL/convergent-functions.hlsl diff --git a/clang/test/CodeGenHLSL/convergent-functions.hlsl b/clang/test/CodeGenHLSL/convergent-functions.hlsl new file mode 100644 index 00..23746ec8703bbe --- /dev/null +++ b/clang/test/CodeGenHLSL/convergent-functions.hlsl @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.4-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck -check-prefixes=CHECK,CONVFUNC %s + +// CHECK: attributes +// NOCONVFUNC-NOT: convergent +// CONVFUNC-SAME: convergent +// CHECK-SAME: } +void fn() { +}; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [HLSL] enforce unsigned types for reversebits (PR #86720)
https://github.com/farzonl updated https://github.com/llvm/llvm-project/pull/86720 >From 13296921ee46ab93d703eb7446479ce8aa7fadce Mon Sep 17 00:00:00 2001 From: Farzon Lotfi Date: Tue, 26 Mar 2024 15:15:03 -0400 Subject: [PATCH] [HLSL] enforce unsigned types for reversebits fixes #86719 `SemaChecking.cpp` - Adds unsigned semaChecks to `__builtin_elementwise_bitreverse` `hlsl_intrinsics.h` - remove signed `reversebits` apis --- clang/lib/Headers/hlsl/hlsl_intrinsics.h | 31 clang/lib/Sema/SemaChecking.cpp | 13 .../CodeGenHLSL/builtins/reversebits.hlsl | 75 --- .../SemaHLSL/BuiltIns/reversebits-errors.hlsl | 12 +++ 4 files changed, 25 insertions(+), 106 deletions(-) create mode 100644 clang/test/SemaHLSL/BuiltIns/reversebits-errors.hlsl diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h b/clang/lib/Headers/hlsl/hlsl_intrinsics.h index 18472f728eefe0..d47eab453f8747 100644 --- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h +++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h @@ -1147,19 +1147,6 @@ float4 pow(float4, float4); /// \param Val The input value. #ifdef __HLSL_ENABLE_16_BIT -_HLSL_AVAILABILITY(shadermodel, 6.2) -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int16_t reversebits(int16_t); -_HLSL_AVAILABILITY(shadermodel, 6.2) -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int16_t2 reversebits(int16_t2); -_HLSL_AVAILABILITY(shadermodel, 6.2) -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int16_t3 reversebits(int16_t3); -_HLSL_AVAILABILITY(shadermodel, 6.2) -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int16_t4 reversebits(int16_t4); - _HLSL_AVAILABILITY(shadermodel, 6.2) _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) uint16_t reversebits(uint16_t); @@ -1174,15 +1161,6 @@ _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) uint16_t4 reversebits(uint16_t4); #endif -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int reversebits(int); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int2 reversebits(int2); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int3 reversebits(int3); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int4 reversebits(int4); - _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) uint reversebits(uint); _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) @@ -1192,15 +1170,6 @@ uint3 reversebits(uint3); _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) uint4 reversebits(uint4); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int64_t reversebits(int64_t); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int64_t2 reversebits(int64_t2); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int64_t3 reversebits(int64_t3); -_HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) -int64_t4 reversebits(int64_t4); - _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) uint64_t reversebits(uint64_t); _HLSL_BUILTIN_ALIAS(__builtin_elementwise_bitreverse) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 08449581330934..dedbd3a737de2e 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5541,6 +5541,14 @@ bool CheckNoDoubleVectors(Sema *S, CallExpr *TheCall) { checkDoubleVector); } +bool CheckUnsignedIntRepresentation(Sema *S, CallExpr *TheCall) { + auto checkAllUnsignedTypes = [](clang::QualType PassedType) -> bool { +return !PassedType->hasUnsignedIntegerRepresentation(); + }; + return CheckArgsTypesAreCorrect(S, TheCall, S->Context.UnsignedIntTy, + checkAllUnsignedTypes); +} + void SetElementTypeAsReturnType(Sema *S, CallExpr *TheCall, QualType ReturnType) { auto *VecTyA = TheCall->getArg(0)->getType()->getAs(); @@ -5628,6 +5636,11 @@ bool Sema::CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) { } // Note these are llvm builtins that we want to catch invalid intrinsic // generation. Normal handling of these builitns will occur elsewhere. + case Builtin::BI__builtin_elementwise_bitreverse: { +if (CheckUnsignedIntRepresentation(this, TheCall)) + return true; +break; + } case Builtin::BI__builtin_elementwise_cos: case Builtin::BI__builtin_elementwise_sin: case Builtin::BI__builtin_elementwise_log: diff --git a/clang/test/CodeGenHLSL/builtins/reversebits.hlsl b/clang/test/CodeGenHLSL/builtins/reversebits.hlsl index 6da7d289f82e80..a319417e97a436 100644 --- a/clang/test/CodeGenHLSL/builtins/reversebits.hlsl +++ b/clang/test/CodeGenHLSL/builtins/reversebits.hlsl @@ -3,31 +3,6 @@ // RUN: -emit-llvm -disable-llvm-passes -O3 -o - | FileCheck %s #ifdef __HLSL_ENABLE_16_BIT -// CHECK: define noundef i16 @ -// CHECK: call i16 @llvm.bitreverse.i16( -int16_t test_bitreverse_short(int16_t p0) -{ - return reversebits(p0); -} -// CHECK: define noundef <2 x i16> @ -// CHECK: call <2 x i1
[clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r e82765bf07a978674c0e75c8b2e20f154ae24a4c...c7d6289573379be760ca9d691d62e0f87ba0ee8a .github/workflows/version-check.py clang-tools-extra/clang-tidy/add_new_check.py clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py clang-tools-extra/docs/clang-tidy/checks/gen-static-analyzer-docs.py clang/docs/tools/dump_ast_matchers.py clang/test/Analysis/check-analyzer-fixit.py clang/utils/ABITest/TypeGen.py compiler-rt/lib/asan/scripts/asan_symbolize.py cross-project-tests/debuginfo-tests/dexter/dex/command/ParseCommand.py cross-project-tests/lit.cfg.py libcxx/test/libcxx/transitive_includes.gen.py libcxx/utils/generate_escaped_output_table.py libcxx/utils/generate_width_estimation_table.py lld/test/MachO/tools/validate-unwind-info.py lld/utils/benchmark.py lldb/examples/python/crashlog.py lldb/examples/python/delta.py lldb/examples/python/gdbremote.py lldb/examples/python/jump.py lldb/examples/python/performance.py lldb/examples/python/symbolication.py lldb/packages/Python/lldbsuite/test/decorators.py lldb/packages/Python/lldbsuite/test/lldbpexpect.py lldb/packages/Python/lldbsuite/test/lldbtest.py lldb/packages/Python/lldbsuite/test/test_runner/process_control.py lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py lldb/test/API/commands/command/backticks/TestBackticksInAlias.py lldb/test/API/commands/expression/memory-allocation/TestMemoryAllocSettings.py lldb/test/API/commands/expression/test/TestExprs.py lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py lldb/test/API/commands/help/TestHelp.py lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py lldb/test/API/commands/register/register/TestRegistersUnavailable.py lldb/test/API/commands/settings/TestSettings.py lldb/test/API/commands/target/basic/TestTargetCommand.py lldb/test/API/commands/target/dump-separate-debug-info/dwo/TestDumpDwo.py lldb/test/API/commands/target/dump-separate-debug-info/oso/TestDumpOso.py lldb/test/API/commands/trace/TestTraceDumpInfo.py lldb/test/API/commands/trace/TestTraceEvents.py lldb/test/API/commands/trace/TestTraceStartStop.py lldb/test/API/commands/trace/TestTraceTSC.py lldb/test/API/driver/quit_speed/TestQuitWithProcess.py lldb/test/API/functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py lldb/test/API/functionalities/data-formatter/type_summary_list_arg/TestTypeSummaryListArg.py lldb/test/API/functionalities/memory-region/TestMemoryRegion.py lldb/test/API/functionalities/target_var/TestTargetVar.py lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py lldb/test/API/lang/c/function_types/TestFunctionTypes.py lldb/test/API/lang/c/register_variables/TestRegisterVariables.py lldb/test/API/lang/c/set_values/TestSetValues.py lldb/test/API/lang/c/strings/TestCStrings.py lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py lldb/test/API/lang/cpp/char1632_t/TestChar1632T.py lldb/test/API/lang/cpp/class_static/TestStaticVariables.py lldb/test/API/lang/cpp/class_types/TestClassTypes.py lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py lldb/test/API/lang/cpp/namespace/TestNamespace.py lldb/test/API/lang/cpp/signed_types/TestSignedTypes.py lldb/test/API/lang/cpp/unsigned_types/TestUnsignedTypes.py lldb/test/API/lang/mixed/TestMixedLanguages.py lldb/test/API/lang/objc/foundation/TestObjCMethods.py lldb/test/API/lang/objc/foundation/TestObjCMethodsNSArray.py lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py lldb/test/API/lang/objc/foundation/TestObjCMethodsString.py lldb/test/API/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
[clang] [llvm] [RISCV] RISCV vector calling convention (1/2) (PR #77560)
4vtomat wrote: Rebase and squash https://github.com/llvm/llvm-project/pull/77560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits