RFR: 8342651: Refactor array constant to use an array of jbyte [v2]

Tobias Hartmann thartmann at openjdk.org
Wed Oct 23 11:50:18 UTC 2024


On Sun, 20 Oct 2024 16:40:32 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> This small patch refactors array constants in C2 to use an array of `jbyte`s instead of an array of `jvalue`. The former is much easier to work with and we can do `memcpy` with them trivially.
>> 
>> Since code buffers support alignment of the constant section, I have also allowed constant tables to be aligned more than 8 bytes and used it for constant vectors on machines not supporting `SSE3`. I also fixed an issue with code buffer relocation where the temporary buffer is not correctly aligned.
>> 
>> This patch is extracted from https://github.com/openjdk/jdk/pull/21229. Tests passed with `UseSSE=2` where 16-byte constants would be generated, as well as normal testing routines.
>> 
>> Please take a look and leave your reviews, thanks a lot.
>
> Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
> 
>  - Merge branch 'master' into constanttable
>  - refactor array constant, fix codebuffer reallocation

Looks good to me overall but reviewers who looked at [JDK-8278947](https://bugs.openjdk.org/browse/JDK-8278947) (@vnkozlov, @iwanowww) should have a look at this as well.

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 1715:

> 1713:   } else {
> 1714:     switch (vlen_in_bytes) {
> 1715:     case 4:  movflt(dst, src); break;

Indentation of the case statements needs to be fixed. Also above.

src/hotspot/cpu/x86/x86.ad line 2743:

> 2741:     case T_BYTE: val->at(i) = con; break;
> 2742:     case T_SHORT: {
> 2743:       jshort c = con;

Why are these casts needed? Isn't `T con` already of the appropriate j-type?

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

PR Review: https://git.openjdk.org/jdk/pull/21596#pullrequestreview-2388280645
PR Review Comment: https://git.openjdk.org/jdk/pull/21596#discussion_r1812526891
PR Review Comment: https://git.openjdk.org/jdk/pull/21596#discussion_r1812556856


More information about the hotspot-compiler-dev mailing list