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

[Xiaohong Gong]

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.

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

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

May I ask why we need to fall-back to java implementation if the indices are in-effective for constant vals?  While if the `start_val` and `step_val` are not all constants, for in-effective-indices, it subs to the lanecount? Is there any difference for constant and variable inputs?

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

> 624:   }
> 625: 
> 626:   bool step_multiply = !step_val->is_con() || !is_power_of_2(step_val->get_con());

Is it better moving this definition above its usage (e.g. line-634)?

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

> 681:   if(do_wrap)  {
> 682:     // Wrap the indices greater than lane count.
> 683:      res = gvn().transform(VectorNode::make(Op_AndV, res, bcast_mod, vt));

Identity Style: please remove one space before `res = `

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14700#discussion_r1247502942
PR Review Comment: https://git.openjdk.org/jdk/pull/14700#discussion_r1247475844
PR Review Comment: https://git.openjdk.org/jdk/pull/14700#discussion_r1247476595