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

[Vladimir Kozlov]

On Tue, 14 Jan 2025 08:08:58 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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);`

Yes, it is better. We see when we return from method clear now.

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

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