https://github.com/dmikushin updated https://github.com/llvm/llvm-project/pull/202292
>From 38dba947bb84cde82364ba06fa2d7630502fff03 Mon Sep 17 00:00:00 2001 From: Dmitry Mikushin <[email protected]> Date: Sun, 7 Jun 2026 00:39:55 +0200 Subject: [PATCH] [Clang][OpenMP] Fix '*' reduction identity for class types Clang accepts OpenMP built-in reductions over C++ class types such as std::complex<T> as an extension (the OpenMP standard only defines the implicitly-declared reduction identities for arithmetic and, in Clang, for _Complex types; GCC rejects the construct outright). For such class types the private reduction copy was left value-initialized, which yields the *additive* identity (e.g. std::complex(0,0)). For '+' this happens to be correct, but for '*' it is wrong: every thread starts from (0,0), so the product collapses to (0,0) on the host -- even with a single thread, since this is purely a wrong identity element and not a parallel-combine problem. Fix the BO_Mul case in actOnOMPReductionKindClause to also synthesize the integer-literal '1' initializer for C++ record types, mirroring the existing scalar/_Complex path. The converting constructor then builds the multiplicative identity (e.g. std::complex(1) == (1,0)). If the type is not constructible from '1', AddInitializerToDecl diagnoses it and the list item is dropped, so we never silently miscompile (no ICE). BO_Add is intentionally left relying on value-initialization for class types, because there the (0,0) default is the correct additive identity in the common case. Adds a codegen test verifying the private copy is initialized to (1,0) for '*' and (0,0) for '+', a -verify test covering the diagnostic for a class type that is not constructible from '1', and a release note. --- clang/docs/ReleaseNotes.rst | 4 ++ clang/lib/Sema/SemaOpenMP.cpp | 21 ++++++++- .../for_reduction_class_identity_codegen.cpp | 43 +++++++++++++++++++ .../for_reduction_class_identity_messages.cpp | 40 +++++++++++++++++ 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 clang/test/OpenMP/for_reduction_class_identity_codegen.cpp create mode 100644 clang/test/OpenMP/for_reduction_class_identity_messages.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index bf917b3f642bc..25a2e9e59e9b3 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -969,6 +969,10 @@ OpenMP Support ``fallback`` modifier (``fb_nullify`` or ``fb_preserve``) with OpenMP >= 61. - Added support for ``local`` clause with declare_target directive when OpenMP >= 60. +- Fixed the identity element used for ``reduction(* : x)`` over C++ class types + (e.g. ``std::complex``). The private copy is now initialized to the + multiplicative identity instead of being value-initialized, which previously + produced a wrong result (the product collapsed to the additive identity). SYCL Support ------------ diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 76b40a5039180..1d11776824976 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -20911,9 +20911,28 @@ static bool actOnOMPReductionKindClause( Init = S.ActOnIntegerConstant(ELoc, /*Val=*/0).get(); break; case BO_Mul: + // '*' reduction op - initializer is '1'. + // For C++ class types (e.g. std::complex) the OpenMP built-in + // reduction identifiers are an extension: the standard only defines + // identities for arithmetic (and, in Clang, _Complex) types. Without + // an explicit initializer the private copy would be value-initialized, + // which yields the *additive* identity (e.g. std::complex(0,0)) and is + // wrong for multiplication. Initialize from the integer literal '1' + // instead and let the converting constructor build the multiplicative + // identity (e.g. std::complex(1) == (1,0)). If the type is not + // constructible from '1', AddInitializerToDecl below diagnoses it and + // the list item is dropped, so we never silently miscompile. + // Note: BO_Add deliberately keeps relying on value-initialization for + // class types, because there the (0,0) default is the correct additive + // identity for the common case; only '*' needs this special handling. + if (Type->isScalarType() || Type->isAnyComplexType() || + (S.getLangOpts().CPlusPlus && Type->isRecordType())) { + Init = S.ActOnIntegerConstant(ELoc, /*Val=*/1).get(); + } + break; case BO_LAnd: if (Type->isScalarType() || Type->isAnyComplexType()) { - // '*' and '&&' reduction ops - initializer is '1'. + // '&&' reduction ops - initializer is '1'. Init = S.ActOnIntegerConstant(ELoc, /*Val=*/1).get(); } break; diff --git a/clang/test/OpenMP/for_reduction_class_identity_codegen.cpp b/clang/test/OpenMP/for_reduction_class_identity_codegen.cpp new file mode 100644 index 0000000000000..34640f51b31b2 --- /dev/null +++ b/clang/test/OpenMP/for_reduction_class_identity_codegen.cpp @@ -0,0 +1,43 @@ +// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s +// expected-no-diagnostics + +struct Complex { + double Re; + double Im; + Complex(double Re = 0.0, double Im = 0.0) : Re(Re), Im(Im) {} + Complex &operator+=(const Complex &RHS) { + Re += RHS.Re; + Im += RHS.Im; + return *this; + } + Complex &operator*=(const Complex &RHS) { + double NewRe = Re * RHS.Re - Im * RHS.Im; + Im = Re * RHS.Im + Im * RHS.Re; + Re = NewRe; + return *this; + } +}; + +void add_reduction(Complex *Data) { + Complex Sum; +#pragma omp parallel for reduction(+ : Sum) + for (int I = 0; I < 4; ++I) + Sum += Data[I]; +} + +void mul_reduction(Complex *Data) { + Complex Product; +#pragma omp parallel for reduction(* : Product) + for (int I = 0; I < 4; ++I) + Product *= Data[I]; +} + +// CHECK-LABEL: define {{.*}}void @_Z13add_reductionP7Complex +// CHECK: call {{.*}} @_ZN7ComplexC1Edd(ptr {{[^,]*}}%{{[^,]*}}, double {{.*}}0.000000e+00, double {{.*}}0.000000e+00) +// CHECK-LABEL: define internal {{.*}}void @_Z13add_reductionP7Complex.omp_outlined +// CHECK: call {{.*}} @_ZN7ComplexC1Edd(ptr {{[^,]*}}%{{[^,]*}}, double {{.*}}0.000000e+00, double {{.*}}0.000000e+00) + +// CHECK-LABEL: define {{.*}}void @_Z13mul_reductionP7Complex +// CHECK: call {{.*}} @_ZN7ComplexC1Edd(ptr {{[^,]*}}%{{[^,]*}}, double {{.*}}0.000000e+00, double {{.*}}0.000000e+00) +// CHECK-LABEL: define internal {{.*}}void @_Z13mul_reductionP7Complex.omp_outlined +// CHECK: call {{.*}} @_ZN7ComplexC1Edd(ptr {{[^,]*}}%{{[^,]*}}, double {{.*}}1.000000e+00, double {{.*}}0.000000e+00) diff --git a/clang/test/OpenMP/for_reduction_class_identity_messages.cpp b/clang/test/OpenMP/for_reduction_class_identity_messages.cpp new file mode 100644 index 0000000000000..7cf75ad641d93 --- /dev/null +++ b/clang/test/OpenMP/for_reduction_class_identity_messages.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux-gnu -ferror-limit 100 %s +// RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-linux-gnu -ferror-limit 100 %s + +// Multiplication reduction over a class type that *is* constructible from the +// integer literal '1' (e.g. std::complex-like) is accepted: the multiplicative +// identity is built via the converting constructor. See +// for_reduction_class_identity_codegen.cpp for the value check. +struct WithInt { + double V; + WithInt(int X = 1) : V(X) {} + WithInt &operator*=(const WithInt &RHS) { + V *= RHS.V; + return *this; + } +}; + +void ok(WithInt *Data) { + WithInt Product; +#pragma omp parallel for reduction(* : Product) + for (int I = 0; I < 4; ++I) + Product *= Data[I]; +} + +// Multiplication reduction over a class type that is NOT constructible from '1' +// must be diagnosed rather than silently miscompiled (it previously fell back +// to value-initialization, producing the wrong (additive) identity). +struct NoInt { // expected-note 2 {{candidate constructor}} + double V; + NoInt &operator*=(const NoInt &RHS) { + V *= RHS.V; + return *this; + } +}; + +void bad(NoInt *Data) { + NoInt Product; +#pragma omp parallel for reduction(* : Product) // expected-error {{no viable conversion from 'int' to 'NoInt'}} + for (int I = 0; I < 4; ++I) + Product *= Data[I]; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
