This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch ARM] big-endian support for Neon vext tests


On 17 September 2012 17:21, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 17/09/12 16:13, Christophe Lyon wrote:
>> On 17 September 2012 14:56, Richard Earnshaw <rearnsha@arm.com> wrote:
>>> On 05/09/12 23:14, Christophe Lyon wrote:
>>>> Hello,
>>>>
>>>> Although the recent optimization I have committed to use Neon vext
>>>> instruction for suitable builtin_shuffle calls does not support
>>>> big-endian yet, I have written a patch to the existing testcases such
>>>> they now support big-endian mode.
>>>>
>>>> I think it's worth improving these tests since writing the right masks
>>>> for big-endian (such that the program computes the same results as in
>>>> little-endian) is not always straightforward.
>>>>
>>>> In particular:
>>>> * I have added some comments in a few tests were it took me a while to
>>>> find the right mask.
>>>> * In the case of the test which is executed, I had to force the
>>>> noinline attribute on the helper functions, otherwise the computed
>>>> results are wrong in big-endian. It is probably an overkill workaround
>>>> but it works :-)
>>>>   I am going to file a bugzilla for this problem.
>>>>
>>>> I have checked that replacing calls to builtin_shuffle by the expected
>>>> Neon vext variant produces the expected results in big-endian mode,
>>>> and I arranged the big-endian masks to get the same results.
>>>>
>>>> Christophe.=
>>>>
>>>>
>>>> neon-vext-big-endian-tests.patch
>>>>
>>>>
>>>> N ÂnârÂÂÃÃ)emÃhÃyhià Âw^âÂÃ
>>>>
>>>
>>>
>>> I'm not sure about this.  Looking at the documentation in the manual for
>>> builtin_suffle makes no mention of the results/behaviour being endian
>>> dependent, which makes me wonder why this test needs to be.
>>>
>>> R.
>>
>>
>> Indeed, but I had to modify the mask value in order to get the same
>> results in big and little-endian.
>>
>> If the mask should be the same (it would be much more confortable for
>> the developers indeed), then GCC needs to be changed/fixed.
>>
>
> That's what I'm trying to establish.  I suspect that there is a bug in
> GCC for all big-endian code here.
>
> What happens for a test of uint8x8_t?
>

Well, in my sample testcase in little-endian, I used mask = {2, 3, 4,
5, 6, 7, 8, 9}, which can be optimized into vext #2.

In big-endian mode, explicitly forcing use of vext #2 leads to the
right result, but to achieve it using builtin_shuffle, I had to change
the mask into {14, 15, 0, 1, 2, 3, 4, 5}.

I did read the thread starting at
http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01133.html and the
threads it references, and I must admit that I got a bit confused :-)

IMHO, it's currently impossible for a GCC user to write code using
vector initializers that would be portable on big and little endian
targets. It's too much of a headache....

It was also a purpose of this patch: have someone react if it looked
inappropriate.

Thanks for the review,

Christophe.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]