Re: [Tinycc-devel] Request for "fix float to u64 intrinsics"

[Kyryl Melekhin]

Hello.

I reverted the tests because the same result doesn't apply on every
platform, but as Herman sent me the latest tests results earlier the
patch did not seem to affect anything except x86_64 which is the
intended target.
That's why I'll let the maintainer decide whether he wants to keep
changes or not. I'll fix the formatting later on if first choice, so
that I don't have to revert 2 commits in the latter case.

Thank you,
Kyryl

чт, 17 сент. 2020 г. в 07:04, Herman ten Brugge <hermantenbrugge@home.nl>:
>
> Hello,
>
> Would it be possible to revert:
> fix float to u64 intrinsics
> and:
> add tests for float conversions to u64
>
> until you have a proper fix.
> The reason is that testing new code is much more difficult because test 22 
> fails for most targets.
>
> Regards,
>
>     Herman
>
> On 2020-09-12 17:36, Herman ten Brugge wrote:
>
> With 'fix float to u64 intrinsics' removed (original libtcc1.c, tccgen.c, new 
> 22_floating_point.*) the result is:
>
> x86_64 (linux + macos):
> --- 22_floating_point.expect    2020-09-11 19:25:02.125152055 +0200
> +++ 22_floating_point.output    2020-09-12 17:18:13.870481385 +0200
> @@ -17,6 +17,6 @@
>  3421
>  7855
>  2469
> -18446744073709548195
> -18446744073709543761
> -18446744073709549147
> +3421
> +7855
> +2469
>
> x86_64 (windows), i386 (linux + windows):
> --- 22_floating_point.expect    2020-09-11 19:25:04.775174826 +0200
> +++ 22_floating_point.output    2020-09-12 17:18:49.234749027 +0200
> @@ -17,6 +17,6 @@
>  3421
>  7855
>  2469
> -18446744073709548195
> -18446744073709543761
> -18446744073709549147
> +4294963875
> +4294959441
> +4294964827
>
> arm, arm64, riscv64:
> --- 22_floating_point.expect    2020-09-11 19:49:40.034917990 +0200
> +++ 22_floating_point.output    2020-09-12 17:22:27.576366194 +0200
> @@ -17,6 +17,6 @@
>  3421
>  7855
>  2469
> -18446744073709548195
> -18446744073709543761
> -18446744073709549147
> +0
> +0
> +0
>
> The arm64 and risc code do not use libtcc1.c and arm does not use 
> __fixunsxfdi.
> The test code should probably use 'long long' to fix some 3 testcases.
> But the arm and risc can not be fixed in this way.
> The arm and risc code look more correct then the i386/x86_64 code to me.
> But it is still undefined behavior.
>
> Regards,
>
>     Herman
>
>
> On 2020-09-12 10:38, Kyryl Melekhin wrote:
>
> Hi Herman,
>
> I've seen that tests fail on i386 and arm,riscv. Thing is, I'm not
> even sure if it's relevant to test this way since we are dealing with
> undefined behavior, so maybe it's not right to expect the same output
> everywhere. Could you run this test again but before the patch? I want
> to see how it behaves on arm. I wanted to test this on my own, on my
> other arm computer but I have some difficulty. I'll get it working
> soon or later but it will take some time.
>
> Thanks.
>
> сб, 12 сент. 2020 г. в 05:52, Herman ten Brugge <hermantenbrugge@home.nl>:
>
> I sent this in a private mail to Kyryl but it probably should also be sent to 
> the list.
>
> After 'add tests for float conversions to u64' I get the following output:
>
> x86_64 (linux + macos): ok
>
> x86_64 (windows), i386 (linux + windows):
> --- 22_floating_point.expect    2020-09-11 19:25:04.775174826 +0200
> +++ 22_floating_point.output    2020-09-11 20:54:23.583325674 +0200
> @@ -17,6 +17,6 @@
>  3421
>  7855
>  2469
> -18446744073709548195
> -18446744073709543761
> -18446744073709549147
> +4294963875
> +4294959441
> +4294964827
>
> arm, arm64, riscv64:
> --- 22_floating_point.expect    2020-09-11 19:49:40.034917990 +0200
> +++ 22_floating_point.output    2020-09-11 20:58:33.768829655 +0200
> @@ -17,6 +17,6 @@
>  3421
>  7855
>  2469
> -18446744073709548195
> -18446744073709543761
> -18446744073709549147
> +0
> +0
> +0
>
> The test code only works 2 targets. The other 6 are broken now.
>
> Regards,
>
>     Herman
>
> On 2020-09-11 13:11, Kyryl Melekhin wrote:
>
> If this is undefined behavior, then this line is straight up also UB.
> https://repo.or.cz/tinycc.git/blobdiff/55f8963dfab5c543f7f34589d3ef9d3f2da3db14..310e3b428cfd181b51723276e6563b90d670da06:/tccgen.c
> And because that line is UB, then it causes even more bugs down the drain.
> At least if the output for this UB matches what other compilers generate
> some bugs can be eliminated. But what gets me the most is why doesn't
> gcc or clang compiler warn on any of this. Also how come we can't
> represent a 32 bit
> float in a 64 bit number? On x86_64 we sure can. Then you are citing ISO 17
> while we don't even have full support for C11.
>
> пт, 11 сент. 2020 г. в 14:43, Vincent Lefevre <vincent@vinc17.net>:
>
> On 2020-09-11 10:32:06 +0000, Kyryl Melekhin wrote:
>
> I guess I'll explain the bug here as well.
> consider this code:
>
> float a = -123.987;
> printf("%lu", (unsigned long int)a);
>
> Before the patch output would be 123.
> After the patch output would be 18446744073709551493.
>
> There's no bug. That's undefined behavior (see ISO C17, 6.3.1.4)
> because -123 is not representable in unsigned long int. So, any
> behavior is correct.
>
> --
> Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/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
>
>
>
>