On Fri, 13 Sep 2024 18:27:07 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:
> > Can you please **define** somewhere what it means to `prune indexes`? It > > does not help me much more than the previous "massaging indexes" you had > > before I asked you to change it. > > > Also: I'm a little worried about the semantics change of the > > > RearrangeNode that you did with the changes in RearrangeNode::Ideal. It > > > looks a little "hacky", especially in conjunction with the > > > vector_indexes_needs_massaging method. Can you give a clear definition of > > > the semantics of RearrangeNode and vector_indexes_needs_massaging, please? > > > > > > You have also not responded to this yet. It seems to me that before your > > proposed change, `RearrangeNode` had a clear and easy semantic, and now you > > somehow "hack it" to work with your `vector_indexes_needs_pruning`. Can you > > explain please why this makes sense and add a comment to `RearrangeNode` > > what its semantics is? > > In case target does not directly support two vector selection instruction we > lower the IR to its constituents, this is better than intrinsification > failure as it saves costly vector boxing penalties. > > Consider in terms of desired compiler IR and not rearrange API semantics, > VectorRearrange IR node generally expects shape conformance b/w vector to be > permuted and index vector, since shuffle indices are held in byte array based > backing storage hence compiler injects VectorLoadShuffle nodes to upcast the > byte vector lanes holding indexes to match the input vector lane. Since > selectFrom API already passes the indexes through vector hence we can save > emitting redundant toShuffle() + toVector() operations in some cases apart > from some target specific scenarios e.g. AVX2 targets [do not support direct > short vector permute]instruction "VPERMW", hence we need to [massage the > index > vector](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86.ad#L8771) > to emulate desired permutation using byte permute instruction. > > VectorLoadShuffle abstraction hides target specific index massaging which is > why adding a target specific hook like Matcher::vector_indexes_needs_pruning > compiler to selectively emit VectorLoadShuffle. I still do not see a **definition of the semantics of RearrangeNode**: what inputs does it accept and what does it do with them? Can you put this explanation as comment in the code, please? It sounds like this is what the `massaging` / `pruning` is: `emulate desired permutation using byte permute instruction.` You should find an accordingly more suiting name for the method name. Maybe it is something like `must_emulate_permutation_with...`. Or maybe it is rather a `supported` kind of question? I leave that up to you. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20508#issuecomment-2351426263
