RFR: 8311906: Improve robustness of String constructors with mutable array inputs [v4]

[Tobias Hartmann]

On Thu, 16 Nov 2023 09:31:57 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>>> This cause a crash if I run with -XX:UseAVX=3 -XX:AVX3Threshold=0:
>> 
>> Good catch! Do we have a test for that scenario? If not, one should be added.
>> 
>>> Alternatively [...]
>> 
>> I would suggest to use `jmp(copy_chars_loop)` here for consistency with surrounding code.
>> 
>>> More generally I do wonder if it'd make most sense to make the AVX512 and SSE42 implementations exclusive
>> 
>> We don't mix AVX and SSE code here, right? We just fall back to SSE42 when AVX512 is not available or when the remaining length is below a threshold. Are you suggesting to better encapsulate both implementations (for example, factor them out into separate methods) or only ever using one?
>
> No, we don't mix: the SSE code is used as fallback only when the length is below 32 (if length is above 32 we check the tail with AVX code by shifting). 
> 
> I would suggest factoring out so that the implementations don't mix as much, mainly to reduce the number of possible variants to test and not to constrain one too much with the design of the other.  We now have AVX3-only, AVX3+SSE, SSE-only and plain, and I suggest dropping AVX3+SSE and fixing the AVX3-only so that it more efficiently handles strings of length 16-31 by duplicating (or using AVX instructions for copying 16 and 8 chars at once. Some code duplication perhaps, but simpler flow through each variant.

That seems reasonable but would be out of scope for this RFE.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16425#discussion_r1395452513