8176580: [ppc, s390] CRC32C: wrong checksum result in some cases
authorsimonis
Fri, 24 Mar 2017 16:35:37 +0100
changeset 46386 742f8b16d00c
parent 46385 6b890c9a717f
child 46387 c46632622b17
8176580: [ppc, s390] CRC32C: wrong checksum result in some cases Reviewed-by: simonis, mdoerr Contributed-by: lutz.schmidt@sap.com
hotspot/src/cpu/ppc/vm/c1_LIRGenerator_ppc.cpp
hotspot/src/cpu/ppc/vm/templateInterpreterGenerator_ppc.cpp
hotspot/src/cpu/s390/vm/c1_LIRGenerator_s390.cpp
hotspot/src/cpu/s390/vm/templateInterpreterGenerator_s390.cpp
--- a/hotspot/src/cpu/ppc/vm/c1_LIRGenerator_ppc.cpp	Fri Apr 14 15:47:33 2017 -0700
+++ b/hotspot/src/cpu/ppc/vm/c1_LIRGenerator_ppc.cpp	Fri Mar 24 16:35:37 2017 +0100
@@ -1433,9 +1433,19 @@
       LIRItem crc(x->argument_at(0), this);
       LIRItem buf(x->argument_at(1), this);
       LIRItem off(x->argument_at(2), this);
-      LIRItem len(x->argument_at(3), this);
+      LIRItem end(x->argument_at(3), this);
       buf.load_item();
       off.load_nonconstant();
+      end.load_nonconstant();
+
+      // len = end - off
+      LIR_Opr len  = end.result();
+      LIR_Opr tmpA = new_register(T_INT);
+      LIR_Opr tmpB = new_register(T_INT);
+      __ move(end.result(), tmpA);
+      __ move(off.result(), tmpB);
+      __ sub(tmpA, tmpB, tmpA);
+      len = tmpA;
 
       LIR_Opr index = off.result();
       int offset = is_updateBytes ? arrayOopDesc::base_offset_in_bytes(T_BYTE) : 0;
@@ -1467,9 +1477,9 @@
               arg2 = cc->at(1),
               arg3 = cc->at(2);
 
-      crc.load_item_force(arg1); // We skip int->long conversion here, because CRC32 stub doesn't care about high bits.
+      crc.load_item_force(arg1); // We skip int->long conversion here, because CRC32C stub doesn't care about high bits.
       __ leal(LIR_OprFact::address(a), arg2);
-      len.load_item_force(arg3); // We skip int->long conversion here, , because CRC32 stub expects int.
+      __ move(len, cc->at(2));   // We skip int->long conversion here, because CRC32C stub expects int.
 
       __ call_runtime_leaf(StubRoutines::updateBytesCRC32C(), LIR_OprFact::illegalOpr, result_reg, cc->args());
       __ move(result_reg, result);
--- a/hotspot/src/cpu/ppc/vm/templateInterpreterGenerator_ppc.cpp	Fri Apr 14 15:47:33 2017 -0700
+++ b/hotspot/src/cpu/ppc/vm/templateInterpreterGenerator_ppc.cpp	Fri Mar 24 16:35:37 2017 +0100
@@ -1908,11 +1908,7 @@
   return NULL;
 }
 
-// TODO: generate_CRC32_updateBytes_entry and generate_CRC32C_updateBytes_entry are identical
-//       except for using different crc tables and some block comment strings.
-//       We should provide a common implementation.
 
-// CRC32 Intrinsics.
 /**
  * Method entry for static native methods:
  *   int java.util.zip.CRC32.updateBytes(     int crc, byte[] b,  int off, int len)
@@ -2004,11 +2000,13 @@
   return NULL;
 }
 
-// CRC32C Intrinsics.
+
 /**
- * Method entry for static native methods:
- *   int java.util.zip.CRC32C.updateBytes(           int crc, byte[] b,  int off, int len)
- *   int java.util.zip.CRC32C.updateDirectByteBuffer(int crc, long* buf, int off, int len)
+ * Method entry for intrinsic-candidate (non-native) methods:
+ *   int java.util.zip.CRC32C.updateBytes(           int crc, byte[] b,  int off, int end)
+ *   int java.util.zip.CRC32C.updateDirectByteBuffer(int crc, long* buf, int off, int end)
+ * Unlike CRC32, CRC32C does not have any methods marked as native
+ * CRC32C also uses an "end" variable instead of the length variable CRC32 uses
  **/
 address TemplateInterpreterGenerator::generate_CRC32C_updateBytes_entry(AbstractInterpreter::MethodKind kind) {
   if (UseCRC32CIntrinsics) {
@@ -2052,6 +2050,7 @@
       __ lwa( dataLen, 1*wordSize, argP);  // #bytes to process
       __ lwz( crc,     5*wordSize, argP);  // current crc state
       __ add( data, data, tmp);            // Add byte buffer offset.
+      __ sub( dataLen, dataLen, tmp);      // (end_index - offset)
     } else {                                                         // Used for "updateBytes update".
       BLOCK_COMMENT("CRC32C_updateBytes {");
       // crc     @ (SP + 4W) (32bit)
@@ -2063,6 +2062,7 @@
       __ lwa( tmp,     2*wordSize, argP);  // byte buffer offset
       __ lwa( dataLen, 1*wordSize, argP);  // #bytes to process
       __ add( data, data, tmp);            // add byte buffer offset
+      __ sub( dataLen, dataLen, tmp);      // (end_index - offset)
       __ lwz( crc,     4*wordSize, argP);  // current crc state
       __ addi(data, data, arrayOopDesc::base_offset_in_bytes(T_BYTE));
     }
--- a/hotspot/src/cpu/s390/vm/c1_LIRGenerator_s390.cpp	Fri Apr 14 15:47:33 2017 -0700
+++ b/hotspot/src/cpu/s390/vm/c1_LIRGenerator_s390.cpp	Fri Mar 24 16:35:37 2017 +0100
@@ -1229,9 +1229,19 @@
       LIRItem crc(x->argument_at(0), this);
       LIRItem buf(x->argument_at(1), this);
       LIRItem off(x->argument_at(2), this);
-      LIRItem len(x->argument_at(3), this);
+      LIRItem end(x->argument_at(3), this);
       buf.load_item();
       off.load_nonconstant();
+      end.load_nonconstant();
+
+      // len = end - off
+      LIR_Opr len  = end.result();
+      LIR_Opr tmpA = new_register(T_INT);
+      LIR_Opr tmpB = new_register(T_INT);
+      __ move(end.result(), tmpA);
+      __ move(off.result(), tmpB);
+      __ sub(tmpA, tmpB, tmpA);
+      len = tmpA;
 
       LIR_Opr index = off.result();
       int offset = is_updateBytes ? arrayOopDesc::base_offset_in_bytes(T_BYTE) : 0;
@@ -1262,7 +1272,7 @@
 
       crc.load_item_force(arg1); // We skip int->long conversion here, because CRC32C stub doesn't care about high bits.
       __ leal(LIR_OprFact::address(a), arg2);
-      len.load_item_force(arg3); // We skip int->long conversion here, because CRC32C stub expects int.
+      __ move(len, cc->at(2));   // We skip int->long conversion here, because CRC32C stub expects int.
 
       __ call_runtime_leaf(StubRoutines::updateBytesCRC32C(), LIR_OprFact::illegalOpr, result_reg, cc->args());
       __ move(result_reg, result);
--- a/hotspot/src/cpu/s390/vm/templateInterpreterGenerator_s390.cpp	Fri Apr 14 15:47:33 2017 -0700
+++ b/hotspot/src/cpu/s390/vm/templateInterpreterGenerator_s390.cpp	Fri Mar 24 16:35:37 2017 +0100
@@ -1930,8 +1930,11 @@
   return entry_point;
 }
 
-// Method entry for static native methods:
-//   int java.util.zip.CRC32.update(int crc, int b)
+
+/**
+ * Method entry for static native methods:
+ *   int java.util.zip.CRC32.update(int crc, int b)
+ */
 address TemplateInterpreterGenerator::generate_CRC32_update_entry() {
 
   if (UseCRC32Intrinsics) {
@@ -1980,9 +1983,11 @@
 }
 
 
-// Method entry for static native methods:
-//   int java.util.zip.CRC32.updateBytes(int crc, byte[] b, int off, int len)
-//   int java.util.zip.CRC32.updateByteBuffer(int crc, long buf, int off, int len)
+/**
+ * Method entry for static native methods:
+ *   int java.util.zip.CRC32.updateBytes(     int crc, byte[] b,  int off, int len)
+ *   int java.util.zip.CRC32.updateByteBuffer(int crc, long* buf, int off, int len)
+ */
 address TemplateInterpreterGenerator::generate_CRC32_updateBytes_entry(AbstractInterpreter::MethodKind kind) {
 
   if (UseCRC32Intrinsics) {
@@ -2058,9 +2063,13 @@
 }
 
 
-// Method entry for static native methods:
-//   int java.util.zip.CRC32C.updateBytes(int crc, byte[] b, int off, int len)
-//   int java.util.zip.CRC32C.updateDirectByteBuffer(int crc, long buf, int off, int len)
+/**
+ * Method entry for intrinsic-candidate (non-native) methods:
+ *   int java.util.zip.CRC32C.updateBytes(           int crc, byte[] b,  int off, int end)
+ *   int java.util.zip.CRC32C.updateDirectByteBuffer(int crc, long* buf, int off, int end)
+ * Unlike CRC32, CRC32C does not have any methods marked as native
+ * CRC32C also uses an "end" variable instead of the length variable CRC32 uses
+ */
 address TemplateInterpreterGenerator::generate_CRC32C_updateBytes_entry(AbstractInterpreter::MethodKind kind) {
 
   if (UseCRC32CIntrinsics) {
@@ -2093,7 +2102,8 @@
       __ z_llgf(crc,    5*wordSize, argP);  // current crc state
       __ z_lg(data,     3*wordSize, argP);  // start of byte buffer
       __ z_agf(data,    2*wordSize, argP);  // Add byte buffer offset.
-      __ z_lgf(dataLen, 1*wordSize, argP);  // #bytes to process
+      __ z_lgf(dataLen, 1*wordSize, argP);  // #bytes to process, calculated as
+      __ z_sgf(dataLen, Address(argP, 2*wordSize));  // (end_index - offset)
     } else {                                                                // Used for "updateBytes update".
       // crc     @ (SP + 4W) (32bit)
       // buf     @ (SP + 3W) (64bit ptr to byte array)
@@ -2104,7 +2114,8 @@
       __ z_llgf(crc,    4*wordSize, argP);  // current crc state
       __ z_lg(data,     3*wordSize, argP);  // start of byte buffer
       __ z_agf(data,    2*wordSize, argP);  // Add byte buffer offset.
-      __ z_lgf(dataLen, 1*wordSize, argP);  // #bytes to process
+      __ z_lgf(dataLen, 1*wordSize, argP);  // #bytes to process, calculated as
+      __ z_sgf(dataLen, Address(argP, 2*wordSize));  // (end_index - offset)
       __ z_aghi(data, arrayOopDesc::base_offset_in_bytes(T_BYTE));
     }