RFR: 8371603: C2: assert(_inputs.at(alias_idx) == nullptr || _inputs.at(alias_idx) == load->in(1)) failed

[Emanuel Peter]

On Thu, 4 Dec 2025 10:25:03 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> I see your concern. Make sense to me. Thanks! I'd like keep current implementation of `Matcher::vector_needs_partial_operations` and `gen_masked_vector` because as we discussed in the previous PR that this sounds more reasonable. 
>> 
>> I will update the caller code here to check `nullptr` in addition although it won't generate a `nullptr` now. Code may look like:
>> 
>> if (Matcher::vector_needs_partial_operations(this, vt)) {
>>     Node* n = VectorNode::gen_masked_vector(phase, this, vt);
>>     if (n != nullptr) {
>>       return n;
>>     }
>>  }
>> return ...
>
> Is it better that we add an assertion of non `nullptr` value before returning in `gen_masked_vector` , consider this might make the caller code clean?

If you do the assert inside the method, then later someone may just do `return nullptr` somewhere, and your assert won't catch it, right?

I would just do this:

if (Matcher::vector_needs_partial_operations(this, vt)) {
    Node* n = VectorNode::gen_masked_vector(phase, this, vt);
    if (n != nullptr) { return n; }
 }

Or you could even combine the methods `vector_needs_partial_operations` and `gen_masked_vector` into some `Ideal_partial_operations`:

Node* progress = VectorNode::Ideal_partial_operations(phase, vt, this);
if (progress != nullptr) { return progress; }

That would remove the most clutter from the caller method.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28651#discussion_r2588819337