[master] RFR: 8347710: [Lilliput] Implement 4 byte headers [v8]

[Andrew Dinn]

On Mon, 27 Jan 2025 10:34:38 GMT, Andrew Haley <aph at openjdk.org> wrote:

> Is there ever a time when we have an acquiring load on a narrow klass pointer?

That's a good question! This predicate was added to handle volatile instance field loads (there is a similar set of cases for volatile stores). I believe I unthinkingly allowed for the possibility of the acquire case for Klass and NKlass loads (likewise stores) when I first added the rules that used the predicate (I think I just blindly changed all the rules that did a load/store). At that stage the test was done by traversing the graph but even then it was probably always going to fail for a Klass load/store.

In the current implementation the predicate is implemented by checking field `_mo` of the relevant `LoadNode` which is set at node create and does not appear ever to get reset. Looking at the opto sources it appears that `LoadKlassNode` and `LoadNKlassNode` only ever seem to be created created with `_mo == unordered`. So, I think the Klass load/store rules which check for acquire/release semantics can be dropped.

It would be worth seeking confirmation of that from @rwestrel who was the one who changed the implementation so it relied on the `_mo` fields in `Load/StoreNode`.

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

PR Review Comment: https://git.openjdk.org/lilliput/pull/193#discussion_r1930339522