View Issue Details

IDProjectCategoryView StatusLast Update
0036176FPCCompilerpublic2019-12-18 14:46
ReporterLacaKAssigned ToFlorian 
PrioritynormalSeverityminorReproducibilitysometimes
Status assignedResolutionreopened 
Product Version3.3.1Product Build 
Target Version3.2.0Fixed in Version3.2.0 
Summary0036176: Currency multiply by constant (divisible by 10000) on Win64
DescriptionThere was implemented fix in rev. 38555 and folloved rev. 38558 - "scale constants if possible before currency multiplications to avoid overflows"
(if there is on left or right side of multiplication constant divisible by 10000)

But actualy there is in TRUNK again something wrong. See "Steps To Reproduce".
Steps To Reproduce// OS: Win64, CPU: x86_64
var
   c: currency;
begin
   c:=922337203685.47;
   writeln(c:18:4,' = ', ' Trunc(c*10000)=', Trunc(c*10000)); // expected 9223372036854700, but get -75
   c:=-92233720368547;
   writeln(c:18:4,' = ', ' Trunc(c*10000)=', Trunc(c*10000)); // expected -922337203685470000, but get 7580
end.
Additional InformationOn Win32 there are results as expected.
TagsNo tags attached.
Fixed in Revision43620, 43621
FPCOldBugId
FPCTarget-
Attached Files
  • tcurrency1.pp.diff (611 bytes)
    --- tcurrency1.pp.ori	Fri Mar 11 11:38:28 2016
    +++ tcurrency1.pp	Mon Dec 02 14:47:14 2019
    @@ -1,4 +1,4 @@
    -program tcurrency;
    +program tcurrency1;
     
     { test basic mathematical operations (+,-,*,/) using currency data type }
     
    @@ -66,6 +66,13 @@
       c2 := 1234;
       if i*c1 <> c2 then begin
         writeln('Invalid currency*integer=', i*c1, ', but expected ', c2);
    +    halt(2);
    +  end;
    +  i:=10000;
    +  c1:=92233720368.5477;
    +  c2:=922337203685477;
    +  if c1*i <> c2 then begin
    +    writeln('Invalid currency*integer=', c1*i, ', but expected ', c2);
         halt(2);
       end;
       // division integer
    
    tcurrency1.pp.diff (611 bytes)

Relationships

related to 0033439 resolvedFlorian Multiply x Currency x Win64 
related to 0033755 closedMichael Van Canneyt Update ODBC header function due to problem on Win64 
related to 0033963 resolvedFlorian Klämpfl Wrong result when a currency variable is multiplied by a constant 
related to 0036455 new Various problems with Currency on x86_64 

Activities

LacaK

2019-12-02 14:11

developer   ~0119583

Fix was backported to 3.2, but my todays tests still show this bug in Fixes_3_2

LacaK

2019-12-02 14:52

developer   ~0119584

+Updated test program tests/test/tcurrency1.pp (to demonstrate error on Win64; btw tw36179.pp should also show error)

tcurrency1.pp.diff (611 bytes)
--- tcurrency1.pp.ori	Fri Mar 11 11:38:28 2016
+++ tcurrency1.pp	Mon Dec 02 14:47:14 2019
@@ -1,4 +1,4 @@
-program tcurrency;
+program tcurrency1;
 
 { test basic mathematical operations (+,-,*,/) using currency data type }
 
@@ -66,6 +66,13 @@
   c2 := 1234;
   if i*c1 <> c2 then begin
     writeln('Invalid currency*integer=', i*c1, ', but expected ', c2);
+    halt(2);
+  end;
+  i:=10000;
+  c1:=92233720368.5477;
+  c2:=922337203685477;
+  if c1*i <> c2 then begin
+    writeln('Invalid currency*integer=', c1*i, ', but expected ', c2);
     halt(2);
   end;
   // division integer
tcurrency1.pp.diff (611 bytes)

Florian

2019-12-03 20:32

administrator   ~0119602

Can you please check with r43635 ? I have no win64 at hand by with wine it worked.

LacaK

2019-12-04 07:50

developer   ~0119605

tcurrency1.pp with TRUNK r43635 worked okay.
But original test program /tests/webtbs/tw36179.pp still shows errors!
Can you please rename tw36179.pp to tw36176.pp - I guess, that number should correspond to bug number (there is typo "9" instead of "6")

Btw what is expected result type of expressions (on Win64, where Currency is INT64 and on Win32):
- Currency * integer constant
- Currency * integer variable
- Currency * int64 variable ?

Florian

2019-12-05 21:27

administrator   ~0119642

LacaK: Currency.

LacaK

2019-12-06 07:51

developer   ~0119652

Then probably results of expressions like:
Currency*10000, which is out of <MinCurrency,MaxCurrency> cannot work?

Then for example: 922337203685.47*10000=9223372036854700 which fits into int64, but is greater than MaxCurrency (922337203685477.5807) ... is illegal expression?
(if yes then test program tw36179.pp will never work?)

If so, what is then a proper way how to get result of above mentioned expression (as Int64 or as double)?

Florian

2019-12-06 18:15

administrator   ~0119665

> Currency*10000, which is out of <MinCurrency,MaxCurrency> cannot work?

Yes, keep in mind: currency is meant for currency calculation and almost 10^15 should be still sufficient for this purpose. That Currency*<any int> is Currency should be also clear due to this.

> If so, what is then a proper way how to get result of above mentioned expression (as Int64 or as double)?

Probably using the bcd unit. The only floating point type which would help is extended on i386 as it has a sufficient high precision and range.

LacaK

2019-12-07 10:45

developer   ~0119674

So I have this suggestion:

1. delete tw36179.pp as this is not supposed to work

2. mark this bug report as "won't fix" or "not fixable"

3. reopen bug report 33755 and fix "Trunc(c*10000)" in another way.
According to
https://www.mail-archive.com/fpc-pascal@lists.freepascal.org/msg51867.html use this construct:

var
  c: currency;
  n: int64 absolute c;
// is it safe assume on all platforms that "n" will represent currency*10000 (on Win32, Win64 it is so) ?
// or we must use {$IFDEF FPC_CURRENCY_IS_INT64} ?

var
  c: currency;
  n: int64 absolute c;
begin
  {$IFDEF FPC_CURRENCY_IS_INT64}
  // here "n" represents currency*10000
  {$ELSE}
  // use Trunc(c*10000)
  {$ENDIF}

Here I would like to get help with solution, which will be accepted by core developers.
(as my original solution in bug report 33755 using "absolute" was rejected)

BrunoK

2019-12-14 15:28

reporter   ~0119843

i386 vs x86_64.

The best current solution for users of the Currency type is TO NOT BUILD for x86-64 and stay with i386 targets, so the i87 FPU is used for calculations of money types, BCD being a bit heavy for normal use case.

What could nice, is to have the compiler have a FPU87 OPTION that would switch the more modern mmx etc whatever mode of calculation OFF and retain only code generation (linking ?) using ONLY the 87 floating point processor.

At least a STRONG WARNING should be issued if the Currency type is used in x86_64.

I have been using the currency type since MS Quick Basic Compiler at the end of the 80's that probably had some kind of soft arithmetic for the currency type and never had the problems we have now with financial calculation on x86_64.

I prefer to loose speed and retain reasonable precision/results with a data type that is really the only good one for financial calculations (14 integer positions + 4 decimal covers nearly all everyday use case).

And another point concerning some of the current discussions on floating point divisions, (that was the on on 1/5), calculation. With the i87 FPU and Currency as a result, it will effectively and nearly always ROUND to a correct Int64 value (Currency is an Int64 value).
It IS NOT POSSIBLE to represent 0.2 (base 10) or store it as an exact value in any of the standard Single/Double/Extended floating system due to the fact that these are base 2 storage representations and there is no way to express 0.2 (base 10) as a base 2 finite fractional part.
To illustrate, make a simple exercise in base 10, decimal, what we probably all learned at school and do :
  10 / 3 and store its precise decimal result on a sheet of paper, without using periodicity notation.

Florian

2019-12-14 15:46

administrator   ~0119845

@BrunoK: as mentioned in another bug report: the tracker is not for discussions. The x86-64 support for currency had some bugs, they are fixed. The remaining problems are indeed non-i386 but they appear not on i386 only by coincidence. Currency is made for numbers within the range typical currency values are and within this range everything is fixed now on x86-64. So putting warning makes no the slightest sense.

Issue History

Date Modified Username Field Change
2019-10-14 08:41 LacaK New Issue
2019-10-14 08:41 LacaK Relationship added related to 0033439
2019-10-14 08:42 LacaK Relationship added related to 0033755
2019-10-14 09:05 LacaK Summary Currency multiply by constant on Win64 (regression) => Currency multiply by constant (divisible by 10000) on Win64
2019-10-14 09:05 LacaK FPCTarget => -
2019-12-01 11:43 Florian Target Version => 3.2.0
2019-12-01 21:23 Florian Relationship added related to 0033963
2019-12-01 21:24 Florian Assigned To => Florian
2019-12-01 21:24 Florian Status new => resolved
2019-12-01 21:24 Florian Resolution open => fixed
2019-12-01 21:24 Florian Fixed in Version => 3.3.1
2019-12-01 21:24 Florian Fixed in Revision => 43620
2019-12-01 21:30 Florian Fixed in Version 3.3.1 => 3.2.0
2019-12-01 21:30 Florian Fixed in Revision 43620 => 43620, 43621
2019-12-02 14:11 LacaK Status resolved => feedback
2019-12-02 14:11 LacaK Resolution fixed => reopened
2019-12-02 14:11 LacaK Note Added: 0119583
2019-12-02 14:52 LacaK File Added: tcurrency1.pp.diff
2019-12-02 14:52 LacaK Note Added: 0119584
2019-12-02 14:52 LacaK Status feedback => assigned
2019-12-03 20:32 Florian Note Added: 0119602
2019-12-04 07:50 LacaK Note Added: 0119605
2019-12-05 21:27 Florian Note Added: 0119642
2019-12-06 07:51 LacaK Note Added: 0119652
2019-12-06 18:15 Florian Note Added: 0119665
2019-12-07 10:45 LacaK Note Added: 0119674
2019-12-14 15:28 BrunoK Note Added: 0119843
2019-12-14 15:46 Florian Note Added: 0119845
2019-12-18 14:46 LacaK Relationship added related to 0036455