[lworld] RFR: 8228634: [lworld] ciField::will_link() returns incorrect result for the withfield bytecode

[Frederic Parain]

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
> 
> Thanks,
> Tobias

Hi Tobias,

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.

Thank you,

Fred

-------------

Marked as reviewed by fparain (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/301