Re: [Tinycc-devel] SSE calling convention bug

[Herman ten Brugge]

Done,

    Herman

On 2019-10-29 10:57, Christian Jullien wrote:
> 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