View Issue Details

IDProjectCategoryView StatusLast Update
0036455FPCDocumentationpublic2020-05-13 12:30
ReporterBrunoK Assigned ToMichael Van Canneyt  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionno change required 
Platformx86_64OSWindows 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0036455: Various problems with Currency on x86_64
Descriptionx86_64 Erroneous multiplication for large currencies that have a
result that fits in the currency domain. AFAIK correct on i386
2 Problems :
  1st Reasonably transtyping Currency to a Int64 and reverse
      fails or is wrong.
      a) Int64(cCur1) := cInt64HighDiv1E6; // Fails to compile x86_64
      b) Int64(vCurRes); // Tanstyping incorrect
  2nd Multiplying 2 currency that have a result within the Currency
      domain range gives an erroneous result.
Steps To ReproduceRun joined program on i386 then on trunk x86_64
Additional InformationResults :
CPUi386 FPC 3.0.4 pgmCurrencyInt64Mix_2.exe
-----------------
cCur1=922'337'203.6854
cInt64HighDiv1E6=9223372036854
Int64(vCurRes)=9223376648540018
vCur1=922'337'203.6854 vCur2=1'000.0005 vCurRes=vCur1*vCur2=922'337'664'854.0018
vCur1=922337203.6854 vCur2=1000.0005 vCurRes=vCur1*vCur2=922337664854.0018
Press enter >

CPUX86_64 FPC 3.3.1 at revision 43682 pgmCurrencyInt64Mix_2.exe
-------------------------------------
cCur1=922'337'203.6854
cInt64HighDiv1E6=9223372036854
vInt64Res=4611685242
vCur1=922'337'203.6854 vCur2=1'000.0005 vCurRes=vCur1*vCur2=461'168.5242
vCur1=922337203.6854 vCur2=1000.0005 vCurRes=vCur1*vCur2=461168.5242
Press enter >
TagsNo tags attached.
Fixed in Revision1700
FPCOldBugId
FPCTarget3.2.0
Attached Files

Relationships

related to 0036176 closedFlorian Currency multiply by constant (divisible by 10000) on Win64 

Activities

BrunoK

2019-12-18 13:26

reporter  

pgmCurrencyInt64Mix_2.pas (2,061 bytes)   
program pgmCurrencyInt64Mix_2;

{ Issue : x86_64 Erroneous multiplication for large currencies that have a
          result that fits in the currency domain. AFAIK correct on i386
          2 Problems :
            1st Resonably transtyping Currency to a Int64 and reverse
                fails or is wrong.
                a) Int64(cCur1) := cInt64HighDiv1E6; // Fails to compile x86_64
                b) Int64(vCurRes);                   // Tanstyping fails
            2nd Multipliying 2 currency that have a result within the Currency
                domain range gives an erroneous result. }


uses
  SysUtils;

const
  cInt64HighDiv1E6 : int64=High(int64) div 1000000;
  cCur1 : currency=0;  // 92233720368547.7580;
  cCur2 : currency=1000.0005;
var
  vCur1,vCur2 : Currency;
  vCurRes : Currency;
  vInt64Res : Int64;
begin
{$IFDEF CPUi386}
  WriteLn('CPUi386 FPC 3.0.4 ', ExtractFileName(ParamStr(0)));
  WriteLn('-----------------');
  Int64(cCur1) := cInt64HighDiv1E6; // Fails to compile x86_64
{$ELSE}
  WriteLn('CPUX86_64 FPC 3.3.1 at revision 43682 ', ExtractFileName(ParamStr(0)));
  WriteLn('-------------------------------------');
  // Int64(cCur1) := cInt64HighDiv1E6; // Fails to compile x86_64
  move(cInt64HighDiv1E6, cCur1, SizeOf(Int64));
{$ENDIF}
  WriteLn('cCur1=',FormatCurr(',.0000', cCur1));
  writeln('cInt64HighDiv1E6=',cInt64HighDiv1E6);
  vCur1 := cCur1;
  vCur2 := cCur2;
  vCurRes := vCur1 * vCur2;
{$IFDEF CPUi386}
  WriteLn('Int64(vCurRes)=', Int64(vCurRes)); // Wrong in x86_64
{$ELSE}
  move(vCurRes, vInt64Res, SizeOf(Int64));    // Needs copy in x86_64
  WriteLn('vInt64Res=', vInt64Res);
{$ENDIF}
  writeLn('vCur1=',FormatCurr(',.0000', vCur1),
          ' vCur2=',FormatCurr(',.0000', vCur2),
          ' vCurRes=vCur1*vCur2=', FormatCurr(',.0000', vCurRes));
  writeLn('vCur1=',FormatCurr('.0000', vCur1),
          ' vCur2=',FormatCurr('.0000', vCur2),
          ' vCurRes=vCur1*vCur2=', FormatCurr('.0000', vCurRes));
  Write('Press enter > '); ReadLn;
end.

pgmCurrencyInt64Mix_2.pas (2,061 bytes)   
pgmCurrencyInt64Mix_2.lpi (1,658 bytes)   
<?xml version="1.0" encoding="UTF-8"?>
<CONFIG>
  <ProjectOptions>
    <Version Value="12"/>
    <PathDelim Value="\"/>
    <General>
      <Flags>
        <MainUnitHasCreateFormStatements Value="False"/>
        <MainUnitHasTitleStatement Value="False"/>
        <MainUnitHasScaledStatement Value="False"/>
      </Flags>
      <SessionStorage Value="InProjectDir"/>
      <Title Value="pgmCurrencyInt64Mix_2"/>
      <UseAppBundle Value="False"/>
      <ResourceType Value="res"/>
    </General>
    <BuildModes>
      <Item Name="Default" Default="True"/>
    </BuildModes>
    <PublishOptions>
      <Version Value="2"/>
      <UseFileFilters Value="True"/>
    </PublishOptions>
    <RunParams>
      <FormatVersion Value="2"/>
      <Modes Count="0"/>
    </RunParams>
    <Units>
      <Unit>
        <Filename Value="pgmCurrencyInt64Mix_2.pas"/>
        <IsPartOfProject Value="True"/>
      </Unit>
    </Units>
  </ProjectOptions>
  <CompilerOptions>
    <Version Value="11"/>
    <PathDelim Value="\"/>
    <Target>
      <Filename Value="pgmCurrencyInt64Mix_2"/>
    </Target>
    <SearchPaths>
      <IncludeFiles Value="$(ProjOutDir)"/>
      <UnitOutputDirectory Value="lib\$(TargetCPU)-$(TargetOS)"/>
    </SearchPaths>
    <Other>
      <CustomOptions Value="-CF64"/>
    </Other>
  </CompilerOptions>
  <Debugging>
    <Exceptions Count="3">
      <Item1>
        <Name Value="EAbort"/>
      </Item1>
      <Item2>
        <Name Value="ECodetoolError"/>
      </Item2>
      <Item3>
        <Name Value="EFOpenError"/>
      </Item3>
    </Exceptions>
  </Debugging>
</CONFIG>
pgmCurrencyInt64Mix_2.lpi (1,658 bytes)   

Florian

2019-12-18 22:44

administrator   ~0119935

As explained somewhere else, currency has it's limitations due to it's design, purpose and history. That pushing the limits works on i386 is coincidence. If you really work with currency values with such a range, I think using bcd numbers of the fmtbcd unit is the better solution. I really wonder what application it is where you have to deal with currency values of almost 100 trillion.

BrunoK

2019-12-19 11:16

reporter   ~0119942

> As explained somewhere else, currency has it's limitations due to it's design, purpose and history.
Could you point me to that, please.

> That pushing the limits works on i386 is coincidence.
So said pushing the limits did work in Quick basic compiled without the help of a floating point unit.
Maybe it can be considered that Extended 80 bit float that has a mantissa of 63 bit that match the 63 bits of Currency (one more for the sign) is a happy and very useful coincidence ...

> I really wonder what application it is where you have to deal with currency values of almost 100 trillion.
a) Actually the really interesting thing is not the 100 trillion, it is that for accounting, adding/subtracting values allows the cents when values are correctly computed to be exactly stored, this you cannot garanty with any floating point type.
b) In case of sudden hyperinflation you'll have time to adapt accounting programs, (see Papiermark, Zimbabwean dollar, Venezuelan bolĂ­var or more modestly the Argentine peso) to some kind of double float or maybe, if it exists, to a floating point BCD type.

And still the problems of transtyping to Int64 and back remain.

Thaddy de Koning

2019-12-19 15:23

reporter   ~0119947

Last edited: 2019-12-19 15:27

View 4 revisions

The trans can be solved if you would agree it is limited to a double on all platforms.
80 bit float support is simply not cross platform, even if hardware - still - supports it.
Most banking software has a resolution of 6 digits behind the comma, with just 5 being significant.
This is actually a standard and can easily be expressed in a double.
Quick Basic is useless on 64 bit.

Thaddy de Koning

2019-12-19 15:33

reporter   ~0119950

Last edited: 2019-12-19 15:34

View 2 revisions

QuickBasic is actually wrong: even in that time the 80 bit should truncate from 80 to 64 bits for currency. Yes, truncate.

BrunoK

2020-01-19 17:28

reporter   ~0120552

Last edited: 2020-01-19 17:34

View 2 revisions

Loss of information on currency multiplication and division. Particularly the multiplication.

Trunk 19.01.2020 Intel 64 bit

{ 1} // Commented current code generated for ccurrency * currency
{ 2} mov 0x4a765(%rip),%rax // Load register with multiplier
{ 3} mov 0x4a76e(%rip),%rcx // Load register with multiplicand
{ 4} imul %rax,%rcx // rcx*rax to [rdx,rax]
{ 5} movabs $0x346dc5d63886594b,%rax // Inverted 10000 multiplication ?
{ 6} imul %rcx // rcx*rax to [rdx,rax] !!! Destroys info in rdx
{ 7} // from the previous imul !!!
{ 8} // rdx data discarded !!!
{ 9} sar $0xb,%rdx // Some adjustemnts
{10} shr $0x3f,%rcx
{11} add %rcx,%rdx
{12} mov %rdx,0x4a75b(%rip) // Store

my code that does handle multiplication extending to %rdx

const
  c10e4 = QWord(10000);

                 {%rcx %rdx }
function MulCurr(aMultiplicand: Currency; constref aMultiplicator: Currency):
                 {%rax} Currency; assembler; nostackframe;
asm
.LBegin:
    mov (%rdx),%rax
    imul %rcx // [%rdx,%rax] contains 128 bit product
    mov c10e4, %rbx // Normalization of [rdx,rax]
    idiv %rbx // Normalize.
                                     // May cause "External: SIGFPE"
                                     // overflow -> EIntOverflow
.LQuit:
  ret
end ['RBX'];

and for division and accuracy

                {%rcx %rdx %rax }
function DivCurr(constref aDividend: Currency; constref aDivisor: Currency):Currency;
                                                 assembler; nostackframe;
asm
.LBegin:
    mov (%rcx),%rax; // Dividend to %eax
    mov (%rdx),%rcx // Save divisor
    mov c10e4, %rbx // Premultiply dividend for precision
    imul %rbx // [%rdx,%rax] premultiplied Dividend
    idiv %rcx // Do divide [%rdx,%rax] by divisor. <- edited comment
                                     // May cause "External: SIGFPE"
                                     // overflow -> EIntOverflow
                                     // div by 0 -> EDivByZero
.LQuit:
  ret
end ['RBX'];

Florian

2020-01-19 17:36

administrator   ~0120554

This is a solution which does not work on non x86-64 platforms. Please realize that currency has its limitations. We will not implement random work arounds on certain platforms for corner cases. If you really need a higher precision/range, use the bcd type or a gmp library everything else will cause trouble soon or later.

Florian

2020-02-02 20:47

administrator   ~0120851

I assign it to Michael so this can be documented.

Michael Van Canneyt

2020-05-13 12:30

administrator   ~0122762

I added a note that the currency must be used with care in expressions that involve numbers close to the upper limit

Issue History

Date Modified Username Field Change
2019-12-18 13:26 BrunoK New Issue
2019-12-18 13:26 BrunoK File Added: pgmCurrencyInt64Mix_2.pas
2019-12-18 13:26 BrunoK File Added: pgmCurrencyInt64Mix_2.lpi
2019-12-18 14:46 LacaK Relationship added related to 0036176
2019-12-18 22:44 Florian Note Added: 0119935
2019-12-19 11:16 BrunoK Note Added: 0119942
2019-12-19 15:23 Thaddy de Koning Note Added: 0119947
2019-12-19 15:26 Thaddy de Koning Note Edited: 0119947 View Revisions
2019-12-19 15:27 Thaddy de Koning Note Edited: 0119947 View Revisions
2019-12-19 15:27 Thaddy de Koning Note Edited: 0119947 View Revisions
2019-12-19 15:33 Thaddy de Koning Note Added: 0119950
2019-12-19 15:34 Thaddy de Koning Note Edited: 0119950 View Revisions
2020-01-19 17:28 BrunoK Note Added: 0120552
2020-01-19 17:34 BrunoK Note Edited: 0120552 View Revisions
2020-01-19 17:36 Florian Note Added: 0120554
2020-02-02 19:38 Florian Assigned To => Florian
2020-02-02 19:38 Florian Status new => resolved
2020-02-02 19:38 Florian Resolution open => no change required
2020-02-02 19:38 Florian FPCTarget => -
2020-02-02 20:47 Florian Note Added: 0120851
2020-02-02 20:48 Florian Assigned To Florian => Michael Van Canneyt
2020-02-02 20:48 Florian Status resolved => assigned
2020-02-02 20:48 Florian Category Compiler => Documentation
2020-05-13 12:30 Michael Van Canneyt Status assigned => resolved
2020-05-13 12:30 Michael Van Canneyt Fixed in Version => 3.3.1
2020-05-13 12:30 Michael Van Canneyt Fixed in Revision => 1700
2020-05-13 12:30 Michael Van Canneyt FPCTarget - => 3.2.0
2020-05-13 12:30 Michael Van Canneyt Note Added: 0122762