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.

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

spirv-opt: CCP pass changes module and returns SuccessWithoutChange (2) #3991

Closed
Vasniktel opened this issue Oct 23, 2020 · 4 comments
Closed
Assignees

Comments

@Vasniktel
Copy link
Collaborator

data.zip

The following shader

               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %4 "main"
               OpExecutionMode %4 OriginUpperLeft
          %2 = OpTypeVoid
          %3 = OpTypeFunction %2
          %8 = OpTypeFloat 32
         %12 = OpTypeVector %8 3
         %87 = OpTypeInt 32 0
         %88 = OpConstant %87 0
        %153 = OpTypeBool
        %398 = OpConstant %8 0
        %399 = OpConstantComposite %12 %398 %398 %398
        %533 = OpConstant %8 0.300000012
        %534 = OpConstantComposite %12 %533 %533 %533
          %4 = OpFunction %2 None %3
          %5 = OpLabel
         %20 = OpBitcast %87 %533
       %3184 = OpUGreaterThan %153 %20 %88
               OpBranch %597
        %597 = OpLabel
       %2975 = OpPhi %12 %399 %5 %534 %660
               OpLoopMerge %663 %660 None
               OpBranchConditional %3184 %660 %663
        %660 = OpLabel
               OpBranch %597
        %663 = OpLabel
        %535 = OpExtInst %12 %1 FMix %399 %2975 %534
               OpReturn
               OpFunctionEnd

produces an error

spirv-opt: spirv-repo/source/opt/optimizer.cpp:598: bool spvtools::Optimizer::Run(const uint32_t *, const size_t, std::vector<uint32_t> *, const spv_optimizer_options) const: Assertion `optimized_binary_with_nop.size() == original_binary_size && "Binary size unexpectedly changed despite the optimizer saying " "there was no change"' failed.

when executed with spirv-opt --ccp --validate-after-all <attached file> -o out.spv.

Can be reproduced on 69f07da.

This appears to be some kind of corner case unhandled in #3738.

@Vasniktel
Copy link
Collaborator Author

@dnovillo ping.

@dnovillo dnovillo self-assigned this Oct 23, 2020
@dnovillo
Copy link
Contributor

Thanks. I'll take a look.

dnovillo added a commit to dnovillo/SPIRV-Tools that referenced this issue Oct 27, 2020
The previous attempts at fixing this issue relied on marking the IR
changed only when CCP was able to fold an instruction during
propagation (KhronosGroup#3799,
KhronosGroup#3732).

Those fixes missed the case described in
KhronosGroup#3991.  In this case,
the folder never actually succeeds in folding the instruction, but it
does create constants in the process.

Fixed with this change.
dnovillo added a commit that referenced this issue Oct 28, 2020
Simplify logic to decide whether CCP modified the IR.

The previous attempts at fixing this issue relied on marking the IR
changed only when CCP was able to fold an instruction during
propagation (#3799,
#3732).

Those fixes missed the case described in
#3991.  In this case,
the folder never actually succeeds in folding the instruction, but it
does create constants in the process.

Fixed with this change.
@dnovillo
Copy link
Contributor

@Vasniktel this should now be fixed. If it is, please close the issue. Thanks.

@Vasniktel
Copy link
Collaborator Author

@dnovillo, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants