Recent

Author Topic: Likely incorrect 64bit code generation in FPC v3.0.4  (Read 13412 times)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Likely incorrect 64bit code generation in FPC v3.0.4
« Reply #30 on: March 16, 2019, 05:07:06 am »
Afaik, cardinal is "unsigned" not "positive".

In any case, "cardinal" contains 0. Yes, so does (signed-)integer.
So from that declaration, either can be used. Both will contain all the possible indexes.

So (for the discussions sake) cardinal is chosen (actually the sub-range 0..0 is chosen, and it is chosen as a subrange of cardinal).

Now (with range check, and other checks off)
  SomeCardinal := SomeIntegerVariable // where SomeIntegerVariable = -5
will assign $ffff...fb  as a positive SomeCardinal.

Equally you can also assign numbers to a variable of subrange type, even if the number is outside that range.
  type TNum = 1..5;
  ANum := SomeIntegerVariable // where SomeIntegerVariable = -5
Not tested, but on 64 bit that probably also give $00000000fffffffb
If it does not, that would IMHO be inconsistent.



So using
  SomeArray[SomeInteger]
where the index is assumed (subrange of) cardinal does the same.

In 32 bit the index value is also treated as cardinal.
Only
  SomeAddressOfElemenZero + $fffffffB
is the same as
  SomeAddressOfElemenZero - 5

---------------------
Again, I am not saying that I agree with this design decision. Just pointing out my understanding of the current state.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9791
  • Debugger - SynEdit - and more
    • wiki
Re: Likely incorrect 64bit code generation in FPC v3.0.4
« Reply #31 on: March 16, 2019, 05:18:34 am »
Imagine if in an expression such as:
Code: Pascal  [Select][+][-]
  1. var ANumber : integer;
  2.   ...
  3.   ...
  4.   ANumber + 5;
  5.  
FPC decided to "convert" ANumber into an unsigned type because the constant 5 is positive.  THAT is effectively what FPC is doing when openly ignoring and overriding the declared type of the indexing variable.  As obviously ludicrous as doing something like that would be, it would actually have greater justification since, at least the numeral 5, unlike zero (0), is positive.

It is not the same. 5 is within the range of integer. No conversion needed.
But:
Code: Pascal  [Select][+][-]
  1. AnOtherInteger := AnInteger + cardinal($F1234567)
That does not fit into int.

Afaik Fpc converts both numbers into int64, but the result then get truncated into integer.

Of course do the above, but
Code: Pascal  [Select][+][-]
  1. AnOtherInt64 := AnInt64 + qword($F123456701234567)
Then afaik, the qword becomes an int64.


440bx

  • Hero Member
  • *****
  • Posts: 3944
Re: Likely incorrect 64bit code generation in FPC v3.0.4
« Reply #32 on: March 16, 2019, 06:28:10 am »
Afaik, cardinal is "unsigned" not "positive".
Formally, any value other than zero (0) is signed.  Cardinal/word/dword/qword/etc means a non-negative-only value which causes the interpretation of the high bit to change thereby allowing higher magnitudes and, that interpretation is valid because the compiler has been explicitly told that there are no negative values in the type/range.  It should also be pointed out that non-negative-only includes zero (0) since it is neither negative nor positive.

In any case, "cardinal" contains 0. Yes, so does (signed-)integer.
Int/Int32/int64/integer also contain zero (0), therefore it is not possible to assign a sign to zero since both "unsigned" and signed data types include that value.  It is incorrect to simply choose, or presume, that zero is positive which is what FPC is doing by "concluding" that sign extension should not be done.

In other words, if you see a statement like this:
Code: Pascal  [Select][+][-]
  1. A := 0;
It is not possible to state that A is a signed or unsigned data type because zero (0) does not provide enough information to draw a correct conclusion but, FPC is drawing a conclusion from that case and, as expected, gets it wrong because it cannot be done.

So (for the discussions sake) cardinal is chosen (actually the sub-range 0..0 is chosen, and it is chosen as a subrange of cardinal).
0 is also in a subrange of int16/int32/int64/etc, therefore, it is incorrect to presume it is _only_ in a subrange of an unsigned type such as Cardinal.

  SomeCardinal := SomeIntegerVariable // where SomeIntegerVariable = -5
will assign $ffff...fb  as a positive SomeCardinal.

Equally you can also assign numbers to a variable of subrange type, even if the number is outside that range.
  type TNum = 1..5;
  ANum := SomeIntegerVariable // where SomeIntegerVariable = -5
Not tested, but on 64 bit that probably also give $00000000fffffffb
If it does not, that would IMHO be inconsistent.
But in that example, you are declaring a type and in addition to that, you have values other than zero (0) which provide enough information to unambiguously state that the type/range does not include negative values.

That example does _not_ reflect the situation that causes the bug.

Again, I am not saying that I agree with this design decision. Just pointing out my understanding of the current state.
I understand you are not saying you are in agreement with the decision. I am clear on that. 

What I'd like to make people understand is why what FPC is doing is a legitimate bug.  It is simply wrong.  The "current state", as you put it, is simply incorrect. It's wrong.  It's not a matter of choice, being nice, implementation or "undefined territory", it is provably wrong.  Zero cannot be presumed to be positive nor negative, it is neither.  No type information can be derived from it.

HTH.

 
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

440bx

  • Hero Member
  • *****
  • Posts: 3944
Re: Likely incorrect 64bit code generation in FPC v3.0.4
« Reply #33 on: March 16, 2019, 07:12:06 am »
Just to leave no doubt that FPC's behavior in that case is a bug, I wrote the following simple program:

Code: Pascal  [Select][+][-]
  1. {$APPTYPE CONSOLE}
  2.  
  3. program SignOfZero;
  4.  
  5. uses
  6.   Math
  7.   ;
  8.  
  9. type
  10.   TZero   = 0..0;
  11.  
  12. var
  13.   AQword  : uint64 = 0;
  14.   AnInt64 : int64  = 0;
  15.  
  16.   ATZero  : TZero  = 0;
  17.  
  18. begin
  19.   {$ifdef VER300}
  20.     writeln('Sign of AQword  : ', Sign(AQword));  // no overloads in FPC for unsigned
  21.                                                   // types, Delphi has them.
  22.   {$endif}
  23.  
  24.   writeln('Sign of AnInt64 : ',  Sign(AnInt64));
  25.  
  26.   writeln('Sign of ATZero  : ',  Sign(ATZero));
  27.  
  28.  
  29.   writeln('press enter/return to end this program');
  30.   readln;
  31. end.
The result is zero in all cases which means neither signed nor unsigned.  The bug mentioned in the first post is caused by FPC "concluding" that zero (0) is  positive which, I hope it is clear now, is incorrect.

Because of that "conclusion" it fails to sign extend a variable which is explicitly declared as signed. 

HTH.

ETA:

Quote
It is not the same. 5 is within the range of integer. No conversion needed.
but, it is also in the range of a qword yet, FPC didn't decide to "convert" the variable into a qword, which is what it is doing when failing to sign extend an integer because it is assuming a data type for a constant.

It's fine and rather commonplace to assume a data type for a constant, it is done all the time when calculating the value of expressions but, that _assumed_ type cannot be forced onto variables (which is what FPC is doing when failing to sign extend a signed integer when moving it into a 64bit register.) Particularly not when the value is the constant zero (0).
« Last Edit: March 16, 2019, 07:25:24 am by 440bx »
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

Akira1364

  • Hero Member
  • *****
  • Posts: 561
Re: Likely incorrect 64bit code generation in FPC v3.0.4
« Reply #34 on: March 28, 2019, 02:43:38 am »
I'm not going to comment on all the rest, but in general: when indexing into arrays/pointers/e.t.c, you should always use one of the SizeInt, NativeInt, or PtrInt aliases. Using "integer" (which is really 32-bit longint of course) is not really actually "ok" to do on 64-bit.

I know people do it all the time, and it often works, but issues like this are the precise reason you're not supposed to. Same reason it's a bad idea to use "int" instead of "size_t" in C++, for example. Especially if you're declaring specifically positive index ranges, but going negative with them anyways. (Which isn't particularly advisable either IMO.)

If your program is just changed at line 36 like this, as you know I guess:

Code: Pascal  [Select][+][-]
  1.   RowIdx      : SizeInt;         // row index
  2.   I           : SizeInt;         // column index

It works fine, as the indices are then always pointer-size.
« Last Edit: March 28, 2019, 03:12:29 am by Akira1364 »

440bx

  • Hero Member
  • *****
  • Posts: 3944
Re: Likely incorrect 64bit code generation in FPC v3.0.4
« Reply #35 on: March 28, 2019, 03:38:42 am »
If your program is just changed at line 36 like this, as you know I guess:

Code: Pascal  [Select][+][-]
  1.   RowIdx      : SizeInt;         // row index
  2.   I           : SizeInt;         // column index

It works fine, as the indices are then always pointer-size.
Yes, that is correct and that is the workaround.  As far as indexing an array, as long as the index is numeric and the compiler works as it is supposed to, doing sign extension on signed types for instance, then the size of the integer in use is irrelevant.

There are countless millions of lines of code out there that use int and integer (C/Pascal), no one is going to change all that code to NativeInt, SizeInt, ptrint or any other int because some compiler has "decided" it won't do sign extension when it should.

I believe the advice you gave is good but, it shouldn't be used to circumvent compiler bugs.  It should be used because if someday we go from 64bit to (presumably) 128bit then, those types automatically get the new size which, for things like handles is rather important.

Sadly, that is a bug that won't be fixed. I am most definitely following that advice, I've been following it since I figured out that sign extension occasionally doesn't agree with FPC.

(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

 

TinyPortal © 2005-2018