fpetrogalli created this revision.
fpetrogalli added a reviewer: ABataev.
Herald added subscribers: cfe-commits, danielkiss, guansong, kristof.beyls,
yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
fpetrogalli added a comment.
Hello reviewers,
I know this is not how the fix should be tested (`fprintf` in debug builds...).
This function would benefit of some unitests in the `unittests` folder of
clang, but I don't have a way to export it there as it is private to this
module.
I would like to lift it to some class (as a static function of
`CodeGenFunction`, for example), but that would require exposing the
`ParamAttrTy`. Are you OK with that? I'd rather use the `llvm::VFParameter` of
`llvm/Analisys/VectorUtils.h` as I suggested in
http://lists.llvm.org/pipermail/llvm-dev/2020-April/141057.html, but that would
definitely require a first patch work to remove the uses of `ParamAttrTy` in
favor of `llvm::VFParameter`.
I am open to alternative suggestions, of course!
Kind regards,
Francesco
This change fixes an aarch64-specific bug in the generation of the NDS
and WDS values used to compute the signature of the vector functions
out of OpenMP directives like `declare simd`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78969
Files:
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/declare_simd_aarch64_ndswds.c
Index: clang/test/OpenMP/declare_simd_aarch64_ndswds.c
===================================================================
--- /dev/null
+++ clang/test/OpenMP/declare_simd_aarch64_ndswds.c
@@ -0,0 +1,20 @@
+// REQUIRES: aarch64-registered-target
+// REQUIRES: asserts
+// -fopemp and -fopenmp-simd behavior are expected to be the same.
+
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp -x
c -emit-llvm %s -o - -femit-all-decls 2>&1 | FileCheck %s --check-prefix=AARCH64
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon
-fopenmp-simd -x c -emit-llvm %s -o - -femit-all-decls 2>&1 | FileCheck %s
--check-prefix=AARCH64
+
+#pragma omp declare simd linear(sin) linear(cos) notinbranch
+void SineCosineWithFloat(float in, float *sin, float *cos);
+// AARCH64-DAG: getNDSWDS SineCosineWithFloat 32 32
+
+#pragma omp declare simd notinbranch
+void SineCosineNoLinear(float in, float *sin, float *cos);
+// AARCH64-DAG: getNDSWDS SineCosineNoLinear 32 64
+
+static float *F;
+void do_something() {
+ SineCosineWithFloat(*F, F, F);
+ SineCosineNoLinear(*F, F, F);
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===================================================================
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -11086,7 +11086,7 @@
/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
/// TODO: Add support for references, section 3.2.1, item 1.
static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext &C) {
- if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
+ if (!getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
QualType PTy = QT.getCanonicalType()->getPointeeType();
if (getAArch64PBV(PTy, C))
return C.getTypeSize(PTy);
@@ -11129,9 +11129,15 @@
}) &&
"Invalid size");
- return std::make_tuple(*std::min_element(std::begin(Sizes), std::end(Sizes)),
- *std::max_element(std::begin(Sizes), std::end(Sizes)),
- OutputBecomesInput);
+ const auto Ret =
+ std::make_tuple(*std::min_element(std::begin(Sizes), std::end(Sizes)),
+ *std::max_element(std::begin(Sizes), std::end(Sizes)),
+ OutputBecomesInput);
+#ifndef NDEBUG
+ fprintf(stderr, "getNDSWDS %s %d %d\n", FD->getNameAsString().c_str(),
+ std::get<0>(Ret), std::get<1>(Ret));
+#endif
+ return Ret;
}
/// Mangle the parameter part of the vector function name according to
Index: clang/test/OpenMP/declare_simd_aarch64_ndswds.c
===================================================================
--- /dev/null
+++ clang/test/OpenMP/declare_simd_aarch64_ndswds.c
@@ -0,0 +1,20 @@
+// REQUIRES: aarch64-registered-target
+// REQUIRES: asserts
+// -fopemp and -fopenmp-simd behavior are expected to be the same.
+
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp -x c -emit-llvm %s -o - -femit-all-decls 2>&1 | FileCheck %s --check-prefix=AARCH64
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp-simd -x c -emit-llvm %s -o - -femit-all-decls 2>&1 | FileCheck %s --check-prefix=AARCH64
+
+#pragma omp declare simd linear(sin) linear(cos) notinbranch
+void SineCosineWithFloat(float in, float *sin, float *cos);
+// AARCH64-DAG: getNDSWDS SineCosineWithFloat 32 32
+
+#pragma omp declare simd notinbranch
+void SineCosineNoLinear(float in, float *sin, float *cos);
+// AARCH64-DAG: getNDSWDS SineCosineNoLinear 32 64
+
+static float *F;
+void do_something() {
+ SineCosineWithFloat(*F, F, F);
+ SineCosineNoLinear(*F, F, F);
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===================================================================
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -11086,7 +11086,7 @@
/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
/// TODO: Add support for references, section 3.2.1, item 1.
static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext &C) {
- if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
+ if (!getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
QualType PTy = QT.getCanonicalType()->getPointeeType();
if (getAArch64PBV(PTy, C))
return C.getTypeSize(PTy);
@@ -11129,9 +11129,15 @@
}) &&
"Invalid size");
- return std::make_tuple(*std::min_element(std::begin(Sizes), std::end(Sizes)),
- *std::max_element(std::begin(Sizes), std::end(Sizes)),
- OutputBecomesInput);
+ const auto Ret =
+ std::make_tuple(*std::min_element(std::begin(Sizes), std::end(Sizes)),
+ *std::max_element(std::begin(Sizes), std::end(Sizes)),
+ OutputBecomesInput);
+#ifndef NDEBUG
+ fprintf(stderr, "getNDSWDS %s %d %d\n", FD->getNameAsString().c_str(),
+ std::get<0>(Ret), std::get<1>(Ret));
+#endif
+ return Ret;
}
/// Mangle the parameter part of the vector function name according to
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
