[Clang] Don't adjust align for IBM extended double type

[hubert.reinterpretcast]

clang/lib/CodeGen/TargetInfo.cpp

5055

s/which is/which are/;

and

use getFloatTypeSemantics to reuse logic (including the possible need to handle OpenMP host versus device differences):

} else if (Ty->isRealFloatingType() &&
           getContext().getFloatTypeSemantics(Ty) ==
               &llvm::APFloat::IEEEquad()) {
  // IEEE 128-bit floating numbers are also stored in vector registers, which
  // are quad-word aligned.
  return CharUnits::fromQuantity(16);
}

[qiucf]

use getFloatTypeSemantics

[hubert.reinterpretcast]

clang/test/CodeGen/ppc64le-varargs-f128.c

67–68

Perhaps the argument to the intrinsic should capture a string variable that is used in the later lines to establish the expected relationship of the later lines to this one.

67–68

See comment below.

[jsji]

clang/lib/CodeGen/TargetInfo.cpp

5055

It would be better to add a testcase for OpenMP host/device format mismatch.

5058

I think it is better to refer to ABI rules instead of referring to vector registers.

Section 2.2.3.3. Optional Save Areas
If extended precision floating-point values in IEEE BINARY 128 QUADRUPLE PRECISION format are supported, map them to a single quadword, quadword aligned. This might result in skipped doublewords in the Parameter Save Area.

[qiucf]

Address comments

• Update comment to refer to ABI
• Add test case for OpenMP host & target mismatch

[jsji]

LGTM. Thanks.

[MaskRay]

clang/test/CodeGen/ppc64le-varargs-f128.c

8

Generally %clang is only used in test/Driver and %clang_cc1 should be used for tests.

%clang_cc1 has a lit substitution rule to find the builtin include directory where stdarg.h can be found.

[qiucf]

clang/test/CodeGen/ppc64le-varargs-f128.c

8

Thanks! I moved it in rG222da77a

[MaskRay]

clang/test/CodeGen/ppc64le-varargs-f128.c

8

rG222da77a82d17cbc6b989779e2ba2bb4904bb672 looks more wrong to me: test/Driver tests how the clang driver passes CC1 options to the frontend. The CodeGen should be tested in this directory.

You probably should split %clang to several %clang_cc1 commands and keep the test here.

[hubert.reinterpretcast]

clang/test/CodeGen/ppc64le-varargs-f128.c

8

I don't think moving the test such that a "driver test" checks code gen output is the direction @MaskRay was pointing to. Changing the RUN line to invoke -cc1 (using %clang_cc1) and leaving the test as a code gen test is what I would have expected.

[qiucf]

clang/test/CodeGen/ppc64le-varargs-f128.c

8

Ah, thanks for pointing it out. I simplified the splitted commands from clang -v. https://reviews.llvm.org/D92544 is created for easier view, if that looks fine, I'll commit it :-)

clang/lib/CodeGen/TargetInfo.cpp

CharUnits PPC64_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // passed via reference, smaller types are not aligned).
   if (IsQPXVectorTy(Ty)) {
     if (getContext().getTypeSize(Ty) > 128)
       return CharUnits::fromQuantity(32);
 
     return CharUnits::fromQuantity(16);
   } else if (Ty->isVectorType()) {
     return CharUnits::fromQuantity(getContext().getTypeSize(Ty) == 128 ? 16 : 8);
-  } else if (Ty->isRealFloatingType() && getContext().getTypeSize(Ty) == 128) {
-    // IEEE 128-bit floating numbers are also stored in vector registers.
-    // And both IEEE quad-precision and IBM extended double (ppc_fp128) should
-    // be quad-word aligned.
+  } else if (Ty->isRealFloatingType() &&
+             &getContext().getFloatTypeSemantics(Ty) ==
+                 &llvm::APFloat::IEEEquad()) {
+    // According to ABI document section 'Optional Save Areas': If extended
+    // precision floating-point values in IEEE BINARY 128 QUADRUPLE PRECISION
+    // format are supported, map them to a single quadword, quadword aligned.
     return CharUnits::fromQuantity(16);
   }
 
   // For single-element float/vector structs, we consider the whole type
   // to have the same alignment requirements as its single element.
   const Type *AlignAsType = nullptr;
   const Type *EltType = isSingleElementStruct(Ty, getContext());
   if (EltType) {

clang/test/CodeGen/ppc64le-varargs-f128.c

 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
 // RUN:   -target-cpu pwr9 -target-feature +float128 -mabi=ieeelongdouble \
 // RUN:   -o - %s | FileCheck %s -check-prefix=IEEE
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
 // RUN:   -target-cpu pwr9 -target-feature +float128 \
 // RUN:   -o - %s | FileCheck %s -check-prefix=IBM
 
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -fopenmp-targets=ppc64le -mfloat128 -mabi=ieeelongdouble -mcpu=pwr9 \
+// RUN:   -Xopenmp-target=ppc64le -mcpu=pwr9 -Xopenmp-target=ppc64le \
+// RUN:   -mfloat128 -fopenmp=libomp -o - %s | FileCheck %s -check-prefix=OMP
+
 #include <stdarg.h>
 
-// IEEE-LABEL: define fp128 @f128(i32 signext %n, ...)
-// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
-// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
-// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
-// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
-// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
-// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
-// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
-__float128 f128(int n, ...) {
+void foo_ld(long double);
+void foo_fq(__float128);
+
+// Verify cases when OpenMP target's and host's long-double semantics differ.
+
+// OMP-LABEL: define internal void @.omp_outlined.
+// OMP: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8**
+// OMP: %[[V2:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to ppc_fp128*
+// OMP: %[[V3:[0-9a-zA-Z_.]+]] = load ppc_fp128, ppc_fp128* %[[V2]], align 8
+// OMP: call void @foo_ld(ppc_fp128 %[[V3]])
+
+// OMP-LABEL: define dso_local void @omp
+// OMP: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
+// OMP: call void @llvm.va_start(i8* %[[AP1]])
+// OMP: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]], align 8
+// OMP: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// OMP: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
+// OMP: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
+// OMP: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
+// OMP: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
+// OMP: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
+// OMP: call void @foo_ld(fp128 %[[V4]])
+void omp(int n, ...) {
   va_list ap;
   va_start(ap, n);
-  __float128 x = va_arg(ap, __float128);
+  foo_ld(va_arg(ap, long double));
+  #pragma omp target parallel
+  for (int i = 1; i < n; ++i) {
+    foo_ld(va_arg(ap, long double));
+  }
   va_end(ap);
-  return x;
 }
 
-// IEEE-LABEL: define fp128 @long_double(i32 signext %n, ...)
-// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
-// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
-// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
-// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
-// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
-// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
-// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
-
-// IBM-LABEL: define ppc_fp128 @long_double(i32 signext %n, ...)
-// IBM: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
-// IBM: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
-// IBM: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
-// IBM: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
-// IBM: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to ppc_fp128*
-// IBM: %{{[0-9a-zA-Z_.]+}} = load ppc_fp128, ppc_fp128* %[[P4]], align 16
-// IBM: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
-long double long_double(int n, ...) {
+// IEEE-LABEL: define void @f128
+// IEEE: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
+// IEEE: call void @llvm.va_start(i8* %[[AP1]])
+// IEEE: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]]
+// IEEE: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// IEEE: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
+// IEEE: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
+// IEEE: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
+// IEEE: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
+// IEEE: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
+// IEEE: call void @foo_fq(fp128 %[[V4]])
+// IEEE: %[[AP2:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP]] to i8*
+// IEEE: call void @llvm.va_end(i8* %[[AP2]])
+void f128(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  foo_fq(va_arg(ap, __float128));
+  va_end(ap);
+}
+
+// IEEE-LABEL: define void @long_double
+// IEEE: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
+// IEEE: call void @llvm.va_start(i8* %[[AP1]])
+// IEEE: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]]
+// IEEE: %[[V0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// IEEE: %[[V1:[0-9a-zA-Z_.]+]] = add i64 %[[V0]], 15
+// IEEE: %[[V2:[0-9a-zA-Z_.]+]] = and i64 %[[V1]], -16
+// IEEE: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[V2]] to i8*
+// IEEE: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to fp128*
+// IEEE: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[V3]], align 16
+// IEEE: call void @foo_ld(fp128 %[[V4]])
+// IEEE: %[[AP2:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP]] to i8*
+// IEEE: call void @llvm.va_end(i8* %[[AP2]])
+
+// IBM-LABEL: define void @long_double
+// IBM: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
+// IBM: call void @llvm.va_start(i8* %[[AP1]])
+// IBM: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]]
+// IBM: %[[V3:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to ppc_fp128*
+// IBM: %[[V4:[0-9a-zA-Z_.]+]] = load ppc_fp128, ppc_fp128* %[[V3]], align 8
+// IBM: call void @foo_ld(ppc_fp128 %[[V4]])
+// IBM: %[[AP2:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP]] to i8*
+// IBM: call void @llvm.va_end(i8* %[[AP2]])
+void long_double(int n, ...) {
   va_list ap;
   va_start(ap, n);
-  long double x = va_arg(ap, long double);
+  foo_ld(va_arg(ap, long double));
   va_end(ap);
-  return x;
 }