Recent

Author Topic: Strange behaviour of BOOLEAN type  (Read 4679 times)

alpine

  • Hero Member
  • *****
  • Posts: 1141
Re: Strange behaviour of BOOLEAN type
« Reply #15 on: May 30, 2024, 01:29:44 pm »

Quite possible, but I don't have trunk to check it. And I'm not sure what is Thaddy talking about (expansion).
A boolean is defined as a BYTE value 0--1 in the language description. The Boolean is now expanded to register size on all platforms and all optimizations and that is wrong. There was a very recent discussion about this and why the expansion of Booleans causes problems when interfacing with other languages. Alas the "fix" in trunk causes more problems than it "fixed" and I told so...
They fixed it in the wrong way. It is fixed in a way to solve a single - rare - problem by an OP that seems to have more infuence than me and the fixer(s) would not listen.
Hence in trunk the behavior of Booleans has changed.... to the worse...
In that case I demonstrated the different behavior with and without optimizations.
Sounds disturbing. Please, can you point me to where that discussion took place, I will study it with all due care.

This is related, btw.
Depending on optimization settings, the result of boolean operations indeed differs, also in 3.2.x.

The change in trunk, though, breaks existing code even more.
The FPC is my tool of choice recently, unfortunately new releases producing incorrect code is something that can change this.
"I'm sorry Dave, I'm afraid I can't do that."
—HAL 9000

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11598
  • FPC developer.
Re: Strange behaviour of BOOLEAN type
« Reply #16 on: May 30, 2024, 03:42:57 pm »
Thaddy is awfully sparingly with actual references for his statements in this thread.

File a bug to get a compiler developer's opinion.

Thaddy

  • Hero Member
  • *****
  • Posts: 14844
  • Censorship about opinions does not belong here.
Re: Strange behaviour of BOOLEAN type
« Reply #17 on: May 30, 2024, 07:50:32 pm »
Marco, you very well know about this discussion:
https://gitlab.com/freepascal.org/fpc/source/-/issues/40746

My example in that discussion works in -O1 but fails in -O2. Now it fails in all cases, which is wrong, because my code is correct (Byte bool 0 or 1 = Pascal)

And the "fix"is not a fix. It will come back to haunt the one that "fixed" it.
I even took the trouble to prove that in clang as well.( same fail at higher optimization level, because of expansion to register size)
The "fix" is mimicking a bug, not a feature.
« Last Edit: May 30, 2024, 07:56:49 pm by Thaddy »
bitrate is always calculated like this:sample rate * bitdepth * number of channels.

alpine

  • Hero Member
  • *****
  • Posts: 1141
Re: Strange behaviour of BOOLEAN type
« Reply #18 on: May 31, 2024, 12:19:44 pm »
@Thaddy
Please, read that post entirely.

After playing the entire morning with the issue from the original post, here are my findings:

The minimal code:
Code: Pascal  [Select][+][-]
  1. var
  2.   mas: array[1..200, 1..2] of Boolean;
  3.   i, j, k: Integer;
  4. begin
  5.   k := 1; j := 1;
  6.   for i := 1 to 200 do
  7.     mas[i, j] := (k = 1);
  8.   WriteLn(mas[1, 1]);
  9. end.              
(The array dims are greater than 1 because in higher optimization levels the compiler reduces them to just one assignment)
All compiler targets are x86_64, i.e. 64-bit.

Free Pascal Compiler version 3.2.2-rrelease_3_2_2-0-g0d122c4953 [2022/10/28] for x86_64, optimization level: -O1
Here the result is FALSE which is incorrect. The disassembly:
Code: ASM  [Select][+][-]
  1. project1.lpr:6                            for i := 1 to 200 do
  2. 00000000004010B6 c705900e030000000000     movl   $0x0,0x30e90(%rip)        # 0x431f50 <U_$P$PROGRAM_$$_I>
  3. 00000000004010C0 8b058a0e0300             mov    0x30e8a(%rip),%eax        # 0x431f50 <U_$P$PROGRAM_$$_I>
  4. 00000000004010C6 83c001                   add    $0x1,%eax
  5. 00000000004010C9 8905810e0300             mov    %eax,0x30e81(%rip)        # 0x431f50 <U_$P$PROGRAM_$$_I>
  6. project1.lpr:7                            mas[i, j] := (k = 1);
  7. 00000000004010CF 8b0d8b0e0300             mov    0x30e8b(%rip),%ecx        # 0x431f60 <U_$P$PROGRAM_$$_J>
  8. 00000000004010D5 48d1e0                   shl    %rax
  9. 00000000004010D8 833d910e030001           cmpl   $0x1,0x30e91(%rip)        # 0x431f70 <U_$P$PROGRAM_$$_K>
  10. 00000000004010DF 488d15da0c0300           lea    0x30cda(%rip),%rdx        # 0x431dc0 <U_$P$PROGRAM_$$_MAS>
  11. 00000000004010E6 4801d0                   add    %rdx,%rax
  12. 00000000004010E9 0f944408fd               sete   -0x3(%rax,%rcx,1)
  13. project1.lpr:6                            for i := 1 to 200 do
  14. 00000000004010EE 813d580e0300c8000000     cmpl   $0xc8,0x30e58(%rip)        # 0x431f50 <U_$P$PROGRAM_$$_I>
  15. 00000000004010F8 7cc6                     jl     0x4010c0 <main+48>
  16.  
Here the problem is the line #11 where the add instruction destroys the flags previously set from the cmpl at line $9.

Free Pascal Compiler version 3.3.1-15761-gbf970b29f4-dirty [2024/05/30] for x86_64 trunk, optimization level: -O1
Here the result is TRUE which is correct. The disassembly:
Code: ASM  [Select][+][-]
  1. project1.lpr:6                            for i := 1 to 200 do
  2. 00000000004010B3 488d0586760300           lea    0x37686(%rip),%rax        # 0x438740 <U_$P$PROGRAM_$$_I>
  3. 00000000004010BA c70001000000             movl   $0x1,(%rax)
  4. project1.lpr:7                            mas[i, j] := (k = 1);
  5. 00000000004010C0 488d0579760300           lea    0x37679(%rip),%rax        # 0x438740 <U_$P$PROGRAM_$$_I>
  6. 00000000004010C7 8b10                     mov    (%rax),%edx
  7. 00000000004010C9 488d0580760300           lea    0x37680(%rip),%rax        # 0x438750 <U_$P$PROGRAM_$$_J>
  8. 00000000004010D0 8b00                     mov    (%rax),%eax
  9. 00000000004010D2 48d1e2                   shl    %rdx
  10. 00000000004010D5 488d0d84760300           lea    0x37684(%rip),%rcx        # 0x438760 <U_$P$PROGRAM_$$_K>
  11. 00000000004010DC 833901                   cmpl   $0x1,(%rcx)
  12. 00000000004010DF 488d0dca740300           lea    0x374ca(%rip),%rcx        # 0x4385b0 <U_$P$PROGRAM_$$_MAS>
  13. 00000000004010E6 488d140a                 lea    (%rdx,%rcx,1),%rdx
  14. 00000000004010EA 0f944402fd               sete   -0x3(%rdx,%rax,1)
  15. project1.lpr:6                            for i := 1 to 200 do
  16. 00000000004010EF 488d054a760300           lea    0x3764a(%rip),%rax        # 0x438740 <U_$P$PROGRAM_$$_I>
  17. 00000000004010F6 8b00                     mov    (%rax),%eax
  18. 00000000004010F8 678d5001                 lea    0x1(%eax),%edx
  19. 00000000004010FC 488d053d760300           lea    0x3763d(%rip),%rax        # 0x438740 <U_$P$PROGRAM_$$_I>
  20. 0000000000401103 8910                     mov    %edx,(%rax)
  21. 0000000000401105 488d0534760300           lea    0x37634(%rip),%rax        # 0x438740 <U_$P$PROGRAM_$$_I>
  22. 000000000040110C 8138c8000000             cmpl   $0xc8,(%rax)
  23. 0000000000401112 7eac                     jle    0x4010c0 <main+48>
  24.  
Here between the cmpl at line #11 and sete at line #14 we have just 2 lea instructions. They don't affect flags.

That confirms the rvk assumption from his reply #8.

I have tested the above code in FPC 3.2.2 and FPC 3.3.1(trunk) with all optimization levels: FPC 3.2.2 output is incorrect in all levels, FPC 3.3.1 are all correct.

Furthermore, if we introduce an inner loop as in the original post:
Code: Pascal  [Select][+][-]
  1. begin
  2.   k := 1; j := 1;
  3.   for i := 1 to 200 do
  4.     for j := 1 to 2 do
  5.       mas[i, j] := (k = 1);
  6.  ...

Then FPC 3.2.2 with -O1 or -O2 still gives a wrong result because of the same subtle EA calculation:
Code: ASM  [Select][+][-]
  1. 00000000004010E6 833d830e030001           cmpl   $0x1,0x30e83(%rip)        # 0x431f70 <U_$P$PROGRAM_$$_K>
  2. 00000000004010ED 488d15cc0c0300           lea    0x30ccc(%rip),%rdx        # 0x431dc0 <U_$P$PROGRAM_$$_MAS>
  3. 00000000004010F4 4801d0                   add    %rdx,%rax
  4. 00000000004010F7 0f944408fd               sete   -0x3(%rax,%rcx,1)

while with level -O3 it gives the correct answer because of inner loop unrolled and replaced with 2x assignments at different constant displacements:
Code: ASM  [Select][+][-]
  1. project1.lpr:8                            mas[i, j] := (k = 1);
  2. 00000000004010C7 89c2                     mov    %eax,%edx
  3. 00000000004010C9 833da00e030001           cmpl   $0x1,0x30ea0(%rip)        # 0x431f70 <U_$P$PROGRAM_$$_K>
  4. 00000000004010D0 488d05e90c0300           lea    0x30ce9(%rip),%rax        # 0x431dc0 <U_$P$PROGRAM_$$_MAS>
  5. 00000000004010D7 0f944450fe               sete   -0x2(%rax,%rdx,2)
  6. 00000000004010DC 8b056e0e0300             mov    0x30e6e(%rip),%eax        # 0x431f50 <U_$P$PROGRAM_$$_I>
  7. 00000000004010E2 833d870e030001           cmpl   $0x1,0x30e87(%rip)        # 0x431f70 <U_$P$PROGRAM_$$_K>
  8. 00000000004010E9 488d15d00c0300           lea    0x30cd0(%rip),%rdx        # 0x431dc0 <U_$P$PROGRAM_$$_MAS>
  9. 00000000004010F0 0f944442ff               sete   -0x1(%rdx,%rax,2)
  10.  
That is contrary to the Thaddy statement that higher optimization level will make things worse. Also I don't see any boolean 'expansion' - all is performed at byte level (sete/setz).

Another thing that stands out is that in the trunk the code generation for loops has been changed, in 3.2.2 we have control loop variable assigned to (start-1 ) and pre-increment, in the trunk we have assignment to the (start) and post-increment. Also, the EA calculation for element of multi-dimensional array is done with lea instead of add, which is the right way as it can be seen.

So, the recap is that the FPC 3.2.2 has a subtle issue with assigning a simple boolean expression to an element of a multi-dimensional array.

The question is whether this is worth reporting when it no longer appears in the trunk?



"I'm sorry Dave, I'm afraid I can't do that."
—HAL 9000

vangli

  • New Member
  • *
  • Posts: 46
Re: Strange behaviour of BOOLEAN type
« Reply #19 on: May 31, 2024, 12:22:58 pm »
Hi Thaddy

I just wonder out of curiosity for learning. If the assembler code in https://forum.lazarus.freepascal.org/index.php/topic,67448.msg518962.html#msg518962 :

Code: ASM  [Select][+][-]
  1. /home/alpi/fpcupdeluxe_v2_2_0m/fpcupdeluxe/projects/project1.lpr:26  mas[i].ar[j].a :=  k=1;
  2. 00000000004010C4 8B45FC                   mov eax,[rbp-$04]
  3. 00000000004010C7 8B4DF8                   mov ecx,[rbp-$08]
  4. 00000000004010CA 48C1E003                 shl rax,$03
  5. 00000000004010CE 837DF401                 cmp dword ptr [rbp-$0C],$01
  6. 00000000004010D2 488D15D7F70700           lea rdx,[rip+$0007F7D7]
  7. 00000000004010D9 4801D0                   add rax,rdx
  8. 00000000004010DC 0F944408FB               setz byte ptr [rcx+rax-$05]
  9.  

had been:

Code: ASM  [Select][+][-]
  1. /home/alpi/fpcupdeluxe_v2_2_0m/fpcupdeluxe/projects/project1.lpr:26  mas[i].ar[j].a :=  k=1;
  2. 00000000004010C4 8B45FC                   mov eax,[rbp-$04]
  3. 00000000004010C7 8B4DF8                   mov ecx,[rbp-$08]
  4. 00000000004010CA 48C1E003                 shl rax,$03
  5. 00000000004010D2 488D15D7F70700           lea rdx,[rip+$0007F7D7]
  6. 00000000004010D9 4801D0                   add rax,rdx
  7. 00000000004010CE 837DF401                 cmp dword ptr [rbp-$0C],$01
  8. 00000000004010DC 0F944408FB               setz byte ptr [rcx+rax-$05]
  9.  

would it then run correctly as the rbp register is not touched by the other instructions?

Bent
Regards Bent

rvk

  • Hero Member
  • *****
  • Posts: 6268
Re: Strange behaviour of BOOLEAN type
« Reply #20 on: May 31, 2024, 12:29:40 pm »
I just wonder out of curiosity for learning. If the assembler code in https://forum.lazarus.freepascal.org/index.php/topic,67448.msg518962.html#msg518962 :
[...]
would it then run correctly as the rbp register is not touched by the other instructions?
I also already mentioned that trunk has done this differently from original topic.
But Thaddy said it made it worse.

Here in trunk it uses rip instead of the original rbp and the cmp is moved directly before the setz.

Isn't this already fixed in trunk?

Code: ASM  [Select][+][-]
  1. C:\Users\Rik\AppData\Local\Temp\project1.lpr:21  mas[i].ar[j].a :=  k=1;
  2. 0000000100001760 8B15BA180200             mov edx,[rip+$000218BA]
  3. 0000000100001766 8B05C4180200             mov eax,[rip+$000218C4]
  4. 000000010000176C 48C1E203                 shl rdx,$03
  5. 0000000100001770 488D0D99180200           lea rcx,[rip+$00021899]
  6. 0000000100001777 4801CA                   add rdx,rcx
  7. 000000010000177A 833DBF18020001           cmp dword ptr [rip+$000218BF],$01
  8. 0000000100001781 0F944402FB               setz byte ptr [rax+rdx-$05]

No, it is worse in trunk, Rik.

So I'm not sure why Thaddy thinks it's worse (it does fix the original problem) but he might have his own examples (which weren't shared).

Thaddy

  • Hero Member
  • *****
  • Posts: 14844
  • Censorship about opinions does not belong here.
Re: Strange behaviour of BOOLEAN type
« Reply #21 on: May 31, 2024, 01:35:54 pm »
Rik, it fixes a linking issue (a problem, yes) but it fixes it in such a way that the proper byte boolean get mutilated. So a "fix" at the cost of the language.
Both my clang code and my pascal code demonstrates the issue is NOT solved, but rather a cludge: the issue lies elseware, beyond the scope of Pascal.

The offence is not making sure that a Boolean (Pascal style) can never be expanded. There are plenty of other Boolean types that can be used. Now they ruined the type system..... >:D >:D >:D >:D >:D

And the real fix was simple in this case: he should have used a longbool.
« Last Edit: May 31, 2024, 01:44:12 pm by Thaddy »
bitrate is always calculated like this:sample rate * bitdepth * number of channels.

jamie

  • Hero Member
  • *****
  • Posts: 6304
Re: Strange behaviour of BOOLEAN type
« Reply #22 on: June 01, 2024, 11:29:15 pm »
apparently,.

Using

LongBool(K=1), WordBool(K=1) or any Bool cast other than Boolean seems to solve it.
The only true wisdom is knowing you know nothing

Thaddy

  • Hero Member
  • *****
  • Posts: 14844
  • Censorship about opinions does not belong here.
Re: Strange behaviour of BOOLEAN type
« Reply #23 on: June 02, 2024, 10:41:31 am »
Yes, but that is what I suggested: If you specify such a boolean I have no complaints, but a Pascal Boolean, or even a C bool is strictly 0..1 of size byte.
The problem arizes when compilers expand the byte size and you will get unpredictable results, as my clang and pascal code demonstrates this can and does cause problems on higher optimizations. So now FPC mimics clang... to solve an issue that was not there.
bitrate is always calculated like this:sample rate * bitdepth * number of channels.

cdbc

  • Hero Member
  • *****
  • Posts: 1246
    • http://www.cdbc.dk
Re: Strange behaviour of BOOLEAN type
« Reply #24 on: June 02, 2024, 10:51:47 am »
Hi
@Thaddy: Have I understood you correctly, in that, in trunk a 'boolean' is no longer 0 or 1 (ordinal) but more like "Anything not 0 is true" e.g.: -1 = true;
Won't that get kind of messy?!?
Regards Benny
If it ain't broke, don't fix it ;)
PCLinuxOS(rolling release) 64bit -> KDE5 -> FPC 3.2.2 -> Lazarus 2.2.6 up until Jan 2024 from then on it's: KDE5/QT5 -> FPC 3.3.1 -> Lazarus 3.0

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 10008
  • Debugger - SynEdit - and more
    • wiki
Re: Strange behaviour of BOOLEAN type
« Reply #25 on: June 02, 2024, 11:17:26 am »
As far as I can see, there are 2 issues being mixed up here. I.e. the Issue mentioned by Thaddy has nothing to do with the original issue (other that being about the  boolean type, which applies to lots of things).

About the issue referred by Thaddy: Boolean exists in 2 forms https://www.freepascal.org/docs-html/ref/refsu4.html#x26-270003.1.1 (and has done so long before that issue).
* Boolean, Boolean16..64 are enum. They can have one of 2 value: true or false. Any other value will not work (examples in which they do are random coincidence).
* ByteBool.... That can have any bitpattern stored, with all bits 0 meaning false, and any other meaning true.

The issue referred by Thaddy (from a quick glance) discusses the change of ONE particular alias from one form of boolean to the other form of boolean.






On the other hand the issue from the OT is a bug in the compiler. The asm clearly shows that the result of the comparison is discarded, before it is assigned to the variable. So the variable then holds the wrong value (never mind in which form it is represented).

The original issue may possible have been fixed by this
Quote
6f24c8b4efccea67d092062009f413cc789a052c
*   * x86: Code generation fixes where FLAGS register is not properly allocated.
Though it may have been something else, the above commit is just a guess of mine, based on the wording of the commit and the findings from the asm.

Also in my testing, the original code works, if compiled with -O-
Or if compiled with
Code: Pascal  [Select][+][-]
  1. {$Optimization noPEEPHOLE }

Thaddy

  • Hero Member
  • *****
  • Posts: 14844
  • Censorship about opinions does not belong here.
Re: Strange behaviour of BOOLEAN type
« Reply #26 on: June 02, 2024, 11:26:56 am »
The issue referred by Thaddy (from a quick glance) discusses the change of ONE particular alias from one form of boolean to the other form of boolean.
It is the ONLY definition of Boolean as far as the Pascal formal definition goes. The same is true for clang.
bitrate is always calculated like this:sample rate * bitdepth * number of channels.

cdbc

  • Hero Member
  • *****
  • Posts: 1246
    • http://www.cdbc.dk
Re: Strange behaviour of BOOLEAN type
« Reply #27 on: June 02, 2024, 11:32:25 am »
Hi
@Martin_fr: Thanks for clarifying Martin.
Regards Benny
If it ain't broke, don't fix it ;)
PCLinuxOS(rolling release) 64bit -> KDE5 -> FPC 3.2.2 -> Lazarus 2.2.6 up until Jan 2024 from then on it's: KDE5/QT5 -> FPC 3.3.1 -> Lazarus 3.0

CuriousKit

  • Jr. Member
  • **
  • Posts: 81
Re: Strange behaviour of BOOLEAN type
« Reply #28 on: June 02, 2024, 12:46:11 pm »
I remember that bug and needing to fix it.  One of my peephole optimisations takes place if the flags weren't in use, but it uncovered the fact that the compiler didn't mark the flags as "in use" for a lot of conditional statements (which the aforementioned commit fixes).

For those unfamiliar with assembly language, "cmp" is a comparison between two arguments, and it sets a bunch of flags based on the result, so then "setz" sets a byte to 1 if the zero flag (represented by the 'z' suffix) is set (i.e. the two arguments are equal in value, since "cmp" does a subtraction internally), and 0 if it is clear.

The problem is that "add" also sets the flags based on the results, so if RAX + RDX (the names of a couple of internal storage spaces directly on the CPU) isn't equal to zero, the zero flag is clear and "setz" writes a zero.
Free Pascal Compiler Developer and Freelance Programmer

Zoran

  • Hero Member
  • *****
  • Posts: 1840
    • http://wiki.lazarus.freepascal.org/User:Zoran
Re: Strange behaviour of BOOLEAN type
« Reply #29 on: June 02, 2024, 03:15:00 pm »
It is the ONLY definition of Boolean as far as the Pascal formal definition goes.

Absolutely. We should be able to rely on that. I wonder where they found the motive for messing with that. %)
Is it the infamous Delphi compatibility again? If it is the reason, can it be really put in front of backwards compatibility?

As Martin says, this doesn't seem to be directly tied to the bug discussed here, though. The fix for this should be backported to fixes branch (btw, will the release from fpc fixes branch ever see the light of the day?).

 

TinyPortal © 2005-2018