[15] RFR(M): 8237859: C2: Crash when loads float above range check

[Christian Hagedorn]

Hi Roland

On 20.03.20 11:41, Roland Westrelin wrote:
>> This fix addresses this issue and rewires the control input of data
>> nodes belonging to the main-loop to a range check predicate before the
>> pre-loop to the copied main-loop range check predicates (copied in
>> PhaseIdealLoop::copy_skeleton_predicates_to_main_loop_helper). When
>> unrolling, the range check predicates of the main-loop are updated
>> together with the control edges (done in
>> PhaseIdealLoop::update_main_loop_skeleton_predicates). The data nodes
>> belonging to the post-loop are rewired to the zero-trip guard If node
>> right before the CountedLoopNode (there are no predicates copied down to
>> the post-loop). For example, in [5] (in contrast to [4]), we rewired the
>> control of 247 LoadP to the main-loop and the control of 377 LoadP to
>> the post-loop.
> 
> That doesn't feel right to me. The predicates between the main and pre
> loop were added to avoid inconsistencies during the compilation
> process. And unless the condition constant folds during loop opts, they
> are removed and not included in the final compiled code. But here you
> are using them to fix a code generation issue. So you might end up with
> loads made dependent on a predicate that's removed and then the loads
> would become dependent on the zero trip guard above the main loop.
> 
> Why node make the loads in the main loop dependent on the zero trip
> guard right before the main loop the way you do it for the post loop?

You're right. I somehow wrote that code with the thought that those 
cloned skeleton predicates can fail - but they never do. Therefore, your 
suggestion to rewire the control inputs for those main loop data nodes 
to the zero trip guard before the main loop sounds reasonable.

Here are the updated webrevs with this change:
Full (after push of 8240227):
http://cr.openjdk.java.net/~chagedorn/8237859/webrev.02/
Incr from webrev.incremental.00:
http://cr.openjdk.java.net/~chagedorn/8237859/webrev.incremental.01/

Best regards,
Christian