From 2d7ea717c28fe34ae3c122525722bc129fe9d33a Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Thu, 13 Nov 2025 16:34:24 -0800 Subject: [PATCH 1/3] [AMDGPU] Use std::variant in ArgDescriptor. This replaces the 2 bool flags and the anonymous union. This also removes an implicit conversion from Register to unsigned and a call to MCRegister::id(). The ArgDescriptor constructor was always assigning the union through the MCRegister field even for stack offsets. The change to SIMachineFunctionInfo.h fixes a case where getRegister was being called on an unset ArgDescriptor. Since it was only this case, it seemed cleaner to fix it at the caller. The other option would be to make getRegister() return MCRegister() for an unset ArgDescriptor. --- .../Target/AMDGPU/AMDGPUArgumentUsageInfo.h | 42 +++++++++---------- .../lib/Target/AMDGPU/SIMachineFunctionInfo.h | 4 +- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h index 8838a94a639eb..4c382a0046b64 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h @@ -13,6 +13,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/CodeGen/Register.h" #include "llvm/Pass.h" +#include namespace llvm { @@ -27,54 +28,49 @@ struct ArgDescriptor { friend struct AMDGPUFunctionArgInfo; friend class AMDGPUArgumentUsageInfo; - union { - MCRegister Reg; - unsigned StackOffset; - }; + std::variant Val; // Bitmask to locate argument within the register. unsigned Mask; - bool IsStack : 1; - bool IsSet : 1; - public: - ArgDescriptor(unsigned Val = 0, unsigned Mask = ~0u, bool IsStack = false, - bool IsSet = false) - : Reg(Val), Mask(Mask), IsStack(IsStack), IsSet(IsSet) {} + ArgDescriptor(unsigned Mask = ~0u) : Mask(Mask) {} static ArgDescriptor createRegister(Register Reg, unsigned Mask = ~0u) { - return ArgDescriptor(Reg, Mask, false, true); + ArgDescriptor Ret(Mask); + Ret.Val = Reg.asMCReg(); + return Ret; } static ArgDescriptor createStack(unsigned Offset, unsigned Mask = ~0u) { - return ArgDescriptor(Offset, Mask, true, true); + ArgDescriptor Ret(Mask); + Ret.Val = Offset; + return Ret; } static ArgDescriptor createArg(const ArgDescriptor &Arg, unsigned Mask) { - return ArgDescriptor(Arg.Reg.id(), Mask, Arg.IsStack, Arg.IsSet); + // Copy the descriptor, then change the mask. + ArgDescriptor Ret(Arg); + Ret.Mask = Mask; + return Ret; } - bool isSet() const { - return IsSet; - } + bool isSet() const { return !std::holds_alternative(Val); } explicit operator bool() const { return isSet(); } - bool isRegister() const { - return !IsStack; - } + bool isRegister() const { return std::holds_alternative(Val); } MCRegister getRegister() const { - assert(!IsStack); - return Reg; + assert(isRegister()); + return std::get(Val); } unsigned getStackOffset() const { - assert(IsStack); - return StackOffset; + assert(std::holds_alternative(Val)); + return std::get(Val); } unsigned getMask() const { diff --git a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h index 019c3b79e5fe5..ca3c35067a923 100644 --- a/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h +++ b/llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.h @@ -1014,7 +1014,9 @@ class SIMachineFunctionInfo final : public AMDGPUMachineFunction, void setNumWaveDispatchVGPRs(unsigned Count) { NumWaveDispatchVGPRs = Count; } Register getPrivateSegmentWaveByteOffsetSystemSGPR() const { - return ArgInfo.PrivateSegmentWaveByteOffset.getRegister(); + if (ArgInfo.PrivateSegmentWaveByteOffset) + return ArgInfo.PrivateSegmentWaveByteOffset.getRegister(); + return MCRegister(); } /// Returns the physical register reserved for use as the resource From fc351943cbb3a971305a02cd9d8b07353d24df09 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Thu, 13 Nov 2025 19:08:14 -0800 Subject: [PATCH 2/3] fixup! Remove asserts --- llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h index 4c382a0046b64..6a4d72446d4ef 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h @@ -64,12 +64,10 @@ struct ArgDescriptor { bool isRegister() const { return std::holds_alternative(Val); } MCRegister getRegister() const { - assert(isRegister()); return std::get(Val); } unsigned getStackOffset() const { - assert(std::holds_alternative(Val)); return std::get(Val); } From 32e7e7a20909a7ae323737651a15d2c567a1b95f Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Thu, 13 Nov 2025 21:55:19 -0800 Subject: [PATCH 3/3] fixup! clang-format --- llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h index 6a4d72446d4ef..cb7f63639aee3 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h @@ -63,13 +63,9 @@ struct ArgDescriptor { bool isRegister() const { return std::holds_alternative(Val); } - MCRegister getRegister() const { - return std::get(Val); - } + MCRegister getRegister() const { return std::get(Val); } - unsigned getStackOffset() const { - return std::get(Val); - } + unsigned getStackOffset() const { return std::get(Val); } unsigned getMask() const { // None of the target SGPRs or VGPRs are expected to have a 'zero' mask.