On 03/19/2011 08:15 AM, Jakub Jelinek wrote:
On Sat, Mar 19, 2011 at 08:08:48AM -0400, Kenneth Zadeck wrote:
On 03/19/2011 05:19 AM, Paolo Bonzini wrote:
On Fri, Mar 18, 2011 at 19:18, Kenneth Zadeck<zad...@naturalbridge.com> wrote:
yes, but i think that the reason this is a pr is that it seems to be needed
for correctness.
Note that df_get_bb_dirty is defaulting to "return false", not
"abort". This is what tricked crossjumping and caused the bug.
where does the compiler fail if you change it to abort, (after doing
the check from jakub's patch)?
With neither of the patches of mine for this issue, df_get_bb_dirty
would ICE with added gcc_assert (df&& df_live);
The point is that df_get_bb_dirty was silently returning false even for
blocks which had lr not recumputed when live wasn't computed at all.
So live isn't needed for crossjumping correctness, assuming df_get_bb_dirty
is compatible with what df_get_live_out does (the first patch).
Jakub
i think that there are two separate questions here:
1) should your original patch go in as you did it, or should it go in
with the last "return false" be an abort?
2) which dataflow problems should be used at what points during
optimization at any level. There is, and will always be, a question of
precision vs cost. Live is both more expensive and more precise.
The questions about the second patch with respect to crossjumping is:
does the extra precision either make any difference? and is required for
correctness? My guess is that it most likely is not required for
correctness and may not even make any difference. However, I never
looked closely at what cross jumping did - i get the feeling that Paolo
has more experience here so I defer to him on this.
I do believe in the principal that if someone specifies something like
cross jumping explicitly, that they get substantially the same
optimization at whatever -O level they run it. So if live does make
any difference in the quality of the optimization, then we should turn
on live to run it.
However, if it does not, then some form of your first patch is all that
is necessary.
kenny
