This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Follow-up regcprop fixes
On 3/27/19 4:24 PM, Jakub Jelinek wrote:
> On Wed, Mar 27, 2019 at 04:32:15PM +0100, Jakub Jelinek wrote:
>> On Wed, Mar 27, 2019 at 09:24:46AM -0600, Jeff Law wrote:
>>>> 2) another thing I've noticed today, we queue up the debug insn changes,
>>>> but if we iterate the second time for any bb, we overwrite and thus lose any
>>>> of the debug insn queued changes from the first iteration, so those changes
>>>> aren't applied (wrong-debug?)
>>> This is actually what worries me, both with the current bits and with
>>> any bits that change to a worklist of blocks to reprocess. As is I
>>> think we preserve the debug stuff across the iterations which should
>>> keep debug info correct. Managing that in a world where we queue up the
>>> iteration step is going to be tougher.
>>
>> The patch I've posted would do df_analyze, all bbs once, then df_analyze,
>> process queued debug changes, and if anything needs to be repeated (just
>> once), redo the bbs from worklist and repeat the above.
>> That seems to me the easiest way to get rid of the per-bb df_analyze calls
>> as well as fixing the debug stuff. Because otherwise, we'd need to somehow
>> merge the queued debug insns from the first and second pass and figure out
>> how to deal with that.
>
> Here is everything in patch on top of current trunk which contains your
> regcprop.c changes.
>
> In addition to the previously mentioned changes, as you've requested it
> clears the visited sbitmap between the two passes, fixes clearing of the
> all_vd[bb->index].e[regno].debug_insn_changes pointers after
> cprop_hardreg_debug processes them and fixes up the n_debug_insn_changes
> updates, so that the invariant that it always is the sum of length of the
> debug_insn_changes chains for all hard regs in the bb is true (before that
> n_debug_insn_changes could be higher, and in the final debug insn processing
> case wasn't actually decremented at all, so the == 0 check was useless
> there).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk if it
> succeeds on some other targets too?
It's run through my tester without any issues.
>
> 2019-03-27 Jakub Jelinek <jakub@redhat.com>
>
> * regcprop.c (copyprop_hardreg_forward_1): Remove redundant INSN_P
> test.
> (cprop_hardreg_bb, cprop_hardreg_debug): New functions.
> (pass_cprop_hardreg::execute): Use those. Don't repeat bb processing
> immediately after first one with df_analyze in between, but rather
> process all bbs, queueing ones that need second pass in a worklist,
> df_analyze, process queued debug insn changes and if second pass is
> needed, process bbs from worklist, df_analyze, process queued debug
> insns again.
OK.
Jeff