changeset 34194:213af0859e7e

8141137: C2 fails rematerializing nodes using flag registers. Summary: Don't rem. if input stretches several live ranges. If rem., don't add SpillCopy on RegFlags edge. Reviewed-by: kvn, vlivanov
author goetz
date Mon, 02 Nov 2015 15:52:37 +0100
parents 9389b61f7eee
children 89011d12ebd3
files hotspot/src/share/vm/opto/machnode.cpp hotspot/src/share/vm/opto/machnode.hpp hotspot/src/share/vm/opto/reg_split.cpp
diffstat 3 files changed, 62 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/hotspot/src/share/vm/opto/machnode.cpp	Tue Nov 17 23:35:55 2015 +0100
+++ b/hotspot/src/share/vm/opto/machnode.cpp	Mon Nov 02 15:52:37 2015 +0100
@@ -433,36 +433,49 @@
   if (is_MachTemp()) return true;
 
   uint r = rule();              // Match rule
-  if( r <  Matcher::_begin_rematerialize ||
-      r >= Matcher::_end_rematerialize )
+  if (r <  Matcher::_begin_rematerialize ||
+      r >= Matcher::_end_rematerialize) {
     return false;
+  }
 
   // For 2-address instructions, the input live range is also the output
-  // live range.  Remateralizing does not make progress on the that live range.
-  if( two_adr() )  return false;
+  // live range. Remateralizing does not make progress on the that live range.
+  if (two_adr()) return false;
 
   // Check for rematerializing float constants, or not
-  if( !Matcher::rematerialize_float_constants ) {
+  if (!Matcher::rematerialize_float_constants) {
     int op = ideal_Opcode();
-    if( op == Op_ConF || op == Op_ConD )
+    if (op == Op_ConF || op == Op_ConD) {
       return false;
+    }
   }
 
-  // Defining flags - can't spill these!  Must remateralize.
-  if( ideal_reg() == Op_RegFlags )
+  // Defining flags - can't spill these! Must remateralize.
+  if (ideal_reg() == Op_RegFlags) {
     return true;
+  }
 
   // Stretching lots of inputs - don't do it.
-  if( req() > 2 )
+  if (req() > 2) {
     return false;
+  }
+
+  if (req() == 2 && in(1) && in(1)->ideal_reg() == Op_RegFlags) {
+    // In(1) will be rematerialized, too.
+    // Stretching lots of inputs - don't do it.
+    if (in(1)->req() > 2) {
+      return false;
+    }
+  }
 
   // Don't remateralize somebody with bound inputs - it stretches a
   // fixed register lifetime.
   uint idx = oper_input_base();
   if (req() > idx) {
     const RegMask &rm = in_RegMask(idx);
-    if (rm.is_bound(ideal_reg()))
+    if (rm.is_bound(ideal_reg())) {
       return false;
+    }
   }
 
   return true;
--- a/hotspot/src/share/vm/opto/machnode.hpp	Tue Nov 17 23:35:55 2015 +0100
+++ b/hotspot/src/share/vm/opto/machnode.hpp	Mon Nov 02 15:52:37 2015 +0100
@@ -578,8 +578,8 @@
 
 
 #ifndef PRODUCT
-  virtual const char *Name() const {
-    switch (_spill_type) {
+  static const char *spill_type(SpillType st) {
+    switch (st) {
       case TwoAddress:
         return "TwoAddressSpillCopy";
       case PhiInput:
@@ -612,6 +612,10 @@
     }
   }
 
+  virtual const char *Name() const {
+    return spill_type(_spill_type);
+  }
+
   virtual void format( PhaseRegAlloc *, outputStream *st ) const;
 #endif
 };
--- a/hotspot/src/share/vm/opto/reg_split.cpp	Tue Nov 17 23:35:55 2015 +0100
+++ b/hotspot/src/share/vm/opto/reg_split.cpp	Mon Nov 02 15:52:37 2015 +0100
@@ -55,13 +55,15 @@
 // Get a SpillCopy node with wide-enough masks.  Use the 'wide-mask', the
 // wide ideal-register spill-mask if possible.  If the 'wide-mask' does
 // not cover the input (or output), use the input (or output) mask instead.
-Node *PhaseChaitin::get_spillcopy_wide(MachSpillCopyNode::SpillType spill_type, Node *def, Node *use, uint uidx ) {
+Node *PhaseChaitin::get_spillcopy_wide(MachSpillCopyNode::SpillType spill_type, Node *def, Node *use, uint uidx) {
   // If ideal reg doesn't exist we've got a bad schedule happening
   // that is forcing us to spill something that isn't spillable.
   // Bail rather than abort
   int ireg = def->ideal_reg();
-  if( ireg == 0 || ireg == Op_RegFlags ) {
-    assert(false, "attempted to spill a non-spillable item");
+  if (ireg == 0 || ireg == Op_RegFlags) {
+    assert(false, "attempted to spill a non-spillable item: %d: %s <- %d: %s, ireg = %d, spill_type: %s",
+           def->_idx, def->Name(), use->_idx, use->Name(), ireg,
+           MachSpillCopyNode::spill_type(spill_type));
     C->record_method_not_compilable("attempted to spill a non-spillable item");
     return NULL;
   }
@@ -308,14 +310,16 @@
 
 //------------------------------split_Rematerialize----------------------------
 // Clone a local copy of the def.
-Node *PhaseChaitin::split_Rematerialize( Node *def, Block *b, uint insidx, uint &maxlrg, GrowableArray<uint> splits, int slidx, uint *lrg2reach, Node **Reachblock, bool walkThru ) {
+Node *PhaseChaitin::split_Rematerialize(Node *def, Block *b, uint insidx, uint &maxlrg,
+                                        GrowableArray<uint> splits, int slidx, uint *lrg2reach,
+                                        Node **Reachblock, bool walkThru) {
   // The input live ranges will be stretched to the site of the new
   // instruction.  They might be stretched past a def and will thus
   // have the old and new values of the same live range alive at the
   // same time - a definite no-no.  Split out private copies of
   // the inputs.
-  if( def->req() > 1 ) {
-    for( uint i = 1; i < def->req(); i++ ) {
+  if (def->req() > 1) {
+    for (uint i = 1; i < def->req(); i++) {
       Node *in = def->in(i);
       uint lidx = _lrg_map.live_range_id(in);
       // We do not need this for live ranges that are only defined once.
@@ -327,12 +331,29 @@
 
       Block *b_def = _cfg.get_block_for_node(def);
       int idx_def = b_def->find_node(def);
-      Node *in_spill = get_spillcopy_wide(MachSpillCopyNode::InputToRematerialization, in, def, i );
-      if( !in_spill ) return 0; // Bailed out
-      insert_proj(b_def,idx_def,in_spill,maxlrg++);
-      if( b_def == b )
-        insidx++;
-      def->set_req(i,in_spill);
+      // Cannot spill Op_RegFlags.
+      Node *in_spill;
+      if (in->ideal_reg() != Op_RegFlags) {
+        in_spill = get_spillcopy_wide(MachSpillCopyNode::InputToRematerialization, in, def, i);
+        if (!in_spill) { return 0; } // Bailed out
+        insert_proj(b_def, idx_def, in_spill, maxlrg++);
+        if (b_def == b) {
+          insidx++;
+        }
+        def->set_req(i, in_spill);
+      } else {
+        // The 'in' defines a flag register. Flag registers can not be spilled.
+        // Register allocation handles live ranges with flag registers
+        // by rematerializing the def (in this case 'in'). Thus, this is not
+        // critical if the input can be rematerialized, too.
+        if (!in->rematerialize()) {
+          assert(false, "Can not rematerialize %d: %s. Prolongs RegFlags live"
+                 " range and defining node %d: %s may not be rematerialized.",
+                 def->_idx, def->Name(), in->_idx, in->Name());
+          C->record_method_not_compilable("attempted to spill a non-spillable item with RegFlags input");
+          return 0; // Bailed out
+        }
+      }
     }
   }