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: PR80357: Negative register pressure


On 04/14/2017 04:37 AM, Richard Sandiford wrote:
In the PR testcase, there were two instructions that had
a large number of insn_reg_use records for the same register.
model_recompute was instead expecting the records to be unique
and so decremented the register pressure for each one.  We then
ended up with a negative pressure.

I think the records *should* be unique here, and that this is
really a bug in the generic -fsched-pressure code.  Making them
unique could be too invasive for GCC 7 though.  There are at
least two problems:

(1) sched-deps.c uses rtx_insn_lists instead of bitmaps to record
     the set of instructions that use a live register.
     sched-rgn.c then propagates this information between blocks
     in a region using list concatenation:

       succ_rl->uses = concat_INSN_LIST (pred_rl->uses, succ_rl->uses);

     So dependencies for common predecessors will appear multiple
     times in the list.

     In this case (and for the PR), it might be enough to make
     setup_insn_reg_uses detect duplicate entries.  However...

(2) setup_insn_reg_uses adds entries for all queued uses of a register
     at the point that the register is expected to die.  It looks like
     this doesn't work reliably for REG_INC registers: if a register R
     is auto-incremented in I1 and then used for the last time in I2,
     setup_insn_reg_uses will first treat the original R as dying
     in I1 (rightly IMO).  But that use of R in I1 is still in the
     reg_last->uses list when processing I2, so setup_insn_reg_uses
     adds it a second time.

There might be more reasons for multiple records: I stopped looking
at this point.

I think for GCC 7 it'd be more pragmatic to live with the duplicate
entries and keep the fix specific to SCHED_PRESSURE_MODEL.

Tested on aarch64-linux-gnu (which uses SCHED_PRESSURE_MODEL by default)
and on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	PR rtl-optimization/80357
	* haifa-sched.c (tmp_bitmap): New variable.
	(model_recompute): Handle duplicate use records.
	(alloc_global_sched_pressure_data): Initialize tmp_bitmap.
	(free_global_sched_pressure_data): Free it.

gcc/testsuite/
	PR rtl-optimization/80357
	* gcc.c-torture/compile/pr80357.c: New test.
OK.

I'm going to go ahead and commit.

Jeff


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