View Issue Details

IDProjectCategoryView StatusLast Update
0038053FPCCompilerpublic2021-04-15 19:16
ReporterChris Rorden Assigned ToJonas Maebe  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformApple Developer Transition KitOSDarwin 
Product Version3.3.1 
Fixed in Version3.2.2 
Summary0038053: AArch64 -O2 bug not seen with -O1
DescriptionCompiling the example software with -O2 reports a bogus value for variable fsz:
  fpc -O2 fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif
But same code works when compiled as -O1. In this example, removing the unused variable "ok1" resolves the problem. Things seem to get confused when there are too many variables being juggled.
Steps To ReproduceIncorrect file size reported when compiled with -O2:

$fpc -O2 fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif
Free Pascal Compiler version 3.3.1 [2020/11/07] for aarch64
Copyright (c) 1993-2020 by Florian Klaempfl and others
Target OS: Darwin for AArch64
Compiling fsz.pas
fsz.pas(80,11) Note: Local variable "ok1" not used
Assembling fsz
Linking fsz
264 lines compiled, 0.2 sec
1 note(s) issued
-rw-r--r--@ 1 chris staff 1050090 Oct 4 2016 dipso002.tif
>>> 1050090
Filesize 1050090
>>> 1050090
TIFF Filesize 248984
ReportTiff Error:IFDstart offset (1048584) exceeds filesize (248984)


Correct file size reported when compiled with -O1:

$fpc -O1 fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif
Free Pascal Compiler version 3.3.1 [2020/11/07] for aarch64
Copyright (c) 1993-2020 by Florian Klaempfl and others
Target OS: Darwin for AArch64
Compiling fsz.pas
fsz.pas(80,11) Note: Local variable "ok1" not used
Assembling fsz
Linking fsz
264 lines compiled, 0.2 sec
1 note(s) issued
-rw-r--r--@ 1 chris staff 1050090 Oct 4 2016 dipso002.tif
>>> 1050090
Filesize 1050090
>>> 1050090
TIFF Filesize 1050090
ReportTiff: OK
TagsNo tags attached.
Fixed in Revision49206, 49207 - 49208
FPCOldBugId
FPCTarget3.2.2
Attached Files

Relationships

related to 0038055 closedFlorian Compiling Lazarus for AArch64 with -O3 creates non-functional code 
related to 0038129 resolvedFlorian Wrong code generated on x86-64 
related to 0038695 resolvedFlorian [Patch] AArch64 incorrect number generation fix 

Activities

Chris Rorden

2020-11-08 22:10

reporter  

aarch64.zip (373,372 bytes)

Jan Bruns

2020-11-10 15:42

reporter   ~0126822

I failed to reproduce on linux64 with release and current trunk builds.
Could you tell the exact svn version your're using?

Chris Rorden

2020-11-10 21:41

reporter   ~0126828

Jan Bruns: Issue is specific to AArch64 (ARM) CPUs. This does not impact x86-64 compiles. Tested with SVN is 47362

J. Gareth Moreton

2021-01-20 05:35

developer   ~0128443

This one seems to be a bit more tricky. I can see the problem in the disassembly, although I'm not sure why the code is going bad. What I can tell is that it's writing an offset to a register that's in use (the same register that contains the actual correct size) and the bug seems to be in the node's second pass that coverts the tree into assembly language (I used DEBUG_NODE_XML to export the node trees, and there's nothing apparently wrong with it for fsz).

J. Gareth Moreton

2021-01-20 14:11

developer   ~0128450

Last edited: 2021-01-20 14:12

View 2 revisions

It looks like there's more than one issue at play with this one - building the compiler with optimisations turned off causes the final binary to generate different code for fsz (in particular, AArch64's a_load_const_reg generates inefficient code when optimisations are turned on, but thankfully still produces correct constants) - however, the main bug of it overwriting a register that's in use is still present.

Investigations continue.

For clarity, with optimisations turned off when building the compiler, code for the constant "248984" is the following:

movz x1,52376
movk x1,3,lsl 16

But under -O2, we get the following instead:

movn x1,13159
movk x1,3,lsl 16
movk x1,0,lsl 32
movk x1,0,lsl 48

Somehow the condition controlled by "doInverted" gets flipped.

J. Gareth Moreton

2021-04-02 22:46

developer   ~0130053

Last edited: 2021-04-02 22:46

View 2 revisions

Chris, can you confirm if the problem still exists as of 49104?

Chris Rorden

2021-04-03 11:16

reporter   ~0130059

I compiled the latest Git release(66065a43077633d9e66d43d44735b16560329b5a) which I presume includes 49104 (not sure how to compare git and SVN versions). The prior error still exists at O2, though as before O1 works:

aarch64 > fpc -O2 fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif
Free Pascal Compiler version 3.3.1 [2021/04/02] for aarch64
Copyright (c) 1993-2021 by Florian Klaempfl and others
Target OS: Darwin for AArch64
Compiling fsz.pas
fsz.pas(80,11) Note: Local variable "ok1" not used
Assembling (pipe) fsz.s
Linking fsz
264 lines compiled, 0.2 sec
1 note(s) issued
-rw-r--r--@ 1 chrisrorden staff 1050090 Oct 4 2016 dipso002.tif
>>> 1050090
Filesize 1050090
>>> 1050090
TIFF Filesize 248984
ReportTiff Error:IFDstart offset (1048584) exceeds filesize (248984)

aarch64 > fpc -O1 fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif
Free Pascal Compiler version 3.3.1 [2021/04/02] for aarch64
Copyright (c) 1993-2021 by Florian Klaempfl and others
Target OS: Darwin for AArch64
Compiling fsz.pas
fsz.pas(80,11) Note: Local variable "ok1" not used
Assembling (pipe) fsz.s
Linking fsz
264 lines compiled, 0.1 sec
1 note(s) issued
-rw-r--r--@ 1 chrisrorden staff 1050090 Oct 4 2016 dipso002.tif
>>> 1050090
Filesize 1050090
>>> 1050090
TIFF Filesize 1050090
ReportTiff: OK

Chris Rorden

2021-04-03 11:35

reporter   ~0130060

Kit, the issue seems to have to do with REGVAR
  https://wiki.freepascal.org/Optimization
the following compiles both create the error:

fpc -O1 -OoREGVAR fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif
fpc -O1 -OoNoPEEPHOLE -OoREGVAR fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif

However, the following creates the correct solution
 fpc -O4 -OoNoREGVAR fsz.pas; ls -l dipso002.tif; ./fsz dipso002.tif

J. Gareth Moreton

2021-04-03 16:32

developer   ~0130067

That's a little irritating. I'll keep at it.

Jonas Maebe

2021-04-14 21:00

manager   ~0130383

There were indeed two issues:
* one in the definition of the kind of operation type of movk (it both reads and writes its first operand, rather than only write it)
* one in the generation of spilling instructions for integer registers when the spilling offset cannot be encoded directly in the load/store instruction

When you see code that does not match at all what the code generator is generating, a good starting approach is to compile with -al -sr. This will generate the assembly file pre-register allocation. If that one looks good, then you know the issue is in the register allocator/spilling code. That said, while the movk issue was fairly easy to find afterwards, the second one was still quite hard to figure out.

J. Gareth Moreton

2021-04-15 04:28

developer   ~0130387

Last edited: 2021-04-15 04:32

View 2 revisions

I'm so glad it's finally fixed! Thanks Jonas.

In all honesty I don't think I would have worked that out without help. I've learnt a few things.

Chris Rorden

2021-04-15 10:52

reporter   ~0130391

Confirmed that this fixes the issue Thanks!

Issue History

Date Modified Username Field Change
2020-11-08 22:10 Chris Rorden New Issue
2020-11-08 22:10 Chris Rorden File Added: aarch64.zip
2020-11-08 22:58 J. Gareth Moreton Assigned To => J. Gareth Moreton
2020-11-08 22:58 J. Gareth Moreton Status new => assigned
2020-11-10 15:42 Jan Bruns Note Added: 0126822
2020-11-10 21:41 Chris Rorden Note Added: 0126828
2020-11-25 19:50 J. Gareth Moreton Relationship added related to 0038055
2020-11-28 15:00 J. Gareth Moreton Relationship added related to 0038129
2021-01-20 05:35 J. Gareth Moreton Note Added: 0128443
2021-01-20 14:11 J. Gareth Moreton Note Added: 0128450
2021-01-20 14:12 J. Gareth Moreton Note Edited: 0128450 View Revisions
2021-04-02 01:06 J. Gareth Moreton Relationship added related to 0038695
2021-04-02 22:46 J. Gareth Moreton Status assigned => feedback
2021-04-02 22:46 J. Gareth Moreton FPCTarget => -
2021-04-02 22:46 J. Gareth Moreton Note Added: 0130053
2021-04-02 22:46 J. Gareth Moreton Note Edited: 0130053 View Revisions
2021-04-03 11:16 Chris Rorden Note Added: 0130059
2021-04-03 11:16 Chris Rorden Status feedback => assigned
2021-04-03 11:35 Chris Rorden Note Added: 0130060
2021-04-03 16:32 J. Gareth Moreton Note Added: 0130067
2021-04-14 21:00 Jonas Maebe Assigned To J. Gareth Moreton => Jonas Maebe
2021-04-14 21:00 Jonas Maebe Status assigned => resolved
2021-04-14 21:00 Jonas Maebe Resolution open => fixed
2021-04-14 21:00 Jonas Maebe Fixed in Version => 3.3.1
2021-04-14 21:00 Jonas Maebe Fixed in Revision => 49206, 49207
2021-04-14 21:00 Jonas Maebe FPCTarget - => 3.2.2
2021-04-14 21:00 Jonas Maebe Note Added: 0130383
2021-04-15 04:28 J. Gareth Moreton Note Added: 0130387
2021-04-15 04:32 J. Gareth Moreton Note Edited: 0130387 View Revisions
2021-04-15 10:52 Chris Rorden Status resolved => closed
2021-04-15 10:52 Chris Rorden Note Added: 0130391
2021-04-15 19:16 Jonas Maebe Fixed in Version 3.3.1 => 3.2.2
2021-04-15 19:16 Jonas Maebe Fixed in Revision 49206, 49207 => 49206, 49207 - 49208