RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)
Civlin, Jan
jan.civlin at intel.com
Thu Oct 8 23:14:35 UTC 2015
Vladimir, Igor,
The webrev was uploaded to this location:
http://cr.openjdk.java.net/~mcberg/8136725/webrev.03/
-----Original Message-----
From: Civlin, Jan
Sent: Tuesday, October 06, 2015 1:44 AM
To: 'Vladimir Kozlov'
Cc: 'hotspot compiler'; Civlin, Jan
Subject: RE: RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)
Seems the attached file was stripped by firewall. Trying to send again.
-----Original Message-----
From: Civlin, Jan
Sent: Monday, October 5, 2015 6:54 PM
To: Vladimir Kozlov
Cc: hotspot compiler; Civlin, Jan
Subject: RE: RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)
Vladimir, thank you for the review.
Here is the updated patch.
I decided to stick to simple if(TraceLoopOpts) condition for checking in printing, since these messages are printed on error only.
Regards,
Jan.
-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
Sent: Monday, October 05, 2015 1:08 AM
To: Civlin, Jan
Cc: hotspot compiler
Subject: Re: RFR(M): 8136725 Provide utility for creation a counted loop reserve copy (clone)
remove .hgtags changes
Trace outputs in create_reserve() are not under trace flag. I would suggest to remove them or put under TraceLoopOpts && Verbose combination.
Also, please, split next and similar lines according to our code stile:
if (!_lp_reserved->is_CountedLoop()) return false;
---
if (!_lp_reserved->is_CountedLoop()) {
return false;
}
The rest looks fine.
Thanks,
Vladimir
On 10/1/15 4:31 AM, Igor Veresov wrote:
> Here’s the updated webrev:
>
> 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
>> <mailto: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
>>> <mailto:jan.civlin at intel.com>>
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~iveresov/loop-reverse/webrev-091515/
>>>
>>> igor
>>>
>> <webrev-r9100r9092.tar.bz2>
>
More information about the hotspot-compiler-dev
mailing list