RFR: 8309531: Incorrect result with unwrapped iotaShuffle. [v3]

[Sandhya Viswanathan]

On Thu, 29 Jun 2023 17:23:47 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Patch fixes following two issues in iotaShuffle inline expander with unwrapped indices.
>>  1) Disable intrinsification if effective index do not lie within byte value range.
>>  2) Use GT predicate while computing comparison mask for all the indices above vector length.
>> 
>> No performance degradation seen with existing slice/unslice operations which internally calls wrapped iotaShuffle.
>> 
>> This interim patch addresses incorrectness around iotaShuffle till we introduce modified shuffle implementations
>> with JDK-8310691.
>> 
>> Kindly review and share feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adding string overflow range check, NULL to nullptr replacements.

Thanks for looking into this. I have couple of comments.

src/hotspot/share/opto/vectorIntrinsics.cpp line 616:

> 614:     int effective_min_index = start_val->get_con();
> 615:     int effective_max_index = start_val->get_con() + step_val->get_con() * (num_elem - 1);
> 616:     effective_indices_in_range = effective_max_index > effective_min_index && effective_min_index >= -128 && effective_max_index <= 127;

effective_max_index >= effective_min_index  (step_val could be zero).

src/hotspot/share/opto/vectorIntrinsics.cpp line 688:

> 686: 
> 687:     // Make the indices greater than lane count as -ve values to match the java side implementation.
> 688:     res = gvn().transform(VectorNode::make(Op_AndV, res, bcast_mod, vt));

Is it correct that here we are setting the mask to be true for within range good lane indices. 
What happens if the index is -ve? The BoolTest:gt would not catch that as it would still be true.
We could instead check for equality of indices before and after the AndV at line 688 below. If not equal then value was out of range.
May be I am missing something here.

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

PR Review: https://git.openjdk.org/jdk/pull/14700#pullrequestreview-1506214378
PR Review Comment: https://git.openjdk.org/jdk/pull/14700#discussion_r1247230251
PR Review Comment: https://git.openjdk.org/jdk/pull/14700#discussion_r1247239408