[Constants][PowerPC] Check exactlyValue for ppc_fp128 in isNullValue

[jsji]

Summary

PPC_FP128 determines isZero/isNan/isInf using high-order double value
only. Checking isZero/isNegative might return the isNullValue unexpectedly.
eg:

0xM0000000000000000FFFFFFFFFFFFFFFFF

isZero, but it is not NullValue.

[efriedma]

If Constant::isNullValue() isn't returning the right result, please fix it directly. Getting it wrong could have other unexpected consequences.

[jsji]

In D103634#2796960, @efriedma wrote:

If Constant::isNullValue() isn't returning the right result, please fix it directly. Getting it wrong could have other unexpected consequences.

Thanks @efriedma for the comments.

In this case Constant::isNullValue() is correct, as the value is indeed a NullValue for ppc_fp128.
per definition of a long double in 3.1.5. Extended Precision https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html

Yes, we will need to deal with this special cases carefully in other optimizations as well to avoid unexpected consequences.

The fix is for BitcodeWriters, so that we won't end up clear bits unexpected, which causes unexpected different behaviors using bitcode vs. IR.

[efriedma]

Constant::isNullValue() is supposed to mean "is this value all zero bits"; various parts of the code use it that way. It shouldn't care whether the value is semantically equal to zero. Anything doing floating-point arithmetic should be using the appropriate ConstantFP/APFloat methods.

[jsji]

In D103634#2797005, @efriedma wrote:

Constant::isNullValue() is supposed to mean "is this value all zero bits"; various parts of the code use it that way. It shouldn't care whether the value is semantically equal to zero. Anything doing floating-point arithmetic should be using the appropriate ConstantFP/APFloat methods.

Oh... Thanks for the info, I will check and update.

[jsji]

Fix isNullValue for ppc_fp128.

[efriedma]

LGTM with one minor comment.

llvm/lib/IR/Constants.cpp

96 ↗

(On Diff #349648)

We can use isExactlyValue() for all floating-point types; no reason to have two separate codepaths.

[jsji]

Use isExactlyValue() for all floating-point types.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

if (const InlineAsm *IA = dyn_cast<InlineAsm>(V)) {
       Record.clear();
       continue;
     }
     const Constant *C = cast<Constant>(V);
     unsigned Code = -1U;
     unsigned AbbrevToUse = 0;
     if (C->isNullValue()) {
       Code = bitc::CST_CODE_NULL;
+      const ConstantFP *CFP = dyn_cast<ConstantFP>(C);
+      // PPC_FP128 determines isZero/isNan/isInf using high-order double value
+      // only. If we write NULL bits directly, we may clear low-order double
+      // values unexpectedly.
+      // eg:
+      //    0xM0000000000000000FFFFFFFFFFFFFFFFF
+      // => 0xM000000000000000000000000000000000
+      if (CFP && CFP->getType()->isPPC_FP128Ty()) {
+        Code = bitc::CST_CODE_FLOAT;
+        APInt api = CFP->getValueAPF().bitcastToAPInt();
+        const uint64_t *p = api.getRawData();
+        Record.push_back(p[0]);
+        Record.push_back(p[1]);
+      }
     } else if (isa<PoisonValue>(C)) {
       Code = bitc::CST_CODE_POISON;
     } else if (isa<UndefValue>(C)) {
       Code = bitc::CST_CODE_UNDEF;
     } else if (const ConstantInt *IV = dyn_cast<ConstantInt>(C)) {
       if (IV->getBitWidth() <= 64) {
         uint64_t V = IV->getSExtValue();
         emitSignedInt64(Record, V);

llvm/test/CodeGen/PowerPC/ppc_fp128-bcwriter.ll

 ; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s
 
 ;CHECK-LABEL: main
-;CHECK: store ppc_fp128 0xM0000000000000000000000000000000
+;CHECK: store ppc_fp128 0xM0000000000000000FFFFFFFFFFFFFFFF
 
 define i32 @main() local_unnamed_addr {
 _main_entry:
   %e3 = alloca ppc_fp128, align 16
   store ppc_fp128 0xM0000000000000000FFFFFFFFFFFFFFFF, ppc_fp128* %e3, align 16
   %0 = call i64 @foo( ppc_fp128* nonnull %e3)
   ret i32 undef
 }
 
 declare i64 @foo(ppc_fp128 *)