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

[Vladimir Kozlov]

Thank you, Kris

I wish you become official Reviewer soon :)

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 :)

> 
> 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,
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
>