[libcxx-commits] [PATCH] D74163: [demangler] Fix the parsing of long double literals for PowerPC and S390

[Xing Xue via Phabricator via libcxx-commits]

xingxue added a comment.

Hi @uweigand, Thanks for your comments.  Please see my explanations below.

In D74163#1887981 <https://reviews.llvm.org/D74163#1887981>, @uweigand wrote:

> In D74163#1887301 <https://reviews.llvm.org/D74163#1887301>, @xingxue wrote:
>
> > @uweigand, Hi, I've addressed your comments.  Any further comments?
>
>
> I'm not very familiar with this code base.  However, I am somewhat confused by your proposed change to "parseExprPrimary".   In particular, where you now parse 'e' literals as "double" on powerpc/s390x, and 'g' literals as "long double" everywhere.  This seems incorrect to me.


In mangled names, floating-point literals are encoded using a fixed-length lowercase hexadecimal string corresponding to the internal representation, high-order bytes first.  For example, `float` literal `-1.0f` is encoded as "fbf800000". For a 64-bit `long double` literal on `powerpc` and `s390x`, the encoded form is type code 'e' followed by 16 hexadecimal digits.  For a 128-bit `long double` literal on `powerpc` and `s390x`, the encoded form is type code 'g' followed by 32 hexadecimal digits.  So, the proposed the change allows the parser to treat type code 'e' as a `double` (64-bit) and take the following 16 hexadecimal digits as the internal representation of the literal, instead of treating it as a 128-bit `long double` and looking for 32 hexadecimal digits after it.   When the type code is 'g', the parser will be looking for 32 hexadecimal digits. These are changes for parsing literals in the mangled names.

> 'e' literals really should be of "long double" *type* always.  It's just that on powerpc and s390x, in an old ABI selected via -mlong-double-64, the "long double" type was implemented as IEEE-64 (just like "double", but the type is nevertheless still "long double", not "double").

When printing out the demangled names, it still prints out "long double" for type code 'e' as usual, and "long double" for type code 'g' for `powerpc` and `s390x` (see lines 3789-3801 of the code).

> 'g' literals on the other hand really should be of type "long double" only on powerpc and s390x; on some other platforms, in particular x86, they should be of type "__float128" (on yet other platforms, 'g' is not used at all).

Right, please see lines 3793-3801 of the proposed change.

> But what confuses me even more is how this whole routine is even supposed to work in non-native mode: it just uses the native "double" or "long double" types, but the host implementation of those may be different from the one active on the target (whether this is because of cross-compilation to another target, or simply to another ABI mode like -mlong-double-64 vs. -mlong-double-128).   Is this routine only ever to be called natively?

`__cxa_demangle()` is a routine in runtime library libc++abi that is only built natively.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74163