RFR: 8319889: Vector API tests trigger VM crashes with -XX:+StressIncrementalInlining

[Vladimir Ivanov]

On Wed, 13 Mar 2024 17:19:40 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> This bug fix patch fixes following two issues:-
> 
> 1) Removing memory operand based masked shift instruction selection patterns. As per Java specification section 15.19 shift count is rounded to fit within valid shift range by performing a bitwise AND with shift_mask this results into creation of an AndV IR after loading original mask into vector. Existing patterns will not be able to match this graph shape, extending the patten to cover AndV IR will associate memory operand with And operation and we will need to emit additional vectorAND instruction before shift instruction, existing memory operand patten for AndV already handle such a graph shape.
> 
> 2) Crash occurs due to combined effect of bi-morphic inlining, exception handling, randomized incremental inlining. In this case top level slice API is invoked using concrete 256 bit vector, some of the intermediate APIs within sliceTemplate are marked for lazy inlining because due to randomized IncrementalInlining, these APIs returns an abstract vector which when used for virtual dispatch of subsequent APIs results into bi-morphic inlining on account of multiple profile based receiver types. Consider following code snippet.
> 
> 
>     ByteVector sliceTemplate(int origin, Vector<Byte> v1) {
>         ByteVector that = (ByteVector) v1;
>         that.check(this);
>         Objects.checkIndex(origin, length() + 1);
>         VectorShuffle<Byte> iota = iotaShuffle();
>         VectorMask<Byte> blendMask = iota.toVector().compare(VectorOperators.LT, (broadcast((byte)(length() - origin))));  [A]
>         iota = iotaShuffle(origin, 1, true);     [B]    
>         return that.rearrange(iota).blend(this.rearrange(iota), blendMask); [C]
>     }
> 
> 
> 
> Receiver for sliceTemplate is a 256 bit vector, parser defers inlining of toVector() API (see code at line A) and generates a Call IR returning an abstract vector. This abstract vector then virtually dispatches compare API. Compiler observes multiple profile based receiver types (128 and 256 bit byte vectors) for compare API and parser generates a chain of PredictedCallGenerators for bi-morphically inlining it.
>       
>          PredictedCallGenerators (Vector.compare)
>               PredictedCallGenerators (Byte256Vector.compare)
>                  ParseGenerator (Byte256Vector.compare)               [D]
>                  UncommonTrap (receiver other than Byte256Vector)
>               PredictedCallGenerators (Byte128Vector.compare)
>                  ParseGenerator (Byte128Vector.compare)               [E...

Overall, both fixes look good. 

I suggest to handle the bugs separately (as 2 bug fixes).

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

> 162: Node* GraphKit::unbox_vector(Node* v, const TypeInstPtr* vbox_type, BasicType elem_bt, int num_elem, bool shuffle_to_vector) {
> 163:   assert(EnableVectorSupport, "");
> 164:   const TypePtr* vbox_type_v = gvn().type(v)->isa_ptr();

You can use `isa_instptr()` and check for `nullptr` instead.


  const TypeInstPtr* vbox_type_v = gvn().type(v)->isa_instptr();
  if (vbox_type_v == nullptr || vbox_type->instance_klass() != vbox_type_v->instance_klass()) {
    return nullptr; // arguments don't agree on vector shapes
  }

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

Marked as reviewed by vlivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18282#pullrequestreview-1935065932
PR Review Comment: https://git.openjdk.org/jdk/pull/18282#discussion_r1523825589