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

[Tue Ly via Phabricator via libc-commits]

lntue marked an inline comment as done.
lntue added inline comments.


================
Comment at: libc/src/__support/FPUtil/x86_64/LongDoubleBits.h:135
+    return cpp::bit_cast<long double>(bits);
+  }
+
----------------
mikhail.ramalho wrote:
> 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
Do you mind creating a github issue for me or @michaelrj to investigate the problem?  It's possible that we might need to clean up some comparison / testing codes for x86 long double instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152459