Re: [PATCH 1/6] aarch64: Sync system register information with Binutils

[Victor Do Nascimento]

On 10/9/23 01:02, Ramana Radhakrishnan wrote:




On 5 Oct 2023, at 14:04, Victor Do Nascimento  
wrote:

External email: Use caution opening links or attachments


On 10/5/23 12:42, Richard Earnshaw wrote:



On 03/10/2023 16:18, Victor Do Nascimento wrote:

This patch adds the `aarch64-sys-regs.def' file to GCC, teaching
the compiler about system registers known to the assembler and how
these can be used.

The macros used to hold system register information reflect those in
use by binutils, a design choice made to facilitate the sharing of data
between different parts of the toolchain.

By aligning the representation of data common to different parts of
the toolchain we can greatly reduce the duplication of work,
facilitating the maintenance of the aarch64 back-end across different
parts of the toolchain; any `SYSREG (...)' that is added in one
project can just as easily be added to its counterpart.

GCC does not implement the full range of ISA flags present in
Binutils.  Where this is the case, aliases must be added to aarch64.h
with the unknown architectural extension being mapped to its
associated base architecture, such that any flag present in Binutils
and used in system register definitions is understood in GCC.  Again,
this is done such that flags can be used interchangeably between
projects making use of the aarch64-system-regs.def file.  This is done
in the next patch in the series.

`.arch' directives missing from the emitted assembly files as a
consequence of this aliasing are accounted for by the compiler using
the S encoding of system registers when
issuing mrs/msr instructions.  This design choice ensures the
assembler will accept anything that was deemed acceptable by the
compiler.

gcc/ChangeLog:

* gcc/config/aarch64/aarch64-system-regs.def: New.
---
  gcc/config/aarch64/aarch64-sys-regs.def | 1059 +++
  1 file changed, 1059 insertions(+)
  create mode 100644 gcc/config/aarch64/aarch64-sys-regs.def


This file is supposed to be /identical/ to the one in GNU Binutils,
right?


You're right Richard.

We want the same file to be compatible with both parts of the toolchain
and, consequently, there is no compelling reason as to why the copy of
the file found in GCC should in any way diverge from its Binutils
counterpart.


If so, I think it needs to continue to say that it is part of
GNU Binutils, not part of GCC.  Ramana, has this happened before?  If
not, does the SC have a position here?



I’ve not had the time to delve into the patch, apologies.


Is the intention here to keep a copy of the file with the main copy being in 
binutils i.e. modifications are made in binutils and then sync’d with GCC at 
the same time ?


In which case the comments in the file should make the mechanics of updates 
abundantly clear.


That is indeed correct.
I will make this clear in the comments for the file.  Thanks for picking 
up on this.



Is there any reason why if the 2 versions were different, you’d have problems 
between gcc and binutils ?

If so, what kinds of problems would they be ? i.e. would they be no more than 
gas not knowing about a system register that GCC claimed to know because 
binutils and gcc were built with different versions of the system register file.


There would be no problem, should the two versions be different for 
whatever reason.  Even the issue you mention of gas not knowing about a 
system register that GCC claimed to know is circumvented.


Gcc is configured to always emit generic register names in the resulting 
asm, decoupling the system register validation mechanisms of the two 
parts of the toolchain.  If gcc deems the requirements of a particular 
system register to be satisfied, it won't trigger the assembler's 
validation mechanism when the assembly stage is reached.  Consequently, 
a stale copy of `aarch64-sys-reg.def' in binutils will bear no impact on 
gcc's execution.


Conversely, a stale `aarch64-sys-reg.def' on gcc's end will result in 
some register names not being recognized by gcc but, as in the above 
scenario, no ill-behavior will be triggered as a consequence of 
mismatches in `aarch64-sys-reg.def' version between different parts of 
the toolchain.



Speaking for myself, I do not see this request being any different from the 
requests for imports from other repositories into the GCC repository.




R.


This does raise a very interesting question on the intellectual property
front and one that is well beyond my competence to opine about.

Nonetheless, this is a question which may arise again if we abstract
away more target description data into such .def files, as has been
discussed for architectural feature flags (for example).

So what might be nice (but not necessarily tenable) is if we had
appropriate provisions in place for where files were shared across
different parts of the toolchain.

Something like "This file is a shared resource of GCC and Binutils."




This model of an additional shared repository with a 

Re: [PATCH 1/6] aarch64: Sync system register information with Binutils

[Ramana Radhakrishnan]

> On 5 Oct 2023, at 14:04, Victor Do Nascimento  
> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> On 10/5/23 12:42, Richard Earnshaw wrote:
>> 
>> 
>> On 03/10/2023 16:18, Victor Do Nascimento wrote:
>>> This patch adds the `aarch64-sys-regs.def' file to GCC, teaching
>>> the compiler about system registers known to the assembler and how
>>> these can be used.
>>> 
>>> The macros used to hold system register information reflect those in
>>> use by binutils, a design choice made to facilitate the sharing of data
>>> between different parts of the toolchain.
>>> 
>>> By aligning the representation of data common to different parts of
>>> the toolchain we can greatly reduce the duplication of work,
>>> facilitating the maintenance of the aarch64 back-end across different
>>> parts of the toolchain; any `SYSREG (...)' that is added in one
>>> project can just as easily be added to its counterpart.
>>> 
>>> GCC does not implement the full range of ISA flags present in
>>> Binutils.  Where this is the case, aliases must be added to aarch64.h
>>> with the unknown architectural extension being mapped to its
>>> associated base architecture, such that any flag present in Binutils
>>> and used in system register definitions is understood in GCC.  Again,
>>> this is done such that flags can be used interchangeably between
>>> projects making use of the aarch64-system-regs.def file.  This is done
>>> in the next patch in the series.
>>> 
>>> `.arch' directives missing from the emitted assembly files as a
>>> consequence of this aliasing are accounted for by the compiler using
>>> the S encoding of system registers when
>>> issuing mrs/msr instructions.  This design choice ensures the
>>> assembler will accept anything that was deemed acceptable by the
>>> compiler.
>>> 
>>> gcc/ChangeLog:
>>> 
>>>* gcc/config/aarch64/aarch64-system-regs.def: New.
>>> ---
>>>  gcc/config/aarch64/aarch64-sys-regs.def | 1059 +++
>>>  1 file changed, 1059 insertions(+)
>>>  create mode 100644 gcc/config/aarch64/aarch64-sys-regs.def
>> 
>> This file is supposed to be /identical/ to the one in GNU Binutils,
>> right?
> 
> You're right Richard.
> 
> We want the same file to be compatible with both parts of the toolchain
> and, consequently, there is no compelling reason as to why the copy of
> the file found in GCC should in any way diverge from its Binutils
> counterpart.
> 
>> If so, I think it needs to continue to say that it is part of
>> GNU Binutils, not part of GCC.  Ramana, has this happened before?  If
>> not, does the SC have a position here?
>> 

I’ve not had the time to delve into the patch, apologies.


Is the intention here to keep a copy of the file with the main copy being in 
binutils i.e. modifications are made in binutils and then sync’d with GCC at 
the same time ?


In which case the comments in the file should make the mechanics of updates 
abundantly clear.

Is there any reason why if the 2 versions were different, you’d have problems 
between gcc and binutils ? 

If so, what kinds of problems would they be ? i.e. would they be no more than 
gas not knowing about a system register that GCC claimed to know because 
binutils and gcc were built with different versions of the system register 
file. 

Speaking for myself, I do not see this request being any different from the 
requests for imports from other repositories into the GCC repository.



>> R.
> 
> This does raise a very interesting question on the intellectual property
> front and one that is well beyond my competence to opine about.
> 
> Nonetheless, this is a question which may arise again if we abstract
> away more target description data into such .def files, as has been
> discussed for architectural feature flags (for example).
> 
> So what might be nice (but not necessarily tenable) is if we had
> appropriate provisions in place for where files were shared across
> different parts of the toolchain.
> 
> Something like "This file is a shared resource of GCC and Binutils."



This model of an additional shared repository with a build dependency will 
transfer the “copy in every dependent repository” to a dependency on a 
“suitable hash in every dependent repository” problem which is certainly 
something to consider

And then the question comes for the GCC project about how many such 
dependencies with different repositories it tracks :) ? Perhaps git submodules 
can be considered , however I am not sure how much that has been looked at 
after the git conversion. 



regards
Ramana




> 
> Anyway, that's my two cents on the matter :).
> 
> Let's see what Ramana has to say on the matter.
> 
> V.
> 
> 
>>> diff --git a/gcc/config/aarch64/aarch64-sys-regs.def
>>> b/gcc/config/aarch64/aarch64-sys-regs.def
>>> new file mode 100644
>>> index 000..d77fee1d5e3
>>> --- /dev/null
>>> +++ b/gcc/config/aarch64/aarch64-sys-regs.def
>>> @@ -0,0 +1,1059 @@
>>> +/* Copyright (C) 2023 Free Software 

Re: [PATCH 1/6] aarch64: Sync system register information with Binutils

[Victor Do Nascimento]

On 10/5/23 12:42, Richard Earnshaw wrote:



On 03/10/2023 16:18, Victor Do Nascimento wrote:

This patch adds the `aarch64-sys-regs.def' file to GCC, teaching
the compiler about system registers known to the assembler and how
these can be used.

The macros used to hold system register information reflect those in
use by binutils, a design choice made to facilitate the sharing of data
between different parts of the toolchain.

By aligning the representation of data common to different parts of
the toolchain we can greatly reduce the duplication of work,
facilitating the maintenance of the aarch64 back-end across different
parts of the toolchain; any `SYSREG (...)' that is added in one
project can just as easily be added to its counterpart.

GCC does not implement the full range of ISA flags present in
Binutils.  Where this is the case, aliases must be added to aarch64.h
with the unknown architectural extension being mapped to its
associated base architecture, such that any flag present in Binutils
and used in system register definitions is understood in GCC.  Again,
this is done such that flags can be used interchangeably between
projects making use of the aarch64-system-regs.def file.  This is done
in the next patch in the series.

`.arch' directives missing from the emitted assembly files as a
consequence of this aliasing are accounted for by the compiler using
the S encoding of system registers when
issuing mrs/msr instructions.  This design choice ensures the
assembler will accept anything that was deemed acceptable by the
compiler.

gcc/ChangeLog:

* gcc/config/aarch64/aarch64-system-regs.def: New.
---
  gcc/config/aarch64/aarch64-sys-regs.def | 1059 +++
  1 file changed, 1059 insertions(+)
  create mode 100644 gcc/config/aarch64/aarch64-sys-regs.def


This file is supposed to be /identical/ to the one in GNU Binutils, 
right?


You're right Richard.

We want the same file to be compatible with both parts of the toolchain 
and, consequently, there is no compelling reason as to why the copy of 
the file found in GCC should in any way diverge from its Binutils 
counterpart.


If so, I think it needs to continue to say that it is part of 
GNU Binutils, not part of GCC.  Ramana, has this happened before?  If 
not, does the SC have a position here?


R.


This does raise a very interesting question on the intellectual property 
front and one that is well beyond my competence to opine about.


Nonetheless, this is a question which may arise again if we abstract 
away more target description data into such .def files, as has been 
discussed for architectural feature flags (for example).


So what might be nice (but not necessarily tenable) is if we had 
appropriate provisions in place for where files were shared across 
different parts of the toolchain.


Something like "This file is a shared resource of GCC and Binutils."

Anyway, that's my two cents on the matter :).

Let's see what Ramana has to say on the matter.

V.


diff --git a/gcc/config/aarch64/aarch64-sys-regs.def 
b/gcc/config/aarch64/aarch64-sys-regs.def

new file mode 100644
index 000..d77fee1d5e3
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-sys-regs.def
@@ -0,0 +1,1059 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+   Contributed by Arm Ltd
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+/* Array of system registers and their associated arch features.
+
+   Before using #include to read this file, define a macro:
+
+ SYSREG (name, encoding, flags, features)
+
+  The NAME is the system register name, as recognized by the
+  assembler.  ENCODING provides the necessary information for the binary
+  encoding of the system register.  The FLAGS field is a bitmask of
+  relevant behavior information pertaining to the particular register.
+  For example: is it read/write-only? does it alias another register?
+  The FEATURES field maps onto ISA flags and specifies the architectural
+  feature requirements of the system register.  */
+
+  SYSREG ("accdata_el1",    CPENC (3,0,13,0,5),    0,
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el1",    CPENC (3,0,1,0,1),    0,
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el2",    CPENC (3,4,1,0,1),    0,
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el3",    CPENC (3,6,1,0,1),    0,

Re: [PATCH 1/6] aarch64: Sync system register information with Binutils

[Richard Earnshaw]

On 03/10/2023 16:18, Victor Do Nascimento wrote:

This patch adds the `aarch64-sys-regs.def' file to GCC, teaching
the compiler about system registers known to the assembler and how
these can be used.

The macros used to hold system register information reflect those in
use by binutils, a design choice made to facilitate the sharing of data
between different parts of the toolchain.

By aligning the representation of data common to different parts of
the toolchain we can greatly reduce the duplication of work,
facilitating the maintenance of the aarch64 back-end across different
parts of the toolchain; any `SYSREG (...)' that is added in one
project can just as easily be added to its counterpart.

GCC does not implement the full range of ISA flags present in
Binutils.  Where this is the case, aliases must be added to aarch64.h
with the unknown architectural extension being mapped to its
associated base architecture, such that any flag present in Binutils
and used in system register definitions is understood in GCC.  Again,
this is done such that flags can be used interchangeably between
projects making use of the aarch64-system-regs.def file.  This is done
in the next patch in the series.

`.arch' directives missing from the emitted assembly files as a
consequence of this aliasing are accounted for by the compiler using
the S encoding of system registers when
issuing mrs/msr instructions.  This design choice ensures the
assembler will accept anything that was deemed acceptable by the
compiler.

gcc/ChangeLog:

* gcc/config/aarch64/aarch64-system-regs.def: New.
---
  gcc/config/aarch64/aarch64-sys-regs.def | 1059 +++
  1 file changed, 1059 insertions(+)
  create mode 100644 gcc/config/aarch64/aarch64-sys-regs.def


This file is supposed to be /identical/ to the one in GNU Binutils, 
right?  If so, I think it needs to continue to say that it is part of 
GNU Binutils, not part of GCC.  Ramana, has this happened before?  If 
not, does the SC have a position here?


R.


diff --git a/gcc/config/aarch64/aarch64-sys-regs.def 
b/gcc/config/aarch64/aarch64-sys-regs.def
new file mode 100644
index 000..d77fee1d5e3
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-sys-regs.def
@@ -0,0 +1,1059 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+   Contributed by Arm Ltd
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+/* Array of system registers and their associated arch features.
+
+   Before using #include to read this file, define a macro:
+
+ SYSREG (name, encoding, flags, features)
+
+  The NAME is the system register name, as recognized by the
+  assembler.  ENCODING provides the necessary information for the binary
+  encoding of the system register.  The FLAGS field is a bitmask of
+  relevant behavior information pertaining to the particular register.
+  For example: is it read/write-only? does it alias another register?
+  The FEATURES field maps onto ISA flags and specifies the architectural
+  feature requirements of the system register.  */
+
+  SYSREG ("accdata_el1", CPENC (3,0,13,0,5), 0,  
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el1",   CPENC (3,0,1,0,1),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el2",   CPENC (3,4,1,0,1),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el3",   CPENC (3,6,1,0,1),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("afsr0_el1",   CPENC (3,0,5,1,0),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("afsr0_el12",  CPENC (3,5,5,1,0),  F_ARCHEXT,
  AARCH64_FEATURE (V8_1A))
+  SYSREG ("afsr0_el2",   CPENC (3,4,5,1,0),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("afsr0_el3",   CPENC (3,6,5,1,0),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("afsr1_el1",   CPENC (3,0,5,1,1),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("afsr1_el12",  CPENC (3,5,5,1,1),  F_ARCHEXT,
  AARCH64_FEATURE (V8_1A))
+  SYSREG ("afsr1_el2",   CPENC (3,4,5,1,1),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("afsr1_el3",   CPENC (3,6,5,1,1),  0,
  AARCH64_NO_FEATURES)
+  SYSREG ("aidr_el1",   

[PATCH 1/6] aarch64: Sync system register information with Binutils

[Victor Do Nascimento]

This patch adds the `aarch64-sys-regs.def' file to GCC, teaching
the compiler about system registers known to the assembler and how
these can be used.

The macros used to hold system register information reflect those in
use by binutils, a design choice made to facilitate the sharing of data
between different parts of the toolchain.

By aligning the representation of data common to different parts of
the toolchain we can greatly reduce the duplication of work,
facilitating the maintenance of the aarch64 back-end across different
parts of the toolchain; any `SYSREG (...)' that is added in one
project can just as easily be added to its counterpart.

GCC does not implement the full range of ISA flags present in
Binutils.  Where this is the case, aliases must be added to aarch64.h
with the unknown architectural extension being mapped to its
associated base architecture, such that any flag present in Binutils
and used in system register definitions is understood in GCC.  Again,
this is done such that flags can be used interchangeably between
projects making use of the aarch64-system-regs.def file.  This is done
in the next patch in the series.

`.arch' directives missing from the emitted assembly files as a
consequence of this aliasing are accounted for by the compiler using
the S encoding of system registers when
issuing mrs/msr instructions.  This design choice ensures the
assembler will accept anything that was deemed acceptable by the
compiler.

gcc/ChangeLog:

* gcc/config/aarch64/aarch64-system-regs.def: New.
---
 gcc/config/aarch64/aarch64-sys-regs.def | 1059 +++
 1 file changed, 1059 insertions(+)
 create mode 100644 gcc/config/aarch64/aarch64-sys-regs.def

diff --git a/gcc/config/aarch64/aarch64-sys-regs.def 
b/gcc/config/aarch64/aarch64-sys-regs.def
new file mode 100644
index 000..d77fee1d5e3
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-sys-regs.def
@@ -0,0 +1,1059 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+   Contributed by Arm Ltd
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+/* Array of system registers and their associated arch features.
+
+   Before using #include to read this file, define a macro:
+
+ SYSREG (name, encoding, flags, features)
+
+  The NAME is the system register name, as recognized by the
+  assembler.  ENCODING provides the necessary information for the binary
+  encoding of the system register.  The FLAGS field is a bitmask of
+  relevant behavior information pertaining to the particular register.
+  For example: is it read/write-only? does it alias another register?
+  The FEATURES field maps onto ISA flags and specifies the architectural
+  feature requirements of the system register.  */
+
+  SYSREG ("accdata_el1",   CPENC (3,0,13,0,5), 0,  
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el1", CPENC (3,0,1,0,1),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el2", CPENC (3,4,1,0,1),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("actlr_el3", CPENC (3,6,1,0,1),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("afsr0_el1", CPENC (3,0,5,1,0),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("afsr0_el12",CPENC (3,5,5,1,0),  F_ARCHEXT,  
AARCH64_FEATURE (V8_1A))
+  SYSREG ("afsr0_el2", CPENC (3,4,5,1,0),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("afsr0_el3", CPENC (3,6,5,1,0),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("afsr1_el1", CPENC (3,0,5,1,1),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("afsr1_el12",CPENC (3,5,5,1,1),  F_ARCHEXT,  
AARCH64_FEATURE (V8_1A))
+  SYSREG ("afsr1_el2", CPENC (3,4,5,1,1),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("afsr1_el3", CPENC (3,6,5,1,1),  0,  
AARCH64_NO_FEATURES)
+  SYSREG ("aidr_el1",  CPENC (3,1,0,0,7),  F_REG_READ, 
AARCH64_NO_FEATURES)
+  SYSREG ("allint",CPENC (3,0,4,3,0),  F_ARCHEXT,  
AARCH64_FEATURE (V8_8A))
+  SYSREG ("amair_el1", CPENC (3,0,10,3,0), 0,  
AARCH64_NO_FEATURES)
+  SYSREG ("amair_el12",CPENC (3,5,10,3,0), F_ARCHEXT,