RFR: 8343685: C2 SuperWord: refactor VPointer with MemPointer [v14]

[Emanuel Peter]

On Fri, 17 Jan 2025 14:22:39 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Batch 2 for Christian
>
> src/hotspot/share/opto/mempointer.cpp line 513:
> 
>> 511: #endif
>> 512:     return MemPointerAliasing::make_not_or_at_distance(distance.value());
>> 513:   }
> 
> You can remove the `else` and just continue because you return in the if-branch:
> Suggestion:
> 
>  
>    // At runtime, the two object bases can be:
>   //   (1) different: pointers do not alias.
>   //   (2) the same:  implies that (S2) holds. The summands are all the same, and with
>   //                  the Pre-Condition, we know that both pointers are in bounds of the
>   //                  same memory object, i.e. (S1) holds. We have already proven (S0)
>   //                  and (S3), so all 4 conditions for "MemPointer Lemma" are given.
> #ifndef PRODUCT
>   if (trace.is_trace_aliasing()) {
>     tty->print_cr("  -> Aliasing not or at distance = %d.", distance.value());
>   }
> #endif
>   return MemPointerAliasing::make_not_or_at_distance(distance.value());

I have to say I prefer the symmetry here, I would prefer an if-else. Oh but the printing ifs are not correctly aligned.

> src/hotspot/share/opto/vectorization.cpp line 186:
> 
>> 184: void VLoopVPointers::allocate_vpointers_array() {
>> 185:   uint bytes2 = _vpointers_length * sizeof(VPointer);
>> 186:   _vpointers = (VPointer*)_arena->Amalloc(bytes2);
> 
> Why `bytes2`?

woops, that is an artefact from refactoring. reverted it.

> src/hotspot/share/opto/vectorization.cpp line 962:
> 
>> 960:     // adr = base + con + invar + iv_scale * iv
>> 961:     tty->print("  adr = base[%d]", base().object_or_native()->_idx);
>> 962:     tty->print(" + con(%d) + invar + iv_scale(%d) * iv", _vpointer.con(), iv_scale());
> 
> Nit: In previous comments, you put `con` last. Should we also do this here?

swapped it, nice catch!

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1920965073
PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1920965640
PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1920966072