Re: [Qemu-devel] [PATCH 01/20] target-mips: add MSA defines and data structure

[Leon Alrae]

On 24/10/2014 10:35, Yongbok Kim wrote:
> Hi,
> 
> On 22/10/2014 12:35, James Hogan wrote:
>> +union wr_t {
>> +    int8_t  b[MSA_WRLEN/8];
>> +    int16_t h[MSA_WRLEN/16];
>> +    int32_t w[MSA_WRLEN/32];
>> +    int64_t d[MSA_WRLEN/64];
>> This is incorrect on a big endian host. The least significant bits of
>> the lowest indexed element should always alias.
>>
>> With a compiler for little endian this will work fine since b[0] will
>> alias the least significant bits of h[0], w[0], and d[0], whereas with a
>> compiler for big endian, b[0] will alias the upper byte of h[0], w[0],
>> and d[0].
> 
> Yes it wouldn't work for a big endian host.
> However this MSA feature has been fully verified for big and little
> endian targets on a little endian host.
> I can see that the dsp_helper.c has similar problem as well.
> MSA could be forcibly turned off in a big endian host or might be leaved
> as it is just like DSP.
> If we need to implement for big endian host it would take longer time...
> 
> What do you guys think about that?

I don't think it would be reasonable to reject these patches only
because of big-endian host limitation as these patches contain huge
amount of work already (moreover, MSA is quite isolated). However, this
version of patchset has other issues pointed out by me and James in
previous emails - they need to be fixed / cleaned up.

Thanks,
Leon