Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TableGen] Fix concatenation of subreg and artificial subregs #114391

Merged

Conversation

sdesmalen-arm
Copy link
Collaborator

@sdesmalen-arm sdesmalen-arm commented Oct 31, 2024

When CoveredBySubRegs is true and a sub-register consists of two
parts; a regular subreg and an artificial subreg, then TableGen
should consider only concatenating the non-artificial subregs.
For example, S0_S1 is a concatenated subreg from D0_D1,
but S0_S1_HI should not be considered.

When CoveredBySubRegs is true and a sub-register consists of two
parts; a regular subreg and an artificial subreg, then TableGen
should consider both as a concatenation of subregs.

This happens for example when a 64-bit register 'D0' consists of
32-bit 'S0_HI' (artificial) and 'S0', and 'S0' consists of (16-bit)
'H0_HI' (artificial) and 'H0'. Then the concatenation should be:
S0_HI, H0_HI, H0.
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-tablegen

Author: Sander de Smalen (sdesmalen-arm)

Changes

When CoveredBySubRegs is true and a sub-register consists of two
parts; a regular subreg and an artificial subreg, then TableGen
should consider both as a concatenation of subregs.

This happens for example when a 64-bit register 'D0' consists of
32-bit 'S0_HI' (artificial) and 'S0', and 'S0' consists of (16-bit)
'H0_HI' (artificial) and 'H0'. Then the concatenation should be:
S0_HI, H0_HI, H0.


Full diff: https://github.com/llvm/llvm-project/pull/114391.diff

2 Files Affected:

  • (added) llvm/test/TableGen/ArtificialSubregs.td (+407)
  • (modified) llvm/utils/TableGen/Common/CodeGenRegisters.cpp (+4-2)
diff --git a/llvm/test/TableGen/ArtificialSubregs.td b/llvm/test/TableGen/ArtificialSubregs.td
new file mode 100644
index 00000000000000..9c3ffeef8926e3
--- /dev/null
+++ b/llvm/test/TableGen/ArtificialSubregs.td
@@ -0,0 +1,407 @@
+// RUN: llvm-tblgen -gen-register-info -register-info-debug -I %p/../../include %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK
+include "llvm/Target/Target.td"
+
+class MyReg<string n, list<Register> subregs = []>
+  : Register<n> {
+  let Namespace = "Test";
+  let SubRegs = subregs;
+}
+
+class MyClass<int size, list<ValueType> types, dag registers>
+  : RegisterClass<"Test", types, size, registers> {
+  let Size = size;
+}
+
+def ssub    : SubRegIndex< 32,   0>;
+def ssub_hi : SubRegIndex< 32,  32>;
+def dsub    : SubRegIndex< 64,   0>;
+def dsub_hi : SubRegIndex< 64,  64>;
+def qsub    : SubRegIndex<128,   0>;
+def qsub_hi : SubRegIndex<128, 128>;
+
+def S0    : MyReg<"s0">;
+def S1    : MyReg<"s1">;
+def S2    : MyReg<"s2">;
+
+let isArtificial = 1 in {
+def S0_HI : MyReg<"s0_hi">;
+def S1_HI : MyReg<"s1_hi">;
+def S2_HI : MyReg<"s2_hi">;
+
+def D0_HI : MyReg<"D0_hi">;
+def D1_HI : MyReg<"D1_hi">;
+def D2_HI : MyReg<"D2_hi">;
+}
+
+let SubRegIndices = [ssub, ssub_hi], CoveredBySubRegs = 1 in {
+def D0    : MyReg<"d0", [S0, S0_HI]>;
+def D1    : MyReg<"d1", [S1, S1_HI]>;
+def D2    : MyReg<"d2", [S2, S2_HI]>;
+}
+
+let SubRegIndices = [dsub, dsub_hi], CoveredBySubRegs = 1 in {
+def Q0    : MyReg<"q0", [D0, D0_HI]>;
+def Q1    : MyReg<"q1", [D1, D1_HI]>;
+def Q2    : MyReg<"q2", [D2, D2_HI]>;
+}
+
+def SRegs : MyClass<32, [i32], (sequence "S%u", 0, 2)>;
+def DRegs : MyClass<64, [i64], (sequence "D%u", 0, 2)>;
+def QRegs : MyClass<128, [i128], (sequence "Q%u", 0, 2)>;
+
+def dsub0 : SubRegIndex<64>;
+def dsub1 : SubRegIndex<64>;
+def dsub2 : SubRegIndex<64>;
+
+def ssub0 : SubRegIndex<32>;
+def ssub1 : ComposedSubRegIndex<dsub1, ssub>;
+def ssub2 : ComposedSubRegIndex<dsub2, ssub>;
+
+def STuples2 : RegisterTuples<[ssub0, ssub1],
+                             [(shl SRegs, 0), (shl SRegs, 1)]>;
+def STuplesRC2 : MyClass<64, [untyped], (add STuples2)>;
+
+def DTuples2 : RegisterTuples<[dsub0, dsub1],
+                             [(shl DRegs, 0), (shl DRegs, 1)]>;
+def DTuplesRC2 : MyClass<128, [untyped], (add DTuples2)>;
+
+def STuples3 : RegisterTuples<[ssub0, ssub1, ssub2],
+                             [(shl SRegs, 0), (shl SRegs, 1), (shl SRegs, 2)]>;
+def STuplesRC3 : MyClass<96, [untyped], (add STuples3)>;
+
+def DTuples3 : RegisterTuples<[dsub0, dsub1, dsub2],
+                             [(shl DRegs, 0), (shl DRegs, 1), (shl DRegs, 2)]>;
+def DTuplesRC3 : MyClass<192, [untyped], (add DTuples3)>;
+
+def TestTarget : Target;
+
+// CHECK:      RegisterClass SRegs:
+// CHECK-NEXT: 	SpillSize: { Default:32 }
+// CHECK-NEXT: 	SpillAlignment: { Default:32 }
+// CHECK-NEXT: 	NumRegs: 3
+// CHECK-NEXT: 	LaneMask: 0000000000000001
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: 	CoveredBySubRegs: 0
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: S0 S1 S2
+// CHECK-NEXT: 	SubClasses: SRegs
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: RegisterClass DRegs:
+// CHECK-NEXT: 	SpillSize: { Default:64 }
+// CHECK-NEXT: 	SpillAlignment: { Default:64 }
+// CHECK-NEXT: 	NumRegs: 3
+// CHECK-NEXT: 	LaneMask: 0000000000000008
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	CoveredBySubRegs: 1
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: D0 D1 D2
+// CHECK-NEXT: 	SubClasses: DRegs
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: RegisterClass STuplesRC2:
+// CHECK-NEXT: 	SpillSize: { Default:64 }
+// CHECK-NEXT: 	SpillAlignment: { Default:64 }
+// CHECK-NEXT: 	NumRegs: 2
+// CHECK-NEXT: 	LaneMask: 0000000000000030
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	CoveredBySubRegs: 1
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: S0_S1 S1_S2
+// CHECK-NEXT: 	SubClasses: STuplesRC2
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: RegisterClass STuplesRC3:
+// CHECK-NEXT: 	SpillSize: { Default:96 }
+// CHECK-NEXT: 	SpillAlignment: { Default:96 }
+// CHECK-NEXT: 	NumRegs: 1
+// CHECK-NEXT: 	LaneMask: 0000000000000070
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	CoveredBySubRegs: 1
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: S0_S1_S2
+// CHECK-NEXT: 	SubClasses: STuplesRC3
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: RegisterClass QRegs:
+// CHECK-NEXT: 	SpillSize: { Default:128 }
+// CHECK-NEXT: 	SpillAlignment: { Default:128 }
+// CHECK-NEXT: 	NumRegs: 3
+// CHECK-NEXT: 	LaneMask: 0000000000000088
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	CoveredBySubRegs: 1
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: Q0 Q1 Q2
+// CHECK-NEXT: 	SubClasses: QRegs
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: RegisterClass DTuplesRC2:
+// CHECK-NEXT: 	SpillSize: { Default:128 }
+// CHECK-NEXT: 	SpillAlignment: { Default:128 }
+// CHECK-NEXT: 	NumRegs: 2
+// CHECK-NEXT: 	LaneMask: 00000000000001A8
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	CoveredBySubRegs: 1
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: D0_D1 D1_D2
+// CHECK-NEXT: 	SubClasses: DTuplesRC2
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: RegisterClass DTuplesRC3:
+// CHECK-NEXT: 	SpillSize: { Default:192 }
+// CHECK-NEXT: 	SpillAlignment: { Default:192 }
+// CHECK-NEXT: 	NumRegs: 1
+// CHECK-NEXT: 	LaneMask: 00000000000003E8
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	CoveredBySubRegs: 1
+// CHECK-NEXT: 	Allocatable: 1
+// CHECK-NEXT: 	AllocationPriority: 0
+// CHECK-NEXT: 	BaseClassOrder: None
+// CHECK-NEXT: 	Regs: D0_D1_D2
+// CHECK-NEXT: 	SubClasses: DTuplesRC3
+// CHECK-NEXT: 	SuperClasses:
+// CHECK-NEXT: SubRegIndex dsub:
+// CHECK-NEXT: 	LaneMask: 0000000000000088
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex dsub0:
+// CHECK-NEXT: 	LaneMask: 0000000000000088
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex dsub1:
+// CHECK-NEXT: 	LaneMask: 0000000000000120
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex dsub2:
+// CHECK-NEXT: 	LaneMask: 0000000000000240
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex dsub_hi:
+// CHECK-NEXT: 	LaneMask: 0000000000000001
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:64 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex qsub:
+// CHECK-NEXT: 	LaneMask: 0000000000000002
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:128 }
+// CHECK-NEXT: SubRegIndex qsub_hi:
+// CHECK-NEXT: 	LaneMask: 0000000000000004
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:128 }
+// CHECK-NEXT: 	Size: { Default:128 }
+// CHECK-NEXT: SubRegIndex ssub:
+// CHECK-NEXT: 	LaneMask: 0000000000000008
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex ssub0:
+// CHECK-NEXT: 	LaneMask: 0000000000000010
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex ssub1:
+// CHECK-NEXT: 	LaneMask: 0000000000000020
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex ssub2:
+// CHECK-NEXT: 	LaneMask: 0000000000000040
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:0 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex ssub_hi:
+// CHECK-NEXT: 	LaneMask: 0000000000000080
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:32 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex dsub1_then_ssub_hi:
+// CHECK-NEXT: 	LaneMask: 0000000000000100
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:32 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex dsub2_then_ssub_hi:
+// CHECK-NEXT: 	LaneMask: 0000000000000200
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:32 }
+// CHECK-NEXT: 	Size: { Default:32 }
+// CHECK-NEXT: SubRegIndex ssub_ssub1:
+// CHECK-NEXT: 	LaneMask: 0000000000000028
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:65535 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex dsub0_dsub1:
+// CHECK-NEXT: 	LaneMask: 00000000000001A8
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:65535 }
+// CHECK-NEXT: 	Size: { Default:128 }
+// CHECK-NEXT: SubRegIndex dsub1_dsub2:
+// CHECK-NEXT: 	LaneMask: 0000000000000360
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:65535 }
+// CHECK-NEXT: 	Size: { Default:128 }
+// CHECK-NEXT: SubRegIndex ssub_ssub1_ssub2:
+// CHECK-NEXT: 	LaneMask: 0000000000000068
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:65535 }
+// CHECK-NEXT: 	Size: { Default:96 }
+// CHECK-NEXT: SubRegIndex ssub1_ssub2:
+// CHECK-NEXT: 	LaneMask: 0000000000000060
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:65535 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: SubRegIndex ssub0_ssub1:
+// CHECK-NEXT: 	LaneMask: 0000000000000030
+// CHECK-NEXT: 	AllSuperRegsCovered: 1
+// CHECK-NEXT: 	Offset: { Default:65535 }
+// CHECK-NEXT: 	Size: { Default:64 }
+// CHECK-NEXT: Register D0:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg ssub = S0
+// CHECK-NEXT: 	SubReg ssub_hi = S0_HI
+// CHECK-NEXT: Register D1:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg ssub = S1
+// CHECK-NEXT: 	SubReg ssub_hi = S1_HI
+// CHECK-NEXT: Register D2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg ssub = S2
+// CHECK-NEXT: 	SubReg ssub_hi = S2_HI
+// CHECK-NEXT: Register Q0:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg dsub = D0
+// CHECK-NEXT: 	SubReg dsub_hi = D0_HI
+// CHECK-NEXT: 	SubReg ssub = S0
+// CHECK-NEXT: 	SubReg ssub_hi = S0_HI
+// CHECK-NEXT: Register Q1:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg dsub = D1
+// CHECK-NEXT: 	SubReg dsub_hi = D1_HI
+// CHECK-NEXT: 	SubReg ssub = S1
+// CHECK-NEXT: 	SubReg ssub_hi = S1_HI
+// CHECK-NEXT: Register Q2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg dsub = D2
+// CHECK-NEXT: 	SubReg dsub_hi = D2_HI
+// CHECK-NEXT: 	SubReg ssub = S2
+// CHECK-NEXT: 	SubReg ssub_hi = S2_HI
+// CHECK-NEXT: Register S0:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register S1:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register S2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register D0_HI:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register D1_HI:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register D2_HI:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register S0_HI:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register S1_HI:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register S2_HI:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 0
+// CHECK-NEXT: 	HasDisjunctSubRegs: 0
+// CHECK-NEXT: Register D0_D1:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg dsub0 = D0
+// CHECK-NEXT: 	SubReg dsub1 = D1
+// CHECK-NEXT: 	SubReg ssub = S0
+// CHECK-NEXT: 	SubReg ssub1 = S1
+// CHECK-NEXT: 	SubReg ssub_hi = S0_HI
+// CHECK-NEXT: 	SubReg dsub1_then_ssub_hi = S1_HI
+// CHECK-NEXT: 	SubReg ssub_ssub1 = S0_S1
+// CHECK-NEXT: Register D1_D2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg dsub0 = D1
+// CHECK-NEXT: 	SubReg dsub1 = D2
+// CHECK-NEXT: 	SubReg ssub = S1
+// CHECK-NEXT: 	SubReg ssub1 = S2
+// CHECK-NEXT: 	SubReg ssub_hi = S1_HI
+// CHECK-NEXT: 	SubReg dsub1_then_ssub_hi = S2_HI
+// CHECK-NEXT: 	SubReg ssub_ssub1 = S1_S2
+// CHECK-NEXT: Register D0_D1_D2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg dsub0 = D0
+// CHECK-NEXT: 	SubReg dsub1 = D1
+// CHECK-NEXT: 	SubReg dsub2 = D2
+// CHECK-NEXT: 	SubReg ssub = S0
+// CHECK-NEXT: 	SubReg ssub1 = S1
+// CHECK-NEXT: 	SubReg ssub2 = S2
+// CHECK-NEXT: 	SubReg ssub_hi = S0_HI
+// CHECK-NEXT: 	SubReg dsub1_then_ssub_hi = S1_HI
+// CHECK-NEXT: 	SubReg dsub2_then_ssub_hi = S2_HI
+// CHECK-NEXT: 	SubReg ssub_ssub1 = S0_S1
+// CHECK-NEXT: 	SubReg dsub0_dsub1 = D0_D1
+// CHECK-NEXT: 	SubReg dsub1_dsub2 = D1_D2
+// CHECK-NEXT: 	SubReg ssub_ssub1_ssub2 = S0_S1_S2
+// CHECK-NEXT: 	SubReg ssub1_ssub2 = S1_S2
+// CHECK-NEXT: Register S0_S1:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg ssub0 = S0
+// CHECK-NEXT: 	SubReg ssub1 = S1
+// CHECK-NEXT: Register S1_S2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg ssub0 = S1
+// CHECK-NEXT: 	SubReg ssub1 = S2
+// CHECK-NEXT: Register S0_S1_S2:
+// CHECK-NEXT: 	CostPerUse: 0
+// CHECK-NEXT: 	CoveredBySubregs: 1
+// CHECK-NEXT: 	HasDisjunctSubRegs: 1
+// CHECK-NEXT: 	SubReg ssub0 = S0
+// CHECK-NEXT: 	SubReg ssub1 = S1
+// CHECK-NEXT: 	SubReg ssub2 = S2
+// CHECK-NEXT: 	SubReg ssub1_ssub2 = S1_S2
+// CHECK-NEXT: 	SubReg ssub0_ssub1 = S0_S1
diff --git a/llvm/utils/TableGen/Common/CodeGenRegisters.cpp b/llvm/utils/TableGen/Common/CodeGenRegisters.cpp
index 78f6dcdf305ffe..e9f5a4c19471f2 100644
--- a/llvm/utils/TableGen/Common/CodeGenRegisters.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenRegisters.cpp
@@ -399,8 +399,7 @@ CodeGenRegister::computeSubRegs(CodeGenRegBank &RegBank) {
   // user already specified.
   for (unsigned i = 0, e = ExplicitSubRegs.size(); i != e; ++i) {
     CodeGenRegister *SR = ExplicitSubRegs[i];
-    if (!SR->CoveredBySubRegs || SR->ExplicitSubRegs.size() <= 1 ||
-        SR->Artificial)
+    if (!SR->CoveredBySubRegs || SR->Artificial)
       continue;
 
     // SR is composed of multiple sub-regs. Find their names in this register.
@@ -411,6 +410,9 @@ CodeGenRegister::computeSubRegs(CodeGenRegBank &RegBank) {
         Parts.push_back(getSubRegIndex(SR->ExplicitSubRegs[j]));
     }
 
+    if (Parts.size() < 2)
+      continue;
+
     // Offer this as an existing spelling for the concatenation of Parts.
     CodeGenSubRegIndex &Idx = *ExplicitSubRegIndices[i];
     Idx.setConcatenationOf(Parts);

@@ -0,0 +1,407 @@
// RUN: llvm-tblgen -gen-register-info -register-info-debug -I %p/../../include %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK
include "llvm/Target/Target.td"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment the test purpose

@jayfoad
Copy link
Contributor

jayfoad commented Oct 31, 2024

This happens for example when a 64-bit register 'D0' consists of
32-bit 'S0_HI' (artificial) and 'S0', and 'S0' consists of (16-bit)
'H0_HI' (artificial) and 'H0'. Then the concatenation should be:
S0_HI, H0_HI, H0.

But that's not what your patch does, is it? The Parts vector only accumulates non-artificial indices (H0 in your example) and then you bail out because Parts only contains one part, so no concatenation is recorded.

@jayfoad
Copy link
Contributor

jayfoad commented Oct 31, 2024

Personally I'd prefer to see a minimal testcase that shows the effect of the change. (But maybe the 400 line test case is useful too, to catch any intentional or unintentional changes in future?)

@sdesmalen-arm
Copy link
Collaborator Author

This happens for example when a 64-bit register 'D0' consists of
32-bit 'S0_HI' (artificial) and 'S0', and 'S0' consists of (16-bit)
'H0_HI' (artificial) and 'H0'. Then the concatenation should be:
S0_HI, H0_HI, H0.

But that's not what your patch does, is it? The Parts vector only accumulates non-artificial indices (H0 in your example) and then you bail out because Parts only contains one part, so no concatenation is recorded.

You're right, the commit message was rubbish. I've updated it, hopefully it makes more sense now.

@sdesmalen-arm
Copy link
Collaborator Author

Personally I'd prefer to see a minimal testcase that shows the effect of the change. (But maybe the 400 line test case is useful too, to catch any intentional or unintentional changes in future?)

I initially started tried to prune the CHECK lines, but then figured it would be useful to catch (un)intentional changes in the future (such as needed for #114392), so opted to keep all lines in the end.

// \ /
// qsub
//
// Where the _hi parts are artificial and where ubregs ssub, dsub and qsub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type "ubregs"

@@ -411,6 +410,9 @@ CodeGenRegister::computeSubRegs(CodeGenRegBank &RegBank) {
Parts.push_back(getSubRegIndex(SR->ExplicitSubRegs[j]));
}

if (Parts.size() < 2)
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we continue here if any of the subreg indices of SR are artificial? (Either instead of or as well as the Parts.size() condition.) E.g. if SR had two real subreg indices and one artificial one, we should give up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point @jayfoad!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jayfoad are you happy with the latest change? (apologies for the prod, I wasn't sure if you noticed I had addressed your comment)

@jayfoad
Copy link
Contributor

jayfoad commented Nov 4, 2024

The code change looks fine to me. I'd still prefer to see minimal tests for the original problem, and for the new cases that 588de85 will catch, but I won't insist.

@@ -411,6 +410,9 @@ CodeGenRegister::computeSubRegs(CodeGenRegBank &RegBank) {
Parts.push_back(getSubRegIndex(SR->ExplicitSubRegs[j]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could break out early if I.Artificial.

@sdesmalen-arm
Copy link
Collaborator Author

The code change looks fine to me. I'd still prefer to see minimal tests for the original problem, and for the new cases that 588de85 will catch, but I won't insist.

Thanks for giving it another look! I've reduced the CHECK lines a bit, which hopefully makes the test a bit easier to parse.

@sdesmalen-arm sdesmalen-arm merged commit 9a211fe into main Nov 4, 2024
5 of 6 checks passed
@sdesmalen-arm sdesmalen-arm deleted the users/sdesmalen-arm/srlt-fix-tablegen-artificial-concat branch November 4, 2024 15:51
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
…14391)

When CoveredBySubRegs is true and a sub-register consists of two
parts; a regular subreg and an artificial subreg, then TableGen
should consider only concatenating the non-artificial subregs. 
For example, S0_S1 is a concatenated subreg from D0_D1,
but S0_S1_HI should not be considered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants