Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 19, 2025

Allow a target to steal the definition of a generic pseudoinstruction
and remap the operands. This works by defining a new instruction, which
will simply swap out the emitted entry in the InstrInfo table.

This is intended to eliminate the C++ half of the implementation
of PointerLikeRegClass. With RegClassByHwMode, the remaining usecase
for PointerLikeRegClass are the common codegen pseudoinstructions.
Every target maintains its own copy of the generic pseudo operand
definitions anyway, so we can stub out the register operands with
an appropriate class instead of waiting for runtime resolution.

In the future we could probably take this a bit further. For example,
there is a similar problem for ADJCALLSTACKUP/DOWN since they depend
on target register definitions for the stack pointer register.

Copy link
Contributor Author

arsenm commented Sep 19, 2025

@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2025

@llvm/pr-subscribers-llvm-globalisel
@llvm/pr-subscribers-backend-systemz

@llvm/pr-subscribers-tablegen

Author: Matt Arsenault (arsenm)

Changes

Allow a target to steal the definition of a generic pseudoinstruction
and remap the operands. This works by defining a new instruction, which
will simply swap out the emitted entry in the InstrInfo table.

This is intended to eliminate the C++ half of the implementation
of PointerLikeRegClass. With RegClassByHwMode, the remaining usecase
for PointerLikeRegClass are the common codegen pseudoinstructions.
Every target maintains its own copy of the generic pseudo operand
definitions anyway, so we can stub out the register operands with
an appropriate class instead of waiting for runtime resolution.

In the future we could probably take this a bit further. For example,
there is a similar problem for ADJCALLSTACKUP/DOWN since they depend
on target register definitions for the stack pointer register.


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

4 Files Affected:

  • (modified) llvm/include/llvm/Target/Target.td (+93)
  • (added) llvm/test/TableGen/target-specialized-pseudos.td (+101)
  • (modified) llvm/utils/TableGen/Common/CodeGenTarget.cpp (+12-1)
  • (modified) llvm/utils/TableGen/InstrInfoEmitter.cpp (+37)
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 13175177edd3e..4a759a99d1d25 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1574,6 +1574,99 @@ def CONVERGENCECTRL_GLUE : StandardPseudoInstruction {
 }
 }
 
+/// Allow a target to replace the instruction definition of a
+/// StandardPseudoInstruction. A target should only define one
+/// instance of this per instruction.
+///
+/// This is intended to allow targets to specify the register class
+/// used for pointers. It should not be used to change the fundamental
+/// operand structure (e.g., this should not add or remove operands,
+/// or change the operand types).
+class TargetSpecializedStandardPseudoInstruction<
+  StandardPseudoInstruction base_inst> : Instruction {
+
+  StandardPseudoInstruction Instruction = base_inst;
+  let OutOperandList = base_inst.OutOperandList;
+  let InOperandList = base_inst.InOperandList;
+
+  // TODO: Copy everything
+  let usesCustomInserter = base_inst.usesCustomInserter;
+  let hasSideEffects = base_inst.hasSideEffects;
+  let mayLoad = base_inst.mayLoad;
+  let mayStore = base_inst.mayStore;
+  let isTerminator = base_inst.isTerminator;
+  let isBranch = base_inst.isBranch;
+  let isIndirectBranch = base_inst.isIndirectBranch;
+  let isEHScopeReturn = base_inst.isEHScopeReturn;
+  let isReturn = base_inst.isReturn;
+  let isCall = base_inst.isCall;
+  let hasCtrlDep = base_inst.hasCtrlDep;
+  let isReMaterializable = base_inst.isReMaterializable;
+  let isMeta = base_inst.isMeta;
+  let Size = base_inst.Size;
+  let isAsCheapAsAMove = base_inst.isAsCheapAsAMove;
+  let isPseudo = true;
+  let hasNoSchedulingInfo = true;
+  let isNotDuplicable = base_inst.isNotDuplicable;
+  let isConvergent = base_inst.isConvergent;
+  let hasExtraSrcRegAllocReq = base_inst.hasExtraSrcRegAllocReq;
+  let hasExtraDefRegAllocReq = base_inst.hasExtraDefRegAllocReq;
+}
+
+// All pseudo instructions which need a pointer register class, which
+// should be specialized by a target.
+defvar PseudosWithPtrOps = [
+  LOAD_STACK_GUARD,
+  PREALLOCATED_ARG,
+  PATCHABLE_EVENT_CALL,
+  PATCHABLE_TYPED_EVENT_CALL
+];
+
+
+/// Replace PointerLikeRegClass operands in OperandList with new_rc.
+class RemapPointerOperandList<dag OperandList, RegisterClassLike new_rc> {
+  // Collect the set of names so we can query and rewrite them.
+  list<string> op_names = !foreach(i, !range(!size(OperandList)),
+                                      !getdagname(OperandList, i));
+
+  // Beautiful language. This would be a lot easier if !getdagarg
+  // didn't require a specific type. We can't just collect a list of
+  // the operand values and reconstruct the dag, since there isn't a
+  // common base class for all the field kinds used in
+  // pseudoinstruction definitions; therefore everything must be
+  // maintained as a dag, so use a foldl. Additionally, ? doesn't
+  // evaluate as false so we get even more noise.
+  dag ret =
+    !foldl(OperandList, op_names, acc, name,
+      !cond(
+        !initialized(!getdagarg<PointerLikeRegClass>(OperandList, name))
+          : !setdagarg(acc, name, new_rc),
+        !initialized(!getdagarg<unknown_class>(OperandList, name)) : acc,
+        !initialized(!getdagarg<DAGOperand>(OperandList, name)) : acc
+      )
+    );
+}
+
+/// Define an override for a pseudoinstruction which uses a pointer
+/// register class, specialized to the target's pointer type.
+class RemapPointerOperands<StandardPseudoInstruction inst,
+                           RegisterClassLike new_rc> :
+  TargetSpecializedStandardPseudoInstruction<inst> {
+  let OutOperandList =
+    RemapPointerOperandList<inst.OutOperandList, new_rc>.ret;
+  let InOperandList =
+    RemapPointerOperandList<inst.InOperandList, new_rc>.ret;
+}
+
+/// Helper to replace all pseudoinstructions using pointers to a
+/// target register class. Most targets should use this
+multiclass RemapAllTargetPseudoPointerOperands<
+  RegisterClassLike default_ptr_rc> {
+  foreach inst = PseudosWithPtrOps in {
+    def : RemapPointerOperands<inst, default_ptr_rc>;
+  }
+}
+
 // Generic opcodes used in GlobalISel.
 include "llvm/Target/GenericOpcodes.td"
 
diff --git a/llvm/test/TableGen/target-specialized-pseudos.td b/llvm/test/TableGen/target-specialized-pseudos.td
new file mode 100644
index 0000000000000..99c63f3ec29d9
--- /dev/null
+++ b/llvm/test/TableGen/target-specialized-pseudos.td
@@ -0,0 +1,101 @@
+// 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
+
+// CHECK: namespace llvm::MyTarget {
+// CHECK: enum {
+// CHECK: LOAD_STACK_GUARD = [[LOAD_STACK_GUARD_OPCODE:[0-9]+]],
+// CHECK: PREALLOCATED_ARG = [[PREALLOCATED_ARG_OPCODE:[0-9]+]],
+// CHECK: PATCHABLE_EVENT_CALL = [[PATCHABLE_EVENT_CALL_OPCODE:[0-9]+]],
+// CHECK: PATCHABLE_TYPED_EVENT_CALL = [[PATCHABLE_TYPED_EVENT_CALL_OPCODE:[0-9]+]],
+
+// Make sure no enum entry is emitted for MY_LOAD_STACK_GUARD
+// CHECK: G_UBFX = [[G_UBFX_OPCODE:[0-9]+]],
+// CHECK-NEXT: MY_MOV = [[MY_MOV_OPCODE:[0-9]+]],
+// CHECK-NEXT: INSTRUCTION_LIST_END = [[INSTR_LIST_END_OPCODE:[0-9]+]]
+
+
+// CHECK: extern const MyTargetInstrTable MyTargetDescs = {
+// CHECK-NEXT: {
+// 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_
+// ALLCASES: { [[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 },  // anonymous_
+
+// 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 },
+// ALLCASES: /* [[PATCHABLE_TYPED_EVENT_CALL_OP_ENTRY]] */ { -1, 0, MCOI::OPERAND_UNKNOWN, 0 }, { MyTarget::XRegsRegClassID, 0, MCOI::OPERAND_REGISTER, 0 }, { -1, 0, MCOI::OPERAND_UNKNOWN, 0 },
+
+
+// CHECK: const char MyTargetInstrNameData[] = {
+// CHECK: /* {{[0-9]+}} */ "LOAD_STACK_GUARD\000"
+
+include "llvm/Target/Target.td"
+
+class MyReg<string n>
+  : Register<n> {
+  let Namespace = "MyTarget";
+}
+
+class MyClass<int size, list<ValueType> types, dag registers>
+  : RegisterClass<"MyTarget", types, size, registers> {
+  let Size = size;
+}
+
+def X0 : MyReg<"x0">;
+def X1 : MyReg<"x1">;
+def XRegs : RegisterClass<"MyTarget", [i64], 64, (add X0, X1)>;
+
+
+class TestInstruction : Instruction {
+  let Size = 2;
+  let Namespace = "MyTarget";
+  let hasSideEffects = false;
+}
+
+#ifdef ONECASE
+
+// Example setting the pointer register class manually
+def MY_LOAD_STACK_GUARD :
+  TargetSpecializedStandardPseudoInstruction<LOAD_STACK_GUARD> {
+  let Namespace = "MyTarget";
+  let OutOperandList = (outs XRegs:$dst);
+}
+
+#endif
+
+#ifdef ALLCASES
+
+defm my_remaps : RemapAllTargetPseudoPointerOperands<XRegs>;
+
+#endif
+
+
+#ifdef ERROR
+
+def MY_LOAD_STACK_GUARD_0 : TargetSpecializedStandardPseudoInstruction<LOAD_STACK_GUARD>;
+
+// ERROR: :[[@LINE+1]]:5: error: multiple overrides of 'LOAD_STACK_GUARD' defined
+def MY_LOAD_STACK_GUARD_1 : TargetSpecializedStandardPseudoInstruction<LOAD_STACK_GUARD>;
+
+#endif
+
+def MY_MOV : TestInstruction {
+  let OutOperandList = (outs XRegs:$dst);
+  let InOperandList = (ins XRegs:$src);
+  let AsmString = "my_mov $dst, $src";
+}
+
+
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
diff --git a/llvm/utils/TableGen/Common/CodeGenTarget.cpp b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
index 3db0d07eec88f..21073f1f805d3 100644
--- a/llvm/utils/TableGen/Common/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
@@ -278,21 +278,32 @@ void CodeGenTarget::ComputeInstrsByEnum() const {
     const CodeGenInstruction *Instr = GetInstByName(Name, InstMap, Records);
     assert(Instr && "Missing target independent instruction");
     assert(Instr->Namespace == "TargetOpcode" && "Bad namespace");
+
     InstrsByEnum.push_back(Instr);
   }
   unsigned EndOfPredefines = InstrsByEnum.size();
   assert(EndOfPredefines == getNumFixedInstructions() &&
          "Missing generic opcode");
 
+  unsigned SkippedInsts = 0;
+
   for (const auto &[_, CGIUp] : InstMap) {
     const CodeGenInstruction *CGI = CGIUp.get();
     if (CGI->Namespace != "TargetOpcode") {
+
+      if (CGI->TheDef->isSubClassOf(
+              "TargetSpecializedStandardPseudoInstruction")) {
+        ++SkippedInsts;
+        continue;
+      }
+
       InstrsByEnum.push_back(CGI);
       NumPseudoInstructions += CGI->TheDef->getValueAsBit("isPseudo");
     }
   }
 
-  assert(InstrsByEnum.size() == InstMap.size() && "Missing predefined instr");
+  assert(InstrsByEnum.size() + SkippedInsts == InstMap.size() &&
+         "Missing predefined instr");
 
   // All of the instructions are now in random order based on the map iteration.
   llvm::sort(
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index 176e4b250b82a..ed32f537d4b5d 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -71,6 +71,13 @@ class InstrInfoEmitter {
   typedef std::vector<OperandInfoTy> OperandInfoListTy;
   typedef std::map<OperandInfoTy, unsigned> OperandInfoMapTy;
 
+  DenseMap<const CodeGenInstruction *, const CodeGenInstruction *>
+      TargetSpecializedPseudoInsts;
+
+  /// Compute mapping of opcodes which should have their definitions overridden
+  /// by a target version.
+  void buildTargetSpecializedPseudoInstsMap();
+
   /// Generate member functions in the target-specific GenInstrInfo class.
   ///
   /// This method is used to custom expand TIIPredicate definitions.
@@ -215,6 +222,10 @@ InstrInfoEmitter::CollectOperandInfo(OperandInfoListTy &OperandInfoList,
   const CodeGenTarget &Target = CDP.getTargetInfo();
   unsigned Offset = 0;
   for (const CodeGenInstruction *Inst : Target.getInstructions()) {
+    auto OverrideEntry = TargetSpecializedPseudoInsts.find(Inst);
+    if (OverrideEntry != TargetSpecializedPseudoInsts.end())
+      Inst = OverrideEntry->second;
+
     OperandInfoTy OperandInfo = GetOperandInfo(*Inst);
     if (OperandInfoMap.try_emplace(OperandInfo, Offset).second) {
       OperandInfoList.push_back(OperandInfo);
@@ -882,6 +893,25 @@ void InstrInfoEmitter::emitTIIHelperMethods(raw_ostream &OS,
   }
 }
 
+void InstrInfoEmitter::buildTargetSpecializedPseudoInstsMap() {
+  ArrayRef<const Record *> SpecializedInsts = Records.getAllDerivedDefinitions(
+      "TargetSpecializedStandardPseudoInstruction");
+  const CodeGenTarget &Target = CDP.getTargetInfo();
+
+  for (const Record *SpecializedRec : SpecializedInsts) {
+    const CodeGenInstruction &SpecializedInst =
+        Target.getInstruction(SpecializedRec);
+    const Record *BaseInstRec = SpecializedRec->getValueAsDef("Instruction");
+
+    const CodeGenInstruction &BaseInst = Target.getInstruction(BaseInstRec);
+
+    if (!TargetSpecializedPseudoInsts.insert({&BaseInst, &SpecializedInst})
+             .second)
+      PrintFatalError(SpecializedRec, "multiple overrides of '" +
+                                          BaseInst.getName() + "' defined");
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // Main Output.
 //===----------------------------------------------------------------------===//
@@ -904,6 +934,8 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
 
   // Collect all of the operand info records.
   Timer.startTimer("Collect operand info");
+  buildTargetSpecializedPseudoInstsMap();
+
   OperandInfoListTy OperandInfoList;
   OperandInfoMapTy OperandInfoMap;
   unsigned OperandInfoSize =
@@ -970,6 +1002,11 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
   for (const CodeGenInstruction *Inst : reverse(NumberedInstructions)) {
     // Keep a list of the instruction names.
     InstrNames.add(Inst->getName());
+
+    auto OverrideEntry = TargetSpecializedPseudoInsts.find(Inst);
+    if (OverrideEntry != TargetSpecializedPseudoInsts.end())
+      Inst = OverrideEntry->second;
+
     // Emit the record into the table.
     emitRecord(*Inst, --Num, InstrInfo, EmittedLists, OperandInfoMap, OS);
   }

let isConvergent = base_inst.isConvergent;
let hasExtraSrcRegAllocReq = base_inst.hasExtraSrcRegAllocReq;
let hasExtraDefRegAllocReq = base_inst.hasExtraDefRegAllocReq;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The unfortunate thing is that we have to duplicate all these members here.
But we don't need them all, right? Only those that are overridden by the instructions from the list below. E.g., we don't need isMeta, Size, isRematerializable etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isRematerializable is used. And I'd rather be safe than sorry and copy literally everything

@s-barannikov
Copy link
Contributor

s-barannikov commented Sep 20, 2025

Can I propose an alternative? Define these instructions with placeholder operand derived from bodyless ptr_rc, and require targets to provide an override by deriving from the same ptr_rc in addition to RegisterClass/RegClassByHwMode.

Something like:

// Target.td
class ptr_rc;
def placeholder_ptr_rc : ptr_rc;

def LOAD_STACK_GUARD : StandardPseudoInstruction {
  let OutOperandList = (outs placeholder_ptr_rc:$dst);
  ...
}

// X86InstrOperands.td
def x86_ptr_rc : ptr_rc, RegClassByHwMode<...>;

TableGen will replace placeholder_ptr_rd with the target-specific subclass of ptr_rc where necessary (checking that there is exactly one override).
I can see an advantage that we can support different address spaces by just adding a template parameter. (not sure this will ever be needed)

@arsenm
Copy link
Contributor Author

arsenm commented Sep 20, 2025

That might work, but that also is more limiting. In particular that doesn't allow you to set implicit uses / defs like you would want for adjcallstack pseudos

@s-barannikov
Copy link
Contributor

That might work, but that also is more limiting.

I think it is a good thing. All target-independent pseudo instructions should have the same semantics across all targets, otherwise they become target-dependent. If a target can override a standard pseudo instruction in any way it wishes, then there is nothing in common between these instructions across targets except for the opcode. This makes them unsuitable for use in generic code.

The instructions from the list have few uses in generic code. It looks like the point in introducing them was to guarantee they have certain properties. If a target can override the properties or get them wrong accidentally, it creates opportunities for bugs to creep in.

In particular that doesn't allow you to set implicit uses / defs like you would want for adjcallstack pseudos

Those are no different from other target-specific instructions IMO. They just happen to have similar names on most targets. To make the argument more obvious/convincing, consider CALL instruction, which goes in the same row. All targets have call instructions, but does that fact alone makes CALL plausible for promotion? I don't think so. Call instructions are too different across targets and most targets have multiple variants of them.

@arsenm
Copy link
Contributor Author

arsenm commented Sep 22, 2025

I think it is a good thing. All target-independent pseudo instructions should have the same semantics across all targets, otherwise they become target-dependent. If a target can override a standard pseudo instruction in any way it wishes, then there is nothing in common between these instructions across targets except for the opcode. This makes them unsuitable for use in generic code.

If you use the correct APIs it wouldn't really matter. Plus we could enforce what is allowed to differ

The instructions from the list have few uses in generic code. It looks like the point in introducing them was to guarantee they have certain properties. If a target can override the properties or get them wrong accidentally, it creates opportunities for bugs to creep in.

In particular that doesn't allow you to set implicit uses / defs like you would want for adjcallstack pseudos

Those are no different from other target-specific instructions IMO. They just happen to have similar names on most targets.

They are quite different from other target instructions. The call sequence pseudos are generically processed

To make the argument more obvious/convincing, consider CALL instruction, which goes in the same row. All targets have call instructions, but does that fact alone makes CALL plausible for promotion? I don't think so. Call instructions are too different across targets and most targets have multiple variants of them.

Calls are quite different and there can be many per target

@s-barannikov
Copy link
Contributor

The call sequence pseudos are generically processed

So are calls, returns, branches etc. That's why we have isCall, isReturn, isBranch.

Calls are quite different and there can be many per target

The fact that we can only have one adjcallstack up/down instruction is a limitation. Some targets could have multiple variants of them if allowed. This limitation doesn't make them special.

If you use the correct APIs it wouldn't really matter. Plus we could enforce what is allowed to differ

What else can differ? Certainly not the number of operands and properties like isCall. The enforcement could be quite expensive compared to the benefits the generalization theoretically gives. And we may end up allowing only the pointer operand types to change, which could be implemented much simpler like I suggested above.

@arsenm
Copy link
Contributor Author

arsenm commented Sep 25, 2025

The call sequence pseudos are generically processed

So are calls, returns, branches etc. That's why we have isCall, isReturn, isBranch.

No, the type of processing is very different. The callstack pseudos are passed to the TargetInstrInfo constructor, and must have the exact same immediate operands for the default frame management. They only differ by the physical register reference

Calls are quite different and there can be many per target

The fact that we can only have one adjcallstack up/down instruction is a limitation. Some targets could have multiple variants of them if allowed.

I don't see what the added complexity would buy you. You just need a vehicle for 2 stack offset adjustment amounts, and what the register is.

If you use the correct APIs it wouldn't really matter. Plus we could enforce what is allowed to differ

What else can differ? Certainly not the number of operands and properties like isCall.

Implicit register uses, for the call stack adjustments, some of the heuristic bits like isAsCheapAsAMove

The enforcement could be quite expensive

I don't see how checking instruction operand lists can be expensive

Allow a target to steal the definition of a generic pseudoinstruction
and remap the operands. This works by defining a new instruction, which
will simply swap out the emitted entry in the InstrInfo table.

This is intended to eliminate the C++ half of the implementation
of PointerLikeRegClass. With RegClassByHwMode, the remaining usecase
for PointerLikeRegClass are the common codegen pseudoinstructions.
Every target maintains its own copy of the generic pseudo operand
definitions anyway, so we can stub out the register operands with
an appropriate class instead of waiting for runtime resolution.

In the future we could probably take this a bit further. For example,
there is a similar problem for ADJCALLSTACKUP/DOWN since they depend
on target register definitions for the stack pointer register.
@arsenm arsenm force-pushed the users/arsenm/amdgpu/use-regclassbyhwmode-av-align-registers branch from 5b4d86d to c1ac2e0 Compare October 4, 2025 10:06
@arsenm arsenm force-pushed the users/arsenm/tablegen/support-target-specialized-pseudos branch from 8f62ad4 to 74bae94 Compare October 4, 2025 10:06
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