RFR: Handle checkcast arraycopy correctly in barriers. Make arraycopy+barriers a single loop.

[Aleksey Shipilev]

On 02/26/2018 09:08 PM, Roman Kennke wrote:
> Which is implemented here:
> http://cr.openjdk.java.net/~rkennke/arraycopy-loop/webrev.00/

*) The test should capture bugs in this code. It should be randomized, *and* it should validate what
have been written with the copy itself, that would save us time debugging the cryptic failures in
larger scenarios. It should probably be separate from checkcast test.

*) It is a good style to split the line by "static"/"dynamic" parameters. In other words, I should
*see* the structure of the signature, e.g.:

   template <typename T>
   bool arraycopy_loop_1(T* src, T* dst, size_t length, Klass* bound,
                         bool checkcast, bool satb, bool matrix,
                         ShenandoahBarrierSet::ArrayCopyStoreValMode storeval_mode);


*) No need to put comments like "/* checkcast */" in the expansion, it is evident what template
parameter you are juggling, and it only adds to clutter, do e.g.:

  99   if (checkcast) {
 100     return arraycopy_loop_2<T, true>(src, dst, length, bound, satb, matrix, storeval_mode);
 101   } else {
 102     return arraycopy_loop_2<T, false>(src, dst, length, bound, satb, matrix, storeval_mode);
 103   }

*) Aren't switches indented wrong? "case" should be indented inside the switch block. Select the
switch block, and do Ctrl+Alt+L.

*) Isn't storeval_str switch redundant?

*) The asserts for every single "other" state should probably just assert the exact state we want to
be in.

Stylistic changes:
 http://cr.openjdk.java.net/~shade/shenandoah/arraycopy-shade-fixes.patch

-Aleksey