[Bug target/66930] [5 Regression]: gengtype.c is miscompiled during stage2

["olegendo at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

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

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Kazumoto Kojima from comment #6)
> Created attachment 36040 [details]
> .i file for gengtype.c
> 
> I've confirmed a miscompile for gengtype.c with -O1 on my 5/6
> compilers.  With them,
> 
>      if (union_p)
>        {
> 	 oprintf (d->of, "%*sbreak;\n", d->indent, "");
> 	 d->indent -= 2;
>        }
> 
> lines in gengtype.c:walk_type function are compiled like as:
> 
>         bf      .L2253
>         mov.l   .L2593,r7
>         mov.l   @(36,r12),r6
>         mov.l   .L2580,r5
>         mov.l   .L2595,r1
>         jsr     @r1
> 	...
> 
> i.e. the instruction testing union_p variable is removed.
> The resulted gengtype produces files without "break" in many
> cases.  Although it's the other way around with the reported
> full of "break" symptom, I think the both are the same issue.
> The deletion has happened in sh_split_movrt_negc_to_movt_xor
> which is called by movrt_negc insn_and_split.  It seems that
> that splitting is applied for the case
> 
> 	tst	reg,reg
> 	mov	#-1,reg1
> 	negc	reg1,reg1
>         ...
>         call ...
>         ...
> 	tst	reg,reg
> 
> and the last tst was removed.  Oleg, could you take a look into
> this?

Somehow I couldn't find the code patterns above in my version of the compiled
code, so I've compared the compiled asm code of gengtype.ii with
sh_split_movrt_negc_to_movt_xor and without (always returns false) at -O1 and
hit this diff:

without:
        mov.l   @(40,r15),r0
        cmp/eq  #5,r0
        mov     #-1,r0
        negc    r0,r0
        mov.b   r0,@(2,r8)
        mov     r12,r5
        mov.l   .L2664,r1
        jsr     @r1
        mov.l   @(8,r10),r4
        mov     r14,r0
        mov.b   r0,@(2,r8)
        mov.l   .L2631,r1
        jsr     @r1
        mov     r13,r4
        mov.l   @(40,r15),r0
        cmp/eq  #5,r0
        bf      .L2286

with:
        mov.l   @(44,r15),r0
        cmp/eq  #5,r0
        movt    r0
        xor     #1,r0
        mov.b   r0,@(2,r8)
        mov     r12,r5
        mov.l   .L2664,r1
        jsr     @r1
        mov.l   @(8,r10),r4
        mov     r14,r0
        mov.b   r0,@(2,r8)
        mov.l   .L2631,r1
        jsr     @r1
        mov     r13,r4
        bf      .L2286

The original idea of this transformation is to not clobber the T bit with the
negc-movrt insn and re-use the T bit later.  It seems that it's missing the
fact that the T bit is clobbered by calls.  The following seems to fix it:

Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c  (revision 225987)
+++ gcc/config/sh/sh.c  (working copy)
@@ -13888,6 +13888,7 @@
       && !sh_insn_operands_modified_between_p (t_before_negc.insn,
                                               t_before_negc.insn,
                                               t_after_negc.insn)
+      && !modified_between_p (get_t_reg_rtx (), curr_insn, t_after_negc.insn)
       && !sh_unspec_insn_p (t_after_negc.insn)
       && !volatile_insn_p (PATTERN (t_after_negc.insn))
       && !side_effects_p (PATTERN (t_after_negc.insn))

I've added this code as part of PR 63986.  I've checked with make -k check-gcc
RUNTESTFLAGS="sh.exp --target_board=sh-sim\{-m2a/-mb,-m4/-ml,-m4/-mb}" that
there are no new SH specific fails, so I assume the patch above should be OK
(although all the checks should probably be combined to avoid walking the insns
repeatedly).  Could you please add it to your test run?  I'm sorry for this
nasty bug.  Obviously it's not only a problem for -O1 but for anything > -O0.