RFR: 8371649: ZGC: AArch64: redundant OrderAccess::fence in ZBarrierSetAssembler::patch_barrier_relocation

[Andrew Haley]

On Thu, 13 Nov 2025 10:09:42 GMT, Andrew Haley <aph at openjdk.org> wrote:

>>> Hi Erik (@fisk),
>>> 
>>> Could you also please take a look, just in case the fence was intentionally put there?
>> 
>> The way I look at it, the fence was there for hardware that is unsophisticated enough to require manual cache flushing instead of having cache coherency that understands instruction edits, and at the same time has unsophisticated enough fences that are not speculated across such that the buffered store hits the cache before invalidating the cache, and not after, which would be awkward.
>> 
>> It is certainly possible that in practice the cache invalidation facilities also do the right level of fencing. So this is mostly just defensive programming.
>> 
>> If I flip the question around - how confident do you feel on a scale from 1 to 10 that the cache invalidation mechanism guarantees across all implementations, that the preceding store is flushed out to the caches before the cache is flushed? This is an area of the code where I don't want to take chances and slip unless we feel a high level of confidence.
>
>> The way I look at it, the fence was there for hardware that is unsophisticated enough to require manual cache flushing instead of having cache coherency that understands instruction edits, and at the same time has unsophisticated enough fences that are not speculated across such that the buffered store hits the cache before invalidating the cache, and not after, which would be awkward.
> 
> [edited]
> 
> Understood. But there are two caches, and `OrderAccess::fence` does not affect icache. So `OrderAccess::fence` cannot do anything to help. in order to make sure the buffered store hits the icache we need `DSB; ISB`, which  `ICache::invalidate` does.
> 
> On the other hand, the cost of `OrderAccess::fence` is small in comparison with `ICache::invalidate_word`, so there's a question about why we're bothering to remove it.

> @theRealAph
> 
> > On the other hand, the cost of OrderAccess::fence is small in comparison with `ICache::invalidate_word`, so there's a question about why we're bothering to remove it.
> 
> This change is not about performance. It's about logical inconsistency: not using this everywhere, absence of history and contradiction to Arm ARM.

I see. So there is little or no performance benefit, but we're doing this for reasons of formal consistency.

> Also, an assumption of a needed fence complicates a fix of [JDK-8370947](https://bugs.openjdk.org/browse/JDK-8370947). See `ICacheInvalidationContext::fence` in Alex's solution: [master...xmas92:jdk:deferred_icache_invalidation](https://github.com/openjdk/jdk/compare/master...xmas92:jdk:deferred_icache_invalidation)

I guess so, but there is no assumption of a needed fence.

A question is whether some future Arm system with fully-coherent i- and d-caches might want to supply a weaker version of `ICache::invalidate_word`. But even if it did, it would at the very least have to be a `DMB`, so there isn't an issue.

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

PR Comment: https://git.openjdk.org/jdk/pull/28244#issuecomment-3528240337