On 12-10-12 4:12 AM, Steven Bosscher wrote:
On Fri, Oct 12, 2012 at 6:16 AM, Vladimir Makarov wrote:
On 12-10-11 4:17 PM, Steven Bosscher wrote:
Hello,
IRA uses record_loop_exits() to cache the loop exit edges, but due to
a code ordering bug the edges are not actually recorded.
record_loop_exits() starts with:
if (!current_loops)
return;
I have no idea why record_loop_exits is here. Loop exits are not used in
IRA. I think it should be removed.
Actually they are used alright:
$ grep -n get_loop_exit_edges ira*
ira-build.c:169: edges = get_loop_exit_edges (loop);
ira-build.c:1774: edges = get_loop_exit_edges (loop_node->loop);
ira-build.c:1972: edges = get_loop_exit_edges (loop);
ira-color.c:2023: edges = get_loop_exit_edges (loop_node->loop);
Especially the last one, in ira_loop_edge_freq, seems to be called
frequently enough to justify caching loop exit edges.
Ops. Sorry, Steven. I did a wrong conclusion because I thought I would
have found such code generation problem if it had an affect.
I've just tried about 20 tests and did not find a difference in code
generation when I moved current_loops assignment above record_loop_exits ().
I did some investigation and it seems to me it is important to have
the right loop exits when such exits are abnormal edges. It would be
problem if there is a generation of reg shuffle insns on such edges.
But it does not happen (if it happens it should result in compiler
crash), probably IRA makes the same allocation of pseudos somehow on
different ends of such edges or such cases are extremely rare.
Another problem would be a possibility of different allocation when loop
has a pseudo living at the exit edges but not on the enters which I
think is also a rare situation.
Still it is good to have right loop exits for some rare corner cases.
So your patch is ok. Thanks for working on this.
So ira.c should set current_loops before calling record_loop_exits.
With the current order, the exists are not actually recorded. Also,
ira.c should release recorded exits and loops, this releases a
considerable amount of GC memory that can be re-used in the next IRA
iteration.
It's not clear to me why IRA has to re-compute the cfgloop tree from
scratch, though, so I've added a "???" comment for that.
I am not a specialist in cfgloop code. I did not find how to update it
incrementally. So I just rebuild it.
My be you can advice how to do this. IRA can generate insns on the BB
edges, it results in creation of new BBs on the region exits/enters.
AFAIU this mostly happens automatically for simple edge splitting
operations, and for more complex transformations there is
fix_loop_structure. But I'm not sure, perhaps Richi can give some
advice.
