[lworld] RFR: 8228634: [lworld] ciField::will_link() returns incorrect result for the withfield bytecode
fparain at openjdk.java.net
Fri Dec 11 15:16:10 UTC 2020
On Fri, 11 Dec 2020 12:44:57 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:
> In `GraphBuilder::withfield`, `ciField::will_link()` can return false even if the holder klass is loaded (see bug comments). In that case we should not deoptimize but patch the store instruction that writes the new field value. That is required even if the field offset is known because the field could not be accessible/available and we would need to throw an exception during patching. The load/store instructions emitted by `copy_inline_content` to initialize the fields of the new inline type buffer copy should never require patching: We know their offsets (because the holder is loaded) and we don't need to check access restrictions.
> Unrelated changes:
> - Array loads should not be delayed if the subsequent getfield requires patching (see changes to `GraphBuilder::load_indexed`)
> - Replaced the `WithField` and `DefaultValue` IR nodes by a `Deoptimize` node
> - Refactoring and removal of dead code
With the discussion we had off-line about nestmates, changes look good to me.
Suggestion for a future improvement: the Deoptimize node in the HIR is a very nice improvement. However, if it looks like a general purpose node in the HIR, the LIRGenerator has a hard coded deopt reason for it: "Reason_unloaded". It would be nice to have a more flexible Deoptmize node where the reason could be specified at the HIR level.
Marked as reviewed by fparain (Committer).
More information about the valhalla-dev