On Sat, 25 May 2024 00:15:03 GMT, Sandhya Viswanathan <sviswanat...@openjdk.org> wrote:
>> Scott Gibbons has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix test; review comments > > src/hotspot/cpu/x86/c2_stubGenerator_x86_64_string.cpp line 316: > >> 314: // Set up jump tables. Used when needle size <= NUMBER_OF_CASES >> 315: setup_jump_tables(ae, L_returnRBP, L_checkRangeAndReturn, >> L_bigCaseFixupAndReturn, >> 316: &big_jump_table, &small_jump_table, _masm); > > We could directly use L_returnError here instead of L_returnRBP. OK > src/hotspot/cpu/x86/c2_stubGenerator_x86_64_string.cpp line 476: > >> 474: // Used to check and return value in rbp - usually error >> 475: __ bind(L_returnRBP); >> 476: __ movq(rax, rbp); > > This seems spurious as rax is being overwritten at line 489. Did you intend > to return -1? Removed all references to L_returnRBP. Replaced with L_returnError. > src/hotspot/cpu/x86/c2_stubGenerator_x86_64_string.cpp line 1816: > >> 1814: byte_compare_helper(i + 1, L_loopTop, L_fixup, needle, >> needle_val, hs_ptr, eq_mask, set_bit, >> 1815: rTmp4, ae, _masm); >> 1816: } > > L_checkRange on NoMatch could be L_error instead. Fixed ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1614900796 PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1614903860 PR Review Comment: https://git.openjdk.org/jdk/pull/16753#discussion_r1614901577