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

[Tom Rodriguez]

On Apr 11, 2012, at 9:15 PM, Vladimir Kozlov wrote:

> On 4/11/12 8:49 PM, Tom Rodriguez wrote:
>> 
>> On Apr 11, 2012, at 6:52 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?

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?

> 
> If you are asking about next line in my fix I will change it. After sending webrev I realized that the assert which use it is useless for the reason you asked:
> 
>   DEBUG_ONLY( const Type* opd_t = velt_type(!in_bb(opd) ? p0 : opd); )
> 
>    assert(opd_t == velt_type(!in_bb(in) ? pi : in), "all same type");

I assumed you were just replicating the existing logic.

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

tom

> 
> Vladimir
> 
>> 
>> tom
>> 
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>