Merge pull request #6938 from fjeremic/fix-indexof

Fix off-by-one issue in intrinsicIndexOfString* intrinsic on Z
eclipse-openj9 · Sep 30, 2019 · 1edbcd1 · 1edbcd1
2 parents 92f3b43 + 7b4b511
commit 1edbcd1
Show file tree

Hide file tree

Showing 7 changed files with 495 additions and 273 deletions.
diff --git a/jcl/src/java.base/share/classes/com/ibm/jit/JITHelpers.java b/jcl/src/java.base/share/classes/com/ibm/jit/JITHelpers.java
@@ -588,7 +588,7 @@ public char getCharFromArrayByIndex(Object obj, int index) {
 	 * @return        the index (in number of characters) of the target array within the source array, or -1 if the
 	 *                target array is not found within the source array.
 	 */
-	public int intrinsicIndexOfStringLatin1(char[] s1Value, int s1len, char[] s2Value, int s2len, int start) {
+	public int intrinsicIndexOfStringLatin1(Object s1Value, int s1len, Object s2Value, int s2len, int start) {
 		char firstChar = byteToCharUnsigned(getByteFromArrayByIndex(s2Value, 0));
 
 		while (true) {
@@ -634,8 +634,8 @@ public int intrinsicIndexOfStringLatin1(char[] s1Value, int s1len, char[] s2Valu
 	 * @return        the index (in number of characters) of the target array within the source array, or -1 if the
 	 *                target array is not found within the source array.
 	 */
-	public int intrinsicIndexOfStringUTF16(char[] s1Value, int s1len, char[] s2Value, int s2len, int start) {
-		char firstChar = s2Value[0];
+	public int intrinsicIndexOfStringUTF16(Object s1Value, int s1len, Object s2Value, int s2len, int start) {
+		char firstChar = getCharFromArrayByIndex(s2Value, 0);
 
 		while (true) {
 			int i = intrinsicIndexOfUTF16(s1Value, firstChar, start, s1len);
@@ -648,7 +648,7 @@ public int intrinsicIndexOfStringUTF16(char[] s1Value, int s1len, char[] s2Value
 			int o1 = i;
 			int o2 = 0;
 
-			while ((++o2 < s2len) && (s1Value[++o1] == s2Value[o2]))
+			while (++o2 < s2len && getCharFromArrayByIndex(s1Value, ++o1) == getCharFromArrayByIndex(s2Value, o2))
 				;
 
 			if (o2 == s2len) {

diff --git a/runtime/compiler/env/j9method.cpp b/runtime/compiler/env/j9method.cpp
@@ -3403,8 +3403,8 @@ void TR_ResolvedJ9Method::construct()
       {
       {x(TR::com_ibm_jit_JITHelpers_is32Bit,                                  "is32Bit", "()Z")},
       {x(TR::com_ibm_jit_JITHelpers_isArray,                                  "isArray", "(Ljava/lang/Object;)Z")},
-      {x(TR::com_ibm_jit_JITHelpers_intrinsicIndexOfStringLatin1,             "intrinsicIndexOfStringLatin1", "([CI[CII)I")},
-      {x(TR::com_ibm_jit_JITHelpers_intrinsicIndexOfStringUTF16,              "intrinsicIndexOfStringUTF16", "([CI[CII)I")},
+      {x(TR::com_ibm_jit_JITHelpers_intrinsicIndexOfStringLatin1,             "intrinsicIndexOfStringLatin1", "(Ljava/lang/Object;ILjava/lang/Object;II)I")},
+      {x(TR::com_ibm_jit_JITHelpers_intrinsicIndexOfStringUTF16,              "intrinsicIndexOfStringUTF16", "(Ljava/lang/Object;ILjava/lang/Object;II)I")},
       {x(TR::com_ibm_jit_JITHelpers_intrinsicIndexOfLatin1,                   "intrinsicIndexOfLatin1", "(Ljava/lang/Object;BII)I")},
       {x(TR::com_ibm_jit_JITHelpers_intrinsicIndexOfUTF16,                    "intrinsicIndexOfUTF16", "(Ljava/lang/Object;CII)I")},
 #ifdef TR_TARGET_32BIT

diff --git a/runtime/compiler/z/codegen/J9TreeEvaluator.cpp b/runtime/compiler/z/codegen/J9TreeEvaluator.cpp
@@ -236,8 +236,8 @@ doubleMaxMinHelper(TR::Node *node, TR::CodeGenerator *cg, bool isMaxOp)
  * StringUTF16.indexOf(s1Value, s1Length, s2Value, s2Length, fromIndex);
  *
  * For Java 8:
- * com.ibm.jit.JITHelpers.intrinsicIndexOfStringLatin1(char[] s1Value, int s1len, char[] s2Value, int s2len, int start);
- * com.ibm.jit.JITHelpers.intrinsicIndexOfStringUTF16(char[] s1Value, int s1len, char[] s2Value, int s2len, int start);
+ * com.ibm.jit.JITHelpers.intrinsicIndexOfStringLatin1(Object s1Value, int s1len, Object s2Value, int s2len, int start);
+ * com.ibm.jit.JITHelpers.intrinsicIndexOfStringUTF16(Object s1Value, int s1len, Object s2Value, int s2len, int start);
  *
  * Assumptions:
  *
@@ -516,7 +516,7 @@ inlineVectorizedStringIndexOf(TR::Node* node, TR::CodeGenerator* cg, bool isUTF1
       generateS390LabelInstruction(cg, TR::InstOpCode::LABEL, node, labelExtractFirstCharPos);
 
       generateVRScInstruction(cg, TR::InstOpCode::VLGV, node, char1IndexReg, tmpVReg, generateS390MemoryReference(7, cg), 0);
-      generateRIEInstruction(cg, TR::InstOpCode::getCmpRegAndBranchRelOpCode(), node, char1IndexReg, loadLenReg, labelStringNotFound, TR::InstOpCode::COND_BNLR);
+      generateRIEInstruction(cg, TR::InstOpCode::getCmpRegAndBranchRelOpCode(), node, char1IndexReg, loadLenReg, labelStringNotFound, TR::InstOpCode::COND_BHR);
 
       generateRRInstruction(cg, TR::InstOpCode::getAddRegOpCode(), node, char1IndexReg, s1VecStartIndexReg);
       generateRIEInstruction(cg, TR::InstOpCode::getCmpRegAndBranchRelOpCode(), node, char1IndexReg, maxIndexReg, labelStringNotFound, TR::InstOpCode::COND_BHR);

diff --git a/test/functional/Java8andUp/playlist.xml b/test/functional/Java8andUp/playlist.xml
@@ -1292,7 +1292,10 @@
 			<group>functional</group>
 		</groups>
     <variations>
-      <variation>'-Xjit:{*Test_JITHelpers*}(count=0)'</variation>
+      <variation>-Xjit:count=0 -XX:-CompactStrings</variation>
+      <variation>-Xjit:count=0 -XX:+CompactStrings</variation>
+      <variation>-Xjit:count=1,disableAsyncCompilation -XX:-CompactStrings</variation>
+      <variation>-Xjit:count=1,disableAsyncCompilation -XX:+CompactStrings</variation>
     </variations>
 		<impls>
 			<impl>openj9</impl>
@@ -1319,7 +1322,10 @@
 			<group>functional</group>
 		</groups>
     <variations>
-      <variation>'-Xjit:{*Test_JITHelpers*}(count=0)'</variation>
+      <variation>-Xjit:count=0 -XX:-CompactStrings</variation>
+      <variation>-Xjit:count=0 -XX:+CompactStrings</variation>
+      <variation>-Xjit:count=1,disableAsyncCompilation -XX:-CompactStrings</variation>
+      <variation>-Xjit:count=1,disableAsyncCompilation -XX:+CompactStrings</variation>
     </variations>
 		<impls>
 			<impl>openj9</impl>
@@ -2159,30 +2165,6 @@
 			<impl>ibm</impl>
 		</impls>
 	</test>
-	<test>
-		<testCaseName>testStringIndexOfString</testCaseName>
-		<variations>
-			<variation>NoOptions</variation>
-			<variation>$(SQ)-Xjit:disableAsyncCompilation,tryToInline={java/lang/String.indexOf*},limit={org/openj9/test/string/StringIndexOfString.doTest*},{org/openj9/test/string/StringIndexOfString.doTest*}(count=50)$(SQ)</variation>
-		</variations>
-		<command>$(ADD_JVM_LIB_DIR_TO_LIBPATH) $(JAVA_COMMAND) $(JVM_OPTIONS) \
-	-cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)GeneralTest.jar$(Q) \
-	org.testng.TestNG -d $(REPORTDIR) $(Q)$(TEST_RESROOT)$(D)testng.xml$(Q) \
-	-testnames testStringIndexOfString \
-	-groups $(TEST_GROUP) \
-	-excludegroups $(DEFAULT_EXCLUDE); \
-	$(TEST_STATUS)</command>
-		<levels>
-			<level>sanity</level>
-		</levels>
-		<groups>
-			<group>functional</group>
-		</groups>
-		<impls>
-			<impl>openj9</impl>
-			<impl>ibm</impl>
-		</impls>
-	</test>
 
 	<test>
 		<testCaseName>testStringStreams</testCaseName>