RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)

[Igor Veresov]

Here’s the updated webrev:

http://cr.openjdk.java.net/~iveresov/loop-reverse/webrev.02/ <http://cr.openjdk.java.net/~iveresov/loop-reverse/webrev.02/>

igor

> On Sep 28, 2015, at 2:25 AM, Civlin, Jan <jan.civlin at intel.com> wrote:
> 
> Vladimir,
> 
> Thank you for the review.
> I'm attaching the patch reflecting your comments.
> 
> Regards,
> 
> Jan
> 
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
> Sent: Wednesday, September 23, 2015 3:38 AM
> To: Civlin, Jan; hotspot compiler
> Subject: Re: RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)
> 
> in loopUnswitch.cpp debug output should be under nonproduct flag (could be TraceLoopOpts or separate new flag). Use #ifndef PRODUCT around print code instead of repeated NOT_PRODUCT() macro. And I don't think you need all output you added in final version.
> 
> All constants are IGVNed so you don't need to pre-generate const_0. 
> Since it is not connected to graph it could be eliminated.
> And I don't think you need next for the same reason:
> 
>  311   lk->set_const_0(const_0);
>  312   lk->set_const_1(const_1);
> 
> In ~CountedLoopReserveKit() and switch_to_reserved() you can make new
> const_0 since you have pointer to IGVN:
> 
> _phase->_igvn.intcon(0);
> 
> You can't use subsume() since it will replace all users of const_1:
> 
> +    //TODO bug? could not use here _const_1->subsume_by(_const_0,
> _phase->C);
> 
> It is correct to use set_req() for constants here.
> 
> superword.cpp I don't think you need to keep _CountedLoopReserveKit_test 
> code in final version after you verified it works during development:
> 
> +  //!///    Testing that switching to reserved copy works
> 
> Thanks,
> Vladimir
> 
> On 9/19/15 1:05 AM, Civlin, Jan wrote:
>> As I was working on modification of SuperWord::output - and this is a function where you modify the ideal graph on the fly and cannot exit on error - I was wondering how can I protect my algorithm in a case some wrong decision was made on the previous stage of the SuperWord but discovered only at the final stage, when the graph is already half modified.
>> 
>> So here is my solution:
>> - the graph to be modified is cloned.
>> - five nodes are added: intcon(0) is disconnected, intcon(1)->If-> IfTrue->orig_loop: IfFalse->clone_loop.
>> 
>> Then any optimization is executed on the original loop, and if it finishes ok, nothing else need to be done, the clone_loop will be removed as dead in the consecutive stages of compiler.
>> At any moment, if an error occurs, the node intcon(1) may be subsumed by node intcon(0), therefore  the clone_loop becomes "active" and the original "unfinished in modification" loop will be removed later.
>> 
>> This utility is implemented in class CountedLoopReserveKit.
>> The loop cloning and 5 nodes adding is realized in the ctor and possible subsuming intcon(1) by intcon(0) in dtor, so a simple return from the modifying graph function will do graph correction (switching in choice) of the graph.
>> Basically, it is sufficient the create a local object of CountedLoopReserveKit class, the scoped dtor makes the choice: modified or original loop.
>> 
>> SuperWord::output in this submission is included only to illustrate how to use the CountedLoopReserveKit.
>> The actual new SuperWord::output where CountedLoopReserveKit  is indeed substantial is coming in the next patch.
>> 
>> This reserve loop cloning in SuperWord may be disabled by the global flag DoReserveCopyInSuperWord.
>> 
>> -----Original Message-----
>> From: Igor Veresov [mailto:igor.veresov at oracle.com]
>> Sent: Thursday, September 17, 2015 2:31 PM
>> To: hotspot compiler
>> Cc: Civlin, Jan
>> Subject: RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)
>> 
>> Provide utility for creation a counted loop reserve copy (clone).
>> May be used in any graph optimization for simple roll back to the original loop, in partially will be used in SuperWord, where the loop modification goes on-the-fly and potentially may not finish correctly (the patch for SuperWord is coming soon).
>> 
>> This is contributed by Jan Civlin <jan.civlin at intel.com>
>> 
>> Webrev: http://cr.openjdk.java.net/~iveresov/loop-reverse/webrev-091515/
>> 
>> igor
>> 
> <webrev-r9100r9092.tar.bz2>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150930/5cd93d9d/attachment.html>