Request for reviews (XS): 7192964: assert(false) failed: bad AD file

[Krystal Mo]

On 08/22/2012 05:17 AM, Vladimir Kozlov wrote:
> Thank you, Kris
>
> I wish you become official Reviewer soon :)
>
Thanks. Me too :-) I'll be working towards that.

> Krystal Mo wrote:
>>
>> Hi Vladimir,
>>
>> Looks good to me. Some minor comments below:
>>
>> superword.cpp, line 1058,
>>
>> It wasn't immediately obvious to me why
>>
>> in_bb(p0->in(2))
>>
>> makes the shift's count a loop variant.
>> Let's see if my understanding is correct: It relies on the fact that 
>> SuperWord::_bb is the entire loop body and there's no control flow in 
>> it, so that, if a value is produced outside this basic block, it must 
>> be loop invariant.
>
> That is correct. We vectorize only unrolled "main" counted loops which 
> have only one exit at the end. So all data nodes between loop->head() 
> and loop->tail() (CountedLoopNode and CountedLoopEndNode nodes) which 
> have loop's head as associated control (PhaseIdealLoop::get_ctrl(n) == 
> loop->head()) are considered inside loop and could be vectorized. 
> in_bb() means "in basic block".
>
>>
>> I'm okay with the change as-is, but it'd be better is this kind of 
>> usage is factored out into a helper function with comments.
>
> I will leave renaming and factoring for later. I have 3 others vectors 
> bugs to fix today :)
>
Yes, I'm fine with that.

>>
>> P.S. (Not directly related to this change)
>> Reading through the code, there are two occurrences of a typo in 
>> vectornode.cpp, line 183 and 185, "invarient" -> invariant. Could 
>> that be fixed as well?
>
> Done.
>

Thanks,
Kris

> Thanks,
> Vladimir
>
>>
>> Regards,
>> Kris
>>
>> On 08/22/2012 02:29 AM, Vladimir Kozlov wrote:
>>> http://cr.openjdk.java.net/~kvn/7192964/webrev
>>>
>>> 7192964: assert(false) failed: bad AD file
>>>
>>> Shifts with loop variant counts "a[i]=1<<b[i];" should not be 
>>> vectorized since hw does not support it.
>>>
>>> Thanks,
>>> Vladimir
>>