Re: GCN: Remove 'FIRST_{SGPR,VGPR,AVGPR}_REG', 'LAST_{SGPR,VGPR,AVGPR}_REG' from machine description

[Andrew Stubbs]

On 31/01/2024 17:21, Thomas Schwinge wrote:

Hi!

On 2018-12-12T11:52:23+, Andrew Stubbs  wrote:

This patch contains the machine description portion of the GCN back-end.  [...]



--- /dev/null
+++ b/gcc/config/gcn/gcn.md



+;; {{{ Constants and enums
+
+; Named registers
+(define_constants
+  [(FIRST_SGPR_REG  0)
+   (LAST_SGPR_REG   101)
+   (FLAT_SCRATCH_REG102)
+   (FLAT_SCRATCH_LO_REG 102)
+   (FLAT_SCRATCH_HI_REG 103)
+   (XNACK_MASK_REG  104)
+   (XNACK_MASK_LO_REG   104)
+   (XNACK_MASK_HI_REG   105)
+   (VCC_REG 106)
+   (VCC_LO_REG  106)
+   (VCC_HI_REG  107)
+   (VCCZ_REG108)
+   (TBA_REG 109)
+   (TBA_LO_REG  109)
+   (TBA_HI_REG  110)
+   (TMA_REG 111)
+   (TMA_LO_REG  111)
+   (TMA_HI_REG  112)
+   (TTMP0_REG   113)
+   (TTMP11_REG  124)
+   (M0_REG  125)
+   (EXEC_REG126)
+   (EXEC_LO_REG 126)
+   (EXEC_HI_REG 127)
+   (EXECZ_REG   128)
+   (SCC_REG 129)
+   (FIRST_VGPR_REG  160)
+   (LAST_VGPR_REG   415)])
+
+(define_constants
+  [(SP_REGNUM 16)
+   (LR_REGNUM 18)
+   (AP_REGNUM 416)
+   (FP_REGNUM 418)])


Oops, these last two are actually wrong, since AVGPRs were inserted!



Generally, shouldn't there be a better way, that avoids duplication and
instead shares such definitions between 'gcn.h' and 'gcn.md'?


I think this is stuff we originally inherited from the Honza's partial 
port and I just never questioned?


If the definitions are unused then it's fine to remove them (I imagine 
the TBA, TMA, and TTMP registers are also unused). Is there something 
about define_constants that is different to external macros? Does it 
affect the mddump? -fdump output? ICE messages? If there's no difference 
then I'm happy with just deleting the lot and use the gcn.h definitions 
exclusively.



Until that's settled, OK to push the attached
"GCN: Remove 'FIRST_{SGPR,VGPR,AVGPR}_REG', 'LAST_{SGPR,VGPR,AVGPR}_REG' from 
machine description"?
(I assume "still builds" is sufficient validation of this change.)


The patch is OK.

Andrew

GCN: Remove 'FIRST_{SGPR,VGPR,AVGPR}_REG', 'LAST_{SGPR,VGPR,AVGPR}_REG' from machine description (was: [PATCH v3 04/10] GCN machine description)

[Thomas Schwinge]

Hi!

On 2018-12-12T11:52:23+, Andrew Stubbs  wrote:
> This patch contains the machine description portion of the GCN back-end.  
> [...]

> --- /dev/null
> +++ b/gcc/config/gcn/gcn.md

> +;; {{{ Constants and enums
> +
> +; Named registers
> +(define_constants
> +  [(FIRST_SGPR_REG0)
> +   (LAST_SGPR_REG 101)
> +   (FLAT_SCRATCH_REG  102)
> +   (FLAT_SCRATCH_LO_REG   102)
> +   (FLAT_SCRATCH_HI_REG   103)
> +   (XNACK_MASK_REG104)
> +   (XNACK_MASK_LO_REG 104)
> +   (XNACK_MASK_HI_REG 105)
> +   (VCC_REG   106)
> +   (VCC_LO_REG106)
> +   (VCC_HI_REG107)
> +   (VCCZ_REG  108)
> +   (TBA_REG   109)
> +   (TBA_LO_REG109)
> +   (TBA_HI_REG110)
> +   (TMA_REG   111)
> +   (TMA_LO_REG111)
> +   (TMA_HI_REG112)
> +   (TTMP0_REG 113)
> +   (TTMP11_REG124)
> +   (M0_REG125)
> +   (EXEC_REG  126)
> +   (EXEC_LO_REG   126)
> +   (EXEC_HI_REG   127)
> +   (EXECZ_REG 128)
> +   (SCC_REG   129)
> +   (FIRST_VGPR_REG160)
> +   (LAST_VGPR_REG 415)])
> +
> +(define_constants
> +  [(SP_REGNUM 16)
> +   (LR_REGNUM 18)
> +   (AP_REGNUM 416)
> +   (FP_REGNUM 418)])

Generally, shouldn't there be a better way, that avoids duplication and
instead shares such definitions between 'gcn.h' and 'gcn.md'?

Until that's settled, OK to push the attached
"GCN: Remove 'FIRST_{SGPR,VGPR,AVGPR}_REG', 'LAST_{SGPR,VGPR,AVGPR}_REG' from 
machine description"?
(I assume "still builds" is sufficient validation of this change.)


Grüße
 Thomas


>From 6af4774b4574086f5d4925333406eab4fed7f9a5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 31 Jan 2024 13:27:34 +0100
Subject: [PATCH] GCN: Remove 'FIRST_{SGPR,VGPR,AVGPR}_REG',
 'LAST_{SGPR,VGPR,AVGPR}_REG' from machine description

They're not used there, and we avoid potentially out-of-sync definitions.

	gcc/
	* config/gcn/gcn.md (FIRST_SGPR_REG, LAST_SGPR_REG)
	(FIRST_VGPR_REG, LAST_VGPR_REG, FIRST_AVGPR_REG, LAST_AVGPR_REG):
	Don't 'define_constants'.
---
 gcc/config/gcn/gcn.md | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/gcc/config/gcn/gcn.md b/gcc/config/gcn/gcn.md
index 1f3c692b7a67..b3235eeea1b6 100644
--- a/gcc/config/gcn/gcn.md
+++ b/gcc/config/gcn/gcn.md
@@ -23,9 +23,7 @@
 
 ; Named registers
 (define_constants
-  [(FIRST_SGPR_REG		 0)
-   (CC_SAVE_REG			 22)
-   (LAST_SGPR_REG		 101)
+  [(CC_SAVE_REG			 22)
(FLAT_SCRATCH_REG		 102)
(FLAT_SCRATCH_LO_REG		 102)
(FLAT_SCRATCH_HI_REG		 103)
@@ -49,11 +47,7 @@
(EXEC_LO_REG			 126)
(EXEC_HI_REG			 127)
(EXECZ_REG			 128)
-   (SCC_REG			 129)
-   (FIRST_VGPR_REG		 160)
-   (LAST_VGPR_REG		 415)
-   (FIRST_AVGPR_REG		 416)
-   (LAST_AVGPR_REG		 671)])
+   (SCC_REG			 129)])
 
 (define_constants
   [(SP_REGNUM 16)
-- 
2.43.0