Request for reviews (S): 7042327: assert(opaq->outcnt() == 1 && opaq->in(1) == limit)

[Vladimir Kozlov]

Thank you, Tom

I ran into small problem with last fix during Match because one IfNode has Con 
as condition but was not optimized out. I added the missing push to IGVN 
worklist in adjust_check() and updated webrev. It happened because Opaque2 node 
I added prevented constant propagation early. So I want to add Identity() to 
Opaque2 node but would like to get your opinion before I do this. Opaque2 node 
was originally (before this fix) used only for guarding loop's trip-counter. But 
with my change the limit expression could be transformed into constant or final 
value of trip-count could be transformed to constant if loop was empty. So I 
think it is safe to do this Identity transformation since it is not trip-count 
any more. What do you think?

+Node *Opaque2Node::Identity( PhaseTransform *phase ) {
+  return (in(1)->is_Con()) ? in(1) : this;
+}

Thanks,
Vladimir

Tom Rodriguez wrote:
> Looks good.
> 
> tom
> 
> On May 5, 2011, at 5:30 PM, Vladimir Kozlov wrote:
> 
>> Thank you, Tom
>>
>> Thinking about what you said and, yes, such situation could happen. To be safe I need always put Opaque2 for limit for unrolled loop. I updated webrev.
>>
>> Thanks,
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> It seems ok.  Can't this situation occur as a result of other optimizations?  If limit is behind by some expressions that later gets optimized away it seems like the same configuration could happen.  Or isn't that possible?
>>> tom
>>> On May 5, 2011, at 4:03 PM, Vladimir Kozlov wrote:
>>>> http://cr.openjdk.java.net/~kvn/7042327/webrev
>>>>
>>>> Fixed 7042327: assert(opaq->outcnt() == 1 && opaq->in(1) == limit)
>>>>
>>>> New loop unrolling code calculate new_limit = limit-stride. If limit is
>>>> trip-counter (phi+stride) with the same stride from a previous loop then new_limit will be optimized to pre-incremented value: new_limit = phi.
>>>> reorg_offsets() optimization will create a separate Opaque2 node for each use of trip-counter (phi) and as result zero trip guard limit will be different from loop limit and it causes the assert to fail.
>>>>
>>>> Separate limit by Opaque2 node when calculating new limit for unroll if limit is an incremented variable from previous loop to avoid using pre-incremented value and reduce register pressure.
>>>> I also removed code which creates dead loops since limit is input to new_limit.
>>>>
>>>> Tested with failed case and CTW.
>