RFR: AArch64: compiler/7196199/Test7196199.java fails on AArch64-Linux with MaxVectorSize > 8

[Edward Nevill]

On Wed, 2015-09-09 at 22:39 +0800, Felix Yang wrote:
> Hi JIT developers,
> 
>     The testcase passes if MaxVectorSize < 16 or UseLoopSafepoints option
> is turned off.
>     After warmup, the test_incrc method failed verification once loop safe
> point interrupt triggers for the outer loop.

Hi,

Thanks for finding this!

I am unable to provoke it using the test case you mention, but it is definitely a bug. What command do you use to provoke it? The command I tried was

/home/ed/images/jdk/bin/java -jar lib/jtreg.jar -nr -conc:48 -timeout:3 -othervm -jdk:/home/ed/images/jdk -vmoption:-XX:MaxVectorSize=16 -v1 -a -ignore:quiet /home/ed/jdk9-dev/hs-comp/hotspot/test/compiler/runtime/7196199/Test7196199.java

> +void MacroAssembler::push_CPU_state(bool save_vectors) {
> +  push(0x3fffffff, sp);         // integer registers except lr & sp
> +
> +  if (!save_vectors) {
>      for (int i = 30; i >= 0; i -= 2)
>        stpd(as_FloatRegister(i), as_FloatRegister(i+1),
>     Address(pre(sp, -2 * wordSize)));

Your patch has TABS in it which means it does not apply cleanly. You may like to turn off tab compression/expansion in your editor.

> +#ifdef COMPILER2
> +  if (save_vectors) {
> +    assert(MaxVectorSize == 16, "only 128bit vectors are supported now");
------------------------------^^ ???
> +    // Save upper half of vector registers
> +    int vect_words = 32 * 8 / wordSize;
> +    additional_frame_words += vect_words;
> +  }
> +#else
> +  assert(!save_vectors, "vectors are generated only by C2");
> +#endif


> -void RegisterSaver::restore_live_registers(MacroAssembler* masm) {
> -  __ pop_CPU_state();
> +void RegisterSaver::restore_live_registers(MacroAssembler* masm, bool
> restore_vectors) {
> +#ifdef COMPILER2
> +  if (restore_vectors) {
> +    assert(MaxVectorSize == 16, "only 128bit vectors are supported now");
-------------------------------^^ ???
> +  }
> +#else
> +  assert(!restore_vectors, "vectors are generated only by C2");
> +#endif
> +  __ pop_CPU_state(restore_vectors);
>    __ leave();
>  }
> 
> @@ -176,9 +194,9 @@
>  }

> +
> +#ifdef COMPILER2
> +  MaxVectorSize = 16;
--------------------^^ ???
> +#endif
>  }

Why is it necessary to restrict the vector size to 16? This stops the user from doing, for example, -XX:MaxVectorSize=8.

aarch64.ad ceils the MaxVectorSize at 16

// Vector width in bytes.
const int Matcher::vector_width_in_bytes(BasicType bt) {
  int size = MIN2(16,(int)MaxVectorSize);
  // Minimum 2 values in vector
  if (size < 2*type2aelembytes(bt)) size = 0;
  // But never < 4
  if (size < 4) size = 0;
  return size;
}

All the best,
Ed.