[libc-commits] [PATCH] D152459: [libc] Add Int<> type and fix (U)Int<128> compatibility issues.

[Mikhail Ramalho via Phabricator via libc-commits]

mikhail.ramalho added inline comments.


================
Comment at: libc/src/__support/FPUtil/x86_64/LongDoubleBits.h:135
+    return cpp::bit_cast<long double>(bits);
+  }
+
----------------
lntue wrote:
> mikhail.ramalho wrote:
> > In https://reviews.llvm.org/D149594, I had to set the explicit bit when the value is not zero because some tests start to fail due to:
> > ```
> > diff --git a/libc/src/__support/str_to_float.h b/libc/src/__support/str_to_float.h
> > index b36c73fbe8ef..79a1c8fe6f0b 100644
> > --- a/libc/src/__support/str_to_float.h
> > +++ b/libc/src/__support/str_to_float.h
> > @@ -13,6 +13,7 @@
> >  #include "src/__support/CPP/optional.h"
> >  #include "src/__support/FPUtil/FEnvImpl.h"
> >  #include "src/__support/FPUtil/FPBits.h"
> > +#include "src/__support/FPUtil/dyadic_float.h"
> >  #include "src/__support/UInt128.h"
> >  #include "src/__support/builtin_wrappers.h"
> >  #include "src/__support/common.h"
> > @@ -570,7 +572,9 @@ clinger_fast_path(ExpandedFloat<T> init_num,
> >    }
> >  
> >    fputil::FPBits<T> result;
> > -  T float_mantissa = static_cast<T>(mantissa);
> > +  T float_mantissa =
> > +      static_cast<T>(fputil::DyadicFloat<128>(/*sign*/ false, /*exponent*/ 0,
> > +                                              /*mantissa*/ mantissa));
> >  
> >    if (exp10 == 0) {
> >      result = fputil::FPBits<T>(float_mantissa);
> > -- 
> > 2.39.2
> > ```
> > We need this change to support platforms with no native 128-bit integers.
> > 
> > May I ask if you can double-check that?
> This code path should not be in ARM32 or RISCV-32 platforms?  I thought that for those platform, `long double` is either double or quad precision (`Float128`), right?
It's not used in rv32, but because of the change to the `clinger_fast_path` function, this gets called in other platforms


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152459