[vector] RFR -- removing Shape as type parameter

[Brian Goetz]

Let me know when you've finished reviewing the impact, and whether you 
think this refactor is OK to push.

On 11/28/2018 9:54 PM, Viswanathan, Sandhya wrote:
> Hi Brian,
>
> I tried your noshape patch with a small test to see how it affects the jitted code.  The test I tried is AddTest (test/jdk/jdk/incubator/vector/AddTest.java).  For this test the generated code continues to be efficient, excerpts below. I plan to try out more complex loops tomorrow.
>
> The workload method in AddTest is as below:
>     static void workload() {
>          for (int i = 0; i < a.length; i += SPECIES.length()) {
>              FloatVector av = SPECIES.fromArray(a, i);
>              FloatVector bv = SPECIES.fromArray(b, i);
>              av.add(bv).intoArray(c, i);
>          }
>      }
>
> The generated code for the hot loop in the workload method is as below:
>
> ;; B7: #       B7 B8 <- B6 B7  Loop: B7-B7 inner main of N69 Freq: 128.132
>
>    0x00007f8e47b03c50: vmovdqu 0x10(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03c57: vaddps 0x10(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03c5e: vmovdqu %ymm0,0x10(%r8,%rdi,4)
>    0x00007f8e47b03c65: vmovdqu 0x30(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03c6c: vaddps 0x30(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03c73: vmovdqu %ymm0,0x30(%r8,%rdi,4)
>    0x00007f8e47b03c7a: vmovdqu 0x50(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03c81: vaddps 0x50(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03c88: vmovdqu %ymm0,0x50(%r8,%rdi,4)
>    0x00007f8e47b03c8f: vmovdqu 0x70(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03c96: vaddps 0x70(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03c9d: vmovdqu %ymm0,0x70(%r8,%rdi,4)
>    0x00007f8e47b03ca4: vmovdqu 0x90(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03cae: vaddps 0x90(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03cb8: vmovdqu %ymm0,0x90(%r8,%rdi,4)
>    0x00007f8e47b03cc2: vmovdqu 0xb0(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03ccc: vaddps 0xb0(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03cd6: vmovdqu %ymm0,0xb0(%r8,%rdi,4)
>    0x00007f8e47b03ce0: vmovdqu 0xd0(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03cea: vaddps 0xd0(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03cf4: vmovdqu %ymm0,0xd0(%r8,%rdi,4)
>    0x00007f8e47b03cfe: vmovdqu 0xf0(%r11,%rdi,4),%ymm0
>    0x00007f8e47b03d08: vaddps 0xf0(%r10,%rdi,4),%ymm0,%ymm0
>    0x00007f8e47b03d12: vmovdqu %ymm0,0xf0(%r8,%rdi,4)
>    0x00007f8e47b03d1c: add    $0x40,%edi
>    0x00007f8e47b03d1f: cmp    %ebx,%edi
>    0x00007f8e47b03d21: jl     0x00007f8e47b03c50
>
> I ran the test as follows:
>
> $JAVA_HOME/bin/java --add-modules=jdk.incubator.vector -XX:CompileCommand=print,AddTest.workload -XX:CompileCommand=dontinline,AddTest.workload  -Djdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK=0 -XX:-UseSuperWord -XX:+PrintInlining AddTest
>
>
> Best Regards,
> Sandhya
>
>
> -----Original Message-----
> From: panama-dev [mailto:panama-dev-bounces at openjdk.java.net] On Behalf Of Brian Goetz
> Sent: Wednesday, November 28, 2018 8:55 AM
> To: panama-dev at openjdk.java.net
> Subject: [vector] RFR -- removing Shape as type parameter
>
> The below patch (tests pass) removes the Shape type argument from Vector and subtypes.
>
>       http://cr.openjdk.java.net/~briangoetz/panama/noShape.patch
>
> The use of Shape as a generic type parameter is clever, in that it does a good job of providing compile-time type safety (preventing us from mixing vectors of the wrong species), but I think we might be pushing it too hard here, and I think it can be profitably removed. It is rare to be mixing vectors who have the same element type but different shapes; when we do, these can (and must) be caught by runtime checks (regardless of whether we’re using static type checking to minimize the risk of mixing vectors of incompatible sizes, we still have to check that things match in the implementations (as the user could be using erased types.) The Shape parameter bubbles out into the user code, for relatively little benefit.  I don't think it carries its weight.
>
> Removing the Shape parameter also lets us undo another distortion, which is making the Shape constants into separate classes.  Now they can be a regular enum.  (Similarly, this also clears the way for some downstream improvements to the API.)
>
> (The patch is large as it includes the changes to the generated files and tests, but all changes were made to the underlying templates. Here's a smaller patch, which only contains changes to the source templates and non-templated source files, in case that's easier to review.)
>
> http://cr.openjdk.java.net/~briangoetz/panama/noShapeSourceTemplatesOnly.patch
>
>