CodeGen: Make target overrides of PointerLikeRegClass mandatory #159882
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Sep 19, 2025
arsenm
added
backend:AMDGPU
backend:SystemZ
llvm:codegen
llvm:globalisel
llvm:mc
Graphite App
|
@llvm/pr-subscribers-llvm-mc @llvm/pr-subscribers-backend-systemz Author: Matt Arsenault (arsenm) ChangesMost targets should now use the convenience multiclass to fixup defm : RemapAllTargetPseudoPointerOperands<target_ptr_regclass>; Full diff: https://github.com/llvm/llvm-project/pull/159882.diff 8 Files Affected:
diff --git a/llvm/test/TableGen/DuplicateFieldValues.td b/llvm/test/TableGen/DuplicateFieldValues.td
index 50c77fa88ccec..85cb5bbfb6c56 100644
--- a/llvm/test/TableGen/DuplicateFieldValues.td
+++ b/llvm/test/TableGen/DuplicateFieldValues.td
@@ -82,3 +82,4 @@ let BaseName = "0" in {
def E0 : I, ABCRel, isEForm;
}
+defm : RemapAllTargetPseudoPointerOperands<DFVRegClass>;
diff --git a/llvm/test/TableGen/RegClassByHwMode.td b/llvm/test/TableGen/RegClassByHwMode.td
index 5d813d2bfc83a..606855c7190f2 100644
--- a/llvm/test/TableGen/RegClassByHwMode.td
+++ b/llvm/test/TableGen/RegClassByHwMode.td
@@ -11,6 +11,7 @@ include "llvm/Target/Target.td"
// INSTRINFO-NEXT: namespace llvm::MyTarget {
// INSTRINFO-NEXT: enum {
// INSTRINFO-NEXT: PHI
+// INSTRINFO: LOAD_STACK_GUARD = [[LOAD_STACK_GUARD_OPCODE:[0-9]+]]
// INSTRINFO: };
// INSTRINFO: enum RegClassByHwModeUses : uint16_t {
// INSTRINFO-NEXT: MyPtrRC,
@@ -19,10 +20,20 @@ include "llvm/Target/Target.td"
// INSTRINFO-NEXT: };
// INSTRINFO-NEXT: }
+
+// INSTRINFO: { [[LOAD_STACK_GUARD_OPCODE]], 1, 1, 0, 0, 0, 0, [[LOAD_STACK_GUARD_OP_INDEX:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::Rematerializable)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
+
+// INSTRINFO: /* [[LOAD_STACK_GUARD_OP_INDEX]] */ { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 },
+// INSTRINFO: { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 },
+// INSTRINFO: { -1, 0, MCOI::OPERAND_UNKNOWN, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 },
+// INSTRINFO: { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_UNKNOWN, 0 },
+// INSTRINFO: { -1, 0, MCOI::OPERAND_UNKNOWN, 0 }, { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_UNKNOWN, 0 },
+
// INSTRINFO: { MyTarget::XRegsRegClassID, 0, MCOI::OPERAND_REGISTER, 0 },
// INSTRINFO: { MyTarget::XRegs_EvenRegClassID, 0, MCOI::OPERAND_REGISTER, 0 },
+
// INSTRINFO: { MyTarget::YRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 },
-// INSTRINFO: { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 },
+
// INSTRINFO: { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 }, { MyTarget::MyPtrRC, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 },
// INSTRINFO: { MyTarget::YRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 }, { MyTarget::XRegs_EvenIfRequired, 0|(1<<MCOI::LookupRegClassByHwMode), MCOI::OPERAND_REGISTER, 0 },
@@ -463,5 +474,7 @@ def : Pat<
(MY_LOAD $src)
>;
+defm : RemapAllTargetPseudoPointerOperands<XRegs_EvenIfRequired>;
+
def MyTargetISA : InstrInfo;
def MyTarget : Target { let InstructionSet = MyTargetISA; }
diff --git a/llvm/test/TableGen/def-multiple-operands.td b/llvm/test/TableGen/def-multiple-operands.td
index 5d215056920e8..dc5ea09eff9ba 100644
--- a/llvm/test/TableGen/def-multiple-operands.td
+++ b/llvm/test/TableGen/def-multiple-operands.td
@@ -35,3 +35,5 @@ def InstA : Instruction {
field bits<8> SoftFail = 0;
let hasSideEffects = false;
}
+
+defm : RemapAllTargetPseudoPointerOperands<P1>;
diff --git a/llvm/test/TableGen/get-named-operand-idx.td b/llvm/test/TableGen/get-named-operand-idx.td
index e6f6331cd9c48..4626fb6439edd 100644
--- a/llvm/test/TableGen/get-named-operand-idx.td
+++ b/llvm/test/TableGen/get-named-operand-idx.td
@@ -48,6 +48,8 @@ def InstD : InstBase {
let UseNamedOperandTable = 0;
}
+defm : RemapAllTargetPseudoPointerOperands<RegClass>;
+
// CHECK-LABEL: #ifdef GET_INSTRINFO_OPERAND_ENUM
// CHECK-NEXT: #undef GET_INSTRINFO_OPERAND_ENUM
// CHECK-NEXT: namespace llvm::MyNamespace {
diff --git a/llvm/test/TableGen/get-operand-type-no-expand.td b/llvm/test/TableGen/get-operand-type-no-expand.td
index a0a8fa957f9b6..fcaf3684528b2 100644
--- a/llvm/test/TableGen/get-operand-type-no-expand.td
+++ b/llvm/test/TableGen/get-operand-type-no-expand.td
@@ -46,3 +46,5 @@ def InstA : Instruction {
// CHECK-NOEXPAND: /* InstA */
// CHECK-NOEXPAND-NEXT: i512complex, i8complex, i32imm,
// CHECK-NOEXPAND: #endif // GET_INSTRINFO_OPERAND_TYPE
+
+defm : RemapAllTargetPseudoPointerOperands<RegClass>;
diff --git a/llvm/test/TableGen/get-operand-type.td b/llvm/test/TableGen/get-operand-type.td
index b2f63cafd6a89..49fbb63ac5974 100644
--- a/llvm/test/TableGen/get-operand-type.td
+++ b/llvm/test/TableGen/get-operand-type.td
@@ -18,6 +18,8 @@ def OpB : Operand<i32>;
def RegOp : RegisterOperand<RegClass>;
+defm : RemapAllTargetPseudoPointerOperands<RegClass>;
+
def InstA : Instruction {
let Size = 1;
let OutOperandList = (outs OpA:$a);
diff --git a/llvm/test/TableGen/target-specialized-pseudos.td b/llvm/test/TableGen/target-specialized-pseudos.td
index 99c63f3ec29d9..3953a36101fe0 100644
--- a/llvm/test/TableGen/target-specialized-pseudos.td
+++ b/llvm/test/TableGen/target-specialized-pseudos.td
@@ -1,6 +1,11 @@
-// RUN: llvm-tblgen -gen-instr-info -I %p/../../include %s -DONECASE -o - | FileCheck -check-prefixes=CHECK,ONECASE %s
// RUN: llvm-tblgen -gen-instr-info -I %p/../../include %s -DALLCASES -o - | FileCheck -check-prefixes=CHECK,ALLCASES %s
-// RUN: not llvm-tblgen -gen-instr-info -I %p/../../include %s -DERROR -o /dev/null 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not llvm-tblgen -gen-instr-info -I %p/../../include %s -DONECASE -o /dev/null 2>&1 | FileCheck -check-prefixes=ERROR-MISSING %s
+// RUN: not llvm-tblgen -gen-instr-info -I %p/../../include %s -DMULTIPLE_OVERRIDE_ERROR -o /dev/null 2>&1 | FileCheck -implicit-check-not=error: -check-prefix=MULTIPLE-OVERRIDE-ERROR %s
+// RUN: not llvm-tblgen -gen-instr-info -I %p/../../include %s -DALLCASES -DERROR_NONPSEUDO -o /dev/null 2>&1 | FileCheck -implicit-check-not=error: -check-prefix=ERROR-NONPSEUDO %s
+
+
+// def PREALLOCATED_ARG : StandardPseudoInstruction {
+
// CHECK: namespace llvm::MyTarget {
// CHECK: enum {
@@ -20,8 +25,6 @@
// CHECK-NEXT: { [[MY_MOV_OPCODE]], 2, 1, 2, 0, 0, 0, {{[0-9]+}}, MyTargetImpOpBase + 0, 0|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // MY_MOV
// CHECK-NEXT: { [[G_UBFX_OPCODE]], 4, 1, 0, 0, 0, 0, {{[0-9]+}}, MyTargetImpOpBase + 0, 0|(1ULL<<MCID::PreISelOpcode)|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // G_UBFX
-// ONECASE: { [[LOAD_STACK_GUARD_OPCODE]], 1, 1, 0, 0, 0, 0, [[LOAD_STACK_GUARD_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::Rematerializable)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // MY_LOAD_STACK_GUARD
-
// ALLCASES: { [[PATCHABLE_TYPED_EVENT_CALL_OPCODE]], 3, 0, 0, 0, 0, 0, [[PATCHABLE_TYPED_EVENT_CALL_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::Call)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::MayStore)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
// ALLCASES: { [[PATCHABLE_EVENT_CALL_OPCODE]], 2, 0, 0, 0, 0, 0, [[PATCHABLE_EVENT_CALL_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::Call)|(1ULL<<MCID::MayLoad)|(1ULL<<MCID::MayStore)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
// ALLCASES: { [[PREALLOCATED_ARG_OPCODE]], 3, 1, 0, 0, 0, 0, [[PREALLOCATED_ARG_OP_ENTRY:[0-9]+]], MyTargetImpOpBase + 0, 0|(1ULL<<MCID::Pseudo)|(1ULL<<MCID::UsesCustomInserter)|(1ULL<<MCID::UnmodeledSideEffects)|(1ULL<<MCID::ExtraSrcRegAllocReq)|(1ULL<<MCID::ExtraDefRegAllocReq), 0x0ULL }, // anonymous_
@@ -29,8 +32,6 @@
// CHECK: /* 0 */ { -1, 0, MCOI::OPERAND_UNKNOWN, 0 },
-// ONECASE: /* [[LOAD_STACK_GUARD_OP_ENTRY]] */ { MyTarget::XRegsRegClassID, 0, MCOI::OPERAND_REGISTER, 0 },
-
// ALLCASES: /* [[LOAD_STACK_GUARD_OP_ENTRY]] */ { MyTarget::XRegsRegClassID, 0, MCOI::OPERAND_REGISTER, 0 },
// ALLCASES: /* [[PREALLOCATED_ARG_OP_ENTRY]] */ { MyTarget::XRegsRegClassID, 0, MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 }, { -1, 0, MCOI::OPERAND_IMMEDIATE, 0 },
// ALLCASES: /* [[PATCHABLE_EVENT_CALL_OP_ENTRY]] */ { MyTarget::XRegsRegClassID, 0, MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_UNKNOWN, 0 },
@@ -72,6 +73,10 @@ def MY_LOAD_STACK_GUARD :
let OutOperandList = (outs XRegs:$dst);
}
+// ERROR-MISSING: error: missing target override for pseudoinstruction using PointerLikeRegClass
+// ERROR-MISSING note: target should define equivalent instruction with RegisterClassLike replacement; (use RemapAllTargetPseudoPointerOperands?)
+
+
#endif
#ifdef ALLCASES
@@ -81,15 +86,28 @@ defm my_remaps : RemapAllTargetPseudoPointerOperands<XRegs>;
#endif
-#ifdef ERROR
+#ifdef MULTIPLE_OVERRIDE_ERROR
def MY_LOAD_STACK_GUARD_0 : TargetSpecializedStandardPseudoInstruction<LOAD_STACK_GUARD>;
-// ERROR: :[[@LINE+1]]:5: error: multiple overrides of 'LOAD_STACK_GUARD' defined
+// MULTIPLE-OVERRIDE-ERROR: :[[@LINE+1]]:5: error: multiple overrides of 'LOAD_STACK_GUARD' defined
def MY_LOAD_STACK_GUARD_1 : TargetSpecializedStandardPseudoInstruction<LOAD_STACK_GUARD>;
#endif
+#ifdef ERROR_NONPSEUDO
+
+// FIXME: Double error
+// ERROR-NONPSEUDO: [[@LINE+2]]:5: error: non-pseudoinstruction user of PointerLikeRegClass
+// ERROR-NONPSEUDO: [[@LINE+1]]:5: error: non-pseudoinstruction user of PointerLikeRegClass
+def NON_PSEUDO : TestInstruction {
+ let OutOperandList = (outs XRegs:$dst);
+ let InOperandList = (ins ptr_rc:$src);
+ let AsmString = "non_pseudo $dst, $src";
+}
+
+#endif
+
def MY_MOV : TestInstruction {
let OutOperandList = (outs XRegs:$dst);
let InOperandList = (ins XRegs:$src);
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index ed32f537d4b5d..84b4e1470183e 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -161,9 +161,21 @@ InstrInfoEmitter::GetOperandInfo(const CodeGenInstruction &Inst) {
Res += ", ";
} else if (OpR->isSubClassOf("RegisterClass"))
Res += getQualifiedName(OpR) + "RegClassID, ";
- else if (OpR->isSubClassOf("PointerLikeRegClass"))
- Res += utostr(OpR->getValueAsInt("RegClassKind")) + ", ";
- else
+ else if (OpR->isSubClassOf("PointerLikeRegClass")) {
+ if (Inst.isPseudo) {
+ // TODO: Verify this is a fixed pseudo
+ PrintError(Inst.TheDef,
+ "missing target override for pseudoinstruction "
+ "using PointerLikeRegClass");
+ PrintNote(OpR->getLoc(),
+ "target should define equivalent instruction "
+ "with RegisterClassLike replacement; (use "
+ "RemapAllTargetPseudoPointerOperands?)");
+ } else {
+ PrintError(Inst.TheDef,
+ "non-pseudoinstruction user of PointerLikeRegClass");
+ }
+ } else
// -1 means the operand does not have a fixed register class.
Res += "-1, ";
|
|
Hi @arsenm , I haven't followed the discussion on all that. Cheers, |
Most targets should now use the convenience multiclass to fixup the operand definitions of pointer-using pseudoinstructions: defm : RemapAllTargetPseudoPointerOperands<target_ptr_regclass>;
arsenm
force-pushed
the
users/arsenm/codegen/make-target-overrides-pointerlikeregclass-mandatory
branch
from
669b2f8 to
a8f9037
Compare
arsenm
force-pushed
the
users/arsenm/codegen/override-ptr-pseudos-all-targets
branch
from
9d77e6b to
64f0e18
Compare
I want to eliminate getPointerRegClass. It's a layering violation. We should be able to interpret an MCOperandInfo without codegen throughout MC. This only depends on the MCInstrInfo and HwMode/Subtarget. Each target's InstrInfo table defines copies of all of the common pseudo instructions anyway, so this series of patches just swaps out the operand definitions to use the correct register class for the pointer types |
Make sense, thanks for the context! |
qcolombet
approved these changes
Oct 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most targets should now use the convenience multiclass to fixup
the operand definitions of pointer-using pseudoinstructions:
defm : RemapAllTargetPseudoPointerOperands<target_ptr_regclass>;