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

[Vladimir Kozlov]

Thank you, Christian

Vladimir

Christian Thalinger wrote:
> I think this looks good.  -- Chris
> 
> On Apr 12, 2012, at 3:29 PM, Vladimir Kozlov wrote:
> 
>> 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
>>>>>>>
>