Fix compressed val register.

[wks]

MMTkBarrierSetAssembler::store_at calls object_reference_write_post after calling BarrierSetAssembler::store_at. However, BarrierSetAssembler::store_at modifies the val register to compress its value in place. Consequently, object_reference_write_post will see a compressed pointer in the val register.

We decompress the pointer in the slow path before calling into MMTk core. This makes the barrier general, and fixes the assertion error when the compressed target value is not aligned, although our current ObjectBarrier doesn't use the slot and the target arguments.

Known issue: The fixed MMTkObjectBarrierSetAssembler::object_reference_write_post depends on the internal implementation of the BarrierSetAssembler::store_at method in OpenJDK. However, a general solution needs to save and restore the val register before and after calling BarrierSetAssembler::store_at, which adds an overhead in the fast path.

Fixes: #291

[qinsoon]

Looks good to me.

[qinsoon]

I added tests here as a separate job: #292. We can either remove the test here, or close that PR.

[wks]

I'll remove this test because #292 tests with multiple plans, and GenImmix may not be the only plan affected by the compressed oops.

I'll keep the one I added to ci-test-minimal.sh so that it will be executed for every new commit in mmtk-core.

[wenyuzhao]

LGTM. I don't think we have reached a conclusion in our meeting due to the time. Should we fix this parameter or remove it?

[wenyuzhao]

FYI looks like it's still beneficial to have the target value. Our current object barrier conservatively remembers all the references, including mature->mature. This can be filtered out by checking the target object address. For generational GC, we only care about mature->young

[wks]

LGTM. I don't think we have reached a conclusion in our meeting due to the time. Should we fix this parameter or remove it?

I think one thing we all agree with is that if we know what barrier we are implementing, we can do whatever we want to exploit the semantics of the barrier. In this case, we are implementing ObjectBarrier, and we know that the barrier behaves the same regardless of the value of the target argument. So we can pass null to the target parameter and expect it to continue to work.

To be safe, I'll create another PR that simply passes null to the target argument, and we discuss this further on Zulip to confirm that we agree with this idea.

[wks]

FYI looks like it's still beneficial to have the target value. Our current object barrier conservatively remembers all the references, including mature->mature. This can be filtered out by checking the target object address. For generational GC, we only care about mature->young

I think that'll not be the (standard) object-remembering barrier. It will be a barrier specific to generational GC, and is not applicable to, for example, coalescing RC. To make it working, we need a GenObjectBarrier which implements object_reference_write_post differently (with an additional check). The OpenJDK binding, knowing the semantic difference, will not omit the target argument when using that barrier when calling object_reference_write_post. But since the slow path (remembering the object) doesn't need the target, we can probably omit the target when calling object_reference_write_slow.

But whether the new GenObjectBarrier is better than the current ObjectBarrier will be another topic. I think @steveblackburn mentioned before that each has its pros and cons, and the unconditional ObjectBarrier is faster for some reasons.

[wks]

Our discussion indicates it's OK to do optimization by passing nullptr to those unused arguments. So I am closing this PR in favor for #294