[PATCH] D54911: [compiler-rt][builtins][PowerPC] Add ___fixunstfti builtin compiler-rt method support for PowerPC

[Robert Enenkel via Phabricator via llvm-commits]

renenkel added a comment.

I don't understand why you need the if statement in SCALE_DOUBLE.  Since you are converting to unsigned int128, I don't know why you cast to (unsigned long long) in one case and (long long) in the other.  Check this with Masoud, but I think you can do both the same way.
SCALE_DOUBLE deserves more explanatory comments.  
doubleType is confusing.  It's not a type.  It's either 0 or 1 for the high of low double.
ldStructure doesn't appear in the macro arguments, but is assumed, which is confusing.
I've also changed the union fields according to my other comment.
The name SCALE_DOUBLE is misleading, since, although it does scale, it really converts a double to 64-bit int.
Operand order is inconsistent, the output is one of the middle arguments.
How about this?

// Scale selected double of input ld union to allow conversion to 64-bit int.
// hilo selects the desired double:  0=hi double, 1=lo double.
// exponent = unbiased exponent of selected double.
// result is ld.d[hilo] scaled to have exponent 53, converted to 64-bit int, and shifted to undo scaling.
#define DOUBLE_TO_INT64 (ld, hilo, exponent, result)                             \

  shift = exponent - 54;                                                       \
  ld.ull[hilo] &= 0x800FFFFFFFFFFFFFll;           \
  ld.ull[hilo] |= 0x4350000000000000ll;           \                                                \
  result = (unsigned long long)ld.d[hilo];          \                                                                      \
  result <<= shift;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54911/new/

https://reviews.llvm.org/D54911