[Bug target/54272] New: [SH] Add support for addv / subv instructions

[Bug target/54272] New: [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

             Bug #: 54272
           Summary: [SH] Add support for addv / subv instructions
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
        AssignedTo: olegendo@gcc.gnu.org
        ReportedBy: olegendo@gcc.gnu.org
            Target: sh*-*-*


The addv and subv instructions can be used for at least two things:

1) Implementing trapping signed integer arithmetic (-ftrapv)

   Currently, for the SH target, trapping insns (e.g. addvsi3) are
   supposed to be implemented by library functions in libgcc.
   However, it seems that for addvsi3 and subvsi3 normal addsi3 and
   subsi3 patterns are expanded by the middle-end, which is wrong.
   Probably PR 35412 is related to this.
   After adding the addvsi3 pattern to sh.md, the middle-end picks it up
   as expected.
   However, I'm unsure how to realize the actual trapping part.
   The library functions in libgcc just invoke 'abort ()' on overflow.
   I think the trapa instruction could be used here to get relatively
   compact code, like:
           addv  r4,r5
           bf    0f
           trapa #???
         0:
   However, I have no idea which trapa immediate value would be
   suitable for this, especially for GNU/Linux environments.
   Maybe it's better to make this user configurable through an -m option?

2) Saturating arithmetic

   E.g. the pattern ssaddsi3 can be implemented with the following
   sequence (if I'm not mistaken):
          mov.l  .Lintmin, r1  ! r1 = 0x80000000
          cmp/pz r4      ! only one sign matters in case of overflow 
          negc   r1,r2   ! r5 = 0 - r1 - T
          addv   r4,r5
          bf     0f
          mov    r2,r5
      0:
   However, I don't know how to make the middle-end expand patterns that
   contain 'ss_plus'.  It seems that other targets provide the ssaddsi3
   pattern through target specific built-in functions only...

[Bug target/54272] [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu.org

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-15 16:17:24 UTC ---
Kaz, can you please provide some feedback on the idea of using the trapa
instruction for -ftrapv?

[Bug target/54272] [SH] Add support for addv / subv instructions

[kkojima at gcc dot gnu.org]

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

--- Comment #2 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-08-16 11:23:42 UTC ---
(In reply to comment #1)

For linux environment, libc's abort function is a rather complex
function and trapa handler should do equivalent things to keep
complete binary compatibility.  I guess that it isn't worth to do
so.  You can introduce simple trapa sequence as an ABI changing -m
option on linux, though trap numbers < 32 are allocated for the
syscalls and all other numbers are given for debug purposes and
generate SIGTRAP with modified pc which points to trapa instruction
itself.  If the user program uses its own SIGTRAP handlers, this
might be problematic.  Perhaps the "sleep" instruction will be
a candidate because current libc uses it for ABORT_INSTRUCTION,
though it would be bad for bare metal cases.
BTW, there is a known SH4 core bug for trapa instruction and we
are using 5 or r0,r0 instructions just after trapa.  See libitm/
config/sh/futex_bits.h for example.

[Bug target/54272] [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-16 12:43:09 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
> 
> For linux environment, libc's abort function is a rather complex
> function and trapa handler should do equivalent things to keep
> complete binary compatibility.  I guess that it isn't worth to do
> so.  You can introduce simple trapa sequence as an ABI changing -m
> option on linux, though trap numbers < 32 are allocated for the
> syscalls and all other numbers are given for debug purposes and
> generate SIGTRAP with modified pc which points to trapa instruction
> itself.  If the user program uses its own SIGTRAP handlers, this
> might be problematic.  Perhaps the "sleep" instruction will be
> a candidate because current libc uses it for ABORT_INSTRUCTION,
> though it would be bad for bare metal cases.
> BTW, there is a known SH4 core bug for trapa instruction and we
> are using 5 or r0,r0 instructions just after trapa.  See libitm/
> config/sh/futex_bits.h for example.

Thanks a lot for the hints!

Just for the record, the SH7750 / SH7751 trapa bug is described here
http://documentation.renesas.com/doc/products/mpumcu/tu/tnsh7456ae.pdf

Taking this into account, the trapa insn suddenly seems very unattractive for
this
purpose ;)
How about small variation:

    mov.l  .Loverflow_trap, r1
    addv   r4,r5
    bf       0f
    jsr      @r1
0:
    nop
    ....

'overflow_trap' then would be a library function in libgcc lib1funcs.S,  like
e.g. the dynamic shift functions for SH2.  The default implementation of
overflow_trap could simply invoke abort to
stay in line with the other trapping operations in libgcc.
For bare metal cases, users can provide their own libgcc implementation and do
whatever they
want.

[Bug target/54272] [SH] Add support for addv / subv instructions

[kkojima at gcc dot gnu.org]

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

--- Comment #4 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-08-16 13:00:37 UTC ---
(In reply to comment #3)
> How about small variation:

Sounds reasonable to me.

[Bug target/54272] [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-08-16
     Ever Confirmed|0                           |1

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-16 20:34:53 UTC ---
Regarding saturating arithmetic, I took the function FIXED_SSADD from
libgcc/fixed-bit.c and substituted FIXED_C_TYPE with int.

int test_01 (int a, int b)
{
  int c;
  int x, y, z;
  x = a;
  y = b;
  z = x + y;
  if ((((x ^ y) >> 31) & 1) == 0)
    {
      if (((z ^ x) >> 31) & 1)
        {
          z = 1;
          z = z << 31;
          if (x >= 0)
            z--;
        }
    }
  c = z;
  return c;
}

compiled with -O2 -m4 -ml:
        div0s   r4,r5
        mov     r4,r0
        bt/s    .L4
        add     r5,r0
        div0s   r0,r4
        bt/s    .L6
        cmp/pz  r4
.L4:
        rts
        nop
        .align 1
.L6:
        mov.l   .L7,r0
        movt    r1
        rts
        sub     r1,r0
.L8:
        .align 2
.L7:
        .long   -2147483648

In this case combine can successfully match the div0s sign comparison patterns
that I added in PR 52933.  However, after matching those, it does not try to
combine the patterns any further (probably because of the conditional
branches). I guess this would be a job for an optimization at the GIMPLE level.

[Bug target/54272] [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Another example where addv could be used:

int test (int a)
{
  if (a == 0x7FFFFFFF)
    return a;

  return a + 1;
}

currently compiles to:

-O2 -m2a:
        mov.l   .L6,r1
        cmp/eq  r1,r4
        bt.s    .L4
        mov     r4,r0
        add     #1,r0
.L4:
        rts/n

        .align 2
.L6:
        .long   2147483647


-O2 -m4:
        mov.l   .L4,r1
        mov     #-1,r0
        cmp/eq  r1,r4
        negc    r0,r0
        rts
        add     r4,r0

        .align 2
.L4:
        .long   2147483647

better:

SH4 with zero displacement branch:
        mov     #1,r0
        addv    r4,r0  // T = r0 == 0x80000000 (overflow)
        bt      0f
        mov     r4,r0
0:
        rts
        nop

branch-free:
        mov    #1,r0
        mov    #0,r1
        addv    r4,r0  // T = r0 == 0x80000000 (overflow)
        rts
        subc    r1,r0  // r0 = r0 - r1 - T

[Bug target/54272] [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #0)
> The addv and subv instructions can be used for at least two things:
> 
> 1) Implementing trapping signed integer arithmetic (-ftrapv)
> 
>    Currently, for the SH target, trapping insns (e.g. addvsi3) are
>    supposed to be implemented by library functions in libgcc.
>    However, it seems that for addvsi3 and subvsi3 normal addsi3 and
>    subsi3 patterns are expanded by the middle-end, which is wrong.

See also PR 52478.

[Bug target/54272] [SH] Add support for addv / subv instructions

[olegendo at gcc dot gnu.org]

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

--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Recently some __builtin_*_overflow functions have been added, of which some
could utilize the addv and subv instructions.  See also
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html