Re: [Tinycc-devel] SSE calling convention bug

[Christian Jullien]

Sorry if I annoy you with details.

I just want to say that you added a pure C test that should work in any cases 
whether backend uses sse or not.
It appears that it was broken when sse is used (as different tests could also 
break on different architecture).
My own opinion, is that generic test names should not be named using a given 
backend.

As your test case is related to passing floats that why I suggest to move it to 
22_floating_point C generic code with comment that explains it uses to fail 
with sse. But something like 109_float_struct_calling also works for me.

It is curious to see 109_sse_calling on ARM 32/64. One wonder what this test 
could do on ARM?

I can also leave with '109_sse_calling'

C.



-----Original Message-----
From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=address@hidden] On 
Behalf Of Herman ten Brugge via Tinycc-devel
Sent: Tuesday, October 29, 2019 10:37
To: address@hidden
Cc: Herman ten Brugge
Subject: Re: [Tinycc-devel] SSE calling convention bug

Do you mean that I should call the testcase '109_calling.c' and add a 
comment in this file that this is for x86_64 sse calling?

This is why I called the file '109_sse_calling.c'.

The file name indicates that this is only solving a bug for x86_64.

     Herman



On 2019-10-29 08:04, Christian Jullien wrote:
> Than you for your recent patch on mod.
>
> I just have a little remark about ' 109_sse_calling' file name.
> This new test is executed also on Windows and arm32/64 (which is fine). 
> However, those backend don't use sse.
> IMHO, this test should not have sse in its name but a comment could explain 
> that the test revealed a bug when see is used as with Linux.
>
> Another solution is to add this new test in 22_floating_point + comment about 
> sse
>
> -----Original Message-----
> From: Tinycc-devel [mailto:tinycc-devel-bounces+eligis=address@hidden] On 
> Behalf Of Michael Matz
> Sent: Monday, October 28, 2019 10:32
> To: Herman ten Brugge via Tinycc-devel
> Cc: Herman ten Brugge
> Subject: *** SPAM *** Re: [Tinycc-devel] SSE calling convention bug
>
> Hi,
>
> On Sun, 27 Oct 2019, Herman ten Brugge via Tinycc-devel wrote:
>
>> Look like the code in x86_64-gen.c is wrong. It should read:
>>
>>                  if (sse_reg) { /* avoid redundant movaps %xmm0, %xmm0
>> */
>>                      /* movaps %xmm1, %xmmN */
>>                      o(0x280f);
>>                      o(0xc1 + ((sse_reg+1) << 3));
>>                      /* movaps %xmm0, %xmmN */
>>                      o(0x280f);
>>                      o(0xc0 + (sse_reg << 3));
>>                  }
> Yes.
>
>
> Ciao,
> Michael.
>
>> The problem occurs when sse_reg == 1. Because then xmm1 is overwritten
>> before it is copied.
>>
>>      Herman
>>
>> On 2019-10-27 07:32, Christian Jullien wrote:
>>> Trying your sample with mod on Windows -m64/-m32 I get:
>>>
>>> c: >tcc -m32 foo.c && foo
>>> 5.000000
>>>
>>> c:>tcc -m64 foo.c && foo
>>> 5.000000
>>>
>>> It only return 3.0000 on Linux x64 (I've not tested your code on Linux x86).
>>>
>>> C.
>>>
>>>
>>> -----Original Message-----
>>> From: Tinycc-devel
>>> [mailto:tinycc-devel-bounces+eligis=address@hidden]
>>> On Behalf Of Shachaf Ben-Kiki
>>> Sent: Saturday, October 26, 2019 12:59
>>> To: address@hidden
>>> Subject: [Tinycc-devel] SSE calling convention bug
>>>
>>> Hello,
>>>
>>> I ran into a bug in the SSE function call code in x86_64-gen.c. It's
>>> in the following lines, in gfunc_call:
>>>
>>>                   if (sse_reg) { /* avoid redundant movaps %xmm0, %xmm0 */
>>>                       /* movaps %xmm0, %xmmN */
>>>                       o(0x280f);
>>>                       o(0xc0 + (sse_reg << 3));
>>>                       /* movaps %xmm1, %xmmN */
>>>                       o(0x280f);
>>>                       o(0xc1 + ((sse_reg+1) << 3));
>>>                   }
>>>
>>> When sse_reg is %xmm1, this generates
>>>
>>>       0f 28 c8                movaps %xmm0,%xmm1
>>>       0f 28 d1                movaps %xmm1,%xmm2
>>>
>>> Such that the first mov overwrites xmm1 before the second mov uses it.
>>> Since the registers are used in reverse order and only one or two at
>>> a time, I think swapping the order of the movs should be sufficient
>>> to fix it.
>>>
>>> Here's a test case:
>>>
>>> #include <stdio.h>
>>>
>>> struct Point {
>>>     float x;
>>>     float y;
>>> };
>>>
>>> struct Rect {
>>>     struct Point top_left;
>>>     struct Point size;
>>> };
>>>
>>> float foo(struct Point p, struct Rect r) {
>>>     return r.size.x;
>>> }
>>>
>>> int main(int argc, char **argv) {
>>>     struct Point p = {1, 2};
>>>     struct Rect r = {{3, 4}, {5, 6}};
>>>     printf("%f\n", foo(p, r));
>>>     return 0;
>>> }
>>>
>>> This program should print 5, but it prints 3 in tcc.
>>>
>>> Is this the right place to post this? I can post it elsewhere, or
>>> send a patch (it took a while to track this down but I think the fix
>>> should be easy).
>>>
>>> Thanks,
>>>       Shachaf
>>>
>>> _______________________________________________
>>> Tinycc-devel mailing list
>>> address@hidden
>>> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>>>
>>>
>>
>> _______________________________________________
>> Tinycc-devel mailing list
>> address@hidden
>> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>>
>>


_______________________________________________
Tinycc-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/tinycc-devel