https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/174414
>From 6ca88ea5dfb94268dfbc6823ad4a8cd25e09482a Mon Sep 17 00:00:00 2001 From: Folkert de Vries <[email protected]> Date: Tue, 6 Jan 2026 15:51:59 +0100 Subject: [PATCH] [X86] use `VPMADDWD` for widening adjacent addition (#174149) The `_mm256_madd_epi16` intrinsic performs first a pointwise widening multiplication, and then adds adjacent elements. In SIMD versions of the adler32 checksum algorithm, a trivial multiplication by an all-ones vector is used to get just the widening and addition behavior. In the rust standard library, we like to implement intrinsics in terms of simpler building blocks, so that all backends can implement a small set of primitives instead of supporting all of LLVM's intrinsics. When we try that for `_mm256_madd_epi16` in isolation it works, but when one of the arguments is an all-ones vector, the multiplication is optimized out long before the `vpmaddwd` instruction can be selected. This PR recognizes the widening adjacent addition pattern that adler32 uses directly, and manually inserts a trivial multiplication by an all-ones vector. Experimentally, performing this optimization increases adler32 throughput from 41 gb/s to 67 gb/s (https://github.com/rust-lang/rust/issues/150560#issue-3774170588) cc https://github.com/rust-lang/stdarch/pull/1985 cc https://github.com/rust-lang/rust/issues/150560 --- .../bugprone/UnsafeFunctionsCheck.cpp | 2 +- .../bugprone/UnsafeFunctionsCheck.h | 2 +- clang-tools-extra/clang-tidy/utils/Matchers.h | 6 ++-- .../modernize/use-std-format-custom.cpp | 2 +- .../modernize/use-std-print-custom.cpp | 2 +- llvm/lib/Target/X86/X86ISelLowering.cpp | 36 +++++++++++++------ llvm/test/CodeGen/X86/combine-pmadd.ll | 29 +++++++++++++++ 7 files changed, 62 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index 52bb355ad44d0..ecd67f1e62609 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -155,7 +155,7 @@ parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) { Result.push_back( {Name.trim().str(), - matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()), + matchers::MatchesAnyListedRegexNameMatcher::NameMatcher(Name.trim()), Replacement.trim().str(), Reason.trim().str()}); } diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h index 99bb91a173187..58d9b2830dafc 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h @@ -35,7 +35,7 @@ class UnsafeFunctionsCheck : public ClangTidyCheck { struct CheckedFunction { std::string Name; - matchers::MatchesAnyListedNameMatcher::NameMatcher Pattern; + matchers::MatchesAnyListedRegexNameMatcher::NameMatcher Pattern; std::string Replacement; std::string Reason; }; diff --git a/clang-tools-extra/clang-tidy/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h index 566b93304ab54..b0572860815a0 100644 --- a/clang-tools-extra/clang-tidy/utils/Matchers.h +++ b/clang-tools-extra/clang-tidy/utils/Matchers.h @@ -77,10 +77,10 @@ AST_MATCHER(Expr, hasUnevaluatedContext) { // A matcher implementation that matches a list of type name regular expressions // against a NamedDecl. If a regular expression contains the substring "::" // matching will occur against the qualified name, otherwise only the typename. -class MatchesAnyListedNameMatcher +class MatchesAnyListedRegexNameMatcher : public ast_matchers::internal::MatcherInterface<NamedDecl> { public: - explicit MatchesAnyListedNameMatcher(llvm::ArrayRef<StringRef> NameList) { + explicit MatchesAnyListedRegexNameMatcher(llvm::ArrayRef<StringRef> NameList) { std::transform( NameList.begin(), NameList.end(), std::back_inserter(NameMatchers), [](const llvm::StringRef Name) { return NameMatcher(Name); }); @@ -145,7 +145,7 @@ class MatchesAnyListedNameMatcher inline ::clang::ast_matchers::internal::Matcher<NamedDecl> matchesAnyListedName(llvm::ArrayRef<StringRef> NameList) { return ::clang::ast_matchers::internal::Matcher( - new MatchesAnyListedNameMatcher(NameList)); + new MatchesAnyListedRegexNameMatcher(NameList)); } // Predicate that verify if statement is not identical to one bound to ID node. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp index 0f3458e61856a..493ded3a407fa 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp @@ -44,7 +44,7 @@ std::string StrFormat_strict_conversion() { // CHECK-FIXES-STRICT: return fmt::format("Integer {} from unsigned char\n", static_cast<signed char>(uc)); } -// Ensure that MatchesAnyListedNameMatcher::NameMatcher::match() can cope with a +// Ensure that MatchesAnyListedRegexNameMatcher::NameMatcher::match() can cope with a // NamedDecl that has no name when we're trying to match unqualified_strprintf. std::string A(const std::string &in) { diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp index 2c6a651b679d6..81a17179f1be0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-custom.cpp @@ -80,7 +80,7 @@ int fprintf_uses_return_value(int i) { // CHECK-FIXES-NOT: std::println(stderr, "return value {}", i); } -// Ensure that MatchesAnyListedNameMatcher::NameMatcher::match() can cope with a +// Ensure that MatchesAnyListedRegexNameMatcher::NameMatcher::match() can cope with a // NamedDecl that has no name when we're trying to match unqualified_printf. void no_name(const std::string &in) { diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp index 0883281edcf88..6501aa3746a0f 100644 --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -58336,7 +58336,8 @@ static SDValue matchPMADDWD(SelectionDAG &DAG, SDNode *N, // (extract_elt Mul, 3), // (extract_elt Mul, 5), // ... - // and identify Mul. + // and identify Mul. Mul must be either ISD::MUL, or can be ISD::SIGN_EXTEND + // in which case we add a trivial multiplication by 1. SDValue Mul; for (unsigned i = 0, e = VT.getVectorNumElements(); i != e; i += 2) { SDValue Op0L = Op0->getOperand(i), Op1L = Op1->getOperand(i), @@ -58367,7 +58368,8 @@ static SDValue matchPMADDWD(SelectionDAG &DAG, SDNode *N, // with 2X number of vector elements than the BUILD_VECTOR. // Both extracts must be from same MUL. Mul = Vec0L; - if (Mul.getOpcode() != ISD::MUL || + if ((Mul.getOpcode() != ISD::MUL && + Mul.getOpcode() != ISD::SIGN_EXTEND) || Mul.getValueType().getVectorNumElements() != 2 * e) return SDValue(); } @@ -58376,16 +58378,30 @@ static SDValue matchPMADDWD(SelectionDAG &DAG, SDNode *N, return SDValue(); } - // Check if the Mul source can be safely shrunk. - ShrinkMode Mode; - if (!canReduceVMulWidth(Mul.getNode(), DAG, Mode) || - Mode == ShrinkMode::MULU16) - return SDValue(); - EVT TruncVT = EVT::getVectorVT(*DAG.getContext(), MVT::i16, VT.getVectorNumElements() * 2); - SDValue N0 = DAG.getNode(ISD::TRUNCATE, DL, TruncVT, Mul.getOperand(0)); - SDValue N1 = DAG.getNode(ISD::TRUNCATE, DL, TruncVT, Mul.getOperand(1)); + + SDValue N0, N1; + if (Mul.getOpcode() == ISD::MUL) { + // Check if the Mul source can be safely shrunk. + ShrinkMode Mode; + if (!canReduceVMulWidth(Mul.getNode(), DAG, Mode) || + Mode == ShrinkMode::MULU16) + return SDValue(); + + N0 = DAG.getNode(ISD::TRUNCATE, DL, TruncVT, Mul.getOperand(0)); + N1 = DAG.getNode(ISD::TRUNCATE, DL, TruncVT, Mul.getOperand(1)); + } else { + assert(Mul.getOpcode() == ISD::SIGN_EXTEND); + + // Add a trivial multiplication with 1 so that we can make use of VPMADDWD. + N0 = Mul.getOperand(0); + + if (N0.getValueType() != TruncVT) + return SDValue(); + + N1 = DAG.getConstant(1, DL, TruncVT); + } auto PMADDBuilder = [](SelectionDAG &DAG, const SDLoc &DL, ArrayRef<SDValue> Ops) { diff --git a/llvm/test/CodeGen/X86/combine-pmadd.ll b/llvm/test/CodeGen/X86/combine-pmadd.ll index d9283aa8591fc..231b9f97a5e3f 100644 --- a/llvm/test/CodeGen/X86/combine-pmadd.ll +++ b/llvm/test/CodeGen/X86/combine-pmadd.ll @@ -331,3 +331,32 @@ define i1 @pmaddwd_pcmpgt_infinite_loop() { %8 = icmp eq i4 %7, 0 ret i1 %8 } + +; If the shuffle matches, but there is no multiply, introduce a trivial multiply by 1. +define <8 x i32> @sext_pairwise_add(<16 x i16> %x) { +; SSE-LABEL: sext_pairwise_add: +; SSE: # %bb.0: +; SSE-NEXT: pmovsxbw {{.*#+}} xmm2 = [1,1,1,1,1,1,1,1] +; SSE-NEXT: pmaddwd %xmm2, %xmm0 +; SSE-NEXT: pmaddwd %xmm2, %xmm1 +; SSE-NEXT: retq +; +; AVX1-LABEL: sext_pairwise_add: +; AVX1: # %bb.0: +; AVX1-NEXT: vextractf128 $1, %ymm0, %xmm1 +; AVX1-NEXT: vbroadcastss {{.*#+}} xmm2 = [1,1,1,1,1,1,1,1] +; AVX1-NEXT: vpmaddwd %xmm2, %xmm1, %xmm1 +; AVX1-NEXT: vpmaddwd %xmm2, %xmm0, %xmm0 +; AVX1-NEXT: vinsertf128 $1, %xmm1, %ymm0, %ymm0 +; AVX1-NEXT: retq +; +; AVX2-LABEL: sext_pairwise_add: +; AVX2: # %bb.0: +; AVX2-NEXT: vpmaddwd {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %ymm0, %ymm0 # [1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1] +; AVX2-NEXT: retq + %1 = sext <16 x i16> %x to <16 x i32> + %2 = shufflevector <16 x i32> %1, <16 x i32> poison, <8 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14> + %3 = shufflevector <16 x i32> %1, <16 x i32> poison, <8 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15> + %4 = add nsw <8 x i32> %2, %3 + ret <8 x i32> %4 +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
