RFR: 8261914: IfNode::fold_compares_helper faces non-canonicalized bool when running JRuby JSON workload

[Aleksey Shipilev]

On Wed, 24 Feb 2021 15:22:26 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The assert fires because the current IfNode's condition was not
> canonicalized when it's being folded with a dominating
> IfNode. idealize_test() should have taken care of that because it
> executed before fold_compares() but it didn't because:
> 
>   Node* new_b = phase->transform( new BoolNode(b->in(1), bt.negate()) );
>   if( !new_b->is_Bool() ) return NULL;
> 
> caused it to bail out. new_b is a constant. This happens because of
> the order in which nodes are processed by IGVN. The If's current Bool
> would also constant fold but it's in the IGVN worklist and hasn't been
> processed yet.
> 
> The fix I propose is to keep Aleksey's defensive fix but to check that
> the Bool input is indeed about to be transformed by IGVN and that that
> would cause the IfNode to be reprocessed.
> 
> I tried to write a test case but didn't succeed. The 2 If nodes come
> from a tableswitch that's transformed into a series of If based on
> profile data. I couldn't reproduce the profile data with a simple test
> case.

Marked as reviewed by shade (Reviewer).

src/hotspot/share/opto/ifnode.cpp line 984:

> 982:       return false;
> 983:     }
> 984:     assert(this_bool->_test.is_less() && !fail->_con, "incorrect test");

This should lead with "this test was canonicalized" comment? Missed during the move, I think.

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

PR: https://git.openjdk.java.net/jdk/pull/2707