RFR: 8338727: RISC-V: Avoid synthetic data dependency in nmethod barrier on Ztso [v2]

[Robbin Ehn]

On Mon, 26 Aug 2024 02:01:22 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Not sure what you mean, but there is no contradiction here.
>> Manual says:
>> 
>> load guard
>> <guard =>  epoch data dep>
>> load epoch
>> load thread_gurad_epoch //unaffected by data dep.
>> 
>> In RVWMO load thread_gurad_epoch is uneffected yes, and can be loaded eariler, yes.
>> 
>> But we branch on the value of epoch (plus guard), delaying the load of epoch more than neccessary means we delay the branch instruction. As that branch have a control dependency it stop the all following instructions:
>> 
>> Control dependencies behave differently from address and data dependencies in the sense that a
>> control dependency always extends to all instructions following the original target in program order.
>> 
>> 
>> Which means the main goal is get throught the branch as quick as possible.
>> 
>> My comment says delaying the load of epoch in favour of loading thread_gurad_epoch eariler may be slower.
>> I have not look to deep but it seems like we can also move the load of thread_gurad_epoch before data dep? (i.e. before any such fence r,r)
>> 
>> As the load of guard and load epoch cannot overlap, they happen sequentially (due to data dep).
>> `fence r,r` only says the load will happen in global memory order, it do not force them to be sequential.
>> 
>> As we are comming very close to CPU implementation here there maybe differences.
>> So the point of the comment was, maybe revisit this in a few years.
>> 
>> Thanks! I'll wait until we can agree if this is a good comment or if it should be in other wording.
>
> All right then. Can you change following line of comment to be more specific?
> 
> 
> // Embed an synthetic data dependency to order the guard load
> 
> =>
> 
> // Embed a syntactic address dependency to order the guard load

Let me know what you think about the update, thanks!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20661#discussion_r1730727524