Hi Raoni,

Some of this isn't an rs6000 patch, but the subject says it is, so it
might well not draw the attention it needs.

Adding some Cc:s.

On Fri, Sep 04, 2020 at 12:52:30PM -0300, Raoni Fassina Firmino wrote:
> There is one pending question raised by Segher, It is about adding
> documentation, I am not sure if it is needed and if so, where it
> should be. I will quote the relevant part of the conversation[2] from
> the v1 thread for context:
> 
>   > > > +OPTAB_D (fegetround_optab, "fegetround$a")
>   > > > +OPTAB_D (feclearexcept_optab, "feclearexcept$a")
>   > > > +OPTAB_D (feraiseexcept_optab, "feraiseexcept$a")
>   > >␣
>   > > Should those be documented somewhere?  (In gcc/doc/ somewhere).
>   >
>   > I am lost on that one. I took a look on the docs (I hope looking on the
>   > online docs was good enough) and I didn't find a place where i feel it
>   > sits well. On the PowerPC target specific sections (6.60.22 Basic
>   > PowerPC Built-in Functions), I didn't found it mentioning builtins that
>   > are optimizations for the standard library functions, but we do have
>   > many of these for Power.  Then, on the generic section (6.59 Other
>   > Built-in Functions Provided by GCC) it mentions C99 functions that have
>   > builtins but it seems like it mentions builtins that have target
>   > independent implementation, or at least it dos not say that some
>   > builtins may be implemented on only some targets.  And in this case
>   > there is no implementation (for now) for any other target that is not
>   > PowerPc.
>   >
>   > So, I don't know if or where this should be documented.

I don't see much about optabs in the docs either.  Add some text to
optabs.def itself then?

> +(define_expand "feclearexceptsi"
> +  [(use (match_operand:SI 1 "const_int_operand" "n"))
> +   (set (match_operand:SI 0 "gpc_reg_operand")
> +     (const_int 0))]
> +  "TARGET_HARD_FLOAT"
> +{
> +  switch (INTVAL (operands[1]))
> +    {
> +    case 0x2000000:  /* FE_INEXACT */
> +    case 0x4000000:  /* FE_DIVBYZERO */
> +    case 0x8000000:  /* FE_UNDERFLOW */
> +    case 0x10000000: /* FE_OVERFLOW */

Please write 0x02000000 etc. instead?

> +;; int fegraiseexcept(int excepts)

(typo)

> +/* { dg-do run } */
> +/* { dg-require-effective-target fenv_exceptions } */
> +/* { dg-options "-lm -fno-builtin" } */

That -fno-builtin looks very strange...  Comment what it is for?

> +#define FAIL(v, e) printf("ERROR, __builtin_fegetround() returned %d," \
> +                          " not the expecected value %d\n", v, e);

(Typo, "expected")

The rs6000 part is okay for trunk (with those modifications), after the
generic parts is approved.  Thanks!


Segher

Reply via email to