RFR: 8317720: RISC-V: Implement Adler32 intrinsic [v15]
ArsenyBochkarev
duke at openjdk.org
Mon Jul 15 20:24:11 UTC 2024
On Mon, 15 Jul 2024 06:00:16 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Sorry for such a late reply. It is a good catch, I see it, thanks! However, it is not correct for all cases: consider having -63 after `L_by16_loop_unroll`. In current case (the correct one) we would just jump to `L_by1` and to `L_do_mod`, ending the execution. With unconditional fallthrough the excessive job would have been performed, which is not correct. So as far as I can see the best option would be just to add another condition for `L_by1`, checking for proper `len` sizes and performing `adler32_process_bytes` when needed. Please correct me if I'm wrong
>
>> Sorry for such a late reply. It is a good catch, I see it, thanks! However, it is not correct for all cases: consider having -63 after `L_by16_loop_unroll`
>
> Hmm ... I don't understand how this could happen. I see `len >= 64` is ensured when we first enter `L_by16_loop_unroll`. And you do `len -= 64` and compare the result with 64 at the end of each iteration, which makes sure that the updated `len` is still >= 64 for the next iteration. So I think the final `len` when we exit this `L_by16_loop_unroll)` loop will be >= 0, right?
>
> BTW: Could you please avoid using `count` as scratch register when say compare `len` with sizes like step_16/64. It's better to uses others like temp0-3.
Ohh, I'm sorry, that's my mistake. The `len` is >= 0 after `L_by16_loop_unroll`, you're right. So I removed the `blt` from `L_by16_loop_unroll` end and excessive `adler32_process_bytes` (introduced in [this](https://github.com/openjdk/jdk/pull/18382/commits/0ce230a4ecf1ca81d5f615e42e9758b6be36f498) commit) from `L_by1`, and ran a `test/hotspot/jtreg/compiler/intrinsics/zip/TestAdler32.java` test, which is OK. Thanks for your patience!
Also I used `t2` instead of a `count` for sizes comparisons
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18382#discussion_r1678345823
More information about the hotspot-compiler-dev
mailing list