Skip to content

[DirectX][NFC] Refactoring DirectX backend to not use llvm::to_underlying in switch cases. #151032

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.

Sign up for GitHub

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

Closed
wants to merge 13 commits into from
Closed

[DirectX][NFC] Refactoring DirectX backend to not use llvm::to_underlying in switch cases. #151032

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ea35c41
remove the to_undernlying switch
Jul 28, 2025
3c1fc51
remove comment
Jul 28, 2025
591d12a
remove compiling errors
Jul 28, 2025
0d55d28
improve comment, maybe
Jul 29, 2025
7841a83
Merge branch 'main' into refactoring/remove_to_undeling
joaosaffran Aug 6, 2025
98be089
Merge branch 'main' into refactoring/remove_to_undeling
Aug 6, 2025
25ee6d7
fixing merge issues
Aug 6, 2025
94fef90
Merge branch 'main' into refactoring/remove_to_undeling
Aug 15, 2025
3924438
fix
Aug 15, 2025
5f26016
finish changing
Aug 15, 2025
c975cc2
fix tests
joaosaffran Aug 18, 2025
1d080aa
Merge branch 'main' into refactoring/remove_to_undeling
joaosaffran Aug 18, 2025
3b5f497
clean up
joaosaffran Aug 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "llvm/IR/Constants.h"
#include "llvm/MC/DXContainerRootSignature.h"
#include "llvm/Support/Compiler.h"
#include <cstdint>

namespace llvm {
class LLVMContext;
Expand All @@ -28,9 +29,9 @@ class Metadata;
namespace hlsl {
namespace rootsig {

template <typename T>
template <typename T, typename ET = T>
class RootSignatureValidationError
: public ErrorInfo<RootSignatureValidationError<T>> {
: public ErrorInfo<RootSignatureValidationError<T, ET>> {
public:
static char ID;
StringRef ParamName;
Expand All @@ -40,7 +41,7 @@ class RootSignatureValidationError
: ParamName(ParamName), Value(Value) {}

void log(raw_ostream &OS) const override {
OS << "Invalid value for " << ParamName << ": " << Value;
OS << "Invalid value for " << ParamName << ": " << static_cast<ET>(Value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can probably eliminate the ET parameter type by doing something like this:

Suggested change
OS << "Invalid value for " << ParamName << ": " << static_cast<ET>(Value);
OS << "Invalid value for " << ParamName << ": ";
if constexpr (std::is_enum<T>)
OS << llvm::to_underlying(Value);
else
OS << Value;

}

std::error_code convertToErrorCode() const override {
Expand Down
30 changes: 20 additions & 10 deletions llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Metadata.h"
#include "llvm/Support/ScopedPrinter.h"
#include <cstdint>

using namespace llvm;

Expand All @@ -27,7 +28,7 @@ namespace rootsig {
char GenericRSMetadataError::ID;
char InvalidRSMetadataFormat::ID;
char InvalidRSMetadataValue::ID;
template <typename T> char RootSignatureValidationError<T>::ID;
template <typename T, typename ET> char RootSignatureValidationError<T, ET>::ID;
Copy link
Contributor

Choose a reason for hiding this comment

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

ET seems to be uint32_t in all the used cases, what is the purpose of this? Couldn't we just have the static_cast above use uint32_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want to always cast to uint32_t, only when dealing with enums. Otherwise, I want to preserve the original type.


static std::optional<uint32_t> extractMdIntValue(MDNode *Node,
unsigned int OpId) {
Expand Down Expand Up @@ -331,7 +332,8 @@ Error MetadataParser::parseDescriptorRange(mcdxbc::DescriptorTable &Table,
.Case("CBV", dxbc::DescriptorRangeType::CBV)
.Case("SRV", dxbc::DescriptorRangeType::SRV)
.Case("UAV", dxbc::DescriptorRangeType::UAV)
.Case("Sampler", dxbc::DescriptorRangeType::Sampler);
.Case("Sampler", dxbc::DescriptorRangeType::Sampler)
.Default(static_cast<dxbc::DescriptorRangeType>(-1));

if (!verifyRangeType(Range.RangeType))
return make_error<GenericRSMetadataError>("Invalid Descriptor Range type.",
Expand Down Expand Up @@ -532,7 +534,8 @@ Error MetadataParser::validateRootSignature(
if (!dxbc::isValidShaderVisibility(Info.Header.ShaderVisibility))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::ShaderVisibility>>(
make_error<
RootSignatureValidationError<dxbc::ShaderVisibility, uint32_t>>(
"ShaderVisibility", Info.Header.ShaderVisibility));

assert(dxbc::isValidParameterType(Info.Header.ParameterType) &&
Expand Down Expand Up @@ -610,25 +613,29 @@ Error MetadataParser::validateRootSignature(
if (!hlsl::rootsig::verifySamplerFilter(Sampler.Filter))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::SamplerFilter>>(
make_error<
RootSignatureValidationError<dxbc::SamplerFilter, uint32_t>>(
"Filter", Sampler.Filter));

if (!hlsl::rootsig::verifyAddress(Sampler.AddressU))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::TextureAddressMode>>(
make_error<
RootSignatureValidationError<dxbc::TextureAddressMode, uint32_t>>(
"AddressU", Sampler.AddressU));

if (!hlsl::rootsig::verifyAddress(Sampler.AddressV))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::TextureAddressMode>>(
make_error<
RootSignatureValidationError<dxbc::TextureAddressMode, uint32_t>>(
"AddressV", Sampler.AddressV));

if (!hlsl::rootsig::verifyAddress(Sampler.AddressW))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::TextureAddressMode>>(
make_error<
RootSignatureValidationError<dxbc::TextureAddressMode, uint32_t>>(
"AddressW", Sampler.AddressW));

if (!hlsl::rootsig::verifyMipLODBias(Sampler.MipLODBias))
Expand All @@ -645,13 +652,15 @@ Error MetadataParser::validateRootSignature(
if (!hlsl::rootsig::verifyComparisonFunc(Sampler.ComparisonFunc))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::ComparisonFunc>>(
make_error<
RootSignatureValidationError<dxbc::ComparisonFunc, uint32_t>>(
"ComparisonFunc", Sampler.ComparisonFunc));

if (!hlsl::rootsig::verifyBorderColor(Sampler.BorderColor))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::StaticBorderColor>>(
make_error<
RootSignatureValidationError<dxbc::StaticBorderColor, uint32_t>>(
"BorderColor", Sampler.BorderColor));

if (!hlsl::rootsig::verifyLOD(Sampler.MinLOD))
Expand Down Expand Up @@ -679,7 +688,8 @@ Error MetadataParser::validateRootSignature(
if (!dxbc::isValidShaderVisibility(Sampler.ShaderVisibility))
DeferredErrs = joinErrors(
std::move(DeferredErrs),
make_error<RootSignatureValidationError<dxbc::ShaderVisibility>>(
make_error<
RootSignatureValidationError<dxbc::ShaderVisibility, uint32_t>>(
"ShaderVisibility", Sampler.ShaderVisibility));
}

Expand Down
34 changes: 17 additions & 17 deletions ...t/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,100 +55,100 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
;DXC:- Name: RTS0
;DXC-NEXT: Size: 380
;DXC-NEXT: RootSignature:
;DXC-NEXT: Version: 2
;DXC-NEXT: Version: V1_1
;DXC-NEXT: NumRootParameters: 1
;DXC-NEXT: RootParametersOffset: 24
;DXC-NEXT: NumStaticSamplers: 0
;DXC-NEXT: StaticSamplersOffset: 0
;DXC-NEXT: Parameters:
;DXC-NEXT: - ParameterType: 0
;DXC-NEXT: ShaderVisibility: 0
;DXC-NEXT: - ParameterType: DescriptorTable
;DXC-NEXT: ShaderVisibility: All
;DXC-NEXT: Table:
;DXC-NEXT: NumRanges: 14
;DXC-NEXT: RangesOffset: 44
;DXC-NEXT: Ranges:
;DXC-NEXT: - RangeType: 3
;DXC-NEXT: - RangeType: Sampler
;DXC-NEXT: NumDescriptors: 1
;DXC-NEXT: BaseShaderRegister: 0
;DXC-NEXT: RegisterSpace: 1
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
;DXC-NEXT: - RangeType: 3
;DXC-NEXT: - RangeType: Sampler
;DXC-NEXT: NumDescriptors: 1
;DXC-NEXT: BaseShaderRegister: 0
;DXC-NEXT: RegisterSpace: 3
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
;DXC-NEXT: - RangeType: 3
;DXC-NEXT: - RangeType: Sampler
;DXC-NEXT: NumDescriptors: 1
;DXC-NEXT: BaseShaderRegister: 0
;DXC-NEXT: RegisterSpace: 4
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
;DXC-NEXT: - RangeType: 0
;DXC-NEXT: - RangeType: SRV
;DXC-NEXT: NumDescriptors: 1
;DXC-NEXT: BaseShaderRegister: 0
;DXC-NEXT: RegisterSpace: 5
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
;DXC-NEXT: - RangeType: 1
;DXC-NEXT: - RangeType: UAV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 6
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
;DXC-NEXT: - RangeType: 2
;DXC-NEXT: - RangeType: CBV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 7
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DATA_VOLATILE: true
;DXC-NEXT: - RangeType: 0
;DXC-NEXT: - RangeType: SRV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 8
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DATA_STATIC: true
;DXC-NEXT: - RangeType: 1
;DXC-NEXT: - RangeType: UAV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 9
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true
;DXC-NEXT: - RangeType: 2
;DXC-NEXT: - RangeType: CBV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 10
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
;DXC-NEXT: DATA_VOLATILE: true
;DXC-NEXT: - RangeType: 0
;DXC-NEXT: - RangeType: SRV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 11
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DESCRIPTORS_VOLATILE: true
;DXC-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true
;DXC-NEXT: - RangeType: 1
;DXC-NEXT: - RangeType: UAV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 12
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
;DXC-NEXT: - RangeType: 2
;DXC-NEXT: - RangeType: CBV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 13
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DATA_VOLATILE: true
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
;DXC-NEXT: - RangeType: 0
;DXC-NEXT: - RangeType: SRV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 14
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DATA_STATIC: true
;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
;DXC-NEXT: - RangeType: 1
;DXC-NEXT: - RangeType: UAV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 15
Expand Down
10 changes: 5 additions & 5 deletions ...CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,24 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC: - Name: RTS0
; DXC-NEXT: Size: 84
; DXC-NEXT: RootSignature:
; DXC-NEXT: Version: 1
; DXC-NEXT: Version: V1_0
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
; DXC-NEXT: StaticSamplersOffset: 0
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 0
; DXC-NEXT: ShaderVisibility: 0
; DXC-NEXT: - ParameterType: DescriptorTable
; DXC-NEXT: ShaderVisibility: All
; DXC-NEXT: Table:
; DXC-NEXT: NumRanges: 2
; DXC-NEXT: RangesOffset: 44
; DXC-NEXT: Ranges:
; DXC-NEXT: - RangeType: 3
; DXC-NEXT: - RangeType: Sampler
; DXC-NEXT: NumDescriptors: 1
; DXC-NEXT: BaseShaderRegister: 1
; DXC-NEXT: RegisterSpace: 0
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
; DXC-NEXT: - RangeType: 1
; DXC-NEXT: - RangeType: UAV
; DXC-NEXT: NumDescriptors: 5
; DXC-NEXT: BaseShaderRegister: 1
; DXC-NEXT: RegisterSpace: 10
Expand Down
54 changes: 27 additions & 27 deletions llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,30 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }

; DXC: - Name: RTS0
; DXC-NEXT: Size: 92
; DXC-NEXT: RootSignature:
; DXC-NEXT: Version: 2
; DXC-NEXT: NumRootParameters: 1
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
; DXC-NEXT: StaticSamplersOffset: 0
; DXC-NEXT: Parameters:
; DXC-NEXT: - ParameterType: 0
; DXC-NEXT: ShaderVisibility: 0
; DXC-NEXT: Table:
; DXC-NEXT: NumRanges: 2
; DXC-NEXT: RangesOffset: 44
; DXC-NEXT: Ranges:
; DXC-NEXT: - RangeType: 0
; DXC-NEXT: NumDescriptors: 1
; DXC-NEXT: BaseShaderRegister: 1
; DXC-NEXT: RegisterSpace: 0
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
; DXC-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true
; DXC-NEXT: - RangeType: 1
; DXC-NEXT: NumDescriptors: 5
; DXC-NEXT: BaseShaderRegister: 1
; DXC-NEXT: RegisterSpace: 10
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
; DXC-NEXT: DATA_VOLATILE: true
;DXC: - Name: RTS0
;DXC-NEXT: Size: 92
;DXC-NEXT: RootSignature:
;DXC-NEXT: Version: V1_1
;DXC-NEXT: NumRootParameters: 1
;DXC-NEXT: RootParametersOffset: 24
;DXC-NEXT: NumStaticSamplers: 0
;DXC-NEXT: StaticSamplersOffset: 0
;DXC-NEXT: Parameters:
;DXC-NEXT: - ParameterType: DescriptorTable
;DXC-NEXT: ShaderVisibility: All
;DXC-NEXT: Table:
;DXC-NEXT: NumRanges: 2
;DXC-NEXT: RangesOffset: 44
;DXC-NEXT: Ranges:
;DXC-NEXT: - RangeType: SRV
;DXC-NEXT: NumDescriptors: 1
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 0
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
;DXC-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true
;DXC-NEXT: - RangeType: UAV
;DXC-NEXT: NumDescriptors: 5
;DXC-NEXT: BaseShaderRegister: 1
;DXC-NEXT: RegisterSpace: 10
;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
;DXC-NEXT: DATA_VOLATILE: true
20 changes: 10 additions & 10 deletions llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout


; DXC: - Name: RTS0
; DXC-NEXT: Size: 24
; DXC-NEXT: RootSignature:
; DXC-NEXT: Version: 2
; DXC-NEXT: NumRootParameters: 0
; DXC-NEXT: RootParametersOffset: 24
; DXC-NEXT: NumStaticSamplers: 0
; DXC-NEXT: StaticSamplersOffset: 0
; DXC-NEXT: Parameters: []
; DXC-NEXT: AllowInputAssemblerInputLayout: true
;DXC: - Name: RTS0
;DXC-NEXT: Size: 24
;DXC-NEXT: RootSignature:
;DXC-NEXT: Version: V1_1
;DXC-NEXT: NumRootParameters: 0
;DXC-NEXT: RootParametersOffset: 24
;DXC-NEXT: NumStaticSamplers: 0
;DXC-NEXT: StaticSamplersOffset: 0
;DXC-NEXT: Parameters: []
;DXC-NEXT: AllowInputAssemblerInputLayout: true
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/DirectX/ContainerData/RootSignature-MultipleEntryFunctions.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }

; CHECK-LABEL: Definition for 'main':
; CHECK-NEXT: Flags: 0x000001
; CHECK-NEXT: Version: 2
; CHECK-NEXT: Version: 0x2
; CHECK-NEXT: RootParametersOffset: 24
; CHECK-NEXT: NumParameters: 0
; CHECK-NEXT: NumStaticSamplers: 0
; CHECK-NEXT: StaticSamplersOffset: 0

; CHECK-LABEL: Definition for 'anotherMain':
; CHECK-NEXT: Flags: 0x000002
; CHECK-NEXT: Version: 2
; CHECK-NEXT: Version: 0x2
; CHECK-NEXT: RootParametersOffset: 24
; CHECK-NEXT: NumParameters: 0
; CHECK-NEXT: NumStaticSamplers: 0
Expand Down
Loading