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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
