Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-31 Thread Herman ten Brugge via Tinycc-devel

On 7/31/23 12:43, grischka wrote:

On 31.07.2023 13:42, Herman ten Brugge via Tinycc-devel wrote:

I agree with your comments above. The size is incorrect.
I could change gfunc_sret in x86_64-gen.c and then calculate the size 
in tccgen.c
as you suggested. But I am not sure regsize is set correctly all the 
time.


I like this better:
--- a/tccgen.c
+++ b/tccgen.c
@@ -6142,7 +6142,7 @@ special_math_val:
 space.  Assume register size is power of 2. */
  if (regsize > align)
    align = regsize;
-   loc &= -align;
+   size = (size + regsize - 1) & -regsize;
  loc = (loc - size) & -align;
  addr = loc;
  offset = 0;

The size should be a multiple of regsize.
What is your opinion?


Hi,

yes, that seems to make sense.  Then again I think that align should
additionally depend on just 'ret_align', as in

  if (ret_align > align)
   align = ret_align;


I was working on a better fix for riscv and reverted the patch from 
Yao Zi by accident.

I will reapply that change.


Well, as I was trying to say, that change wasn't a good one either.

Anyway, I just applied some more extensive fix for both.  Hope it does 
work

now.  Also yet another pragma_once_normalize patch (which you may or may
not want to comment ;)


I tested all changes and all work on multiple targets. Great work.
I have no comments on the pragma once patch. Sorry 

    Herman


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


Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-31 Thread grischka

On 31.07.2023 13:42, Herman ten Brugge via Tinycc-devel wrote:

I agree with your comments above. The size is incorrect.
I could change gfunc_sret in x86_64-gen.c and then calculate the size in 
tccgen.c
as you suggested. But I am not sure regsize is set correctly all the time.

I like this better:
--- a/tccgen.c
+++ b/tccgen.c
@@ -6142,7 +6142,7 @@ special_math_val:
 space.  Assume register size is power of 2. */
  if (regsize > align)
align = regsize;
-   loc &= -align;
+   size = (size + regsize - 1) & -regsize;
  loc = (loc - size) & -align;
  addr = loc;
  offset = 0;

The size should be a multiple of regsize.
What is your opinion?


Hi,

yes, that seems to make sense.  Then again I think that align should
additionally depend on just 'ret_align', as in

  if (ret_align > align)
   align = ret_align;



I was working on a better fix for riscv and reverted the patch from Yao Zi by 
accident.
I will reapply that change.


Well, as I was trying to say, that change wasn't a good one either.

Anyway, I just applied some more extensive fix for both.  Hope it does work
now.  Also yet another pragma_once_normalize patch (which you may or may
not want to comment ;)

-- gr



 Herman



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


Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-31 Thread Herman ten Brugge via Tinycc-devel

On 7/31/23 10:08, grischka wrote:

On 31.07.2023 09:32, Herman ten Brugge via Tinycc-devel wrote:

On 7/30/23 18:27, Vincent Lefevre wrote:

On 2023-07-30 16:07:29 +0600, Viktor M wrote:

Hello everyone. So today I stumbled upon this bug when doing math
involving conversions between float and double. A minimal example:

[...]

Not really minimal. I could simplify it even further:

I fixed this on mob.
The problem was that stack was overwritten because stack was not 
aligned correctly.


Hi Herman,

hope you don't mind some critic:  At first,  if we want to reserve
some space of 'size' on stack, while respecting alignment of 'align',
then there is nothing wrong with this line:

   loc = (loc - size) & -align;

As such your addition

   loc &= -align;
   loc = (loc - size) & -align;

does not make sense, even if it may fix the problem.

So, what is the problem about really?  The problem is that

   sizeof (struct V { int x, y, z; })

is 12, but when returned in registers (on x86_64-linux), the size of
two registers is 2*8 = 16.

Therefor the problem is not wrong alignment, it is wrong size.

Digging further, it turns out that gfunc_sret() on x86_64-linux for
    struct V { int x, y, z; }
returns: one register of type VT_QLONG with regsize 8.

That does not look right either.  In fact, tcc handles VT_QLONG
as sort of pseudo register, using two processor registers (vtop->r/r2)
so I'd think that for VT_QLONG, it should pass 16 as the 'regsize'.

In the end, it seems that the space to be reserved on stack should be
calculated like this

   size = ret_nregs * regsize;

rather than with 'size = type_size()'

Btw note that the other part or your patch

-    || (align & (ret_align-1))) {
+    && (align & (ret_align-1))) {

exactly undoes a previous patch from Yao Zi

-    && (align & (ret_align-1))) {
+    || (align & (ret_align-1))) {

As I tried to point out in an earlier email, this previous patch was
not the correct fix for the other problem, either.

That's why I think that our patches must strive for two things always:
1) to fix the problem and 2) in a way that logically does make sense ;)


I agree with your comments above. The size is incorrect.
I could change gfunc_sret in x86_64-gen.c and then calculate the size in 
tccgen.c

as you suggested. But I am not sure regsize is set correctly all the time.

I like this better:
--- a/tccgen.c
+++ b/tccgen.c
@@ -6142,7 +6142,7 @@ special_math_val:
    space.  Assume register size is power of 2. */
 if (regsize > align)
   align = regsize;
-   loc &= -align;
+   size = (size + regsize - 1) & -regsize;
 loc = (loc - size) & -align;
 addr = loc;
 offset = 0;

The size should be a multiple of regsize.
What is your opinion?

I was working on a better fix for riscv and reverted the patch from Yao 
Zi by accident.

I will reapply that change.

    Herman



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


Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-31 Thread Herman ten Brugge via Tinycc-devel

On 7/30/23 18:27, Vincent Lefevre wrote:

On 2023-07-30 16:07:29 +0600, Viktor M wrote:

Hello everyone. So today I stumbled upon this bug when doing math
involving conversions between float and double. A minimal example:

[...]

Not really minimal. I could simplify it even further:

I fixed this on mob.
The problem was that stack was overwritten because stack was not aligned 
correctly.


    Herman


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


Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-30 Thread Vincent Lefevre
On 2023-07-30 16:07:29 +0600, Viktor M wrote:
> Hello everyone. So today I stumbled upon this bug when doing math
> involving conversions between float and double. A minimal example:
[...]

Not really minimal. I could simplify it even further:

#include 

struct V {
  int x, y, z;
};

struct V vec(void)
{
  return (struct V) { 0, 0, 1 };
}

void func(float f, struct V v)
{
  printf("%f\n", f);
}

int main(void)
{
  float d = 5.0f;
  func(d, vec());
  return 0;
}

I get successively on x86_64:

-213.969727
-0.01
-0.00
44308718004014403453773152256.00

Putting the 5.0f in the argument instead of d, or reducing the
struct size, or changing float to double makes the bug disappear.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

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


Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-30 Thread C.J. Wagenius
Sorry. Ignore that.



30 juli 2023 16:18 av c...@voidptr.se:

> Use "%lf" for doubles.
>
> /cjw
>
> 30 juli 2023 15:34 av dayllen...@gmail.com:
>
>> Hello everyone. So today I stumbled upon this bug when doing math
>> involving conversions between float and double. A minimal example:
>>
>> ---
>> #include 
>>
>> struct V {
>>  int x, y, z;
>> };
>>
>> struct V vec(void)
>> {
>>  return (struct V) { 0, 0, 1 };
>> }
>>
>> void func(float f, struct V v)
>> {
>>  printf("%f %d %d %d\n", f, v.x, v.y, v.z);
>> }
>>
>> int main(void)
>> {
>>  float f = 5;
>>  double d = f;
>>  func(d, vec());
>>  return 0;
>> }
>> ---
>>
>> The code should print this:
>> 5.00 0 0 1
>>
>> but it prints garbage in the first value like this:
>> -85964.625000 0 0 1
>>
>> I've tried Linux x86_64 and both mob branch and 0.9.27 release.
>> tcc -run also fails.
>>
>> I hope there is an easy fix for this. Thanks.
>>
>> ___
>> Tinycc-devel mailing list
>> Tinycc-devel@nongnu.org
>> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>>
>
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>

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


Re: [Tinycc-devel] A double-to-float conversion bug

2023-07-30 Thread C.J. Wagenius
Use "%lf" for doubles.

/cjw

30 juli 2023 15:34 av dayllen...@gmail.com:

> Hello everyone. So today I stumbled upon this bug when doing math
> involving conversions between float and double. A minimal example:
>
> ---
> #include 
>
> struct V {
>  int x, y, z;
> };
>
> struct V vec(void)
> {
>  return (struct V) { 0, 0, 1 };
> }
>
> void func(float f, struct V v)
> {
>  printf("%f %d %d %d\n", f, v.x, v.y, v.z);
> }
>
> int main(void)
> {
>  float f = 5;
>  double d = f;
>  func(d, vec());
>  return 0;
> }
> ---
>
> The code should print this:
> 5.00 0 0 1
>
> but it prints garbage in the first value like this:
> -85964.625000 0 0 1
>
> I've tried Linux x86_64 and both mob branch and 0.9.27 release.
> tcc -run also fails.
>
> I hope there is an easy fix for this. Thanks.
>
> ___
> Tinycc-devel mailing list
> Tinycc-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/tinycc-devel
>

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


[Tinycc-devel] A double-to-float conversion bug

2023-07-30 Thread Viktor M
Hello everyone. So today I stumbled upon this bug when doing math
involving conversions between float and double. A minimal example:

---
#include 

struct V {
int x, y, z;
};

struct V vec(void)
{
return (struct V) { 0, 0, 1 };
}

void func(float f, struct V v)
{
printf("%f %d %d %d\n", f, v.x, v.y, v.z);
}

int main(void)
{
float f = 5;
double d = f;
func(d, vec());
return 0;
}
---

The code should print this:
5.00 0 0 1

but it prints garbage in the first value like this:
-85964.625000 0 0 1

I've tried Linux x86_64 and both mob branch and 0.9.27 release.
tcc -run also fails.

I hope there is an easy fix for this. Thanks.

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