Skip to content

Commit c71ee1b

Browse files
Check unified/resource heap-stride conflict at option-parse time (#11723)
Per review (@jkwak-work): diagnose the `-spirv-unified-descriptor-heap-stride` / non-zero `-spirv-resource-heap-stride` mutual exclusivity in OptionsParser's post-parse validation (alongside ConflictingProfilesSpecifiedForTarget / ConflictingStagesForEntryPoint), reading both from linkage->m_optionSet, so E57006 fires as soon as the options are parsed -- before any SPIR-V emission and regardless of whether the module declares a resource heap. An explicit `-spirv-resource-heap-stride 0` selects the default OpConstantSizeOfEXT path the unified option modifies and is accepted. Keep diagnoseConflictingDescriptorHeapStrideOptions() at emit time as the compile-API safety net: SPIRVResourceHeapStride / SPIRVUnifiedDescriptorHeapStride are public API options, and entries set directly through the compile API do not flow through OptionsParser, so without the emit-time check that path would silently honor the explicit stride. The two do not double-diagnose: an E57006 at parse time aborts before emission, so the emit-time check only fires for the API path. CONFLICT test variant still produces error[E57006]; build + 4/4 static tests + spirv-val EXIT=0 for both entries green.
1 parent 893f8cc commit c71ee1b

2 files changed

Lines changed: 23 additions & 7 deletions

File tree

source/slang/slang-emit-spirv.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7175,9 +7175,10 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
71757175
// Returns whether resource descriptor-heap runtime arrays should advertise a single unified
71767176
// maximum `ArrayStride` (opt-in via `-spirv-unified-descriptor-heap-stride`). This only affects
71777177
// the auto stride path, i.e. when `-spirv-resource-heap-stride` is 0. Combining it with a
7178-
// non-zero `-spirv-resource-heap-stride` is a conflict (the two express contradictory strides)
7179-
// and is diagnosed in `diagnoseConflictingDescriptorHeapStrideOptions`; an explicit stride of 0
7180-
// selects that same auto path and is not a conflict.
7178+
// non-zero `-spirv-resource-heap-stride` is a conflict (the two express contradictory strides),
7179+
// rejected up front during option processing for the CLI (`OptionsParser` in
7180+
// `slang-options.cpp`) and re-checked here by `diagnoseConflictingDescriptorHeapStrideOptions`
7181+
// for the compile-API path; an explicit stride of 0 selects that same auto path, not a conflict.
71817182
bool isUnifiedResourceHeapStrideEnabled()
71827183
{
71837184
return m_targetProgram->getOptionSet().getBoolOption(
@@ -7309,10 +7310,13 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
73097310
return runtimeArrayType;
73107311
}
73117312

7312-
// Diagnoses the mutually-exclusive `-spirv-resource-heap-stride` / unified-stride option pair:
7313-
// a non-zero explicit literal stride and the unified maximum stride contradict, so combining
7314-
// them is an error rather than letting one silently win. An explicit stride of 0 selects the
7315-
// same default `OpConstantSizeOfEXT` path the unified option modifies, so it is not a conflict.
7313+
// Diagnoses the `-spirv-resource-heap-stride` / `-spirv-unified-descriptor-heap-stride` conflict
7314+
// for the compile-API path. The CLI rejects this at option-parse time (`OptionsParser` in
7315+
// `slang-options.cpp`), which aborts before emission; but options set directly through the
7316+
// public compile API do not flow through that parser, so this re-checks at emit time and fails
7317+
// loudly rather than silently honoring the explicit stride. The condition matches the parser:
7318+
// unified enabled and a non-zero `SPIRVResourceHeapStride` (an explicit 0 selects the default
7319+
// `OpConstantSizeOfEXT` path the unified option modifies, so it is not a conflict).
73167320
void diagnoseConflictingDescriptorHeapStrideOptions()
73177321
{
73187322
if (isUnifiedResourceHeapStrideEnabled() &&

source/slang/slang-options.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4277,6 +4277,18 @@ SlangResult OptionsParser::_parse(int argc, char const* const* argv)
42774277
}
42784278
}
42794279

4280+
// `-spirv-unified-descriptor-heap-stride` and an explicit non-zero
4281+
// `-spirv-resource-heap-stride` express contradictory strides for the same resource heap,
4282+
// so reject the combination here, as soon as the options are parsed, rather than waiting
4283+
// until SPIR-V emission. An explicit stride of 0 selects the default `OpConstantSizeOfEXT`
4284+
// path that the unified option modifies and is therefore not a conflict.
4285+
if (linkage->m_optionSet.getBoolOption(
4286+
CompilerOptionName::SPIRVUnifiedDescriptorHeapStride) &&
4287+
linkage->m_optionSet.getIntOption(CompilerOptionName::SPIRVResourceHeapStride) != 0)
4288+
{
4289+
m_sink->diagnose(Diagnostics::SpirvConflictingDescriptorHeapStrideOptions{});
4290+
}
4291+
42804292
// TODO: do we need to require that a target must have a profile specified,
42814293
// or will we continue to allow the profile to be inferred from the target?
42824294

0 commit comments

Comments
 (0)