Request for review(M): 6627983: G1: Bad oop deference during marking
john.r.rose at oracle.com
Thu Feb 24 17:27:00 PST 2011
On Feb 24, 2011, at 4:49 PM, Igor Veresov wrote:
> Just by code analysis. Did I miss something?
I felt a slight doubt because there are tricky paths through the various subroutine layers in library_call. I see you converted "must_clear_dest" to "suppress_pre_barrier" along two paths. There's a path through generate_block_arraycopy which *almost* needs the flag also, but it allows the flag to default.
To make the code more obviously correct, I suggest making the flag non-optional in library_call. And when the role of the flag changes, a line or two of code could mark the transition like this:
bool use_pre_barrier = (bt == T_OBJECT); // usual case
if (must_clear_dest) use_pre_barrier = false; // 6627983 suppress pre_barrier, if any
If it is simply foo(..., !must_clear_dest), it becomes hard for maintainers to see what the argument means.
In the stub generator the optional boolean is OK as is, although I generally prefer optional booleans to default to 'false'. Given the way the bug is structured, I keep wanting to call it 'suppress_pre_barrier', meaning "just this once, trust me when I tell you not to emit the pre-barrier, if there is one."
More information about the hotspot-compiler-dev