harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Egor Pasko <egor.pa...@gmail.com>
Subject Re: [drlvm][jit] possible ABCD bug
Date Wed, 04 Oct 2006 05:53:53 GMT
On the 0x1F8 day of Apache Harmony Naveen Neelakantam wrote:
> >> 2) Also in updateMemDistanceWithPredecessors, I added an
> >> optimization.  Essentially, we can take advantage of ProveResult
> >> being a lattice (i.e. is monotonic) to prevent some recursive proofs.
> >
> > oh, the optimization is fine, but seems that it would make a
> > suspicious 'Reduced' instead of 'True' sometimes, which still looks
> > like OK for our purposes. It won't give us a big performance gain due
> > to the sparse nature of the inequality graph.. Though, optimizing
> > prematurely makes a lot of fun, so I like it :) I would suggest to
> > move the 'if' statement outside the loop because it is a loop
> > invariant. Elinimanting this 'break' would be a better style.
> 
> The if statement is not loop invariant because the value of "res"
> changes each loop iteration.  Am I missing something?  As for the
> "break", I can definitely rewrite the loop to get rid of it.

Wait a minute, I need to check if my hair became blond. Now "break"
looks fine. Sorry:)

> > One more to say on the patch:
> > +            //            meetBest(Reduced, x) <= Reduced
> >
> > should be:
> > +            //            meetBest(Reduced, x) >= Reduced
> > (just a comment, but still...)
> >
> > so, could you, please, refresh the patch with my suggestions
> > implemented?
> 
> Will do, once we come to agreement above.

we have it now

> >> Once again, your code is very good.  It is also nice that it matches
> >> the algorithm outlined in the ABCD paper wherever possible.
> >
> > heh:) did you notice that "line 9" of the algorithm? (Figure 5 in the
> > paper). I replaced '>' by '<', and only after that it worked
> > (classic_abcd_solver.cpp:645). I suppose, it's a bug in the
> > paper. Could you, please, double check this?
> 
> I did not notice it before, but I agree that the bug is in the paper.

I love this game :)

> >> Is there something else I can do to help?
> >
> > Yeah, now it's my step. We would do our best if I publish my
> > HIR->Igraph conversion scratches (they don't work yet).
> >
> > Unfortunately, I am so busy with ClassLoaderTest which fails after
> > some seemingly correct optimizations in 'memopt'.. I'll open the
> > sources in a couple of days. Can you wait so long?
> 
> No worries.  I am also working on other things, but would like to try
> and move this forward whenever possible.

OK, I'll submit the code this week. Maybe, even tomorrow. (I have some
progress on ClassLoaderTest)

-- 
Egor Pasko, Intel Managed Runtime Division


---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message