Request for reviews (S): 7160610: Unknown Native Code compilation issue

[Vladimir Kozlov]

http://cr.openjdk.java.net/~kvn/7160610/webrev

I updated webrev with added regression test and using BasicType for input's type 
verification. And I ran nsk, full CTW and compiler regression tests with 
-XX:-OptimizeFile and -XX:-TieredCompilation. They passed.

Thanks,
Vladimir

Vladimir Kozlov wrote:
> On 4/11/12 9:34 PM, Tom Rodriguez wrote:
>>
>> On Apr 11, 2012, at 9:15 PM, Vladimir Kozlov wrote:
>>>>> http://cr.openjdk.java.net/~kvn/7160610/webrev
>>>>>
>>>>> 7160610: Unknown Native Code compilation issue
>>>>>
>>>>> When constructing input vector use type of vector's operation which 
>>>>> use it since element's sizes should match. In the bug case byte 
>>>>> value is used for initialization of int[] array. The store vector 
>>>>> is Store2I and input's type (byte) was used to construct init 
>>>>> vector but there is no Replicate2B. Even if Replicate2B existed it 
>>>>> will be wrong since it initialize only lowest 2 bytes.
>>>>
>>>> Why does in_bb(opd) ever affect which type to use?
>>>
>>> If you ask why we had this condition, I don't know. I also think it 
>>> is bogus.
>>
>> Yes, that's what I'm asking.  Maybe it's some kind of shortcut check 
>> for whether it's part of the pack or not?
> 
> I think it was simple shortcut to get type. Nodes in loop has special 
> types velt_type() pre-calculated for them, like next (in container_type()):
> 
>   if (t->basic_type() == T_INT) {
>     if (t->higher_equal(TypeInt::BOOL))  return TypeInt::BOOL;
>     if (t->higher_equal(TypeInt::BYTE))  return TypeInt::BYTE;
> ...
> 
>> We probably need to run a bunch of code past this to make sure it 
>> doesn't break anything.  Don't we have a few regressions tests that 
>> exercise this pretty well?
> 
> I doubt we have such tests for vectors. Vectors were introduced eons ago 
> and we hit this bug only with 7u2. Also OptimizeFill optimization 
> reduced number of cases for vectorized loops. I start writing 
> TestVect.java in my big vectors changes to cover common cases but I 
> think we need to ask QE to write more tests for more coverage. 
> Especially for vector arithmetic.
> 
> It reminds me that I need to add the test from bug report to the 
> compiler regression tests.
> 
> I will run nsk, full CTW and compiler regression tests tomorrow.
> 
>>> I will change it to next to verify that all packed input's has the 
>>> same type (they should, since they come from loop unrolling/node 
>>> cloning):
>>>
>>>    DEBUG_ONLY( const Type* opd_t = opd->bottom_type();
>>>
>>>     assert(opd_t == in->bottom_type(), "all same type");
>>
>> That wouldn't necessarily be true would it?  It could be a more 
>> precise type but still the same BasicType couldn't it?
> 
> You are right, may be I should use just BasicType.
> 
> Vladimir
> 
>>
>> tom
>>
>>>
>>> Vladimir
>>>
>>>>
>>>> tom
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>
>>