Skip to content

Conversation

ehsankianifar
Copy link
Contributor

Add implementation for integral vreductionadd opcode on IBM Z platform. vreductionAdd is not enabled in this PR since it requires float support as well. Float reduction add implementation will be added in phase 2.

@ehsankianifar
Copy link
Contributor Author

@r30shah @hzongaro please take a look at this PR at your convenience.
I broke vreductionadd to 'integral and 'float' parts since instructions are very different to support those two types. I am already working on float instructions.
The changes tested with this unit test: https://github.com/ehsankianifar/omr/blob/Z_ImplementVReductionAddOpcode_DEV/fvtest/compilertriltest/EhsanTest.cpp

@ehsankianifar ehsankianifar force-pushed the Z_ImplementVReductionAddOpcode branch 3 times, most recently from ceb5978 to 7a238ac Compare October 9, 2025 17:48
@ehsankianifar ehsankianifar force-pushed the Z_ImplementVReductionAddOpcode branch from 7a238ac to 8a545de Compare October 15, 2025 20:33
@hzongaro hzongaro self-assigned this Oct 17, 2025
@hzongaro hzongaro self-requested a review October 17, 2025 18:03
Copy link
Contributor

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. Just a couple of minor suggestions regarding comments.

Add initial implementation for the integral vreductionadd opcode on the
IBM Z platform. Note that vreductionadd is not enabled in this commit,
as full support requires floating-point handling, which is not yet
implemented. Support for floating-point reduction will be introduced
in a subsequent PR, after which the opcode can be fully enabled.

signed-off-by: Ehsan Kiani Far <[email protected]>
@ehsankianifar ehsankianifar force-pushed the Z_ImplementVReductionAddOpcode branch from 8a545de to 6402720 Compare October 17, 2025 19:05
Copy link
Contributor

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. I'll wait until @r30shah has had a chance to review before merging.

generateVRIaInstruction(cg, TR::InstOpCode::VGBM, node, scratchReg, 0, 0);
if (needPreReduction) {
// We can not sum all lanes in one operation when the lane size is byte or halfword.
// Calculating the sum of byte or halfword into an intermediate word so we can add all word in the next step.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ehsankianifar - How does this code handles the overflow for byte and half word ? VSUM would zero extend the intermediate sum and place it in the word, but VSUMQ would not do that right ?

Does reduction opcode doubles the element type ? If not, I believe VSUMQ possibly can produce result that is larger than element size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed with @gita-omr that we do not need to handle overflow in this opcode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I would like that to be in comment here - or at least in documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants