Add support for FFI_TYPE_STRUCT_FF/DD on z/OS

[r30shah]

XPLINK on z/OS requires a special treatment for structs which with two members whose base type are same, either float or double. Such parameters are passed to the function as complex type and available FPRs will be used. Changes in this PR adds support for two new type in libffi - FFI_TYPE_STRUCT_FF and FFI_TYPE_STRUCT_DD and also uses it in J9 simiar to how FFI_TYPE_STRUCT is used as for J9 it shouldn't be different. It is just that libffi needs to know if the argument type or return type are considered complex on XPLINK to make sure that appropriate FPRs if available are used to pass parameters or read return values from.

J9 does not need to do the analysis for complex type, that work is done when platform dependant prep_cif is called when initializing the cif object.

Signed-off-by: Rahil Shah [email protected]

[r30shah]

@ChengJin01 Can I get your opinion on the above approach to handle complex type on z/OS , specifically with the change I have made in the VM?

[ChengJin01]

Add @zl-wang for review in the case of libffi with the generic ABI specifics.

[zl-wang]

I am wondering how libffi deals with FFI_TYPE_STRUCT_DD on x86 linux. i knew as a fact xlinux needs to treat less-than-16-byte structs specially, including FFI_TYPE_STRUCT_DD in two xmm registers. FFI_TYPE_STRUCT_FF in one xmm register though. you might not need to add these if you can find that out.

[r30shah]

I am wondering how libffi deals with FFI_TYPE_STRUCT_DD on x86 linux. i knew as a fact xlinux needs to treat less-than-16-byte structs specially, including FFI_TYPE_STRUCT_DD in two xmm registers. FFI_TYPE_STRUCT_FF in one xmm register though. you might not need to add these if you can find that out.

The way x86 is tackling the argument passing is much more sophisticated than how it is done on z/OS. There most of the analysis and set-up for the args to pass-in is done in the C function where they utilize a struct [1] and [2] to classify the arguments and prepare the structure that can be easily used to populate appropriate registers.

I do think that is the best approach and on Z, we should have done that - but that would require some re-design/re-writing the libffi port of z/OS - Something that is still in agenda - but may not be able to accommodate in the required timeline.

[1]. https://github.com/eclipse-openj9/openj9/blob/9bb74b5ec6b47e145d2f7aaa078d9f8b79dfda7d/runtime/libffi/x86/ffi64.c#L63-L70 [2]. https://github.com/eclipse-openj9/openj9/blob/9bb74b5ec6b47e145d2f7aaa078d9f8b79dfda7d/runtime/libffi/x86/ffi64.c#L86

[r30shah]

j9munprotect.s ends with END\n\n (two newlines); the extra one there should be removed, but this file should have (exactly) one final newline.

I am not removing the new line from the assembly file as clean-up work @keithc-ca . The new line in the Assembly causes the compilation on z/OS with OpenXL fail. On XLC it complains about new line , but let the build continue. So the removal of the new line is to make the code compatible to both OpenXL and XLC.

So eventually we would have to make the change, if you do not like that change as a part of this PR, I can get the end like of file back to the state it was before and let @Deigue to change it with his work for OpenXL in OpenJ9.

[keithc-ca]

I am not removing the new line from the assembly file as clean-up work @keithc-ca . The new line in the Assembly causes the compilation on z/OS with OpenXL fail. On XLC it complains about new line , but let the build continue. So the removal of the new line is to make the code compatible to both OpenXL and XLC.

So eventually we would have to make the change, if you do not like that change as a part of this PR, I can get the end like of file back to the state it was before and let @Deigue to change it with his work for OpenXL in OpenJ9.

My point was that j9munprotect.s is not an example of a single final newline causing a problem, and thus isn't justification for removing it from sysvz64.s. Can someone point to a better example?

[Deigue]

Fixed the multiple newline issues via #19720 As discussed, looks like it should be fine to keep single newline at the end. Apologies for the stir up.

[r30shah]

@keithc-ca Friendly ping for this PR, I have mode most of the recommended changes, if you are good with the changes, I can squash the changes to get this ready for merge.

[keithc-ca]

I don't think I'll have time to review this until at least the middle of next week.

There are still some cosmetic changes that I don't think are necessary or helpful. Please revert those and squash.

I also thought I saw as part of the assembly code, recognition of the FF and DD patterns this is concerned with: Please explain why that is not good enough. Why do we want this change at all?

[r30shah]

I also thought I saw as part of the assembly code, recognition of the FF and DD patterns this is concerned with: Please explain why that is not good enough. Why do we want this change at all?

z/OS ABI treats structs / nested structs with effective two same type primitive member specially where they are passed in to floating point register. In the libffi, currently code in runtime/libffi/z/sysvz64.s looks at the type in ffi_cif object and figures out if it is primitive of struct and loads argument list and registers accordingly. Problem happens when you have structs inside struct and it becomes tricky and costly to find out that information in assembly code. New type added is to make sure that we have information available before we enter the ASM routine where it can simply jump to appropriate label to load the parameters correctly.

There are still some cosmetic changes that I don't think are necessary or helpful. Please revert those and squash.

I think, only recommendation I have not addressed is the one in https://github.com/eclipse-openj9/openj9/pull/19645#discussion_r1643235398 , Which I understand you consider cosmetic. I believe the code style in that part of the code is inconsistent with the tabs and space , so I correct that part only. If you still think that I should not touch that, I can revert that back, let me know.

[keithc-ca]

If you think cosmetic changes are appropriate and helpful, please propose them separately (perhaps before) this change. The style I think we should be following is that of https://github.com/libffi/libffi, where z/OS support would ideally belong.

[r30shah]

If you think cosmetic changes are appropriate and helpful, please propose them separately (perhaps before) this change. The style I think we should be following is that of https://github.com/libffi/libffi, where z/OS support would ideally belong.

Ok, I agree and we are working on contributing the changes to open source libffi and currently working on redesigning the z/OS port so that it can be used by other runtimes as well. That is WIP and would take some time to come.

Meanwhile, I have tried to restored the state back to original as much as possible and squashed all commits. . Let me know if you are OK with that or not. W

[r30shah]

@keithc-ca I have tried to revertback most of the cosmetic changes . Friendly ping to get this one going .

[keithc-ca]

Please correct the typos in the commit message:

• "adds support for two new type" -> "adds support for two new types"
• "simiar" -> "similar"
• "analysis for complex type" -> "analysis for complex types"

[r30shah]

Please correct the typos in the commit message:

Done

[keithc-ca]

Jenkins line endings check

[keithc-ca]

Jenkins test sanity amac,zlinux jdk21