On Thu, 26 Sep 2024 16:15:39 GMT, Roman Kennke <rken...@openjdk.org> wrote:
>>> Does this look correct to you? Or better to do it as a follow-up? >> >> I do not feel confident enough to review this part. If you want to include >> https://github.com/rkennke/jdk/commit/097c2afa04397773e514552dfb942aa889bfa2c1 >> in this changeset, I would prefer that the original author of JDK-8320448 >> or at least someone from Intel reviews it, otherwise I think it is fine to >> leave it as a follow-up enhancement. > > @sviswa7 or @asgibbons WDYT about including > https://github.com/rkennke/jdk/commit/097c2afa04397773e514552dfb942aa889bfa2c1 > as part of compact object headers implementation? Otherwise we would have to > disable indexOf intrinsic when running with compact headers, because of the > assumption that array headers are >= 16 bytes, which is no longer true with > compact headers. @rkennke I reviewed [rkennke@ 097c2af](https://github.com/rkennke/jdk/commit/097c2afa04397773e514552dfb942aa889bfa2c1) and the code looks good to me. I would prefer this approach instead of not generating the IndexOf intrinsic. Should the controlling `if` be conditioned on `UseCompactObjectHeaders` instead of `arrayOopDesc::base_offset_in_bytes`? I can see benefits to either - which provides more clarity? I like the assert as it makes the intention clear (thanks!). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1777485078
