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.

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

Jump to bottom

Capabilities System: Inclusive join logic bug #4295

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Capabilities System: Inclusive join logic bug #4295

merged 1 commit into from
Jun 6, 2024

Conversation

ArielG-NV
Copy link
Collaborator

fixes: #4293

Currently the code does:

    void inclusiveJoinConjunction(CapabilitySharedContext& context, CapabilityConjunction& c, List<CapabilityConjunction>& toAddAfter)
    {
        if (c.isImpossible())
            return;
        for (auto& conjunction : conjunctions)
        {
            if (c.implies(conjunction))
                return;
        }

This is incorrect because it means that "if the set we are going to add is a super set of an element, don't add it". This does not make sense since inclusive join is meant to be 'additive' of super sets.

Correct behavior code:

    void inclusiveJoinConjunction(CapabilitySharedContext& context, CapabilityConjunction& c, List<CapabilityConjunction>& toAddAfter)
    {
        if (c.isImpossible())
            return;
        for (auto& conjunction : conjunctions)
        {
            if (conjunction.implies(c))
                return;
        }

This is correct behavior because it means that: "if the set we are going to add is a sub set of an element, ignore it". This makes sense since inclusive join can then add super sets in-place of subsets

@ArielG-NV ArielG-NV added the pr: non-breaking PRs without breaking changes label Jun 6, 2024
@ArielG-NV ArielG-NV changed the title Inclusive join bug in logic Capabilities System: Inclusive join bug in logic Jun 6, 2024
@ArielG-NV ArielG-NV changed the title Capabilities System: Inclusive join bug in logic Capabilities System: Inclusive join logic bug Jun 6, 2024
@ArielG-NV ArielG-NV merged commit a709e02 into shader-slang:master Jun 6, 2024
18 checks passed
aroidzap added a commit to aroidzap/slang that referenced this pull request Jun 20, 2024
@aroidzap
Merge tag 'v2024.1.22' into emit-header-draft
e0f3a10 
v2024.1.22

c00f461 remove inline that crashes old glibc version (shader-slang#4398)
fdef653 Improve Direct SPIRV Backend Test Coverage (shader-slang#4396)
33e81a0 [Metal] Fix global constant array emit. (shader-slang#4392)
a38a4fb Make unknown attributes a warning instead of an error. (shader-slang#4391)
a210091 [Metal] Support SV_TargetN. (shader-slang#4390)
2cc9690 Implement for metal `SV_GroupIndex` (shader-slang#4385)
a6b8348 fix the clang/gcc warning (shader-slang#4382)
cfef0c6 Metal: misc fixes and enable more tests. (shader-slang#4374)
2407966 SPIR-V `SV_InstanceId` support in pixel shader (shader-slang#4368)
fba316f Remove `IRHLSLExportDecoration` and `IRKeepAliveDecoration` for non-CUDA/Torch targets (shader-slang#4364)
f0d40ad (origin/master, origin/HEAD, master) capture/replay: implement infrastructure for capture (shader-slang#4372)
ecc6ecb Fix cuda/cpp/metal crash for when using GLSL style shader inputs (shader-slang#4378)
0bf0bf7 Implement Sampler2D for CPP target (shader-slang#4371)
b970b88 Enable full test on macos. (shader-slang#4327)
0574dca Delete glsl_vulkan and glsl_vulkan_one_desc targets. (shader-slang#4361)
085d1a6 Fix emit logic for getElementPtr. (shader-slang#4362)
8813c61 Capability System: Implicit capability upgrade warning/error (shader-slang#4241)
7447fca Add constant folding for % operator. (shader-slang#4359)
8bf7d11 Fix merge error. (shader-slang#4358)
b7e8243 Add slangc flag to `-zero-initialize` all variables (shader-slang#3987)
ccc26c2 Extend the COM-based API to support whole program compilation. (shader-slang#4355)
318adcc Add compiler option to treat enum types as unscoped. (shader-slang#4354)
ec35feb Fix incorrect drop of decoration when translating glsl global var to entrypoint param. (shader-slang#4353)
c194af8 Fix crash on invalid entrypoint varying parameter. (shader-slang#4349)
7a4757d Implicit register binding for hlsl to non-hlsl targets (shader-slang#4338)
180d6b1 Fix duplicate SPIRV decorations. (shader-slang#4346)
fa8c11e Add option to preserve shader parameter declaration in output SPIRV. (shader-slang#4344)
3fe4a77 Fix crash when using optional type in a generic. (shader-slang#4341)
5da06d4 Fix global value inlining for spirv_asm blocks. (shader-slang#4339)
7e79669 [gfx] Metal improvements (shader-slang#4337)
6909d65 SPIRV backend: add support for tessellation stages, (shader-slang#4336)
ef20d93 Test more texture types in Metal (shader-slang#4333)
5a28968 [gfx] Metal texture fixes (shader-slang#4331)
df0a201 Support integer typed textures for GLSL (shader-slang#4329)
51d3585 Remove duplicate `VkPhysicalDeviceComputeShaderDerivativesFeaturesNV` extension structure in vk-api.h (shader-slang#4335)
6d5ef9b Fix `GetAttributeAtVertex` for spirv and glsl targets. (shader-slang#4334)
21bbebb Address glslang ordering requirments for 'derivative_group_*NV' (shader-slang#4323)
72016f9 Partial implementation of static_assert (shader-slang#4294)
712ce65 enable more metal tests (shader-slang#4326)
38c0bac Fix SPIRV emit for `Flat` decoration and TessLevel builtin. (shader-slang#4318)
b5cdd83 Support all integer typed indices in StructuredBuffer Load/Store/[]. (shader-slang#4311)
6857dd5 [gfx] Metal graphics support (shader-slang#4324)
0974463 Fix typos in the docs (shader-slang#4322)
9a23a9a SPIRV `Block` decoration fixes. (shader-slang#4303)
bc680e7 Add initial draft auto-diff basics and IR overview documents (shader-slang#4216)
ee812d1 Disallow certain types of decls in `interface` to provide better diagnostic message. (shader-slang#4312)
65928af Metal system value overhaul. (shader-slang#4308)
e39ceab Adding functional test for GLSL texture functions (shader-slang#4306)
2a45bc3 Support HLSL `.mips` syntax. (shader-slang#4310)
056a4b9 Small SPIRV emit cleanup around vector element extract. (shader-slang#4309)
f83fe55 Make CTS failure report more obvious (shader-slang#4302)
78d34f3 Improve documentation and example formatting consistency (shader-slang#4299)
7c6faf6 Precompute UIntSet from individual capabilities inside generator (shader-slang#4269)
004fe27 Metal compute tests (shader-slang#4292)
72f10a8 Fixed profile string per request in pr#4268 (shader-slang#4297)
a709e02 Update capability-generator-main.cpp (shader-slang#4295)
d301267 Typo in 06-interfaces-generics.md (shader-slang#4284)
fa664d1 Fix build warnings and treat warnings as error on CI (shader-slang#4276)
f149052 Remove unnecessary call to __requireComputeDerivative (shader-slang#4283)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csyonghe csyonghe csyonghe approved these changes

Assignees
No one assigned
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Capabilities System: Inclusive join logic bug
2 participants
@ArielG-NV @csyonghe