Please review fix for 7120481 storeStore barrier in constructor with final field
john.r.rose at oracle.com
Wed Feb 8 17:04:47 PST 2012
That output code looks very nice. And it looks like a reasonable approach to fixing the ordering problems. But the implementation code needs some adjustments.
I haven't reviewed it all, but I have a couple of comments:
ciMethod.hpp : Please do not put state on the CI types. They are read-only mirrors into the JVM metadata. If you need compiler-local state, use the data structures in src/share/vm/c1. I would look at IRScope as a candidate (compare _number_of_locks).
rewriter.hpp : Please do not make TRAPS an optional argument. That subverts the static checking that the TRAPs macro is designed for.
Also, do not use TRAPS as a shorthand for passing a thread argument. It means something else.
I suggest that you just pass what you need (the constantPoolHandle, not the thread and the instanceKlassHandle).
The logic for checking for final fields is wrong; it looks up the name/type of the field in klass, whereas it should respect the klass named in the constant pool.
Adding a new bytecode (_return_with_membar) is a relatively high-cost change, and requires many touches in many places in the source base. I suspect you have not found them all yet, since a grep for _return_register_finalizer shows files not on your webrev.
But if you are going to use the new-interpreter-instruction tactic, I suggest you merge your new logic into _return_register_finalizer, using that instruction as a general "slow path" form of "_return". (You could rename it as _return_special; check with Tom who wrote it.) There will be no difference in performance. Adding a new interpreter bytecode requires a strong performance justification, and you only need one "special return".
On Feb 8, 2012, at 3:35 PM, Jiangli Zhou wrote:
> Please review the following webrevs for 7120481 storeStore barrier in constructor with final field:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-compiler-dev