[Bug target/67391] New: [SH] Convert clrt addc to normal add insn

[Bug target/67391] New: [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

            Bug ID: 67391
           Summary: [SH] Convert clrt addc to normal add insn
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: olegendo at gcc dot gnu.org
  Target Milestone: ---

Looking through bzip2 compress.c of the CSiBE set, I've spotted sequences such
as:

        mov     r15,r4
        add     #64,r4
        mov.l   @(44,r4),r2
        mov     r15,r0
        mov.w   .L615,r5
        add     #124,r0
        clrt                 <<<
        mov.l   @(16,r0),r0
        mov     r14,r1
        add     r2,r5
        addc    r12,r1       <<<
        mov.l   r5,@(44,r4)
        cmp/eq  r1,r0        <<<
        bf/s    .L126

It seems that this is a left-over of what originally was a 64 bit addition. 
The high word of the 64 bit add result is unused, which makes it effectively a
32 bit addition.  The clrt can be removed and the addc can be converted into a
regular add insn.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

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> ---
This seems to be (indirectly) caused by the extra checks added to addsi3 as
part of PR 55212, in particular attachment 33707, and r218999, r219341.

Removing those with:

@@ -2129,11 +2129,6 @@
 {
   if (TARGET_SHMEDIA)
     operands[1] = force_reg (SImode, operands[1]);
-  else if (! arith_operand (operands[2], SImode))
-    {
-      if (reg_overlap_mentioned_p (operands[0], operands[1]))
-       FAIL;
-    }
 })

 (define_insn "addsi3_media"
@@ -2172,10 +2167,7 @@
   [(set (match_operand:SI 0 "arith_reg_dest" "=r,&u")
        (plus:SI (match_operand:SI 1 "arith_operand" "%0,r")
                 (match_operand:SI 2 "arith_or_int_operand" "rI08,rn")))]
-  "TARGET_SH1
-   && ((rtx_equal_p (operands[0], operands[1])
-        && arith_operand (operands[2], SImode))
-       || ! reg_overlap_mentioned_p (operands[0], operands[1]))"
+  "TARGET_SH1"
   "@
        add     %2,%0
        #"
@@ -2190,6 +2182,37 @@
 }
   [(set_attr "type" "arith")])

Removes the suspicious clrt-addc sequence.  The added reg overlap
checks/restrictions also trigger some problems with ifcvt, because it tries to
emit insns that go like

(set (reg:SI 162)
    (ne:SI (reg:SI 147 t)
        (const_int 0 [0])))

(set (reg:SI 162)
     (plus:SI (reg:SI 162) (const_int 2147483647)))

I haven't tested the patch above (with LRA enabled), but on CSiBE it does:
sum:  3347407 -> 3346163    -1244 / -0.037163 %

Kaz, do you have any memory of the extra checks?  Isn't it enough to just
accept the addsi3 pattern as "rC = rA + {rB|imm}" and insert the reg-reg copy
after register allocation via split2, as it's already done?  Why are the reg
overlap checks needed during matching of the patterns?

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 36373
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36373&action=edit
Proposed patch

Tested on sh-elf, LRA enabled, with make -k check
RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

I don't get any new failures.  Kaz, could you please patch try this in your
setup?

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #1)
> Kaz, do you have any memory of the extra checks?  Isn't it enough to just
> accept the addsi3 pattern as "rC = rA + {rB|imm}" and insert the reg-reg
> copy after register allocation via split2, as it's already done?  Why are
> the reg overlap checks needed during matching of the patterns?

Ugh, those checks look just wrong and I can't remind why I've
added them.  33707 didn't do that and checked overlapping at
the split condition only.  Perhaps I mixed up the final patch
with the one of test codes to collect CSiBE results.  You can
install the above patch as the rather obvious one, though I'm
running "make -k check" on sh4-unknown-linux-gnu.  Sorry for
adding that thinko.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #5 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #4)
> I've just checked, the code is also present in GCC 5.  Because of the funny
> side effects even with LRA disabled (this PR) I'd like to backport this to
> the GCC 5 branch, too.

Yes, this is clearly a 5/6 regression.  My test has passed C and C++ part
with no new failures.  I'll report back when test completed.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #6 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #5)
> Yes, this is clearly a 5/6 regression.  My test has passed C and C++ part
> with no new failures.  I'll report back when test completed.

Test completed with no new failures on sh4-unknown-linux-gnu.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #6)
> Test completed with no new failures on sh4-unknown-linux-gnu.

Thanks!

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Wed Sep 23 11:55:45 2015
New Revision: 228046

URL: https://gcc.gnu.org/viewcvs?rev=228046&root=gcc&view=rev
Log:
gcc/
        PR target/67391
        * config/sh/sh.md (addsi3, *addsi3_compact): Don't check for
overlapping
        regs when matching the pattern.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.md

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Wed Sep 23 11:57:27 2015
New Revision: 228047

URL: https://gcc.gnu.org/viewcvs?rev=228047&root=gcc&view=rev
Log:
gcc/
        Backport from mainline
        2015-09-23  Oleg Endo  <olegendo@gcc.gnu.org>

        PR target/67391
        * config/sh/sh.md (addsi3, *addsi3_compact): Don't check for
overlapping
        regs when matching the pattern.

Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/sh/sh.md

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
The core issue should be fixed.  I'd like to keep this PR open though for a
while.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #11 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #10)
> The core issue should be fixed.  I'd like to keep this PR open though for a
> while.

I've got

/exp/ldroot/dodes/INTEST/trunk/gcc/testsuite/gfortran.dg/matmul_6.f90:19:0:
Error: could not split insn
(insn 2778 2779 94 (set (reg:SI 3 r3)
        (plus:SI (reg:SI 3 r3 [316])
            (const_int 372 [0x174])))
/exp/ldroot/dodes/INTEST/trunk/gcc/testsuite/gfortran.dg/matmul_6.f90:20 64
{*addsi3_compact}
     (nil))

in my local tree.  Clearly it was hidden by the wrong extra checks.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #12 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I already thought that something like this might happen.  I will have a look.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #13 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #11)
> 
> /exp/ldroot/dodes/INTEST/trunk/gcc/testsuite/gfortran.dg/matmul_6.f90:19:0:
> Error: could not split insn
> (insn 2778 2779 94 (set (reg:SI 3 r3)
>         (plus:SI (reg:SI 3 r3 [316])
>             (const_int 372 [0x174])))
> /exp/ldroot/dodes/INTEST/trunk/gcc/testsuite/gfortran.dg/matmul_6.f90:20 64
> {*addsi3_compact}
>      (nil))

I don't know how this insn could be fixed after register allocation.  We need
r3 and a free reg to load the constant.  The only last resort option in this
case is to emit something like ...
   add  #127,r3
   add  #127,r3
   add  #118,r3

but I don't think it's a good idea for large constants.  Of course we can also
do ...
   mov.l  r0,@-r15
   mov.w  .Lconstant,r0
   add    r0,r3
   mov.l  @r15+,r0

But maybe it's better to avoid that in the first place.  We can allow more
relaxed predicates, like arith_or_int_operand, but the splitter for those has
to run before register allocation (in split1) and we should not allow the "n"
constraint.  An "&u" "r" "rI08" alternative would be fine though, since we can
always fix up the I08 case.  In fact, I think we should allow such relaxed
predicates for other insns, too, e.g. PR 65317.

I haven't checked again, but if LRA insists on the "n" alternative for
arbitrary constants, maybe it's better to fix this issue in LRA.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #14 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Created attachment 36387
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36387&action=edit
a trial

Although a bit ugly, how about adding pattern using scratch reg?
Does it get the original clrt+addc issue back again?

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #16 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 36396
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36396&action=edit
Another trail, works with LRA

(In reply to Oleg Endo from comment #15)
> 
> I'm now trying to come up with something that is between your patch and my
> patch to make it work with LRA.  However, somehow I think we are just
> desperately wallpapering LRA deficits here.  It looks a bit fragile.  We
> should try to avoid that if possible.

This patch is a mix of the patches attachment 36387 and attachment 36393.  It
works with LRA (tested only by compiling the CSiBE set) and keeps the code size
improvement without LRA.

First, the original addsi3 expander looks a bit wrong.  It should be
"arith_reg_dest" for operands[0], because also the SHmedia patterns only accept
"arith_reg_dest".  operands[1] should be "arith_reg_operand".  This should be
sufficient and no need to use "force_reg" on it (for SHmedia case), because
usually constants are put in operands[2], not operands[1] when the insn is
expanded.

Then, there's is messy thing with 3 addsi3 patterns ... the order is very
important.  They must be in exactly this order, or else we don't get the code
size improvement without LRA and with LRA it will ICE/segfault...

I've tested this patch with
make -k check
RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

and no new failures.  I'll test it with -mlra enabled by default later.  Kaz,
does this patch fix the issue in c#11 ?

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #17 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #16)
> Kaz, does this patch fix the issue in c#11 ?

Yep, it fixes that ICE.  Thanks!
My 36387 trial patch can cause a similar problem with PR64533 when sp
is taken as the scratch register.  I've modified your patch with

diff --git a/config/sh/sh.md b/config/sh/sh.md
index 3e6a244..8e08d0b 100644
--- a/config/sh/sh.md
+++ b/config/sh/sh.md
@@ -2197,7 +2197,7 @@
   [(set (match_operand:SI 0 "arith_reg_dest" "=r,&u,&u")
        (plus:SI (match_operand:SI 1 "arith_reg_operand" "%0,r,r")
                 (match_operand:SI 2 "arith_or_int_operand" "rI08,r,n")))
-   (clobber (match_scratch:SI 3 "=X,X,&r"))]
+   (clobber (match_scratch:SI 3 "=X,X,&u"))]
   "TARGET_SH1"
   "@
        add     %2,%0

Now the full tests on sh4-unknown-linux-gnu are running.  I'll report
the results when completed.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #18 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #17)
> (In reply to Oleg Endo from comment #16)
> > Kaz, does this patch fix the issue in c#11 ?
> 
> Yep, it fixes that ICE.  Thanks!
> My 36387 trial patch can cause a similar problem with PR64533 when sp
> is taken as the scratch register.  I've modified your patch with
> 
> diff --git a/config/sh/sh.md b/config/sh/sh.md
> index 3e6a244..8e08d0b 100644
> --- a/config/sh/sh.md
> +++ b/config/sh/sh.md
> @@ -2197,7 +2197,7 @@
>    [(set (match_operand:SI 0 "arith_reg_dest" "=r,&u,&u")
>         (plus:SI (match_operand:SI 1 "arith_reg_operand" "%0,r,r")
>                  (match_operand:SI 2 "arith_or_int_operand" "rI08,r,n")))
> -   (clobber (match_scratch:SI 3 "=X,X,&r"))]
> +   (clobber (match_scratch:SI 3 "=X,X,&u"))]
>    "TARGET_SH1"
>    "@
>         add     %2,%0
> 
> Now the full tests on sh4-unknown-linux-gnu are running.  I'll report
> the results when completed.

OK.  I've checked CSiBE results with &u instead of &r for the scratch.  There's
a small size increase ...

&r sum:  3345527 -> 3334307    -11220 / -0.335373 %
&u sum:  3345527 -> 3334351    -11176 / -0.334058 %

But doesn't matter.  It's still better than without the patch ;)

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #19 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #16)
> Then, there's is messy thing with 3 addsi3 patterns ... the order is very
> important.  They must be in exactly this order, or else we don't get the
> code size improvement without LRA and with LRA it will ICE/segfault...

I'm just afraid that the same will have to be done for all insns if PR 65317 is
fixed in this way...

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #20 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #16)
> Created attachment 36396 [details]
> Another trail, works with LRA
>
> I've tested this patch with
> make -k check
> RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-
> mb,-m4a/-ml,-m4a/-mb}"
> 
> and no new failures.  I'll test it with -mlra enabled by default later. 
> Kaz, does this patch fix the issue in c#11 ?

Now tested as above with LRA enabled by default.  Looks OK here.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #21 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
No new failures with -mlra here too.  The test without -mlra is still
running, though there is a new failure:

/home/ldroot/dodes/xsh-gcc/gcc/xgcc -B/home/ldroot/dodes/xsh-gcc/gcc/
-fno-diagnostics-show-caret -fdiagnostics-color=never -O1 -ffat-lto-objects -c
-o sync-3.o
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c: In
function 'test_op_ignore':
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:75:3:
note: '__sync_fetch_and_nand' changed semantics in GCC 4.4
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c: In
function 'test_fetch_and_op':
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:164:1:
error: insn does not satisfy its constraints:
(insn 729 728 42 2 (set (reg:SI 7 r7)
        (plus:SI (reg:SI 0 r0)
            (const_int 0 [0])))
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:104
66 {*addsi3}
     (nil))
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:164:1:
internal compiler error: in extract_constrain_insn, at recog.c:2200
0x85f822f _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        ../../LOCAL/trunk/gcc/rtl-error.c:109
0x85f8258 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
        ../../LOCAL/trunk/gcc/rtl-error.c:120
0x85c5430 extract_constrain_insn(rtx_insn*)
        ../../LOCAL/trunk/gcc/recog.c:2200
0x85a6935 reload_cse_simplify_operands
        ../../LOCAL/trunk/gcc/postreload.c:408
0x85a7e9a reload_cse_simplify
        ../../LOCAL/trunk/gcc/postreload.c:194
0x85a7e9a reload_cse_regs_1
        ../../LOCAL/trunk/gcc/postreload.c:233
0x85a7fda reload_cse_regs
        ../../LOCAL/trunk/gcc/postreload.c:81
0x85a7fda execute
        ../../LOCAL/trunk/gcc/postreload.c:2350

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #36396|0                           |1
        is obsolete|                            |

--- Comment #22 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 36400
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36400&action=edit
Another trail, works with LRA

(In reply to Kazumoto Kojima from comment #21)
> No new failures with -mlra here too.  The test without -mlra is still
> running, though there is a new failure:
> 
> /home/ldroot/dodes/xsh-gcc/gcc/xgcc -B/home/ldroot/dodes/xsh-gcc/gcc/
> -fno-diagnostics-show-caret -fdiagnostics-color=never -O1 -ffat-lto-objects
> -c -o sync-3.o
> /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c
> /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:
> In function 'test_op_ignore':
> /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:
> 75:3: note: '__sync_fetch_and_nand' changed semantics in GCC 4.4
> /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:
> In function 'test_fetch_and_op':
> /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.c-torture/compile/sync-3.c:
> 164:1: error: insn does not satisfy its constraints:
> (insn 729 728 42 2 (set (reg:SI 7 r7)
>         (plus:SI (reg:SI 0 r0)
>             (const_int 0 [0])))

Thanks.  I could reproduce it here with "-O1 -m4 -ml
-matomic-model=soft-gusa,strict" and this snippet:

void test_fetch_and_op (void)
{
signed char sc[2];
unsigned char uc[2];
signed short ss[2];
unsigned short us[2];
signed int si[2];
unsigned int ui[2];
signed long sl[2];
unsigned long ul[2];
signed long long sll[2];
unsigned long long ull[2];

  sc[1] = __sync_fetch_and_and (&sc[1], -11);
  uc[1] = __sync_fetch_and_and (&uc[1], -11);
  ss[1] = __sync_fetch_and_and (&ss[1], -11);
  us[1] = __sync_fetch_and_and (&us[1], -11);
  si[1] = __sync_fetch_and_and (&si[1], -11);
  ui[1] = __sync_fetch_and_and (&ui[1], -11);
  sl[1] = __sync_fetch_and_and (&sl[1], -11);
  sll[1] = __sync_fetch_and_and (&sll[1], -11);
  ull[1] = __sync_fetch_and_and (&ull[1], -11);

  sc[1] = __sync_fetch_and_nand (&sc[1], -11);
  uc[1] = __sync_fetch_and_nand (&uc[1], -11);
  ss[1] = __sync_fetch_and_nand (&ss[1], -11);
  us[1] = __sync_fetch_and_nand (&us[1], -11);
  ul[1] = __sync_fetch_and_nand (&ul[1], -11);
}

Maybe I should run sh-elf tests with -matomic-model=<something> to catch
those...

I couldn't really catch where the const_int 0 gets in.  It happens during
reload, but not through the addsi3 expander.  Then *addsi3 ends up with
non-overlapping-regs operands[0] and operands[1] and operands[2] == const0_rtx,
which effectively is a reg-reg move.  So I've added handling of this case to
the splitters and it seems to fix the problem.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #23 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
My tests are done.  Only
gcc.c-torture/compile/sync-3.c   -O1  (internal compiler error)
for -mno-lra is the new test that fails.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #24 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Thanks!

Could you please re-run that test with attachment 36400?

(Because the problem was triggered only by this test, I think we don't need to
fully re-test it)

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #26 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #25)

> Yes, 36400 fixes that failure:
> PASS: gcc.c-torture/compile/sync-3.c   -O1   (test for warnings, line )

OK, I'll commit attachment 36400 to trunk then.  Do you think it's safe for GCC
5 branch, too?  Or shall we test it on the branch?

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #27 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sun Sep 27 11:55:55 2015
New Revision: 228176

URL: https://gcc.gnu.org/viewcvs?rev=228176&root=gcc&view=rev
Log:
gcc/
        PR target/67391
        * config/sh/sh-protos.h (sh_lra_p): Declare.
        * config/sh/sh.c (sh_lra_p): Make non-static.
        * config/sh/sh.md (addsi3): Use arith_reg_dest for operands[0] and
        arith_reg_operand for operands[1].  Remove TARGET_SHMEDIA case.
        Expand into addsi3_scr if operands[2] if needed.
        (*addsi3_compact): Rename to *addsi3_compact_lra.  Use
        arith_reg_operand for operands[1].  Allow it only when LRA is enabled.
        (addsi3_scr, *addsi3): New insn_and_split patterns.

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

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #28 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I've created a new PR for the LRA addsi3 thing .. PR 67732.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[kkojima at gcc dot gnu.org]

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

--- Comment #29 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #26)
> OK, I'll commit attachment 36400 [details] to trunk then.  Do you think it's
> safe for GCC 5 branch, too?  Or shall we test it on the branch?

I think so, though we should test it on 5-branch.  I'm running tests
on 5-branch now.

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #30 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #29)
> I think so, though we should test it on 5-branch.  I'm running tests
> on 5-branch now.

No new failures on sh-elf with
make -k check
RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #32 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Mon Sep 28 13:44:32 2015
New Revision: 228201

URL: https://gcc.gnu.org/viewcvs?rev=228201&root=gcc&view=rev
Log:
gcc/
        Backport from mainline
        2015-09-27  Oleg Endo  <olegendo@gcc.gnu.org>
                    Kaz Kojima  <kkojima@gcc.gnu.org>

        PR target/67391
        * config/sh/sh-protos.h (sh_lra_p): Declare.
        * config/sh/sh.c (sh_lra_p): Make non-static.
        * config/sh/sh.md (addsi3): Use arith_reg_dest for operands[0] and
        arith_reg_operand for operands[1].  Remove TARGET_SHMEDIA case.
        Expand into addsi3_scr if operands[2] if needed.
        (*addsi3_compact): Rename to *addsi3_compact_lra.  Use
        arith_reg_operand for operands[1].  Allow it only when LRA is enabled.
        (addsi3_scr, *addsi3): New insn_and_split patterns.

Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/sh/sh-protos.h
    branches/gcc-5-branch/gcc/config/sh/sh.c
    branches/gcc-5-branch/gcc/config/sh/sh.md

[Bug target/67391] [SH] Convert clrt addc to normal add insn

[olegendo at gcc dot gnu.org]

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

--- Comment #33 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Maybe we should leave this one open for a little bit longer...