[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.

In the summary, we're not using DD.h any more, so that struct code should be changed to the new version.



================
Comment at: compiler-rt/lib/builtins/ppc/fixunstfti.c:23
+            (__uint128_t) 0x0000000000000000ll;
+  }
+
----------------
masoud.ataei wrote:
> Here we are returning NaN bit pattern for NaN input. It was originally my suggestion but I still don't know if it is good idea. Maybe it is better to remove this if statement and let the rest of algorithm create an output. In fact, any output is junk. (Since there is no NaN in int.)  
I agree that the integer conversion of a NaN should be undefined.  If we want to match the meaningless number that gcc produces, we can do that.  Or we can do what Masoud did, which was to return the NaN bit pattern.  Or we can return anything else.  Does the C standard say anything about what such a conversion should produce?  Hubert Tong might know that.


================
Comment at: compiler-rt/test/builtins/Unit/ppc/fixunstfti_test.h:14
+};
+
+struct testCase testList[] = {
----------------
amyk wrote:
> nemanjai wrote:
> > It might be more readable to define a macro such as:
> > `#define INIT_U128(HI, LO) (((__uint128_t) (HI) << 64) | (LO))`
> > 
> > And use that to initialize the 128-bit int values.
> That is a good point; I agree with your suggestion. I will utilize a macro since it also enhances readability. 
I agree.


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

https://reviews.llvm.org/D54911