[clang][OpenMP] Fix getNDSWDS for aarch64.

[fpetrogalli]

Summary

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. When the directive is used in conjunction with the linear clause, the size of the pointee must be used instead of the size of the pointer to compute NDS and WDS.

The code-fix is strictly related to the behavior for linear, but given that the only way we have to test the NDS and WDS values is to check the resulting <vlen> token in the mangled name of the vector function, the tests have been extended to cover all the possible values of WDS and NDS as defined in the ABI at https://github.com/ARM-software/abi-aa/tree/master/vfabia64.

[fpetrogalli]

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

[jdoerfert]

Is the NDS and WDS number never visible in the IR, e.g., as part of the name?

[fpetrogalli]

In D78969#2012591, @jdoerfert wrote:

Is the NDS and WDS number never visible in the IR, e.g., as part of the name?

Hang on, thanks for asking the question, maybe I can work something out here! :)

They won't be visible in IR directly, but they are part of an expression that is used to compute the number of lanes in the mangled name.

I'll come up with an IR check.

Thanks!

Francesco

[fpetrogalli]

I have added (indirect) testing of the values of NDS and WDS.

[fpetrogalli]

Removed single use variable definition.

[jdoerfert]

LGTM.

clang/lib/CodeGen/CGOpenMPRuntime.cpp

static bool getAArch64PBV(QualType QT, ASTContext &C) {
 
   return false;
 }
 
 /// Computes the lane size (LS) of a return type or of an input parameter,
 /// 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);
   }
   if (getAArch64PBV(QT, C))
     return C.getTypeSize(QT);
 
   return C.getTypeSize(C.getUIntPtrType());

clang/test/OpenMP/aarch64_vfabi_NarrowestDataSize.c

+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp      -x c -emit-llvm %s -o - -femit-all-decls | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon -fopenmp-simd -x c -emit-llvm %s -o - -femit-all-decls | FileCheck %s
+
+// REQUIRES: aarch64-registered-target
+// Note: -fopemp and -fopenmp-simd behavior are expected to be the same.
+
+// This test checks the values of Narrowest Data Size (NDS), as defined in
+// https://github.com/ARM-software/abi-aa/tree/master/vfabia64
+//
+// NDS is used to compute the <vlen> token in the name of AdvSIMD
+// vector functions when no `simdlen` is specified, with the rule:
+//
+// if NDS(f) = 1, then VLEN = 16, 8;
+// if NDS(f) = 2, then VLEN = 8, 4;
+// if NDS(f) = 4, then VLEN = 4, 2;
+// if NDS(f) = 8 or NDS(f) = 16, then VLEN = 2.
+
+// NDS(NDS_is_sizeof_char) = 1
+#pragma omp declare simd notinbranch
+char NDS_is_sizeof_char(short in);
+// CHECK-DAG: _ZGVnN16v_NDS_is_sizeof_char
+// CHECK-DAG: _ZGVnN8v_NDS_is_sizeof_char
+// CHECK-NOT: _ZGV{{.*}}_NDS_is_sizeof_char
+
+// NDS(NDS_is_sizeof_short) = 2
+#pragma omp declare simd notinbranch
+int NDS_is_sizeof_short(short in);
+// CHECK-DAG: _ZGVnN8v_NDS_is_sizeof_short
+// CHECK-DAG: _ZGVnN4v_NDS_is_sizeof_short
+// CHECK-NOT: _ZGV{{.*}}_NDS_is_sizeof_short
+
+// NDS(NDS_is_sizeof_float_with_linear) = 4, and not 2, because the pointers are
+// marked as `linear` and therefore the size of the pointee realizes
+// the NDS.
+#pragma omp declare simd linear(sin) notinbranch
+void NDS_is_sizeof_float_with_linear(double in, float *sin);
+// Neon accepts only power of 2 values as <vlen>.
+// CHECK-DAG: _ZGVnN4vl4_NDS_is_sizeof_float_with_linear
+// CHECK-DAG: _ZGVnN2vl4_NDS_is_sizeof_float_with_linear
+// CHECK-NOT: _ZGV{{.*}}_NDS_is_sizeof_float_with_linear
+
+// NDS(NDS_is_size_of_float) = 4
+#pragma omp declare simd notinbranch
+double NDS_is_size_of_float(float in);
+// CHECK-DAG: _ZGVnN4v_NDS_is_size_of_float
+// CHECK-DAG: _ZGVnN2v_NDS_is_size_of_float
+// CHECK-NOT: _ZGV{{.*}}_NDS_is_size_of_float
+
+// NDS(NDS_is_sizeof_double) = 8
+#pragma omp declare simd linear(sin) notinbranch
+void NDS_is_sizeof_double(double in, double *sin);
+// CHECK-DAG: _ZGVnN2vl8_NDS_is_sizeof_double
+// CHECK-NOT: _ZGV{{.*}}_NDS_is_sizeof_double
+
+// NDS(double_complex) = 16
+#pragma omp declare simd notinbranch
+double _Complex double_complex(double _Complex);
+// CHECK-DAG: _ZGVnN2v_double_complex
+// CHECK-NOT: _ZGV{{.*}}_double_complex
+
+// NDS(double_complex_linear_char) = 1, becasue `x` is marked linear.
+#pragma omp declare simd linear(x) notinbranch
+double _Complex double_complex_linear_char(double _Complex y, char *x);
+// CHECK-DAG: _ZGVnN8vl_double_complex_linear_char
+// CHECK-DAG: _ZGVnN16vl_double_complex_linear_char
+// CHECK-NOT: _ZGV{{.*}}_double_complex_linear_char
+
+static float *F;
+static double *D;
+static short S;
+static int I;
+static char C;
+static double _Complex DC;
+void do_something() {
+  C = NDS_is_sizeof_char(S);
+  I = NDS_is_sizeof_short(S);
+  NDS_is_sizeof_float_with_linear(*D, F);
+  *D = NDS_is_size_of_float(*F);
+  NDS_is_sizeof_double(*D, D);
+  DC = double_complex(DC);
+  DC = double_complex_linear_char(DC, &C);
+}

clang/test/OpenMP/aarch64_vfabi_WidestDataSize.c

+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +sve  -fopenmp      -x c -emit-llvm %s -o - -femit-all-decls | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +sve  -fopenmp-simd -x c -emit-llvm %s -o - -femit-all-decls | FileCheck %s
+
+// REQUIRES: aarch64-registered-target
+// Note: -fopemp and -fopenmp-simd behavior are expected to be the same.
+
+// This test checks the values of Widest Data Size (WDS), as defined
+// in https://github.com/ARM-software/abi-aa/tree/master/vfabia64
+//
+// WDS is used to check the accepted values <N> of `simdlen(<N>)` when
+// targeting fixed-length SVE vector function names. The values of
+// `<N>` that are accepted are such that for X = WDS * <N> * 8,
+// 128-bit <= X <= 2048-bit and X is a multiple of 128-bit.
+
+#pragma omp declare simd simdlen(8)
+#pragma omp declare simd simdlen(16)
+#pragma omp declare simd simdlen(256)
+#pragma omp declare simd simdlen(272)
+char WDS_is_sizeof_char(char in);
+// WDS = 1, simdlen(8) and simdlen(272) are not generated.
+// CHECK-DAG: _ZGVsM16v_WDS_is_sizeof_char
+// CHECK-DAG: _ZGVsM256v_WDS_is_sizeof_char
+// CHECK-NOT: _ZGV{{.*}}_WDS_is_sizeof_char
+
+#pragma omp declare simd simdlen(4)
+#pragma omp declare simd simdlen(8)
+#pragma omp declare simd simdlen(128)
+#pragma omp declare simd simdlen(136)
+char WDS_is_sizeof_short(short in);
+// WDS = 2, simdlen(4) and simdlen(136) are not generated.
+// CHECK-DAG: _ZGVsM8v_WDS_is_sizeof_short
+// CHECK-DAG: _ZGVsM128v_WDS_is_sizeof_short
+// CHECK-NOT: _ZGV{{.*}}_WDS_is_sizeof_short
+
+#pragma omp declare simd linear(sin) notinbranch simdlen(2)
+#pragma omp declare simd linear(sin) notinbranch simdlen(4)
+#pragma omp declare simd linear(sin) notinbranch simdlen(64)
+#pragma omp declare simd linear(sin) notinbranch simdlen(68)
+void WDS_is_sizeof_float_pointee(float in, float *sin);
+// WDS = 4, simdlen(2) and simdlen(68) are not generated.
+// CHECK-DAG: _ZGVsM4vl4_WDS_is_sizeof_float_pointee
+// CHECK-DAG: _ZGVsM64vl4_WDS_is_sizeof_float_pointee
+// CHECK-NOT: _ZGV{{.*}}_WDS_is_sizeof_float_pointee
+
+#pragma omp declare simd linear(sin) notinbranch simdlen(2)
+#pragma omp declare simd linear(sin) notinbranch simdlen(4)
+#pragma omp declare simd linear(sin) notinbranch simdlen(32)
+#pragma omp declare simd linear(sin) notinbranch simdlen(34)
+void WDS_is_sizeof_double_pointee(float in, double *sin);
+// WDS = 8 because of the linear clause, simdlen(34) is not generated.
+// CHECK-DAG: _ZGVsM2vl8_WDS_is_sizeof_double_pointee
+// CHECK-DAG: _ZGVsM4vl8_WDS_is_sizeof_double_pointee
+// CHECK-DAG: _ZGVsM32vl8_WDS_is_sizeof_double_pointee
+// CHECK-NOT: _ZGV{{.*}}_WDS_is_sizeof_double_pointee
+
+#pragma omp declare simd simdlen(2)
+#pragma omp declare simd simdlen(4)
+#pragma omp declare simd simdlen(32)
+#pragma omp declare simd simdlen(34)
+double WDS_is_sizeof_double(double in);
+// WDS = 8, simdlen(34) is not generated.
+// CHECK-DAG: _ZGVsM2v_WDS_is_sizeof_double
+// CHECK-DAG: _ZGVsM4v_WDS_is_sizeof_double
+// CHECK-DAG: _ZGVsM32v_WDS_is_sizeof_double
+// CHECK-NOT: _ZGV{{.*}}_WDS_is_sizeof_double
+
+static char C;
+static short S;
+static float F;
+static double D;
+
+void do_something() {
+  C = WDS_is_sizeof_char(C);
+  C = WDS_is_sizeof_short(S);
+  WDS_is_sizeof_float_pointee(F, &F);
+  WDS_is_sizeof_double_pointee(F, &D);
+  D = WDS_is_sizeof_double(D);
+}