Vector API Get Implementation patch

[Kandu, Rahul]

Vladimir,

Thanks for your input. I'll upload the updated patch shortly. 

Rahul

-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com] 
Sent: Thursday, June 14, 2018 9:44 AM
To: Kandu, Rahul <rahul.kandu at intel.com>; panama-dev at openjdk.java.net
Subject: Re: Vector API Get Implementation patch

Rahul,

Some comments/questions/ suggestions:

   (1) I don't see any non-AVX rules. Do you plan to add them later?


   (2) No matcher rules for ExtractI as well. Same question: left for later?


   (3) AVX-512 rules should be guarded by "UseAVX > 2" predicate:

+instruct extract32s(rRegI dst, vecZ src, vecZ tmp, immI idx) %{
+  predicate(UseAVX > 0 && n->as_Vector()->length() == 32 && 
n->bottom_type()->basic_type() == T_SHORT);


   (4) Is extract4b ever used? I don't see any 32-bit vectors present.

+instruct extract4b(rRegI dst, vecX src, immI idx) %{
+  predicate(UseAVX > 0 && n->as_Vector()->length() == 4 && 
n->bottom_type()->basic_type() == T_BYTE);


    (5) I suggest to migrate to immU? (recently introduced) and get rid 
of index masking (see [1] for examples):
    +    int midx = 0x3F & $idx$$constant;

Applying similar refactorings to rvinsert64B, the following:

+instruct extract64b(rRegI dst, vecZ src, vecZ tmp, immI idx) %{
...
+    int midx = 0x3F & $idx$$constant;
+    if (midx == 0) {
+      __ movdl($dst$$Register, $src$$XMMRegister);
+          __ movsbl($dst$$Register, $dst$$Register);
+    }
+    else if (midx >= 1 && midx <= 15) {
+      __ pextrb($dst$$Register, $src$$XMMRegister, midx);
+          __ movsbl($dst$$Register, $dst$$Register);
+    }
+    else {
+      int extr_idx1 = midx / 16;
+      int extr_idx2 = midx % 16;
+      __ vextracti32x4($tmp$$XMMRegister, $src$$XMMRegister, extr_idx1);
+      __ pextrb($dst$$Register, $tmp$$XMMRegister, extr_idx2);
+          __ movsbl($dst$$Register, $dst$$Register);
+    }

can be converted to into:

+instruct extract64b(rRegI dst, vecZ src, vecZ tmp, immU6 idx) %{
...
+    uint x_idx = $idx$$constant & right_n_bits(4);
+    uint z_idx = ($idx$$constant >> 4) & 2;
+    if (z_idx == 0) {
+      if (x_idx == 0) {
+        __ movdl($dst$$Register, $src$$XMMRegister);
+      } else {
+        __ pextrb($dst$$Register, $src$$XMMRegister, x_idx);
+      }
+    } else { // z_idx > 0
+      __ vextracti32x4($tmp$$XMMRegister, $src$$XMMRegister, z_idx);
+      __ pextrb($dst$$Register, $tmp$$XMMRegister, x_idx);
+    }
+    __ movsbl($dst$$Register, $dst$$Register);

or even:

+    uint x_idx = $idx$$constant & right_n_bits(4);
+    uint z_idx = ($idx$$constant >> 4) & 2;
+    Register r_i32 = $src$$XMMRegister;
+    if (z_idx > 0) {
+      r_i32 = $tmp$$XMMRegister;
+      __ vextracti32x4(r_i32, $src$$XMMRegister, z_idx);
+    }
+    if (x_idx == 0) {
+      __ movdl($dst$$Register, r_i32);
+    } else {
+      __ pextrb($dst$$Register, r_i32, x_idx);
+    }
+    __ movsbl($dst$$Register, $dst$$Register);


Also, please, fix code formatting. Many lines have wrong indentation.

Best regards,
Vladimir Ivanov

[1] http://hg.openjdk.java.net/panama/dev/rev/e00750ed585a

On 14/06/2018 03:10, Kandu, Rahul wrote:
> Hi All,
> 
> Please find the Get Implementation patch for the Vector API. The following patch also includes Tests for Get operations.
> 
> http://cr.openjdk.java.net/~rlupusoru/panama/webrevrGetImplF/
> 
> regards,
> Rahul
>