Re: [PATCH v4 00/13] port C-SKY to glibc

[Joseph Myers <joseph at codesourcery dot com>]

On Wed, 12 Sep 2018, Mao Han wrote:

> 38245425a9add7bd22f8732219e0085432f223b6 (6 sep). Two abi combinations
> are supported with this patch: C-SKY ABIV2 with (soft float & little endian,
> hard float & little endian). CK807(ef), CK810(ef), CK860 are the

Could you please clarify whether those are the same ABI (compatible for 
function calling, structure layout for types from glibc headers, etc.) or 
different ABIs?

If they are different ABIs, they should have different dynamic linker 
names (of course you need to make the GCC port reflect the dynamic linker 
name used for each ABI), and your bits/fenv.h should disable most of its 
contents for soft float, like e.g. MIPS does (define only __FE_UNDEFINED 
and FE_TONEAREST as rounding modes in that case, define FE_ALL_EXCEPT to 0 
and no other exception macros, do not define FE_NOMASK_ENV).  Then you 
shouldn't need math-tests-exceptions.h and math-tests-rounding.h in the 
nofpu directory because things will be handled automatically when 
bits/fenv.h avoids defining unsupported things.

If they are the same ABI, I don't see any use for the CSKY_HARD_FLOAT 
macro defined in preconfigure; nothing seems to test it, so it's only 
relevant if shlib-versions is testing it (i.e. if they are different ABIs 
with different dynamic linker names).

Does C-SKY hard float support exception traps or not?  math-tests-trap.h 
has a comment saying not.  But you have an implementation of 
feenableexcept that implies it does support exception traps.

* If exception traps are never supported, everything related to them 
(including the definition of FE_NOMASK_ENV) should be removed; the default 
feenableexcept and fedisableexcept and fegetexcept implementations should 
suffice.

* If exception traps are always supported, your math-tests-trap.h is wrong 
and should be removed.

* If they are conditionally supported, like on Arm and AArch64, your 
math-tests-trap.h is appropriate with a different comment explaining the 
conditional support - but various fenv.h functions like feenableexcept 
need to check for the support and give errors if asked to enable 
exception traps they are unable to enable (this includes fesetenv and 
feupdateenv passed FE_NOMASK_ENV - see Arm and AArch64 for examples).

> Several patches will be post to GCC and Binutils to fix some testcase fail
> later this month, while the follow results have these patches applied.

Note that glibc ports can't go in while they depend on such non-upstream 
patches for good results (see the ARC port discussion).  You can send a 
list of all the patches required to get the given test results, but 
they'll need to be upstream (as will the Linux kernel port) before the 
port can go into glibc.

> 5. The following cases fail due to gcc optimize change the sequence of
>    -a * b to -(a * b) and got 1ulps error.
>    math/test-double-tgamma
>    math/test-float-tgamma
>    math/test-float32-tgamma
>    math/test-float32x-tgamma
>    math/test-float64-tgamma
>    math/test-ldouble-tgamma

So that needs a GCC bug fix (just like when the Arm and AArch64 ports of 
GCC used to have such a bug).

-- 
Joseph S. Myers
joseph@codesourcery.com