Skip to content

8338023: Support two vector selectFrom API #20508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 20 commits into from
Closed

8338023: Support two vector selectFrom API #20508

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
45bfe6c
8338023: Support two vector selectFrom API
Aug 8, 2024
82c0b0a
Adding Benchmark
Aug 8, 2024
055fb22
Review suggestions incorporated.
Aug 19, 2024
e24632c
Pass explicit wrap argument to selectFrom API with default value set …
Aug 21, 2024
d7ad688
Defaulting to index wrapping scheme.
Aug 23, 2024
6cb1a46
Removing redundant checkIndex routine
Aug 23, 2024
408a869
Review comments resolutions.
Aug 27, 2024
8d71f17
Adding descriptive comments
Aug 29, 2024
d3ee310
Review resolutions.
Sep 6, 2024
1c00f41
Documentation suggestions from Paul.
Sep 13, 2024
7c80bfc
Disabling VectorLoadShuffle bypassing optimization to comply with rea…
Sep 16, 2024
d72c82e
Review comments resolution.
Sep 17, 2024
2953004
Jcheck clearance
Sep 17, 2024
31a5864
Incorporating review and documentation suggestions.
Sep 18, 2024
42ca80c
Handling NPOT vector length for AArch64 SVE with vector sizes varying…
Sep 24, 2024
7327736
Review comments resolutions.
Oct 1, 2024
130fa3e
Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8338023
Oct 3, 2024
6215ab9
Review comments resolution.
Oct 3, 2024
1cca8e2
Sharpening intrinsic exit check.
Oct 3, 2024
79ee29c
Updating tests to use floorMod
Oct 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/hotspot/share/opto/vectornode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2100,7 +2100,9 @@ Node* SelectFromTwoVectorNode::Ideal(PhaseGVN* phase, bool can_reshape) {
// (VectorRearrange SRC1, INDEX)
// (VectorRearrange SRC2, NORM_INDEX)
// MASK)
//
// This shall prevent an intrinsification failure and associated argument
// boxing penalties.
Copy link
Contributor

Choose a reason for hiding this comment

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

A quick comment about how the mask is computed could be nice.
MASK = INDEX < num_elem

Copy link
Contributor

Choose a reason for hiding this comment

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

@jatin-bhateja very nice, thanks!


auto lane_count_type = [&]() {
switch(elem_bt) {
case T_BYTE:
Expand Down Expand Up @@ -2177,6 +2179,10 @@ Node* VectorRearrangeNode::Ideal(PhaseGVN* phase, bool can_reshape) {
default: return elem_bt;
}
};
// Targets emulating unsupported permutation for certain vector types
// may need to message the indexes to match the users intent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// may need to message the indexes to match the users intent.
// may need to massage the indexes to match the users intent.

Copy link
Contributor

Choose a reason for hiding this comment

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

This optimization for now seems quite specific to your SelectFromTwoVectorNode::Ideal lowering code. Can this conversion not be done there already?

What is the semantics of VectorRearrangeNode? Should its shuffle vector always be bytes, and we now violated that "for a quick second"? Or is it going to be generally the idea to create all sorts of shuffle types and then fix that up? But then why do we need the vector_indexes_needs_massaging?

Can you help me understand the concept/strategy behind this?

Copy link
Member Author

@jatin-bhateja jatin-bhateja Sep 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, variable index permutation instruction on every target expects shape conformance b/w data vector and permute index vector. Rearrange expects indices to be passed through shuffle, idealization routines automatically injects a VectorLoadShuffle after loading indexes held in shuffle's backing storage i.e. a byte array.

In all the cases apart from byte vector permute , VectorLoadShuffle expands the index byte lanes to match the data vector lane. So we always end up emitting a lane expansion instruction before permute instruction (scenario 1). Apart from usual expansions VectorLoadShuffle may also do additional magic for some targets where it may need to prune / massage the index vector if target does not support destination vector type (scenario 2).

For our case, new selectFrom accepts the indices though vectors which save redundant expansions, but to leverage existing backend support for scenario 2 we do target specific pruning

// Lowering index vector to a bytevector followed by an explicit loadshuffle
// will bring the indexes in the consumable format.
int cast_opc = VectorCastNode::opcode(-1, elem_bt, true);
Node* pack_shuf = phase->transform(VectorCastNode::make(cast_opc, in(2), T_BYTE, num_elem));
const TypeVect* newvt = TypeVect::make(get_integal_type(elem_bt), num_elem);
Expand Down