RFR(L): 8186027: C2: loop strip mining

[Vladimir Kozlov]

On 10/30/17 10:02 AM, Roland Westrelin wrote:
> 
> Hi Vladimir,
> 
>> Should we just make _loop_flags field type uint (32-bit) since we hit 16-bit limit?
> 
> We don't hit the limit with this change. I have some other changes for
> which I had to change _loop_flags to uint. That's where the int -> uint
> tweaks are coming from. I can remove them if you like as they are not
> required. Sorry for the confusion.

Please, remove them. Changes are big already.

> 
>> There is confusion (because you did not have enough bits?) about which loops are marked as
>> strip_mined. I thought it is only inner loop but it looks like out (skeleton) loop also marked as
>> such. I would suggest to mark them differently.
> 
> The way it works currently is:
> 
> Opcode() == Op_Loop && is_strip_mined() => outer loop
> Opcode() == Op_CountedLoop && is_strip_mined() => inner loop

Yes, I got it after 3rd read-through. It is not very obvious.

> 
> The outer loop can't be transformed to a counted loop so that scheme
> shouldn't break.

Yes, it is correct code but it is hard to follow.

> 
>> I was thinking may be we should create new Loop node subclass for outer loop. Then you don't need
>> special flag for it and it will be obvious what they are in Ideal Graph. The same for outer loop end
>> node.
> 
> Ok. That sounds like it could clean up the code a bit. Do you want me to
> look into that?

Yes, please.

> 
>> src/hotspot/share/opto/superword.cpp
>>
>> Where next change come from?
>>
>> +      if (t2->Opcode() == Op_AddI && t2 == _lp->as_CountedLoop()->incr()) continue; // don't mess
>> with the iv
> 
> I saw a few cases where t2 is the increment of the CountedLoop
> iv. SuperWord::opnd_positions_match() then swaps the edges of the AddI
> and later CountedLoopEndNode::phi() fails because the edges of the iv's
> AddI are not in the expected order anymore.

Good. But why you need to check Opcode() too? In Counted loops it should be Int type.

Thanks,
Vladimir

> 
> Roland.
>