On 2015-01-12 2:25 PM, Jeff Law wrote:
On 01/08/15 04:00, Ajit Kumar Agarwal wrote:
Hello Vladimir:
We have made the changes in the ira-color.c in ira_loop_edge_freq
and move_spill_restore. The main motivation
behind the change is to reduce the memory instruction with respect to
the Loops. The changes are done to not to
consider the back edge frequency if there are no references of the
allocno inside the loop even though the allocno
are live out at the exit of the Loop and Live In at the entry of the
Loop. This will reduce the calculation of the cost of
putting allocno into memory. If we reduce the cost of putting allocno
into memory, then there are chances of getting
color in the simplification phase of the gcc IRA register allocator.
Enabling this, there are chances of putting the allocno
in the register and the splitting phase will split the live range
across the Loop boundary. This will beneficial with respect
to reducing memory instruction inside the Loop. Other changes is done
to enable some of the allocno to allocate in
memory if the cost is less than equal to zero.
This is the first phase of the change in the IRA core register
module, I would like to get your feedbacks on this change
based on this the actual patch is sent. The changes are accompanied
with the patch created for the change which will
be easier for you to review the change.
We have tested the changes for Microblaze target for MIBENCH and
EEMBC benchmarks and there are no degradation
in the Geomean and some of the Mibench and EEMBC benchmarks is
benefitted with this change.
Please review the changes and let me know your feedbacks and other
steps needs to be done to incorporate the
changes in the Upstream". We would like to get this change for
Microblaze, Please also let us know if the changes are
required to be enabled with any switch flag.
There's a lot of similarities between this and one aspect of Morgan's
register pressure reduction schemes. I believe he refers to objects
of this nature as transparent across the loop (value is live across
the loop, but neither set nor referenced in the loop -- ie, it's just
holding a resource across the loop).
If a loop has high register pressure, then homing these kind of
objects in memory across the loop will reduce the register pressure in
the loop and presumably allow some other objects with more value to
live in a register inside the loop. Of course, if there is little
pressure in the loop, then homing to memory is a waste.
Your approach of ignoring backedge frequency for cost calculation of
these objects is interesting in that it tries to capture the benefits
of Morgan-esque pressure reduction without regressing in cases where
the pressure is not high.
Ignoring loop back-edges is a right idea but it is already implemented
in IRA from the start. On enter we already ignore back edge to the loop
header with condition:
e->src == loop_node->loop->latch
on exit we consider edges only
get_loop_exit_edges (loop_node->loop)
The patch does not exclude back edges (they are already excluded), it
excludes all edges from the consideration for the transparent pseudos.
In other words, the patch ignores any allocation in the parent loop.
IMO it should not do this, with this patch we can spill pseudo even if
the pseudo is allocated to hard reg and there are enough regs to keep
the transparent pseudo in the register.
If you've got data which does this does not regress on a more
mainstream target and that it bootstrapps and regression tests on a
mainstream target (x86_64 is by far the most popular), then I'd
seriously consider approving this.
I've just got results of the patch for the SPEC2000:
The patch worsens the code on x86-64 SPEC2000 (5094 vs
5104 on SPECInt and 7125 vs 7137 on SPECFP). The patch also results
in slightly bigger average code (0.003% and 0.03% correspondingly for
Int and FP benchmarks)
So I believe the patch is no go.