Skip to content

Commit 5e7108e

Browse files
natgavrilenkonatgavrilenko
authored
spirv-val: Update memory semantics rules to match the specification (KhronosGroup#6096)
* spirv-val: Update memory semantics rules to match the specification Updated validation of memory semantics to match the formal Vulkan memory model and the Vulkan specification: - Require non-empty storage class semantics for non-relaxed memory orders - Require non-relaxed memory orders for non-empty storage class semantics - Validate AtomicCompareExchange Unequal semantics with respect to its Equal semantics - Unify Vulkan error messages and validation rules Signed-off-by: Natalia Gavrilenko <[email protected]> * Removed duplicated tests --------- Signed-off-by: Natalia Gavrilenko <[email protected]>
1 parent 31d713b commit 5e7108e

File tree

9 files changed

+1046
-1208
lines changed

9 files changed

+1046
-1208
lines changed

‎source/opt/upgrade_memory_model.cpp‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -700,22 +700,29 @@ void UpgradeMemoryModel::UpgradeBarriers() {
700700
roots.push(e.GetSingleWordInOperand(1u));
701701
if (context()->ProcessCallTreeFromRoots(CollectBarriers, &roots)) {
702702
for (auto barrier : barriers) {
703-
// Add OutputMemoryKHR to the semantics of the barriers.
703+
// Add OutputMemoryKHR to the semantics of the non-relaxed barriers.
704704
uint32_t semantics_id = barrier->GetSingleWordInOperand(2u);
705705
Instruction* semantics_inst =
706706
context()->get_def_use_mgr()->GetDef(semantics_id);
707707
analysis::Type* semantics_type =
708708
context()->get_type_mgr()->GetType(semantics_inst->type_id());
709709
uint64_t semantics_value = GetIndexValue(semantics_inst);
710-
const analysis::Constant* constant =
711-
context()->get_constant_mgr()->GetConstant(
712-
semantics_type,
713-
{static_cast<uint32_t>(semantics_value) |
714-
uint32_t(spv::MemorySemanticsMask::OutputMemoryKHR)});
715-
barrier->SetInOperand(2u, {context()
716-
->get_constant_mgr()
717-
->GetDefiningInstruction(constant)
718-
->result_id()});
710+
const uint64_t memory_order_mask =
711+
uint64_t(spv::MemorySemanticsMask::Acquire |
712+
spv::MemorySemanticsMask::Release |
713+
spv::MemorySemanticsMask::AcquireRelease |
714+
spv::MemorySemanticsMask::SequentiallyConsistent);
715+
if (semantics_value & memory_order_mask) {
716+
const analysis::Constant* constant =
717+
context()->get_constant_mgr()->GetConstant(
718+
semantics_type,
719+
{static_cast<uint32_t>(semantics_value) |
720+
uint32_t(spv::MemorySemanticsMask::OutputMemoryKHR)});
721+
barrier->SetInOperand(2u, {context()
722+
->get_constant_mgr()
723+
->GetDefiningInstruction(constant)
724+
->result_id()});
725+
}
719726
}
720727
}
721728
barriers.clear();

‎source/val/validate_atomics.cpp‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -388,27 +388,6 @@ spv_result_t AtomicsPass(ValidationState_t& _, const Instruction* inst) {
388388
if (auto error = ValidateMemorySemantics(
389389
_, inst, unequal_semantics_index, memory_scope))
390390
return error;
391-
392-
// Volatile bits must match for equal and unequal semantics. Previous
393-
// checks guarantee they are 32-bit constants, but we need to recheck
394-
// whether they are evaluatable constants.
395-
bool is_int32 = false;
396-
bool is_equal_const = false;
397-
bool is_unequal_const = false;
398-
uint32_t equal_value = 0;
399-
uint32_t unequal_value = 0;
400-
std::tie(is_int32, is_equal_const, equal_value) = _.EvalInt32IfConst(
401-
inst->GetOperandAs<uint32_t>(equal_semantics_index));
402-
std::tie(is_int32, is_unequal_const, unequal_value) =
403-
_.EvalInt32IfConst(
404-
inst->GetOperandAs<uint32_t>(unequal_semantics_index));
405-
if (is_equal_const && is_unequal_const &&
406-
((equal_value & uint32_t(spv::MemorySemanticsMask::Volatile)) ^
407-
(unequal_value & uint32_t(spv::MemorySemanticsMask::Volatile)))) {
408-
return _.diag(SPV_ERROR_INVALID_ID, inst)
409-
<< "Volatile mask setting must match for Equal and Unequal "
410-
"memory semantics";
411-
}
412391
}
413392

414393
if (opcode == spv::Op::OpAtomicStore) {

‎source/val/validate_memory_semantics.cpp‎

Lines changed: 173 additions & 147 deletions
Large diffs are not rendered by default.

‎source/val/validation_state.cpp‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2469,10 +2469,6 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
24692469
return VUID_WRAP(VUID-StandaloneSpirv-None-04644);
24702470
case 4645:
24712471
return VUID_WRAP(VUID-StandaloneSpirv-None-04645);
2472-
case 10609:
2473-
return VUID_WRAP(VUID-StandaloneSpirv-OpControlBarrier-10609);
2474-
case 4650:
2475-
return VUID_WRAP(VUID-StandaloneSpirv-OpControlBarrier-04650);
24762472
case 4651:
24772473
return VUID_WRAP(VUID-StandaloneSpirv-OpVariable-04651);
24782474
case 4652:
@@ -2539,14 +2535,6 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
25392535
return VUID_WRAP(VUID-StandaloneSpirv-PhysicalStorageBuffer64-04710);
25402536
case 4711:
25412537
return VUID_WRAP(VUID-StandaloneSpirv-OpTypeForwardPointer-04711);
2542-
case 4730:
2543-
return VUID_WRAP(VUID-StandaloneSpirv-OpAtomicStore-04730);
2544-
case 4731:
2545-
return VUID_WRAP(VUID-StandaloneSpirv-OpAtomicLoad-04731);
2546-
case 4732:
2547-
return VUID_WRAP(VUID-StandaloneSpirv-OpMemoryBarrier-04732);
2548-
case 4733:
2549-
return VUID_WRAP(VUID-StandaloneSpirv-OpMemoryBarrier-04733);
25502538
case 4734:
25512539
return VUID_WRAP(VUID-StandaloneSpirv-OpVariable-04734);
25522540
case 4744:
@@ -2727,6 +2715,36 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
27272715
case 10824:
27282716
// This use to be a standalone, but maintenance9 will set allow_vulkan_32_bit_bitwise now
27292717
return VUID_WRAP(VUID-RuntimeSpirv-None-10824);
2718+
case 10865:
2719+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10865);
2720+
case 10866:
2721+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10866);
2722+
case 10867:
2723+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10867);
2724+
case 10868:
2725+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10868);
2726+
case 10869:
2727+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10869);
2728+
case 10870:
2729+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10870);
2730+
case 10871:
2731+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10871);
2732+
case 10872:
2733+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10872);
2734+
case 10873:
2735+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10873);
2736+
case 10874:
2737+
return VUID_WRAP(VUID-StandaloneSpirv-MemorySemantics-10874);
2738+
case 10875:
2739+
return VUID_WRAP(VUID-StandaloneSpirv-UnequalMemorySemantics-10875);
2740+
case 10876:
2741+
return VUID_WRAP(VUID-StandaloneSpirv-UnequalMemorySemantics-10876);
2742+
case 10877:
2743+
return VUID_WRAP(VUID-StandaloneSpirv-UnequalMemorySemantics-10877);
2744+
case 10878:
2745+
return VUID_WRAP(VUID-StandaloneSpirv-UnequalMemorySemantics-10878);
2746+
case 10879:
2747+
return VUID_WRAP(VUID-StandaloneSpirv-UnequalMemorySemantics-10879);
27302748
case 10880:
27312749
return VUID_WRAP(VUID-StandaloneSpirv-TessLevelInner-10880);
27322750
default:

‎test/opt/upgrade_memory_model_test.cpp‎

Lines changed: 60 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,9 +1562,9 @@ OpFunctionEnd
15621562

15631563
TEST_F(UpgradeMemoryModelTest, TessellationControlBarrierNoChange) {
15641564
const std::string text = R"(
1565-
; CHECK: [[none:%\w+]] = OpConstant {{%\w+}} 0
1566-
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2
1567-
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[none]]
1565+
; CHECK: [[none:%\w+]] = OpConstant {{%\w+}} 0{{\s*$}}
1566+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1567+
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[none]]{{\s*$}}
15681568
OpCapability Tessellation
15691569
OpMemoryModel Logical GLSL450
15701570
OpEntryPoint TessellationControl %func "func"
@@ -1583,11 +1583,11 @@ OpFunctionEnd
15831583
SinglePassRunAndMatch<opt::UpgradeMemoryModel>(text, true);
15841584
}
15851585

1586-
TEST_F(UpgradeMemoryModelTest, TessellationControlBarrierAddOutput) {
1586+
TEST_F(UpgradeMemoryModelTest, TessellationControlBarrierRelaxedNoChange) {
15871587
const std::string text = R"(
1588-
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2
1589-
; CHECK: [[output:%\w+]] = OpConstant {{%\w+}} 4096
1590-
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[output]]
1588+
; CHECK: [[none:%\w+]] = OpConstant {{%\w+}} 0{{\s*$}}
1589+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1590+
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[none]]{{\s*$}}
15911591
OpCapability Tessellation
15921592
OpMemoryModel Logical GLSL450
15931593
OpEntryPoint TessellationControl %func "func" %var
@@ -1610,25 +1610,52 @@ OpFunctionEnd
16101610
SinglePassRunAndMatch<opt::UpgradeMemoryModel>(text, true);
16111611
}
16121612

1613+
TEST_F(UpgradeMemoryModelTest, TessellationControlBarrierAddOutput) {
1614+
const std::string text = R"(
1615+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1616+
; CHECK: [[acqrel_workgroup_output:%\w+]] = OpConstant {{%\w+}} 4360{{\s*$}}
1617+
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[acqrel_workgroup_output]]{{\s*$}}
1618+
OpCapability Tessellation
1619+
OpMemoryModel Logical GLSL450
1620+
OpEntryPoint TessellationControl %func "func" %var
1621+
%void = OpTypeVoid
1622+
%int = OpTypeInt 32 0
1623+
%workgroup = OpConstant %int 2
1624+
%acqrel_workgroup = OpConstant %int 264
1625+
%ptr_int_Output = OpTypePointer Output %int
1626+
%var = OpVariable %ptr_int_Output Output
1627+
%func_ty = OpTypeFunction %void
1628+
%func = OpFunction %void None %func_ty
1629+
%1 = OpLabel
1630+
%ld = OpLoad %int %var
1631+
OpControlBarrier %workgroup %workgroup %acqrel_workgroup
1632+
OpStore %var %ld
1633+
OpReturn
1634+
OpFunctionEnd
1635+
)";
1636+
1637+
SinglePassRunAndMatch<opt::UpgradeMemoryModel>(text, true);
1638+
}
1639+
16131640
TEST_F(UpgradeMemoryModelTest, TessellationMemoryBarrierNoChange) {
16141641
const std::string text = R"(
1615-
; CHECK: [[none:%\w+]] = OpConstant {{%\w+}} 0
1616-
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2
1617-
; CHECK: OpMemoryBarrier [[workgroup]] [[none]]
1642+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1643+
; CHECK: [[acqrel_workgroup:%\w+]] = OpConstant {{%\w+}} 264{{\s*$}}
1644+
; CHECK: OpMemoryBarrier [[workgroup]] [[acqrel_workgroup]]{{\s*$}}
16181645
OpCapability Tessellation
16191646
OpMemoryModel Logical GLSL450
16201647
OpEntryPoint TessellationControl %func "func" %var
16211648
%void = OpTypeVoid
16221649
%int = OpTypeInt 32 0
1623-
%none = OpConstant %int 0
16241650
%workgroup = OpConstant %int 2
1651+
%acqrel_workgroup = OpConstant %int 264
16251652
%ptr_int_Output = OpTypePointer Output %int
16261653
%var = OpVariable %ptr_int_Output Output
16271654
%func_ty = OpTypeFunction %void
16281655
%func = OpFunction %void None %func_ty
16291656
%1 = OpLabel
16301657
%ld = OpLoad %int %var
1631-
OpMemoryBarrier %workgroup %none
1658+
OpMemoryBarrier %workgroup %acqrel_workgroup
16321659
OpStore %var %ld
16331660
OpReturn
16341661
OpFunctionEnd
@@ -1639,16 +1666,16 @@ OpFunctionEnd
16391666

16401667
TEST_F(UpgradeMemoryModelTest, TessellationControlBarrierAddOutputSubFunction) {
16411668
const std::string text = R"(
1642-
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2
1643-
; CHECK: [[output:%\w+]] = OpConstant {{%\w+}} 4096
1644-
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[output]]
1669+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1670+
; CHECK: [[acqrel_workgroup_output:%\w+]] = OpConstant {{%\w+}} 4360{{\s*$}}
1671+
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[acqrel_workgroup_output]]{{\s*$}}
16451672
OpCapability Tessellation
16461673
OpMemoryModel Logical GLSL450
16471674
OpEntryPoint TessellationControl %func "func" %var
16481675
%void = OpTypeVoid
16491676
%int = OpTypeInt 32 0
1650-
%none = OpConstant %int 0
16511677
%workgroup = OpConstant %int 2
1678+
%acqrel_workgroup = OpConstant %int 264
16521679
%ptr_int_Output = OpTypePointer Output %int
16531680
%var = OpVariable %ptr_int_Output Output
16541681
%func_ty = OpTypeFunction %void
@@ -1660,7 +1687,7 @@ OpFunctionEnd
16601687
%sub_func = OpFunction %void None %func_ty
16611688
%2 = OpLabel
16621689
%ld = OpLoad %int %var
1663-
OpControlBarrier %workgroup %workgroup %none
1690+
OpControlBarrier %workgroup %workgroup %acqrel_workgroup
16641691
OpStore %var %ld
16651692
OpReturn
16661693
OpFunctionEnd
@@ -1672,16 +1699,16 @@ OpFunctionEnd
16721699
TEST_F(UpgradeMemoryModelTest,
16731700
TessellationControlBarrierAddOutputDifferentFunctions) {
16741701
const std::string text = R"(
1675-
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2
1676-
; CHECK: [[output:%\w+]] = OpConstant {{%\w+}} 4096
1677-
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[output]]
1702+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1703+
; CHECK: [[acqrel_workgroup_output:%\w+]] = OpConstant {{%\w+}} 4360{{\s*$}}
1704+
; CHECK: OpControlBarrier [[workgroup]] [[workgroup]] [[acqrel_workgroup_output]]{{\s*$}}
16781705
OpCapability Tessellation
16791706
OpMemoryModel Logical GLSL450
16801707
OpEntryPoint TessellationControl %func "func" %var
16811708
%void = OpTypeVoid
16821709
%int = OpTypeInt 32 0
1683-
%none = OpConstant %int 0
16841710
%workgroup = OpConstant %int 2
1711+
%acqrel_workgroup = OpConstant %int 264
16851712
%ptr_int_Output = OpTypePointer Output %int
16861713
%var = OpVariable %ptr_int_Output Output
16871714
%func_ty = OpTypeFunction %void
@@ -1701,7 +1728,7 @@ OpReturnValue %ld
17011728
OpFunctionEnd
17021729
%barrier_func = OpFunction %void None %func_ty
17031730
%3 = OpLabel
1704-
OpControlBarrier %workgroup %workgroup %none
1731+
OpControlBarrier %workgroup %workgroup %acqrel_workgroup
17051732
OpReturn
17061733
OpFunctionEnd
17071734
%st_func = OpFunction %void None %st_func_ty
@@ -1717,21 +1744,22 @@ OpFunctionEnd
17171744

17181745
TEST_F(UpgradeMemoryModelTest, ChangeControlBarrierMemoryScope) {
17191746
std::string text = R"(
1720-
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2
1721-
; CHECK: [[queuefamily:%\w+]] = OpConstant {{%\w+}} 5
1722-
; CHECK: OpControlBarrier [[workgroup]] [[queuefamily]]
1747+
; CHECK: [[workgroup:%\w+]] = OpConstant {{%\w+}} 2{{\s*$}}
1748+
; CHECK: [[acqrel_workgroup:%\w+]] = OpConstant {{%\w+}} 264{{\s*$}}
1749+
; CHECK: [[queuefamily:%\w+]] = OpConstant {{%\w+}} 5{{\s*$}}
1750+
; CHECK: OpControlBarrier [[workgroup]] [[queuefamily]] [[acqrel_workgroup]]{{\s*$}}
17231751
OpCapability Shader
17241752
OpMemoryModel Logical GLSL450
17251753
OpEntryPoint GLCompute %func "func"
17261754
%void = OpTypeVoid
17271755
%int = OpTypeInt 32 0
1728-
%none = OpConstant %int 0
17291756
%device = OpConstant %int 1
17301757
%workgroup = OpConstant %int 2
1758+
%acqrel_workgroup = OpConstant %int 264
17311759
%func_ty = OpTypeFunction %void
17321760
%func = OpFunction %void None %func_ty
17331761
%1 = OpLabel
1734-
OpControlBarrier %workgroup %device %none
1762+
OpControlBarrier %workgroup %device %acqrel_workgroup
17351763
OpReturn
17361764
OpFunctionEnd
17371765
)";
@@ -1741,19 +1769,20 @@ OpFunctionEnd
17411769

17421770
TEST_F(UpgradeMemoryModelTest, ChangeMemoryBarrierMemoryScope) {
17431771
std::string text = R"(
1744-
; CHECK: [[queuefamily:%\w+]] = OpConstant {{%\w+}} 5
1745-
; CHECK: OpMemoryBarrier [[queuefamily]]
1772+
; CHECK: [[acqrel_workgroup:%\w+]] = OpConstant {{%\w+}} 264{{\s*$}}
1773+
; CHECK: [[queuefamily:%\w+]] = OpConstant {{%\w+}} 5{{\s*$}}
1774+
; CHECK: OpMemoryBarrier [[queuefamily]] [[acqrel_workgroup]]{{\s*$}}
17461775
OpCapability Shader
17471776
OpMemoryModel Logical GLSL450
17481777
OpEntryPoint GLCompute %func "func"
17491778
%void = OpTypeVoid
17501779
%int = OpTypeInt 32 0
1751-
%none = OpConstant %int 0
17521780
%device = OpConstant %int 1
1781+
%acqrel_workgroup = OpConstant %int 264
17531782
%func_ty = OpTypeFunction %void
17541783
%func = OpFunction %void None %func_ty
17551784
%1 = OpLabel
1756-
OpMemoryBarrier %device %none
1785+
OpMemoryBarrier %device %acqrel_workgroup
17571786
OpReturn
17581787
OpFunctionEnd
17591788
)";

‎test/val/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ add_spvtools_unittest(TARGET val_fghijklmnop
8181
val_literals_test.cpp
8282
val_logicals_test.cpp
8383
val_memory_test.cpp
84+
val_memory_semantics_test.cpp
8485
val_mesh_shading_test.cpp
8586
val_misc_test.cpp
8687
val_modes_test.cpp

0 commit comments

Comments
 (0)