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

Vladimir Kozlov kvn at openjdk.org
Tue Nov 26 18:02:46 UTC 2024


On Wed, 13 Nov 2024 14:09:59 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 four additional commits since the last revision:
> 
>  - indentation
>  - Merge branch 'master' into constanttable
>  - Merge branch 'master' into constanttable
>  - refactor array constant, fix codebuffer reallocation

I have few comments.

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

> 2769:     int offset = i * type2aelembytes(bt);
> 2770:     switch (bt) {
> 2771:       case T_BYTE: val->at(i) = con; break;

I don't like that switch is executed for each copied element. What is typical `len` value?

src/hotspot/share/opto/constantTable.cpp line 65:

> 63: }
> 64: 
> 65: int ConstantTable::alignment() const {

Add comment that it is used for nmethod's constant section size and alignment.

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

PR Review: https://git.openjdk.org/jdk/pull/21596#pullrequestreview-2462310312
PR Review Comment: https://git.openjdk.org/jdk/pull/21596#discussion_r1859020040
PR Review Comment: https://git.openjdk.org/jdk/pull/21596#discussion_r1858997584


More information about the hotspot-compiler-dev mailing list