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

[Emanuel Peter]

On Mon, 13 Jan 2025 20:17:23 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 116 commits:
>> 
>>  - copyright 2025
>>  - Merge branch 'master' into JDK-8343685-VPointer-MemPointer
>>  - manual merge
>>  - fix printing
>>  - rename
>>  - fix up print
>>  - add TestEquivalentInvariants.java
>>  - improve documentation
>>  - hide parser via delegation
>>  - Merge branch 'master' into JDK-8343685-VPointer-MemPointer
>>  - ... and 106 more: https://git.openjdk.org/jdk/compare/84e6432b...b64f9295
>
> src/hotspot/share/opto/superword.cpp line 500:
> 
>> 498: 
>> 499: // We use two comparisons, because a subtraction could underflow.
>> 500: #define RETURN_CMP_VALUE_IF_NOT_EQUAL(a, b) \
> 
> Please use local static function instead of macro - you can't step through macros in debugger.

Hmm, that is tricky here:

// We use two comparisons, because a subtraction could underflow.
#define RETURN_CMP_VALUE_IF_NOT_EQUAL(a, b) \
  if (a < b) { return -1; }                 \
  if (a > b) { return  1; }

The macro only returns if we get a non-equal case. But it does not return when the values are equal.

This is helpful in code where there are repeated comparisons, and we want to only return once we have a non-equal.

Example:

  RETURN_CMP_VALUE_IF_NOT_EQUAL(a_con, b_con);

  RETURN_CMP_VALUE_IF_NOT_EQUAL(a->original_index(), b->original_index());

We first compare the `con`, but if they are the same we do not return but compare the `original_index`.

It simplifies the code. But I suppose I can refactor it now, especially because I use the macro less often. Before refactoring it was something like:

-// To be in the same group, two VPointers must be the same,
-// except for the offset.
-int VPointer::cmp_for_sort_by_group(const VPointer** p1, const VPointer** p2) {
-  const VPointer* a = *p1;
-  const VPointer* b = *p2;
-
-  RETURN_CMP_VALUE_IF_NOT_EQUAL(a->base()->_idx,     b->base()->_idx);
-  RETURN_CMP_VALUE_IF_NOT_EQUAL(a->mem()->Opcode(),  b->mem()->Opcode());
-  RETURN_CMP_VALUE_IF_NOT_EQUAL(a->scale_in_bytes(), b->scale_in_bytes());
-
-  int a_inva_idx = a->invar() == nullptr ? 0 : a->invar()->_idx;
-  int b_inva_idx = b->invar() == nullptr ? 0 : b->invar()->_idx;
-  RETURN_CMP_VALUE_IF_NOT_EQUAL(a_inva_idx,          b_inva_idx);
-
-  return 0; // equal
-}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21926#discussion_r1914376637