On Tue, 12 Aug 2025 05:55:14 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:
>> src/hotspot/share/opto/vectorIntrinsics.cpp line 1714: >> >>> 1712: } >>> 1713: >>> 1714: Node* origin_node = gvn().intcon(origin->get_con() * >>> type2aelembytes(elem_bt)); >> >> Q1: Is it possible that just passing `origin->get_con()` to >> `VectorSliceNode` in case there are architectures that need it directly? Or, >> maybe we'd better add comment telling that the origin passed to >> `VectorSliceNode` is adjust to bytes. >> >> Q2: If `origin` is not a constant, and there is an architecture that support >> the index as a variable, will the code crash here? Can we just limit the >> `origin` to a constant for this intrinsifaction in this PR? We can consider >> to extend it to variable in case any architecture has such requirement. WDYT? > >> Q1: Is it possible that just passing `origin->get_con()` to >> `VectorSliceNode` in case there are architectures that need it directly? Or, >> maybe we'd better add comment telling that the origin passed to >> `VectorSliceNode` is adjust to bytes. >> > > Added comments. > >> Q2: If `origin` is not a constant, and there is an architecture that support >> the index as a variable, will the code crash here? Can we just limit the >> `origin` to a constant for this intrinsifaction in this PR? We can consider >> to extend it to variable in case any architecture has such a requirement. >> WDYT? > > Currently, inline expander only supports constant origin. I have added a > check to fail intrinsification and inline fallback using the hybrid call > generator. Thanks for your updating! So maybe the matcher function `supports_vector_slice_with_non_constant_index()` could also be removed totally? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24104#discussion_r2268675021
