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][GlobalISel] Add specialized opcodes #74823

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

Pierre-vh
Copy link
Contributor

@Pierre-vh Pierre-vh commented Dec 8, 2023

NOTE: This PR is part of a stack, see #74429 to review the first commits

Most users of AddImm and CheckConstantInt only use 1 byte immediates, so I added an opcode variants for those. That way all those instructions save 7 bytes.
Also added an opcode for AddTempRegister for the cases where there are no register flags.

Space savings:
- AMDGPUGenGlobalISel: 470180 bytes to 422564 (-(10%)
- AArch64GenGlobalISel.inc: 383893 bytes to 374046

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 8, 2023

@llvm/pr-subscribers-llvm-globalisel

Author: Pierre van Houtryve (Pierre-vh)

Changes

NOTE: This PR is part of a stack, see #74429 to review the first commits

Most users of AddImm and CheckConstantInt only use 1 byte immediates, so I added an opcode variants for those. That way all those instructions save 7 bytes.
Also added an opcode for AddTempRegister for the cases where there are no register flags.

Space savings:
- AMDGPUGenGlobalISel: 470180 bytes to 422564 (-(10%)
- AArch64GenGlobalISel.inc: 383893 bytes to 374046


Patch is 458.79 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/74823.diff

47 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h (+233-188)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h (+241-195)
  • (modified) llvm/test/TableGen/ContextlessPredicates.td (+49-51)
  • (modified) llvm/test/TableGen/DefaultOpsGlobalISel.td (+194-95)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/builtins/match-table-eraseroot.td (+6-6)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/builtins/match-table-replacerreg.td (+18-18)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-imms.td (+29-29)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-miflags.td (+12-12)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-operand-types.td (+11-11)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-patfrag-root.td (+29-29)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td (+130-130)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-typeof.td (+19-19)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-variadics.td (+24-24)
  • (modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td (+61-61)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-PR39045.td (+2-2)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-SDNodeXForm-timm.td (+6-6)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-atomic_store.td (+4-4)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-immAllZeroOne.td (+6-6)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-immarg-literal-pattern.td (+10-11)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-input-discard.td (+7-7)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-multiple-output-discard.td (+6-6)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-multiple-output.td (+12-12)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-nested-subregs.td (+15-15)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-notype-output-pattern.td (+3-3)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-output-discard.td (+4-4)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-setcc.td (+4-4)
  • (modified) llvm/test/TableGen/GlobalISelEmitter-zero-reg.td (+7-7)
  • (modified) llvm/test/TableGen/GlobalISelEmitter.td (+241-241)
  • (modified) llvm/test/TableGen/GlobalISelEmitterCustomPredicate.td (+144-144)
  • (modified) llvm/test/TableGen/GlobalISelEmitterFlags.td (+6-6)
  • (modified) llvm/test/TableGen/GlobalISelEmitterHwModes.td (+30-30)
  • (modified) llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizer.td (+21-21)
  • (modified) llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td (+33-34)
  • (modified) llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand.td (+1-1)
  • (modified) llvm/test/TableGen/GlobalISelEmitterOverloadedPtr.td (+5-5)
  • (modified) llvm/test/TableGen/GlobalISelEmitterRegSequence.td (+26-26)
  • (modified) llvm/test/TableGen/GlobalISelEmitterSubreg.td (+84-84)
  • (modified) llvm/test/TableGen/GlobalISelEmitterVariadic.td (+11-11)
  • (modified) llvm/test/TableGen/HasNoUse.td (+4-4)
  • (modified) llvm/test/TableGen/address-space-patfrags.td (+16-16)
  • (modified) llvm/test/TableGen/gisel-physreg-input.td (+14-14)
  • (modified) llvm/test/TableGen/immarg-predicated.td (+4-4)
  • (modified) llvm/test/TableGen/immarg.td (+3-3)
  • (modified) llvm/test/TableGen/predicate-patfags.td (+10-10)
  • (modified) llvm/utils/TableGen/GlobalISelMatchTable.cpp (+341-197)
  • (modified) llvm/utils/TableGen/GlobalISelMatchTable.h (+22-24)
  • (modified) llvm/utils/TableGen/GlobalISelMatchTableExecutorEmitter.cpp (+7-2)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index f5d9f5f40881cb..73308925e9142d 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -54,10 +54,30 @@ enum {
   GICXXCustomAction_Invalid = 0,
 };
 
+/// The MatchTable is encoded as an array of bytes.
+/// Thus, opcodes are expected to be <255.
+///
+/// Operands can be variable-sized, their size is always after their name
+/// in the docs, e.g. "Foo(4)" means that "Foo" takes 4 entries in the table,
+/// so 4 bytes. "Foo()"
+///
+/// As a general rule of thumb:
+///   - Instruction & Operand IDs are ULEB128
+///   - LLT IDs are 1 byte
+///   - Predicates and target opcodes, register and register class IDs are 2
+///     bytes.
+///   - Indexes into the table are 4 bytes.
+///   - Inline constants are 8 bytes
+///
+/// Design notes:
+///   - Inst/Op IDs have to be LEB128 because some targets generate
+///     extremely long patterns which need more than 255 temporaries.
+///     We could just use 2 bytes everytime, but then some targets like
+///     X86/AMDGPU that have no need for it will pay the price all the time.
 enum {
   /// Begin a try-block to attempt a match and jump to OnFail if it is
   /// unsuccessful.
-  /// - OnFail - The MatchTable entry at which to resume if the match fails.
+  /// - OnFail(4) - The MatchTable entry at which to resume if the match fails.
   ///
   /// FIXME: This ought to take an argument indicating the number of try-blocks
   ///        to exit on failure. It's usually one but the last match attempt of
@@ -68,100 +88,103 @@ enum {
   GIM_Try,
 
   /// Switch over the opcode on the specified instruction
-  /// - InsnID - Instruction ID
-  /// - LowerBound - numerically minimum opcode supported
-  /// - UpperBound - numerically maximum + 1 opcode supported
-  /// - Default - failure jump target
-  /// - JumpTable... - (UpperBound - LowerBound) (at least 2) jump targets
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - LowerBound(2) - numerically minimum opcode supported
+  /// - UpperBound(2) - numerically maximum + 1 opcode supported
+  /// - Default(4) - failure jump target
+  /// - JumpTable(4)... - (UpperBound - LowerBound) (at least 2) jump targets
   GIM_SwitchOpcode,
 
   /// Switch over the LLT on the specified instruction operand
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - LowerBound - numerically minimum Type ID supported
-  /// - UpperBound - numerically maximum + 1 Type ID supported
-  /// - Default - failure jump target
-  /// - JumpTable... - (UpperBound - LowerBound) (at least 2) jump targets
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - LowerBound(2) - numerically minimum Type ID supported
+  /// - UpperBound(2) - numerically maximum + 1 Type ID supported
+  /// - Default(4) - failure jump target
+  /// - JumpTable(4)... - (UpperBound - LowerBound) (at least 2) jump targets
   GIM_SwitchType,
 
   /// Record the specified instruction.
   /// The IgnoreCopies variant ignores COPY instructions.
-  /// - NewInsnID - Instruction ID to define
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
+  /// - NewInsnID(ULEB128) - Instruction ID to define
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
   GIM_RecordInsn,
   GIM_RecordInsnIgnoreCopies,
 
   /// Check the feature bits
-  /// - Expected features
+  ///   Feature(2) - Expected features
   GIM_CheckFeatures,
 
   /// Check the opcode on the specified instruction
-  /// - InsnID - Instruction ID
-  /// - Expected opcode
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Opc(2) - Expected opcode
   GIM_CheckOpcode,
 
   /// Check the opcode on the specified instruction, checking 2 acceptable
   /// alternatives.
-  /// - InsnID - Instruction ID
-  /// - Expected opcode
-  /// - Alternative expected opcode
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Opc(2) - Expected opcode
+  /// - Opc(2) - Alternative expected opcode
   GIM_CheckOpcodeIsEither,
 
   /// Check the instruction has the right number of operands
-  /// - InsnID - Instruction ID
-  /// - Expected number of operands
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Ops(ULEB128) - Expected number of operands
   GIM_CheckNumOperands,
+
   /// Check an immediate predicate on the specified instruction
-  /// - InsnID - Instruction ID
-  /// - The predicate to test
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Pred(2) - The predicate to test
   GIM_CheckI64ImmPredicate,
   /// Check an immediate predicate on the specified instruction via an APInt.
-  /// - InsnID - Instruction ID
-  /// - The predicate to test
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Pred(2) - The predicate to test
   GIM_CheckAPIntImmPredicate,
   /// Check a floating point immediate predicate on the specified instruction.
-  /// - InsnID - Instruction ID
-  /// - The predicate to test
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Pred(2) - The predicate to test
   GIM_CheckAPFloatImmPredicate,
   /// Check an immediate predicate on the specified instruction
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - The predicate to test
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - Pred(2) - The predicate to test
   GIM_CheckImmOperandPredicate,
+
   /// Check a memory operation has the specified atomic ordering.
-  /// - InsnID - Instruction ID
-  /// - Ordering - The AtomicOrdering value
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - Ordering(ULEB128) - The AtomicOrdering value
   GIM_CheckAtomicOrdering,
   GIM_CheckAtomicOrderingOrStrongerThan,
   GIM_CheckAtomicOrderingWeakerThan,
+
   /// Check the size of the memory access for the given machine memory operand.
-  /// - InsnID - Instruction ID
-  /// - MMOIdx - MMO index
-  /// - Size - The size in bytes of the memory access
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - MMOIdx(ULEB128) - MMO index
+  /// - Size(4) - The size in bytes of the memory access
   GIM_CheckMemorySizeEqualTo,
 
   /// Check the address space of the memory access for the given machine memory
   /// operand.
-  /// - InsnID - Instruction ID
-  /// - MMOIdx - MMO index
-  /// - NumAddrSpace - Number of valid address spaces
-  /// - AddrSpaceN - An allowed space of the memory access
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - MMOIdx(ULEB128) - MMO index
+  /// - NumAddrSpace(ULEB128) - Number of valid address spaces
+  /// - AddrSpaceN(ULEB128) - An allowed space of the memory access
   /// - AddrSpaceN+1 ...
   GIM_CheckMemoryAddressSpace,
 
   /// Check the minimum alignment of the memory access for the given machine
   /// memory operand.
-  /// - InsnID - Instruction ID
-  /// - MMOIdx - MMO index
-  /// - MinAlign - Minimum acceptable alignment
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - MMOIdx(ULEB128) - MMO index
+  /// - MinAlign(ULEB128) - Minimum acceptable alignment
   GIM_CheckMemoryAlignment,
 
   /// Check the size of the memory access for the given machine memory operand
   /// against the size of an operand.
-  /// - InsnID - Instruction ID
-  /// - MMOIdx - MMO index
-  /// - OpIdx - The operand index to compare the MMO against
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - MMOIdx(ULEB128) - MMO index
+  /// - OpIdx(ULEB128) - The operand index to compare the MMO against
   GIM_CheckMemorySizeEqualToLLT,
   GIM_CheckMemorySizeLessThanLLT,
   GIM_CheckMemorySizeGreaterThanLLT,
@@ -170,106 +193,117 @@ enum {
   /// constant. This is valid for both G_BUILD_VECTOR as well as
   /// G_BUILD_VECTOR_TRUNC. For AllOnes refers to individual bits, so a -1
   /// element.
-  /// - InsnID - Instruction ID
+  /// - InsnID(ULEB128) - Instruction ID
   GIM_CheckIsBuildVectorAllOnes,
   GIM_CheckIsBuildVectorAllZeros,
 
   /// Check a trivial predicate which takes no arguments.
   /// This can be used by executors to implement custom flags that don't fit in
   /// target features.
+  /// - Pred(2) - Predicate ID to check.
   GIM_CheckSimplePredicate,
 
   /// Check a generic C++ instruction predicate
-  /// - InsnID - Instruction ID
-  /// - PredicateID - The ID of the predicate function to call
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - PredicateID(2) - The ID of the predicate function to call
   GIM_CheckCxxInsnPredicate,
 
   /// Check if there's no use of the first result.
-  /// - InsnID - Instruction ID
+  /// - InsnID(ULEB128) - Instruction ID
   GIM_CheckHasNoUse,
 
   /// Check the type for the specified operand
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - Expected type
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - Ty(1) - Expected type
   GIM_CheckType,
+
   /// Check the type of a pointer to any address space.
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - SizeInBits - The size of the pointer value in bits.
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - SizeInBits(ULEB128) - The size of the pointer value in bits.
   GIM_CheckPointerToAny,
+
   /// Check the register bank for the specified operand
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - Expected register bank (specified as a register class)
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - RC(2) - Expected register bank (specified as a register class)
   GIM_CheckRegBankForClass,
 
   /// Check the operand matches a complex predicate
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - RendererID - The renderer to hold the result
-  /// - Complex predicate ID
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - RendererID(2) - The renderer to hold the result
+  /// - Pred(2) - Complex predicate ID
   GIM_CheckComplexPattern,
 
   /// Check the operand is a specific integer
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - Expected integer
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - Val(8) Expected integer
   GIM_CheckConstantInt,
+
+  /// Check the operand is a specific 8-bit signed integer
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - Val(1) Expected integer
+  GIM_CheckConstantInt8,
+
   /// Check the operand is a specific literal integer (i.e. MO.isImm() or
   /// MO.isCImm() is true).
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - Expected integer
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - Val(8) - Expected integer
   GIM_CheckLiteralInt,
+
   /// Check the operand is a specific intrinsic ID
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - Expected Intrinsic ID
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - IID(2) - Expected Intrinsic ID
   GIM_CheckIntrinsicID,
 
   /// Check the operand is a specific predicate
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - Expected predicate
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - Pred(2) - Expected predicate
   GIM_CheckCmpPredicate,
 
   /// Check the specified operand is an MBB
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
   GIM_CheckIsMBB,
 
   /// Check the specified operand is an Imm
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
   GIM_CheckIsImm,
 
   /// Check if the specified operand is safe to fold into the current
   /// instruction.
-  /// - InsnID - Instruction ID
+  /// - InsnID(ULEB128) - Instruction ID
   GIM_CheckIsSafeToFold,
 
   /// Check the specified operands are identical.
   /// The IgnoreCopies variant looks through COPY instructions before
   /// comparing the operands.
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - OtherInsnID - Other instruction ID
-  /// - OtherOpIdx - Other operand index
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - OtherInsnID(ULEB128) - Other instruction ID
+  /// - OtherOpIdx(ULEB128) - Other operand index
   GIM_CheckIsSameOperand,
   GIM_CheckIsSameOperandIgnoreCopies,
 
   /// Check we can replace all uses of a register with another.
-  /// - OldInsnID
-  /// - OldOpIdx
-  /// - NewInsnID
-  /// - NewOpIdx
+  /// - OldInsnID(ULEB128)
+  /// - OldOpIdx(ULEB128)
+  /// - NewInsnID(ULEB128)
+  /// - NewOpIdx(ULEB128)
   GIM_CheckCanReplaceReg,
 
   /// Check that a matched instruction has, or doesn't have a MIFlag.
   ///
-  /// - InsnID  - Instruction to check.
-  /// - Flag(s) - (can be one or more flags OR'd together)
+  /// - InsnID(ULEB128) - Instruction to check.
+  /// - Flags(4) - (can be one or more flags OR'd together)
   GIM_MIFlags,
   GIM_MIFlagsNot,
 
@@ -277,15 +311,15 @@ enum {
   /// named operands that will be recorded in RecordedOperands. Names of these
   /// operands are referenced in predicate argument list. Emitter determines
   /// StoreIdx(corresponds to the order in which names appear in argument list).
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - StoreIdx - Store location in RecordedOperands.
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - StoreIdx(ULEB128) - Store location in RecordedOperands.
   GIM_RecordNamedOperand,
 
   /// Records an operand's register type into the set of temporary types.
-  /// - InsnID - Instruction ID
-  /// - OpIdx - Operand index
-  /// - TempTypeIdx - Temp Type Index, always negative.
+  /// - InsnID(ULEB128) - Instruction ID
+  /// - OpIdx(ULEB128) - Operand index
+  /// - TempTypeIdx(1) - Temp Type Index, always negative.
   GIM_RecordRegType,
 
   /// Fail the current try-block, or completely fail to match if there is no
@@ -295,121 +329,133 @@ enum {
   //=== Renderers ===
 
   /// Mutate an instruction
-  /// - NewInsnID - Instruction ID to define
-  /// - OldInsnID - Instruction ID to mutate
-  /// - NewOpcode - The new opcode to use
+  /// - NewInsnID(ULEB128) - Instruction ID to define
+  /// - OldInsnID(ULEB128) - Instruction ID to mutate
+  /// - NewOpcode(2) - The new opcode to use
   GIR_MutateOpcode,
 
   /// Build a new instruction
-  /// - InsnID - Instruction ID to define
-  /// - Opcode - The new opcode to use
+  /// - InsnID(ULEB128) - Instruction ID to define
+  /// - Opcode(2) - The new opcode to use
   GIR_BuildMI,
 
   /// Builds a constant and stores its result in a TempReg.
-  /// - TempRegID - Temp Register to define.
-  /// - Imm - The immediate to add
+  /// - TempRegID(ULEB128) - Temp Register to define.
+  /// - Imm(8) - The immediate to add
   GIR_BuildConstant,
 
   /// Copy an operand to the specified instruction
-  /// - NewInsnID - Instruction ID to modify
-  /// - OldInsnID - Instruction ID to copy from
-  /// - OpIdx - The operand to copy
+  /// - NewInsnID(ULEB128) - Instruction ID to modify
+  /// - OldInsnID(ULEB128) - Instruction ID to copy from
+  /// - OpIdx(ULEB128) - The operand to copy
   GIR_Copy,
 
   /// Copy an operand to the specified instruction or add a zero register if the
   /// operand is a zero immediate.
-  /// - NewInsnID - Instruction ID to modify
-  /// - OldInsnID - Instruction ID to copy from
-  /// - OpIdx - The operand to copy
-  /// - ZeroReg - The zero register to use
+  /// - NewInsnID(ULEB128) - Instruction ID to modify
+  /// - OldInsnID(ULEB128) - Instruction ID to copy from
+  /// - OpIdx(ULEB128) - The operand to copy
+  /// - ZeroReg(2) - The zero register to use
   GIR_CopyOrAddZeroReg,
   /// Copy an operand to the specified instruction
-  /// - NewInsnID - Instruction ID to modify
-  /// - OldInsnID - Instruction ID to copy from
-  /// - OpIdx - The operand to copy
-  /// - SubRegIdx - The subregister to copy
+  /// - NewInsnID(ULEB128) - Instruction ID to modify
+  /// - OldInsnID(ULEB128) - Instruction ID to copy from
+  /// - OpIdx(ULEB128) - The operand to copy
+  /// - SubRegIdx(2) - The subregister to copy
   GIR_CopySubReg,
 
   /// Add an implicit register def to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - RegNum - The register to add
-  /// - Flags - Register Flags
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - RegNum(2) - The register to add
+  /// - Flags(2) - Register Flags
   GIR_AddImplicitDef,
   /// Add an implicit register use to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - RegNum - The register to add
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - RegNum(2) - The register to add
   GIR_AddImplicitUse,
   /// Add an register to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - RegNum - The register to add
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - RegNum(2) - The register to add
+  /// - Flags(2) - Register Flags
   GIR_AddRegister,
 
   /// Marks the implicit def of a register as dead.
-  /// - InsnID - Instruction ID to modify
-  /// - OpIdx - The implicit def operand index
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - OpIdx(ULEB128) - The implicit def operand index
   ///
   /// OpIdx starts at 0 for the first implicit def.
   GIR_SetImplicitDefDead,
 
   /// Set or unset a MIFlag on an instruction.
   ///
-  /// - InsnID  - Instruction to modify.
-  /// - Flag(s) - (can be one or more flags OR'd together)
+  /// - InsnID(ULEB128)  - Instruction to modify.
+  /// - Flags(4) - (can be one or more flags OR'd together)
   GIR_SetMIFlags,
   GIR_UnsetMIFlags,
 
   /// Copy the MIFlags of a matched instruction into an
   /// output instruction. The flags are OR'd together.
   ///
-  /// - InsnID     - Instruction to modify.
-  /// - OldInsnID  - Matched instruction to copy flags from.
+  /// - InsnID(ULEB128)     - Instruction to modify.
+  /// - OldInsnID(ULEB128)  - Matched instruction to copy flags from.
   GIR_CopyMIFlags,
 
   /// Add a temporary register to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - TempRegID - The temporary register ID to add
-  /// - TempRegFlags - The register flags to set
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - TempRegID(ULEB128) - The temporary register ID to add
+  /// - TempRegFlags(2) - The register flags to set
   GIR_AddTempRegister,
 
+  /// Add a temporary register to the specified instruction without
+  /// setting any flags.
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - TempRegID(ULEB128) - The temporary register ID to add
+  GIR_AddSimpleTempRegister,
+
   /// Add a temporary register to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - TempRegID - The temporary register ID to add
-  /// - TempRegFlags - The register flags to set
-  /// - SubRegIndex - The subregister index to set
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - TempRegID(ULEB128) - The temporary register ID to add
+  /// - TempRegFlags(2) - The register flags to set
+  /// - SubRegIndex(2) - The subregister index to set
   GIR_AddTempSubRegister,
 
   /// Add an immediate to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - Imm - The immediate to add
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - Imm(8) - The immediate to add
   GIR_AddImm,
 
+  /// Add signed 8 bit immediate to the specified instruction
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - Imm(1) - The immediate to add
+  GIR_AddImm8,
+
   /// Add an CImm to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - Ty - Type of the constant immediate.
-  /// - Imm - The immediate to add
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /// - Ty(1) - Type of the constant immediate.
+  /// - Imm(8) - The immediate to add
   GIR_AddCImm,
 
   /// Render complex operands to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - RendererID - The renderer to call
+  /// - InsnID(ULEB128) - Instruction ID to modify
+  /...
[truncated]

Copy link

github-actions bot commented Dec 8, 2023

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

@qcolombet
Copy link
Collaborator

What are the code size gain here?
What is the compile time impact?
(I expect that reading ULEB is not going to be free.)

@Pierre-vh
Copy link
Contributor Author

What are the code size gain here? What is the compile time impact? (I expect that reading ULEB is not going to be free.)

This doesn't add ULEB reading to the table, that's in #74429
There are some numbers in the RFC. So far no noticeable impact to performance, it might even be a bit faster due to the massive reduction in table size (about 8Mb less in LLC's binary size)

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM!

Most users of AddImm and CheckConstantInt only use 1 byte immediates, so I added an opcode variants for those. That way all those instructions save 7 bytes.
Also added an opcode for AddTempRegister for the cases where there are no register flags.

Space savings:
 - AMDGPUGenGlobalISel: 470180 bytes to 422564 (-(10%)
 - AArch64GenGlobalISel.inc: 383893 bytes to 374046
@Pierre-vh Pierre-vh merged commit a160536 into llvm:main Dec 13, 2023
3 of 4 checks passed
@Pierre-vh Pierre-vh deleted the gisel-specialize-opcs branch December 13, 2023 08:09
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.

None yet

5 participants