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

[Emanuel Peter]

On Tue, 14 Jan 2025 07:44:27 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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
> -}

Ok, this is my solution now:

+
+    // We use two comparisons, because a subtraction could underflow.
+    template <typename T>
+    static int cmp_code(T a, T b) {
+      if (a < b) { return -1; }
+      if (a > b) { return  1; }
+      return 0;
+    }


And use it like this:

+  int c_con = cmp_code(a_con, b_con);
+  if (c_con != 0) { return c_con; }

Instead of
`RETURN_CMP_VALUE_IF_NOT_EQUAL(a_con, b_con);`

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

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