RFR(M): 8146612: C2: Precedence edges specification violated
martin.doerr at sap.com
Fri Jan 8 11:06:42 UTC 2016
thanks for the review.
I have changed the comments, added assertions and factored out the common functionality of del_req(), del_req_ordered() and rm_prec() into a new private function close_prec_gap_at(). That makes sense.
About your concern about accessing outside of _in array in rm_prec():
Please note that i is decremented before it gets used:
"j == _max-1", "i" will be set to "_max", but decremented in "_in[--i]"
Anyway, I have replaced this code by close_prec_gap_at(), so it doesn't matter anymore.
The new webrev is here:
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
Sent: Donnerstag, 7. Januar 2016 23:08
To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(M): 8146612: C2: Precedence edges specification violated
// Avoid spec violation: multiple prec edge.
I think should be:
// Avoid spec violation: duplicated prec edge.
Should we add assert to rm_prec()?:
assert(j >= _cnt, "not a precedence edge");
Also we may need to check that input index is < _max in set_prec() and rm_prec().
Next access will be outside _in array if j == _max-1 (in rm_prec()):
_in[i] = NULL; // NULL out last element
unless we guarantee that there is always NULL at the end. Which I don't see because set_prec() may set the last prec
edge to not NULL.
Please factor out similar code (search for last non-NULL prec edge) in del_req(), del_req_ordered() and rm_prec() into
On 1/7/16 5:45 AM, Doerr, Martin wrote:
> some time ago, we found out, that C2 doesn't treat precedence edges as specified.
> The description of precedence edges in node.hpp says:
> "They are unordered and not duplicated; they have no embedded NULLs."
> Some functions in the current implementation violate this specification.
> I have fixed this in the following webrev:
> Please review. I will need a sponsor, please.
> Best regards,
More information about the hotspot-compiler-dev