RFR: 8327652: S390x: Implements SLP support [v7]

[Amit Kumar]

On Wed, 27 Mar 2024 16:37:48 GMT, Lutz Schmidt <lucy at openjdk.org> wrote:

>>> > I think we shouldn't allow `MacroAssembler::string_compress(...)` and `MacroAssembler::string_expand(...)` to use vector registers without specifying this effect. That can be solved by adding a KILL effect for all vector registers which are killed. Alternatively, we could revert to the old implementation before [d5adf1d](https://github.com/openjdk/jdk/commit/d5adf1df921e5ecb8ff4c7e4349a12660069ed28) which doesn't use vector registers. The benefit was not huge if I remember correctly.
>>> 
>>> Agreed. My proposed circumvention is a too dirty hack.
>>> 
>>> I would prefer to add KILL effects to the match rules. I believe the vector implementation had a substantial performance effect. Unfortunately, I can't find any records of performance results from back then.
>>> 
>>> Reverting the commit @TheRealMDoerr mentioned is not possible. It contains many additions that may have been used by unrelated code. The vector code is well encapsulated and could be removed by deleting the
>>> 
>>> ```
>>>   if (VM_Version::has_VectorFacility()) {
>>>   } 
>>> ```
>>> 
>>> block. I would not like that, though.
>> 
>> I didn't mean to back out the whole commit. Only the implementation of string_compress and string_expand. The benefit of the vector version certainly depends on what kind of strings are used. (Effect may also be negative in some cases.) I think that classical benchmarks didn't show a significant performance impact, but I don't remember exactly, either. I'll leave the s390 maintainers free to decide if they want to adapt the vector version or go for the short and simple implementation.
>
>> ... I think that classical benchmarks didn't show a significant performance impact, but I don't remember exactly, either. ...
> 
> Yes, you need "long" strings (>= 32 characters) for the vector code to kick in. Once it kicks in, there is a performance improvement. Hard to say which applications might benefit. For too short strings I do not expect a visible performance penalty. It's just a shift and a branch. 
> 
> Let the maintainers decide.

Hi @RealLucy, @TheRealMDoerr 

Can we disable the vector part in the `string_{compress, inflate, const_inflate}` and integrate this for now. I am working on static stub part and need change from s390.ad file. Moreover I need these changes for Late barrier extension changes in the save live register class implementation.

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

PR Comment: https://git.openjdk.org/jdk/pull/18162#issuecomment-2270356558