View Issue Details

IDProjectCategoryView StatusLast Update
0038695FPCCompilerpublic2021-04-02 18:59
ReporterJ. Gareth Moreton Assigned ToFlorian  
PriorityhighSeveritymajorReproducibilityN/A
Status resolvedResolutionfixed 
Platformaarch64-linuxOSLInux (Raspberry Pi OS) 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0038695: [Patch] AArch64 incorrect number generation fix
DescriptionThis patch fixes a bug in AArch64's a_load_const_reg routine where certain constants were not encoded properly. This, ironically, manifested in a_load_const_reg itself and caused inefficient (but correct) code generation for other numbers.
Steps To ReproduceApply patch and confirm both correct code compilation and more efficient constant generation in the system unit's disassembly (among other things).
Additional InformationIn the aforementioned a_load_const_reg routine, the constant $0000FFFFFFFEFFFF was instead generated as $FFFEFFFFFFFEFFFF, causing a case block to not be evaluated correctly. The class of numbers that were incorrectly encoded are those where replacing the upper 16 bits with the 2nd lowest 16 bits produced a valid shifter constant, since this could be encoded compactly as (using $0000FFFFFFFEFFFF as an example):

    orr x0,xzr,0xFFFEFFFFFFFEFFFF
    movk x0,0,lsl 48

Previously, the "movk" instruction was erroneously omitted if the upper 16 bits were zero.

----

For an inefficient code example, the System unit had many such constants - for the number 18.874,385 ($01200011):

    movn x1,65518
    movk x1,288,lsl 16
    movk x1,0,lsl 32
    movk x1,0,lsl 48

This patch now allows the routine to properly encode the constant as expected:

    movz x1,17
    movk x1,288,lsl 16
Tagsaarch64, compiler, patch
Fixed in Revision49104
FPCOldBugId
FPCTarget-
Attached Files

Relationships

related to 0038053 closedJonas Maebe AArch64 -O2 bug not seen with -O1 
child of 0037554 resolvedFlorian AArch64 generates incorrect code for some immediate values 

Activities

J. Gareth Moreton

2021-04-02 03:05

developer  

number-generation-fix.patch (2,685 bytes)   
Index: compiler/aarch64/cgcpu.pas
===================================================================
--- compiler/aarch64/cgcpu.pas	(revision 49099)
+++ compiler/aarch64/cgcpu.pas	(working copy)
@@ -588,7 +588,7 @@
         leftover_a: word;
       begin
 {$ifdef extdebug}
-        list.concat(tai_comment.Create(strpnew('Generating constant ' + tostr(a))));
+        list.concat(tai_comment.Create(strpnew('Generating constant ' + tostr(a) + ' / $' + hexstr(a, 16))));
 {$endif extdebug}
         case a of
           { Small positive number }
@@ -623,7 +623,7 @@
                       Exit;
                     end;
 
-                  { This determines whether this write can be peformed with an ORR followed by MOVK
+                  { This determines whether this write can be performed with an ORR followed by MOVK
                     by copying the 2nd word to the 4th word for the ORR constant, then overwriting
                     the 4th word (unless the word is.  The alternative would require 3 instructions }
                   leftover_a := word(a shr 48);
@@ -644,14 +644,15 @@
                     called for a and it returned False.  Reduces processing time. [Kit] }
                   if (manipulated_a <> a) and is_shifter_const(manipulated_a, size) then
                     begin
+                      { Encode value as:
+                          orr  reg,xzr,manipulated_a
+                          movk reg,#(leftover_a),lsl #48
+                      }
                       list.concat(taicpu.op_reg_reg_const(A_ORR, reg, makeregsize(NR_XZR, size), manipulated_a));
-                      if (leftover_a <> 0) then
-                        begin
-                          shifterop_reset(so);
-                          so.shiftmode := SM_LSL;
-                          so.shiftimm := 48;
-                          list.concat(taicpu.op_reg_const_shifterop(A_MOVK, reg, leftover_a, so));
-                        end;
+                      shifterop_reset(so);
+                      so.shiftmode := SM_LSL;
+                      so.shiftimm := 48;
+                      list.concat(taicpu.op_reg_const_shifterop(A_MOVK, reg, leftover_a, so));
                       Exit;
                     end;
 
@@ -664,7 +665,7 @@
                           stored as the first 16 bits followed by a shifter constant }
                         case a of
                           TCgInt($FFFF0000FFFF0000)..TCgInt($FFFF0000FFFFFFFF):
-                            doinverted := False
+                            doinverted := False;
                           else
                             begin
                               doinverted := True;
number-generation-fix.patch (2,685 bytes)   

J. Gareth Moreton

2021-04-02 03:07

developer   ~0130032

This bug is also possibly a cause behind 0038053

Issue History

Date Modified Username Field Change
2021-04-02 03:05 J. Gareth Moreton New Issue
2021-04-02 03:05 J. Gareth Moreton File Added: number-generation-fix.patch
2021-04-02 03:05 J. Gareth Moreton Tag Attached: patch
2021-04-02 03:05 J. Gareth Moreton Tag Attached: aarch64
2021-04-02 03:05 J. Gareth Moreton Tag Attached: compiler
2021-04-02 03:06 J. Gareth Moreton Relationship added related to 0038053
2021-04-02 03:07 J. Gareth Moreton Priority normal => high
2021-04-02 03:07 J. Gareth Moreton Severity minor => major
2021-04-02 03:07 J. Gareth Moreton Additional Information Updated View Revisions
2021-04-02 03:07 J. Gareth Moreton FPCTarget => -
2021-04-02 03:07 J. Gareth Moreton Note Added: 0130032
2021-04-02 03:11 J. Gareth Moreton Additional Information Updated View Revisions
2021-04-02 03:13 J. Gareth Moreton Additional Information Updated View Revisions
2021-04-02 04:22 J. Gareth Moreton Relationship added child of 0037554
2021-04-02 18:59 Florian Assigned To => Florian
2021-04-02 18:59 Florian Status new => resolved
2021-04-02 18:59 Florian Resolution open => fixed
2021-04-02 18:59 Florian Fixed in Version => 3.3.1
2021-04-02 18:59 Florian Fixed in Revision => 49104