Webrev for vectorIntrinsics branch

[Lupusoru, Razvan A]

Hi Henry,

Thank you for porting this patch over! The code changes look good to me. As far as I can tell, the only real conflicts were: adding method expand_vbox_nodes, calling expand_vbox_nodes, and adding the phase name. If that is the case, then the changes you brought over look good to me. Let me know if there is any other place you are concerned about - otherwise looks good to me.

Thanks,
Razvan

-----Original Message-----
From: Henry Jen [mailto:henry.jen at oracle.com] 
Sent: Saturday, December 16, 2017 1:02 AM
To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>; Vladimir Ivanov <vladimir.x.ivanov at oracle.com>
Cc: panama-dev at openjdk.java.net
Subject: Re: Webrev for vectorIntrinsics branch

I need a review on the merged changeset "Vector Unbox, masking support, and more for Vector API"[1] into the new vectorIntrinsics branch,

The code in question modify macro.cpp:PhaseMacroExpand::expand_vbox_nodes(), which is introduced by code snippet support which is not included in this branch, so I bring over the macro expansion code and the hook in compile.cpp. There are other minor fixes for merge, it would be nice you can have a look as well.

The webrev can be found at http://cr.openjdk.java.net/~henryjen/panama/vi/12895/webrev.

Cheers,
Henry

[1] http://hg.openjdk.java.net/panama/panama/hotspot/rev/6f6fb6578c34

--- macro.cpp
+++ macro.cpp
@@ -2817,12 +2819,28 @@
 //------------------------------expand_macro_nodes----------------------
 //  Returns true if a failure occurred.
 void PhaseMacroExpand::expand_vbox_nodes() {
+  // Run a first phase that solely looks at Vector Unbox nodes. We need 
+ all those to  // be expanded before deciding whether on what to do with the boxing nodes.
   int macro_idx = C->macro_count() - 1;
+  while(macro_idx >= 0) {
+    Node * n = C->macro_node(macro_idx);
+    assert(n->is_macro(), "only macro nodes expected here");
+    if (n->Opcode() == Op_VectorUnbox) {
+      VectorUnboxNode* vunbox = static_cast<VectorUnboxNode*>(n);
+      expand_vectorunbox_node(vunbox);
+    }
+    if (C->failing())  return;
+    macro_idx = MIN2(macro_idx - 1, C->macro_count() - 1);  }
+
+  macro_idx = C->macro_count() - 1;
   while (macro_idx >= 0) {
     Node * n = C->macro_node(macro_idx);
     assert(n->is_macro(), "only macro nodes expected here");
-    if (n->Opcode() == Op_Opaque1 || n->Opcode() == Op_Opaque2) {
+    if (n->Opcode() == Op_Opaque1 || n->Opcode() == Op_Opaque2 || 
+ n->Opcode() == Op_VectorUnbox) {
       // skip
+    } else if (n->is_VectorBox()) {
+      expand_vectorbox_node(n->as_VectorBox());
     } else if (_igvn.type(n) == Type::TOP || n->in(0)->is_top() ) {
       // node is unreachable, so don't try to expand it
 //      C->remove_macro_node(n);


> On Dec 14, 2017, at 4:45 PM, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
> 
> Hi Henry,
> 
> Sounds good to me - please go ahead with the branch creation!
> 
> Also, thank you again for doing this and also being willing to help with some of the additional patch porting.
> 
> --Razvan
> 
> 
> -----Original Message-----
> From: Henry Jen [mailto:henry.jen at oracle.com]
> Sent: Wednesday, December 13, 2017 11:10 AM
> To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>
> Cc: panama-dev at openjdk.java.net
> Subject: Re: Webrev for vectorIntrinsics branch
> 
> 
> 
>> On Dec 13, 2017, at 8:51 AM, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
>> 
>> Thank you for doing this Henry!
>> 
>> You are right - what is under vector-draft should be the code that was migrated to become jdk.incubator.vector. And also it looks like you have the correct patches circa October.
>> 
>> Two questions:
>> - Can you tell us the titles of the patches that you merged together? This will help keep track when looking back to find if everything was included.
> 
> It should have everything relevant up to following patch in JDK and 
> hotspot,
> 
> Double256Vector JDK support for VectorAPI Implementation
> 
> 
>> - Will you or someone else be helping with the port of all other Vector patches to the new branch?
>> 
> 
> Yes, I can help with that once this initial webrev for vectorIntrinsics branch pushed.
> 
> Cheers,
> Henry