RFR(L): 8186027: C2: loop strip mining
vladimir.kozlov at oracle.com
Mon Nov 6 20:55:01 UTC 2017
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
> Ok. That sounds like it could clean up the code a bit. Do you want me to
> look into that?
>> 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.
More information about the hotspot-compiler-dev