diff mbox series

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

Message ID 87jznpl8pd.fsf@euler.schwinge.ddns.net
State New
Headers show
Series GCN: Remove 'FIRST_{SGPR,VGPR,AVGPR}_REG', 'LAST_{SGPR,VGPR,AVGPR}_REG' from machine description (was: [PATCH v3 04/10] GCN machine description) | expand

Commit Message

Thomas Schwinge Jan. 31, 2024, 5:21 p.m. UTC 
Hi!

On 2018-12-12T11:52:23+0000, Andrew Stubbs <ams@codesourcery.com> 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_REG		 102)
> +   (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_REG			 108)
> +   (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_REG			 126)
> +   (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)])

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

Comments

Andrew Stubbs Jan. 31, 2024, 5:41 p.m. UTC | #1 
On 31/01/2024 17:21, Thomas Schwinge wrote:
> Hi!
> 
> On 2018-12-12T11:52:23+0000, Andrew Stubbs <ams@codesourcery.com> 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_REG		 102)
>> +   (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_REG			 108)
>> +   (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_REG			 126)
>> +   (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
diff mbox series

Patch

From 6af4774b4574086f5d4925333406eab4fed7f9a5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwinge@baylibre.com>
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