__builtin_signbit fix for ppcf128 type

[abeserminji]

Summary

Function__builtin_signbit returns wrong value for type ppcf128 on big endian machines. This patch fixes how value is generated in that case, and generate correct value.

[rjmccall]

Just a few comment suggestions, but functionally LGTM.

lib/CodeGen/CGBuiltin.cpp

246 ↗

(On Diff #38660)

Hmm, let's merge these the old and new comments a bit:

We want the sign bit of the higher-order double. The bitcast we just
did works as if the double-double was stored to memory and then
read as an i128. The "store" will put the higher-order double in the
lower address in both little- and big-Endian modes, but the "load"
will treat those bits as a different part of the i128: the low bits in
little-Endian, the high bits in big-Endian. Therefore, on big-Endian
we need to shift the high bits down to the low before truncating.

252 ↗

(On Diff #38660)

Typo: dobule.

Also, we're not quite extracting the sign yet; we're truncating as a way to extract the higher-order double, which we'll extract the sign from in a second.

[abeserminji]

Modified comments as suggested by @rjmccall

[rjmccall]

LGTM, thanks!

[abeserminji]

Comment modified

cfe/trunk/lib/CodeGen/CGBuiltin.cpp

 static Value *EmitSignBit(CodeGenFunction &CGF, Value *V) {
   LLVMContext &C = CGF.CGM.getLLVMContext();
 
   llvm::Type *Ty = V->getType();
   int Width = Ty->getPrimitiveSizeInBits();
   llvm::Type *IntTy = llvm::IntegerType::get(C, Width);
   V = CGF.Builder.CreateBitCast(V, IntTy);
   if (Ty->isPPC_FP128Ty()) {
-    // The higher-order double comes first, and so we need to truncate the
-    // pair to extract the overall sign. The order of the pair is the same
-    // in both little- and big-Endian modes.
+    // We want the sign bit of the higher-order double. The bitcast we just
+    // did works as if the double-double was stored to memory and then
+    // read as an i128. The "store" will put the higher-order double in the
+    // lower address in both little- and big-Endian modes, but the "load"
+    // will treat those bits as a different part of the i128: the low bits in
+    // little-Endian, the high bits in big-Endian. Therefore, on big-Endian
+    // we need to shift the high bits down to the low before truncating.
     Width >>= 1;
+    if (CGF.getTarget().isBigEndian()) {
+      Value *ShiftCst = llvm::ConstantInt::get(IntTy, Width);
+      V = CGF.Builder.CreateLShr(V, ShiftCst);
+    } 
+    // We are truncating value in order to extract the higher-order 
+    // double, which we will be using to extract the sign from.
     IntTy = llvm::IntegerType::get(C, Width);
     V = CGF.Builder.CreateTrunc(V, IntTy);
   }
   Value *Zero = llvm::Constant::getNullValue(IntTy);
   return CGF.Builder.CreateICmpSLT(V, Zero);
 }
 
 static RValue emitLibraryCall(CodeGenFunction &CGF, const FunctionDecl *Fn,

cfe/trunk/test/Analysis/builtin_signbit.cpp

+// RUN: %clang -target powerpc-linux-gnu     -emit-llvm -S -O0 %s -o - | FileCheck %s --check-prefix=CHECK-BE --check-prefix=CHECK
+// RUN: %clang -target powerpc64-linux-gnu   -emit-llvm -S -O0 %s -o - | FileCheck %s --check-prefix=CHECK-BE --check-prefix=CHECK
+// RUN: %clang -target powerpc64le-linux-gnu -emit-llvm -S -O0 %s -o - | FileCheck %s --check-prefix=CHECK-LE --check-prefix=CHECK
+
+bool b;
+double d = -1.0;
+long double ld = -1.0L;
+void test_signbit()
+{
+  b = __builtin_signbit(1.0L);
+  // CHECK: i128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+
+  b = __builtin_signbit(ld);
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+
+  b = __builtin_signbitf(1.0);
+  // CHECK: store i8 0
+
+  b = __builtin_signbitf(d);
+  // CHECK: bitcast
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE-NOT: lshr
+
+  b = __builtin_signbitl(1.0L);
+  // CHECK: i128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+
+  b = __builtin_signbitl(ld);
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+}