This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] profile feedback: -fprofile-use= and -fprofile-correction, correctness fixes and option semantic changes.


2008/4/3 Seongbae Park (박성배, 朴成培) <seongbae.park@gmail.com>:
> On Thu, Apr 3, 2008 at 9:58 AM, Seongbae Park (박성배, 朴成培)
> <seongbae.park@gmail.com> wrote:
>>
>> On Wed, Apr 2, 2008 at 11:39 PM, Jan Hubicka <jh@suse.cz> wrote:
>>  > >
>>  >  > Attached is the separate patch that preserves the value histogram
>>  >  > when replacing a whole statement in set_rhs.
>>  >  > I'm trying to reduce the testcase that causes the ICE
>>  >  > but no luck so far - I'll add the testcase as soon as I can.
>>  >  > Bootstrapped and regtested with other patches.
>>  >  > Ok for mainline ?
>>  >  >
>>  >  > Seongbae
>>  >  >
>>  >  > 2008-04-02  Seongbae Park  <seongbae.park@gmail.com>
>>  >  >
>>  >  >         * tree-ssa-propagate.c (set_rhs): Preserve the histogram.
>>  >  >         * value-prof.c (gimple_move_stmt_histograms): New function.
>>  >  >         * value-prof.h (gimple_move_stmt_histograms): New function declaration.
>>  >
>>  >  The patch looks fine from histograms perspective, but it seems to me
>>  >  that you also need something like
>>  >
>>  >   /* Preserve EH region information from the original statement, if
>>  >      requested by the caller.  */
>>  >   eh_region = lookup_stmt_eh_region (orig_stmt);
>>  >   if (eh_region >= 0)
>>  >     {
>>  >       remove_stmt_from_eh_region (orig_stmt);
>>  >       add_stmt_to_eh_region (stmt, eh_region);
>>  >     }
>>  >
>>  >  To move the EH info around, or do we already take care of that or is
>>  >  there some simple reason why it can't be around I've missed?
>>  >
>>  >  Honza
>>
>>  I don't think this is taken care of anywhere, and I can't convince myself
>>  this won't happen either (I thought I had a good reason to believe
>>  it won't happen, but I can't remember now :P ).
>>  Here's the updated patch.
>>  Bootstrapped on i686. Regtest still in progress.
>>
>>  Seongbae
>>
>>
>>  2008-04-03  Seongbae Park  <seongbae.park@gmail.com>
>>
>>
>>         * tree-ssa-propagate.c (set_rhs): Preserve the histogram
>>         and the eh region information.
>>
>>
>>         * value-prof.c (gimple_move_stmt_histograms): New function.
>>         * value-prof.h (gimple_move_stmt_histograms): New function declaration.
>>
>
> Regtest found one regression.
> It turns out we replace a call to __builtin_strcpy to __builtin_memcpy
> but __builtin_strcpy is not marked with ECF_NOTHROW but memcpy is.
> So the above code does the wrong thing - since it puts builtin_memcpy
> which is ECF_NOTHROW, into eh region.
>
> Of course, probably the right thing to do is mark builtin strcpy as no throw,
> but that's not the point of this patch.
> Attached is the updated patch. Restarted bootstrap & regtest.
> The ChangeLog remains the same.
>
> Seongbae
>

ping^3 (or ^4 counting ping on irc :)) for the above patch and:

http://gcc.gnu.org/ml/gcc-patches/2008-04/msg00112.html

Seongbae

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]