[Bug target/94812] New: ppc incorrect mffs-based emulation of mffsl

[Bug target/94812] New: ppc incorrect mffs-based emulation of mffsl

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94812

            Bug ID: 94812
           Summary: ppc incorrect mffs-based emulation of mffsl
           Product: gcc
           Version: 10.0
               URL: https://gcc.gnu.org/pipermail/gcc-patches/2020-April/5
                    44391.html
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: aoliva at gcc dot gnu.org
                CC: segher at kernel dot crashing.org
  Target Milestone: ---

The emulation of mffsl with mffs does not store the result where requested.

The patch submission starting at this bug's URL has more details.

[Bug target/94812] ppc incorrect mffs-based emulation of mffsl

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94812

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-04-28
             Status|UNCONFIRMED                 |NEW
      Known to fail|                            |10.0, 9.3.0
     Ever confirmed|0                           |1

--- Comment #1 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Confirmed.  On 9 and 10.

[Bug target/94812] ppc incorrect mffs-based emulation of mffsl

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94812

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:50714f45eeaf315a0b55d3db3de3bf8df8e94b04

commit r10-8020-g50714f45eeaf315a0b55d3db3de3bf8df8e94b04
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Thu Apr 23 02:19:55 2020 -0300

    [rs6000] fix mffsl emulation

    The emulation of mffsl with mffs, used when !TARGET_P9_MISC, is going
    through the motions, but not storing the result in the given
    operands[0]; it rather modifies operands[0] without effect.  It also
    creates a DImode pseudo that it doesn't use, overwriting subregs
    instead.

    The patch below fixes all of these, the indentation and a typo.


    I'm concerned about several issues in the mffsl testcase.  First, I
    don't see that comparing the values as doubles rather than as long
    longs is desirable.  These are FPSCR bitfields, not FP numbers.  I
    understand mffs et al use double because they output to FP registers,
    and the bit patterns are subnormal FP numbers, so it works, but given
    the need for bit masking of at least one side, I'm changing the
    compare to long longs.

    Another issue with the test is that, if the compare fails, it calls
    mffsl again to print the value, as if it would yield the same result.
    But part of the FPSCR that mffsl (emulated with mffs or not) copies to
    the output FP register is the FPCC, so the fcmpu used to compare the
    result of the first mffsl will modify FPSCR and thus the result of the
    second mffsl call.  After changing the compare, this is no longer the
    case, but I still think it's better to make absolutely sure what we
    print is what we compared.

    Yet another issue is that the test assumed the mffs bits that are not
    to be extracted by mffsl to be already zero, instead of masking them
    out explicitly.  This is not about the mffs emulation in the mffsl
    implementation, but about the mffs use in the test proper.  The bits
    appear to be zero indeed, as the bits left out are for sticky
    exceptions, but there are reserved parts of FPSCR that might turn out
    to be set in the future, so we're better off masking them out
    explicitly, otherwise those bits could cause the compare to fail.

    If some future mffsl is changed so that it copies additional nonzero
    bits, the test will fail, and then we'll have a chance to adjust it
    and the emulation.


    for  gcc/ChangeLog

            PR target/94812
            * gcc/config/rs6000/rs6000.md (rs6000_mffsl): Copy result to
            output operand in emulation.  Don't overwrite pseudos.

    for  gcc/testsuite/ChangeLog

            PR target/94812
            * gcc.target/powerpc/test_mffsl.c: Call mffsl only once.
            Reinterpret the doubles as long longs for compares.  Mask out
            mffs bits that are not expected from mffsl.

[Bug target/94812] ppc incorrect mffs-based emulation of mffsl

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94812

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Alexandre Oliva
<aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:cd8cc299de624a764a5dcbcd7fb031a6b3e6b855

commit r9-8625-gcd8cc299de624a764a5dcbcd7fb031a6b3e6b855
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Tue May 26 11:28:42 2020 -0300

    [rs6000] fix mffsl emulation

    The emulation of mffsl with mffs, used when !TARGET_P9_MISC, is going
    through the motions, but not storing the result in the given
    operands[0]; it rather modifies operands[0] without effect.  It also
    creates a DImode pseudo that it doesn't use, overwriting subregs
    instead.

    The patch below fixes all of these, the indentation and a typo.


    I'm concerned about several issues in the mffsl testcase.  First, I
    don't see that comparing the values as doubles rather than as long
    longs is desirable.  These are FPSCR bitfields, not FP numbers.  I
    understand mffs et al use double because they output to FP registers,
    and the bit patterns are subnormal FP numbers, so it works, but given
    the need for bit masking of at least one side, I'm changing the
    compare to long longs.

    Another issue with the test is that, if the compare fails, it calls
    mffsl again to print the value, as if it would yield the same result.
    But part of the FPSCR that mffsl (emulated with mffs or not) copies to
    the output FP register is the FPCC, so the fcmpu used to compare the
    result of the first mffsl will modify FPSCR and thus the result of the
    second mffsl call.  After changing the compare, this is no longer the
    case, but I still think it's better to make absolutely sure what we
    print is what we compared.

    Yet another issue is that the test assumed the mffs bits that are not
    to be extracted by mffsl to be already zero, instead of masking them
    out explicitly.  This is not about the mffs emulation in the mffsl
    implementation, but about the mffs use in the test proper.  The bits
    appear to be zero indeed, as the bits left out are for sticky
    exceptions, but there are reserved parts of FPSCR that might turn out
    to be set in the future, so we're better off masking them out
    explicitly, otherwise those bits could cause the compare to fail.

    If some future mffsl is changed so that it copies additional nonzero
    bits, the test will fail, and then we'll have a chance to adjust it
    and the emulation.


    gcc/ChangeLog:

            PR target/94812
            * config/rs6000/rs6000.md (rs6000_mffsl): Copy result to
            output operand in emulation.  Don't overwrite pseudos.

    gcc/testsuite/ChangeLog:

            PR target/94812
            * gcc.target/powerpc/test_mffsl.c: Call mffsl only once.
            Reinterpret the doubles as long longs for compares.  Mask out
            mffs bits that are not expected from mffsl.

    (cherry picked from commit 50714f45eeaf315a0b55d3db3de3bf8df8e94b04)

[Bug target/94812] ppc incorrect mffs-based emulation of mffsl

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94812

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |aoliva at gcc dot gnu.org
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Fixed

[Bug target/94812] ppc incorrect mffs-based emulation of mffsl

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94812

--- Comment #5 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Thanks :-)