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 <ja...@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
