harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Naveen Neelakantam <neela...@uiuc.edu>
Subject Re: [drlvm][jit] possible ABCD bug
Date Wed, 04 Oct 2006 04:34:14 GMT

On Oct 3, 2006, at 12:29 AM, Egor Pasko wrote:

> On the 0x1F7 day of Apache Harmony Naveen Neelakantam wrote:
>> Hello Egor,
>>
>> I finally got a chance to read through your code.  It is well
>> designed, and correct (as far as I can tell).  I especially like the
>> way that the upper bounds solver works as the lower bounds solver
>> simply by flipping a flag (and both operate on the same inequality
>> graph).  Very nice!
>
> Too many good words for me, cannot handle it)) Thnx

:-)

>> 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.

> 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.

>> 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.

>> 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.

> What I did:
> * Created a new optimization pass "classic_abcd"
> * moved the Pi insertion logic to a separate Class (and file
>   insertpi.cpp) to reuse it in "classic_abcd"
> * to use an Opnd in Inequality Graph like an IOpnd, I made an
>   "IOpndProxy : public IOpnd" containing an original operand and
>   tweaking here and there to make them have unique ids :)
> * Started creating Inequality graph (not all info reused, some bugs,
>   crashes, etc.)
>
> I would be happy if you look at that code, and finish it up to an
> initial version while I am stuck with those critical bugz.

I can certainly take a look and try to finish it up.

> Do you have some other ideas how we can collaborate better at this
> point?

I'm happy, so not really.  :-)

Naveen

---------------------------------------------------------------------
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