Option -fprofile-arcs fails for target avr and fr30

Option -fprofile-arcs fails for target avr and fr30

[Jie Zhang]

Hi, I have a question about using -fprofile-arcs option on avr and fr30 
targets for gcc-3.3.

Avr and fr30 targets have no DI mode add instructions, nor add with 
carry instructions. But profile instrument code needs DI mode add. GCC 
has to translate the DImode add into a sequence of instructions. That 
will introduce new jump_insn, which will cause make_edges abort. Testing 
gcc.misc-tests/bprob-1.c reveals this problem.

I think GCC should take this case into acount. But I have no good method 
by now. Does anyone have good solution? Thanks!

- J

[PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[Jie Zhang]

Jie Zhang wrote:

> Hi, I have a question about using -fprofile-arcs option on avr and 
> fr30 targets for gcc-3.3.
>
> Avr and fr30 targets have no DI mode add instructions, nor add with 
> carry instructions. But profile instrument code needs DI mode add. GCC 
> has to translate the DImode add into a sequence of instructions. That 
> will introduce new jump_insn, which will cause make_edges abort. 
> Testing gcc.misc-tests/bprob-1.c reveals this problem.
>
> I think GCC should take this case into acount. But I have no good 
> method by now. Does anyone have good solution? Thanks!
>
> - J 

The following patch may fix this problem.

- J


2003-05-12  Jie Zhang  <zhangjie@magima.com.cn>

        Fix the FAILs of gcc.misc-tests/bprob-1.c and 
gcc.misc-tests/bprob-2.c
        on target avr and fr30.
        * cfgbuild.c (make_edges): Find JUMP_LABELs for the new JUMP_INSNs
        introduced by instrument code after jump optimization pass.

--- cfgbuild.c  2003-01-09 20:40:44.000000000 +0800
+++ cfgbuild-new.c      2003-05-19 09:58:23.000000000 +0800
@@ -398,8 +398,17 @@ make_edges (label_value_list, min, max,
          /* Otherwise, we have a plain conditional or unconditional 
jump.  */
          else
            {
+              rtx tmp;
+
+              /* Instrument code may introduce new JUMP_INSNs after jump
+                 optimization pass. So we find JUMP_LABELs for these new
+                 JUMP_INSNs. */
+              if (! JUMP_LABEL (insn) && (tmp = condjump_label (insn)))
+                JUMP_LABEL (insn) = XEXP (tmp, 0);
+
              if (! JUMP_LABEL (insn))
                abort ();
+
              make_label_edge (edge_cache, bb, JUMP_LABEL (insn), 0);
            }
        }

Re: [PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[Nathan Sidwell]

Jie Zhang wrote:

> The following patch may fix this problem.
> 
> - J
> 
> 
> 2003-05-12  Jie Zhang  <zhangjie@magima.com.cn>
> 
>        Fix the FAILs of gcc.misc-tests/bprob-1.c and 
> gcc.misc-tests/bprob-2.c
>        on target avr and fr30.
>        * cfgbuild.c (make_edges): Find JUMP_LABELs for the new JUMP_INSNs
>        introduced by instrument code after jump optimization pass.
It would probably be sensible to only enable that code when flag_profile_arcs
is true -- otherwise you might be hiding a bug in GCC. Jan, what do you
think?

nathan

> --- cfgbuild.c  2003-01-09 20:40:44.000000000 +0800
> +++ cfgbuild-new.c      2003-05-19 09:58:23.000000000 +0800
> @@ -398,8 +398,17 @@ make_edges (label_value_list, min, max,
>          /* Otherwise, we have a plain conditional or unconditional 
> jump.  */
>          else
>            {
> +              rtx tmp;
> +
> +              /* Instrument code may introduce new JUMP_INSNs after jump
> +                 optimization pass. So we find JUMP_LABELs for these new
> +                 JUMP_INSNs. */
> +              if (! JUMP_LABEL (insn) && (tmp = condjump_label (insn)))
> +                JUMP_LABEL (insn) = XEXP (tmp, 0);
> +
>              if (! JUMP_LABEL (insn))
>                abort ();
> +
>              make_label_edge (edge_cache, bb, JUMP_LABEL (insn), 0);
>            }
>        }
> 


-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
          The voices in my head said this was stupid too
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: [PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[Jan Hubicka]

> Jie Zhang wrote:
> 
> >The following patch may fix this problem.
> >
> >- J
> >
> >
> >2003-05-12  Jie Zhang  <zhangjie@magima.com.cn>
> >
> >       Fix the FAILs of gcc.misc-tests/bprob-1.c and 
> >gcc.misc-tests/bprob-2.c
> >       on target avr and fr30.
> >       * cfgbuild.c (make_edges): Find JUMP_LABELs for the new JUMP_INSNs
> >       introduced by instrument code after jump optimization pass.
> It would probably be sensible to only enable that code when 
> flag_profile_arcs
> is true -- otherwise you might be hiding a bug in GCC. Jan, what do you
> think?

There is code to set the JUMP_LABEL properly in the code to emit insn on
the edges: 
	      insert_insn_on_edge (edge_profile, e);
	      rebuild_jump_labels (e->insns);
rebuild_jump_labels is supposed to update JUMP_LABEL accordingly.
In case the profiling code somewhere does not update labels correctly it
is the bug on place where code is emit.  I will try to figure out what
happens tomorrow. At the time the basic blocks are split, all
JUMP_LABELs are supposed to be up-to-date.

Honza

Re: [PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[Jie Zhang]

Jan Hubicka wrote:

>There is code to set the JUMP_LABEL properly in the code to emit insn on
>the edges: 
>	      insert_insn_on_edge (edge_profile, e);
>	      rebuild_jump_labels (e->insns);
>rebuild_jump_labels is supposed to update JUMP_LABEL accordingly.
>In case the profiling code somewhere does not update labels correctly it
>is the bug on place where code is emit.  I will try to figure out what
>happens tomorrow. At the time the basic blocks are split, all
>JUMP_LABELs are supposed to be up-to-date.
>
>Honza
>
>
>  
>
Sorry, I didn't see that rebuild_jump_labels was added into HEAD of cvs 
on Mar 30, 2003, I'm only tracking 3.3 branch now, which had been 
branched before it was added. Now how do we fix this bug on 3.3 branch?

- Jie

Re: [PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[Jan Hubicka]

> Jan Hubicka wrote:
> 
> >There is code to set the JUMP_LABEL properly in the code to emit insn on
> >the edges: 
> >	      insert_insn_on_edge (edge_profile, e);
> >	      rebuild_jump_labels (e->insns);
> >rebuild_jump_labels is supposed to update JUMP_LABEL accordingly.
> >In case the profiling code somewhere does not update labels correctly it
> >is the bug on place where code is emit.  I will try to figure out what
> >happens tomorrow. At the time the basic blocks are split, all
> >JUMP_LABELs are supposed to be up-to-date.
> >
> >Honza
> >
> >
> > 
> >
> Sorry, I didn't see that rebuild_jump_labels was added into HEAD of cvs 
> on Mar 30, 2003, I'm only tracking 3.3 branch now, which had been 
> branched before it was added. Now how do we fix this bug on 3.3 branch?
Bacporting that patch I guess.  It should be safe.

Honza
> 
> - Jie
> 

Re: [PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[DJ Delorie]

> There is code to set the JUMP_LABEL properly in the code to emit insn on
> the edges: 
> 	      insert_insn_on_edge (edge_profile, e);
> 	      rebuild_jump_labels (e->insns);
> rebuild_jump_labels is supposed to update JUMP_LABEL accordingly.
> In case the profiling code somewhere does not update labels correctly it
> is the bug on place where code is emit.  I will try to figure out what
> happens tomorrow. At the time the basic blocks are split, all
> JUMP_LABELs are supposed to be up-to-date.

When I originally tracked this down, the problem was that profiling is
added after the last time rebuild_jump_labels is called, so any new
jumps won't have jump_labels.

Re: [PATCH] Re: Option -fprofile-arcs fails for target avr and fr30

[Zdenek Dvorak]

Hello,

> Jie Zhang wrote:
> 
> >Hi, I have a question about using -fprofile-arcs option on avr and 
> >fr30 targets for gcc-3.3.
> >
> >Avr and fr30 targets have no DI mode add instructions, nor add with 
> >carry instructions. But profile instrument code needs DI mode add. GCC 
> >has to translate the DImode add into a sequence of instructions. That 
> >will introduce new jump_insn, which will cause make_edges abort. 
> >Testing gcc.misc-tests/bprob-1.c reveals this problem.
> >
> >I think GCC should take this case into acount. But I have no good 
> >method by now. Does anyone have good solution? Thanks!
> >
> >- J 
> 
> The following patch may fix this problem.
> 

you might consider to add a call of rebuild_jump_labels (insns)
to the beginning of commit_one_edge_insertion instead (it would
in effect prevent us from using it to add jump instructions to
outside of the insns, for which I am not sure whether it is needed
anywhere.

Zdenek

Btw. post the patches to the appropriate list too.


> 
> 2003-05-12  Jie Zhang  <zhangjie@magima.com.cn>
> 
>        Fix the FAILs of gcc.misc-tests/bprob-1.c and 
> gcc.misc-tests/bprob-2.c
>        on target avr and fr30.
>        * cfgbuild.c (make_edges): Find JUMP_LABELs for the new JUMP_INSNs
>        introduced by instrument code after jump optimization pass.
> 
> --- cfgbuild.c  2003-01-09 20:40:44.000000000 +0800
> +++ cfgbuild-new.c      2003-05-19 09:58:23.000000000 +0800
> @@ -398,8 +398,17 @@ make_edges (label_value_list, min, max,
>          /* Otherwise, we have a plain conditional or unconditional 
> jump.  */
>          else
>            {
> +              rtx tmp;
> +
> +              /* Instrument code may introduce new JUMP_INSNs after jump
> +                 optimization pass. So we find JUMP_LABELs for these new
> +                 JUMP_INSNs. */
> +              if (! JUMP_LABEL (insn) && (tmp = condjump_label (insn)))
> +                JUMP_LABEL (insn) = XEXP (tmp, 0);
> +
>              if (! JUMP_LABEL (insn))
>                abort ();
> +
>              make_label_edge (edge_cache, bb, JUMP_LABEL (insn), 0);
>            }
>        }
>