[PATCH] D78969: [clang][OpenMP] Fix getNDSWDS for aarch64.

[Francesco Petrogalli via Phabricator via cfe-commits]

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78969.260476.patch
Type: text/x-patch
Size: 2550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200427/6a004d64/attachment.bin>