[jdk8u-dev] RFR: 8303466: C2: failed: malformed control flow. Limit type made precise with MaxL/MinL

[Roman Marchenko]

On Mon, 1 Jul 2024 21:46:07 GMT, Francisco Ferrari Bihurriet <fferrari at openjdk.org> wrote:

>> hotspot/src/share/vm/opto/addnode.hpp line 295:
>> 
>>> 293:   virtual const Type* bottom_type() const { return TypeLong::LONG; }
>>> 294:   virtual uint ideal_reg() const { return Op_RegL; }
>>> 295:   virtual Node* Identity(PhaseGVN* phase);
>> 
>> Is it OK that `MaxLNode::Identity` has different signature than `AddNode::Identity`?
>> 
>>   virtual Node *Identity( PhaseTransform *phase );
>> 
>> Tha same question is for MinLNode and ConvI2LNode.
>
> Yes, as [JDK-8146629](https://bugs.openjdk.org/browse/JDK-8146629 "Make phase->is_IterGVN() accessible from Node::Identity and Node::Value") (openjdk/jdk11u at 69b52aa28be9054ea53bf9587ef7b563a658e510) doesn't have a 8u backport, it would have been neater to have `PhaseTransform* phase`.
> 
> What happened here is that neither the patch application nor the compilation failed, since `PhaseGVN` inherits from `PhaseValues`, which inherits from `PhaseTransform`.
> 
> However, there shouldn't be any issue, as the C++ type system is ensuring that all the callers are passing a `PhaseGVN` pointer.

Compilation didn't fail because not all the warnings are enabled.

Enabling `-Woverloaded-virtual` causes a number of warnings like the following:

hotspot/src/share/vm/opto/addnode.hpp:50:17: error: ‘virtual Node* AddNode::Identity(PhaseTransform*)’ was hidden [-Werror=overloaded-virtual]
   50 |   virtual Node *Identity( PhaseTransform *phase );
hotspot/src/share/vm/opto/addnode.hpp:312:17: error:   by ‘virtual Node* MinLNode::Identity(PhaseGVN*)’ [-Werror=overloaded-virtual]
  312 |   virtual Node* Identity(PhaseGVN* phase);

that may point to a serious bug.

In few words, `MaxLNode::Identity` may be never called in case:

AddNode *obj = new MaxLNode;
obj->Identity(...); // AddNode::Identity() is called.

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

PR Review Comment: https://git.openjdk.org/jdk8u-dev/pull/529#discussion_r1662147447