[Bug target/53988] New: [SH] tst Rm,Rn not used for QI/HImode

[Bug target/53988] New: [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53988

             Bug #: 53988
           Summary: [SH] tst Rm,Rn not used for QI/HImode
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: olegendo@gcc.gnu.org
            Target: sh*-*-*


The following functions:

int test_00 (unsigned char* a, unsigned char* b, int c, int d)
{
  if (*a & *b)
    return c;
  return d;
}

int test_01 (char* a, char* b, int c, int d)
{
  if (*a & *b)
    return c;
  return d;
}

get compiled to:
        mov.b   @r4,r1  ! 10    *extendqisi2_compact_snd
        mov.b   @r5,r0  ! 11    *extendqisi2_compact_snd
        and     r1,r0   ! 12    *andsi_compact/4
        tst     #255,r0 ! 15    tstqi_t_zero
        bf      .L9     ! 16    branch_false
        mov     r7,r6   ! 7    movsi_ie/2
.L9:
        rts             ! 49    *return_i
        mov     r6,r0   ! 24    movsi_ie/2


which should actually result in:
        mov.b   @r4,r1
        mov.b   @r5,r2
        tst     r1,r2
        bf      .L9
        mov     r7,r6
.L9:
        rts
        mov     r6,r0

The same happens for QImode and HImode.

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53988

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-09-03 10:01:46 UTC ---
Omitting the sign/zero extensions is not safe for calculated QImode / HImode
values, only after the extending loads.  However, I guess it would still be a
little bit better to do something like:

  <r5 calculated somehow>
  <r6 calculated somehow>
  extu.b   r5,r1         ! zero-extend only one operand
  tst         r1,r6
  bf          .Lsomething

This would at least reduce R0 pressure.

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53988

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-31 22:08:42 UTC ---
There was a typo in the PR number when committing the patch for this issue.
This is the original commit message that accidentally ended up in PR 54988:

Author: olegendo
Date: Tue Oct 30 09:07:08 2012
New Revision: 192982

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192982
Log:
    PR target/54988
    * config/sh/sh.md (tstqi_t_zero): Rename to *tstqi_t_zero.
    (*tst<mode>_t_zero): New insns.
    * config/sh/iterators.md (lowpart_be, lowpart_le): New mode attributes.

    PR target/54988
    * gcc.target/sh/pr53988.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr53988.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/iterators.md
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

I've fixed the typos in the ChangeLog files.

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53988

Oleg Endo <olegendo at gcc dot gnu.org> changed:

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

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-11-05 21:11:40 UTC ---
Fixed in 4.8.

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2014-12-29
         Resolution|FIXED                       |---
     Ever confirmed|0                           |1

--- Comment #4 from Oleg Endo <olegendo at gcc dot gnu.org> ---
The change in r192982 results in wrong code:

int test (short x, short* y, int z, int w)
{
  return x & y[0] ? z : w;
}

        mov.w   @r5,r1    // sign extended y[0], bits[31:16] = 1
        tst     r4,r1     // r4 bits [31:16] undefined
        bf      .L5
        mov     r7,r0
        rts
        nop
    .align 1
.L5:
        rts
        mov     r6,r0

The *tst<mode>_t_zero insns blindly accept subregs.  They have to check whether
both operands have been sign extended or at least one of them has been zero
extended (to mask out the unwanted high bits).

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Wed Jan 14 23:46:34 2015
New Revision: 219623

URL: https://gcc.gnu.org/viewcvs?rev=219623&root=gcc&view=rev
Log:
gcc/
    PR target/53988
    * config/sh/sh-protos.h (sh_find_set_of_reg): Add option to ignore
    reg-reg copies.
    (sh_extending_set_of_reg): New struct.
    (sh_find_extending_set_of_reg, sh_split_tst_subregs,
    sh_remove_reg_dead_or_unused_notes): New Declarations.
    * config/sh/sh.c (sh_remove_reg_dead_or_unused_notes,
    sh_find_extending_set_of_reg, sh_split_tst_subregs,
    sh_extending_set_of_reg::use_as_extended_reg): New functions.
    * config/sh/sh.md (*tst<mode>_t_zero): Rename to *tst<mode>_t_subregs,
    convert to insn_and_split and use new function sh_split_tst_subregs.

gcc/testsuite/
    PR target/53988
    * gcc.target/sh/pr53988-1.c: New.

Added:
    trunk/gcc/testsuite/gcc.target/sh/pr53988-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh-protos.h
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Mon Jan 19 22:35:53 2015
New Revision: 219864

URL: https://gcc.gnu.org/viewcvs?rev=219864&root=gcc&view=rev
Log:
gcc/
    PR target/53988
    * config/sh/sh-protos.h (sh_find_set_of_reg): Make sure not to return
    nullptr for insn when reaching the first insn.
    * config/sh/sh.c (sh_unspec_insn_p): Rewrite using subrtx_iterator.
    (sh_insn_operands_modified_between_p): Add nullptr check.
    (sh_find_extending_set_of_reg): Fix log message.  Don't accept
    sign extending mem load if the insn contains any UNSPEC or
    UNSPEC_VOLATILE.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh-protos.h
    trunk/gcc/config/sh/sh.c

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Tue Mar 10 08:09:56 2015
New Revision: 221305

URL: https://gcc.gnu.org/viewcvs?rev=221305&root=gcc&view=rev
Log:
gcc/
    PR target/53988
    * config/sh/sh.md (*tst<mode>_t_zero): Remove insns.

gcc/testsuite/
    PR target/53988
    * gcc.target/sh/pr53988.c: Mark tests as xfail.

Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/sh/sh.md
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/sh/pr53988.c

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Tue Mar 10 08:17:31 2015
New Revision: 221307

URL: https://gcc.gnu.org/viewcvs?rev=221307&root=gcc&view=rev
Log:
gcc/
    PR target/53988
    * config/sh/sh.md (*tst<mode>_t_zero): Remove insns.

gcc/testsuite/
    PR target/53988
    * gcc.target/sh/pr53988.c: Mark tests as xfail.

Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/sh/sh.md
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/sh/pr53988.c

[Bug target/53988] [SH] tst Rm,Rn not used for QI/HImode

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

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

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Fixed on 5.  Fixed on 4.9 and 4.8 by removing the problematic patterns, as a
real backport seemed to intrusive.