8240905: assert(mem == (Node*)1 || mem == mem2) failed: multiple Memories being matched at once?

Stop recursion if there are multiple loads with different memory inputs in the tree.

Reviewed-by: kvn, vlivanov

src/hotspot/share/opto/matcher.cpp

@@ -1359,7 +1359,8 @@ MachNode *Matcher::match_tree( const Node *n ) {
   s->_kids[1] = NULL;
   s->_leaf = (Node*)n;
   // Label the input tree, allocating labels from top-level arena
-  Label_Root( n, s, n->in(0), mem );
+  Node* root_mem = mem;
+  Label_Root(n, s, n->in(0), root_mem);
   if (C->failing())  return NULL;
 
   // The minimum cost match for the whole tree is found at the root State
@@ -1473,8 +1474,9 @@ static bool match_into_reg( const Node *n, Node *m, Node *control, int i, bool s
 // Store and the Load must have identical Memories (as well as identical
 // pointers).  Since the Matcher does not have anything for Memory (and
 // does not handle DAGs), I have to match the Memory input myself.  If the
-// Tree root is a Store, I require all Loads to have the identical memory.
-Node *Matcher::Label_Root( const Node *n, State *svec, Node *control, const Node *mem){
+// Tree root is a Store or if there are multiple Loads in the tree, I require
+// all Loads to have the identical memory.
+Node* Matcher::Label_Root(const Node* n, State* svec, Node* control, Node*& mem) {
   // Since Label_Root is a recursive function, its possible that we might run
   // out of stack space.  See bugs 6272980 & 6227033 for more info.
   LabelRootDepth++;
@@ -1498,6 +1500,11 @@ Node *Matcher::Label_Root( const Node *n, State *svec, Node *control, const Node
     if( m->is_Load() ) {
       if( input_mem == NULL ) {
         input_mem = m->in(MemNode::Memory);
+        if (mem == (Node*)1) {
+          // Save this memory to bail out if there's another memory access
+          // to a different memory location in the same tree.
+          mem = input_mem;
+        }
       } else if( input_mem != m->in(MemNode::Memory) ) {
         input_mem = NodeSentinel;
       }
@@ -1521,16 +1528,16 @@ Node *Matcher::Label_Root( const Node *n, State *svec, Node *control, const Node
     // the current tree.  If it finds any, that value is matched as a
     // register operand.  If not, then the normal matching is used.
     if( match_into_reg(n, m, control, i, is_shared(m)) ||
-        //
-        // Stop recursion if this is LoadNode and the root of this tree is a
-        // StoreNode and the load & store have different memories.
+        // Stop recursion if this is a LoadNode and there is another memory access
+        // to a different memory location in the same tree (for example, a StoreNode
+        // at the root of this tree or another LoadNode in one of the children).
         ((mem!=(Node*)1) && m->is_Load() && m->in(MemNode::Memory) != mem) ||
         // Can NOT include the match of a subtree when its memory state
         // is used by any of the other subtrees
         (input_mem == NodeSentinel) ) {
       // Print when we exclude matching due to different memory states at input-loads
       if (PrintOpto && (Verbose && WizardMode) && (input_mem == NodeSentinel)
-        && !((mem!=(Node*)1) && m->is_Load() && m->in(MemNode::Memory) != mem)) {
+          && !((mem!=(Node*)1) && m->is_Load() && m->in(MemNode::Memory) != mem)) {
         tty->print_cr("invalid input_mem");
       }
       // Switch to a register-only opcode; this value must be in a register
@@ -1542,7 +1549,7 @@ Node *Matcher::Label_Root( const Node *n, State *svec, Node *control, const Node
       if( control == NULL && m->in(0) != NULL && m->req() > 1 )
         control = m->in(0);         // Pick up control
       // Else match as a normal part of the match tree.
-      control = Label_Root(m,s,control,mem);
+      control = Label_Root(m, s, control, mem);
       if (C->failing()) return NULL;
     }
   }

src/hotspot/share/opto/matcher.hpp

@@ -131,7 +131,7 @@ private:
   GrowableArray<Node_Notes*>* _old_node_note_array;
 
   // Node labeling iterator for instruction selection
-  Node *Label_Root( const Node *n, State *svec, Node *control, const Node *mem );
+  Node* Label_Root(const Node* n, State* svec, Node* control, Node*& mem);
 
   Node *transform( Node *dummy );
 

test/hotspot/jtreg/compiler/codegen/TestMultiMemInstructionMatching.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8240905
+ * @summary Test matching of instructions that have multiple memory inputs.
+ * @run main/othervm -Xbatch -XX:-TieredCompilation
+ *                   compiler.codegen.TestMultiMemInstructionMatching
+ */
+
+package compiler.codegen;
+
+public class TestMultiMemInstructionMatching {
+
+    static volatile int iFldV = 42;
+    static volatile long lFldV = 42;
+    static int iFld = 42;
+    static long lFld = 42;
+
+    // Integer versions
+
+    static int test_blsiI_rReg_mem_1() {
+        return (0 - iFldV) & iFldV;
+    }
+
+    static int test_blsiI_rReg_mem_2() {
+        int sub = (0 - iFld);
+        iFldV++;
+        return sub & iFld;
+    }
+
+    static int test_blsrI_rReg_mem_1() {
+        return (iFldV - 1) & iFldV;
+    }
+
+    static int test_blsrI_rReg_mem_2() {
+        int sub = (iFld - 1);
+        iFldV++;
+        return sub & iFld;
+    }
+
+    static int test_blsmskI_rReg_mem_1() {
+        return (iFldV - 1) ^ iFldV;
+    }
+
+    static int test_blsmskI_rReg_mem_2() {
+        int sub = (iFld - 1);
+        iFldV++;
+        return sub ^ iFld;
+    }
+
+    // Long versions
+
+    static long test_blsiL_rReg_mem_1() {
+        return (0 - lFldV) & lFldV;
+    }
+
+    static long test_blsiL_rReg_mem_2() {
+        long sub = (0 - lFld);
+        lFldV++;
+        return sub & lFld;
+    }
+
+    static long test_blsrL_rReg_mem_1() {
+        return (lFldV - 1) & lFldV;
+    }
+
+    static long test_blsrL_rReg_mem_2() {
+        long sub = (lFld - 1);
+        lFldV++;
+        return sub & lFld;
+    }
+
+    static long test_blsmskL_rReg_mem_1() {
+        return (lFldV - 1) ^ lFldV;
+    }
+
+    static long test_blsmskL_rReg_mem_2() {
+        long sub = (lFld - 1);
+        lFldV++;
+        return sub ^ lFld;
+    }
+
+    public static void main(String[] args) {
+        for (int i = 0;i < 100_000;++i) {
+            test_blsiI_rReg_mem_1();
+            test_blsiI_rReg_mem_2();
+            test_blsrI_rReg_mem_1();
+            test_blsrI_rReg_mem_2();
+            test_blsmskI_rReg_mem_1();
+            test_blsmskI_rReg_mem_2();
+
+            test_blsiL_rReg_mem_1();
+            test_blsiL_rReg_mem_2();
+            test_blsrL_rReg_mem_1();
+            test_blsrL_rReg_mem_2();
+            test_blsmskL_rReg_mem_1();
+            test_blsmskL_rReg_mem_2();
+        }
+    }
+}