RFR: 8327963: [Umbrella] Incorrect result of C2 compiled code since JDK-8237581 [v2]

[Emanuel Peter]

On Wed, 23 Apr 2025 12:13:08 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   TestIterativeEA fix
>
> src/hotspot/share/opto/escape.cpp line 4878:
> 
>> 4876:       if (mem->Opcode() == Op_NarrowMemProj) {
>> 4877:         const TypePtr* at = mem->adr_type();
>> 4878:         uint idx = (uint) _compile->get_alias_index(at->is_ptr());
> 
> Suggestion:
> 
>         uint alias_idx = (uint) _compile->get_alias_index(at->is_ptr());
> 
> for consistency with the other code

Well, actually it is already being used inconsistently... hmm. More expressive may still be better. Up to you.

> src/hotspot/share/opto/multnode.hpp line 117:
> 
>> 115:     return ProjNode::hash() + _adr_type->hash();
>> 116:   }
>> 117:   virtual bool cmp(const Node &n) const {
> 
> Suggestion:
> 
>   virtual bool cmp(const Node& n) const {

Subjective, up to you

> src/hotspot/share/opto/multnode.hpp line 135:
> 
>> 133:   }
>> 134:   virtual int Opcode() const;
>> 135: };
> 
> Would it make sense to have these overridden? Just so you can print the `_adr_type` :)
> 
> #ifndef PRODUCT
>   virtual void dump_spec(outputStream *st) const;
>   virtual void dump_compact_spec(outputStream *st) const;
> #endif

Ah, or does it already get printed from the `adr_type()` i.e. the virtual method?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24570#discussion_r2055915472
PR Review Comment: https://git.openjdk.org/jdk/pull/24570#discussion_r2055935253
PR Review Comment: https://git.openjdk.org/jdk/pull/24570#discussion_r2055936793