[Bug target/111778] New: PowerPC constant code change uses an undefined shift

[Bug target/111778] New: PowerPC constant code change uses an undefined shift

[meissner at gcc dot gnu.org]

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

            Bug ID: 111778
           Summary: PowerPC constant code change uses an undefined shift
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: meissner at gcc dot gnu.org
  Target Milestone: ---

I was building a cross compiler to PowerPC on my x86_86 workstation with the
latest version of GCC on October 11th.  I could not build the compiler on the
x86_64 system as it died in building libgcc.  I looked into it, and I
discovered the compiler was recursing until it ran out of stack space.  If I
build a native compiler with the same sources on a PowerPC system, it builds
fine.

I traced this down to a change made around October 10th:

commit 8f1a70a4fbcc6441c70da60d4ef6db1e5635e18a (HEAD)
Author: Jiufu Guo <guojiufu@linux.ibm.com>
Date:   Tue Jan 10 20:52:33 2023 +0800

    rs6000: build constant via li/lis;rldicl/rldicr

    If a constant is possible left/right cleaned on a rotated value from
    a negative value of "li/lis".  Then, using "li/lis ; rldicl/rldicr"
    to build the constant.

    gcc/ChangeLog:

            * config/rs6000/rs6000.cc (can_be_built_by_li_lis_and_rldicl): New
            function.
            (can_be_built_by_li_lis_and_rldicr): New function.
            (rs6000_emit_set_long_const): Call
can_be_built_by_li_lis_and_rldicr and
            can_be_built_by_li_lis_and_rldicl.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/const-build.c: Add more tests.


In particular, the code is:

static bool
can_be_built_by_li_lis_and_rldicl (HOST_WIDE_INT c, int *shift,
                                   HOST_WIDE_INT *mask)
{
  /* Leading zeros may be cleaned by rldicl with a mask.  Change leading zeros
     to ones and then recheck it.  */
  int lz = clz_hwi (c);
  HOST_WIDE_INT unmask_c
    = c | (HOST_WIDE_INT_M1U << (HOST_BITS_PER_WIDE_INT - lz));
  int n;

  if (can_be_rotated_to_lowbits (~unmask_c, 15, &n)
      || can_be_rotated_to_negative_lis (unmask_c, &n))
    {
      *mask = HOST_WIDE_INT_M1U >> lz;
      *shift = n == 0 ? 0 : HOST_BITS_PER_WIDE_INT - n;
      return true;
    }

  return false;
}

In particular, if lz is 0 due to the constant having the highest bit set, the
-1 shift to set the mask in unmask_c would do a shift left by 64.

Different machines interpret num << shift differently if shift is at least the
number of bits in num's representation.  It is explicitly undefined behavior in
the C/C++ langauges.

In particular (-1 << 64) on an x86_64 produces -1 and (-1 << 64) on a 64-bit
PowerPC produces 0.

If I add a test for lz being 0 and returning false, the compiler builds fine.

One other note is the ChangeLog date is not correct for Jiufu Guo's changes
that include this change.  The several changes that were submitted list dates
of:

Tue Jan 10 21:40:48 2023 +0800
Tue Jan 10 20:52:33 2023 +0800
Thu Jun 15 21:11:53 2023 +0800
Thu Aug 24 09:08:34 2023 +0800

[Bug target/111778] PowerPC constant code change uses an undefined shift

[meissner at gcc dot gnu.org]

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

Michael Meissner <meissner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major
           Priority|P3                          |P2
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |dje at gcc dot gnu.org,
                   |                            |guojiufu at gcc dot gnu.org,
                   |                            |meissner at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org
              Build|                            |powerpc64le-unknown-linux-g
                   |                            |nu
             Target|                            |powerpc64le-unknown-linux-g
                   |                            |nu
               Host|                            |x86_64-unknown-linux-gnu

[Bug target/111778] PowerPC constant code change uses an undefined shift

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
On the date. It is the author date vs commit date.
You can see that here
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=8f1a70a4fbcc6441c70da60d4ef6db1e5635e18a
.

[Bug target/111778] PowerPC constant code change uses an undefined shift

[guojiufu at gcc dot gnu.org]

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

--- Comment #2 from Jiu Fu Guo <guojiufu at gcc dot gnu.org> ---
Thanks so much for reporting this issue, and thanks for tracing down it!

For the code, if 'lz' is 0, it is correct to return false.

[Bug target/111778] PowerPC constant code change uses an undefined shift

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Michael Meissner <meissner@gcc.gnu.org>:

https://gcc.gnu.org/g:611eef7609f732db65c119a7eab6d50a5fdd5985

commit r14-4600-g611eef7609f732db65c119a7eab6d50a5fdd5985
Author: Michael Meissner <meissner@linux.ibm.com>
Date:   Thu Oct 12 16:17:59 2023 -0400

    PR111778, PowerPC: Do not depend on an undefined shift

    I was building a cross compiler to PowerPC on my x86_86 workstation with
the
    latest version of GCC on October 11th.  I could not build the compiler on
the
    x86_64 system as it died in building libgcc.  I looked into it, and I
    discovered the compiler was recursing until it ran out of stack space.  If
I
    build a native compiler with the same sources on a PowerPC system, it
builds
    fine.

    I traced this down to a change made around October 10th:

    | commit 8f1a70a4fbcc6441c70da60d4ef6db1e5635e18a (HEAD)
    | Author: Jiufu Guo <guojiufu@linux.ibm.com>
    | Date:   Tue Jan 10 20:52:33 2023 +0800
    |
    |   rs6000: build constant via li/lis;rldicl/rldicr
    |
    |   If a constant is possible left/right cleaned on a rotated value from
    |   a negative value of "li/lis".  Then, using "li/lis ; rldicl/rldicr"
    |   to build the constant.

    The code was doing a -1 << 64 which is undefined behavior because different
    machines produce different results.  On the x86_64 system, (-1 << 64)
produces
    -1 while on a PowerPC 64-bit system, (-1 << 64) produces 0.  The x86_64
then
    recurses until the stack runs out of space.

    If I apply this patch, the compiler builds fine on both x86_64 as a PowerPC
    crosss compiler and on a native PowerPC system.

    2023-10-12  Michael Meissner  <meissner@linux.ibm.com>

    gcc/

            PR target/111778
            * config/rs6000/rs6000.cc (can_be_built_by_li_lis_and_rldicl):
Protect
            code from shifts that are undefined.
            (can_be_built_by_li_lis_and_rldicr): Likewise.
            (can_be_built_by_li_and_rldic): Protect code from shifts that
            undefined.  Also replace uses of 1ULL with HOST_WIDE_INT_1U.

[Bug target/111778] PowerPC constant code change uses an undefined shift

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug target/111778] [14 Regression] PowerPC constant code change uses an undefined shift

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
           Keywords|                            |ice-on-valid-code
            Summary|PowerPC constant code       |[14 Regression] PowerPC
                   |change uses an undefined    |constant code change uses
                   |shift                       |an undefined shift

[Bug target/111778] [14 Regression] PowerPC constant code change uses an undefined shift

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zsojka at seznam dot cz

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 111746 has been marked as a duplicate of this bug. ***