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 still changes the module and returns SuccessWithoutChange #3738

Closed
Vasniktel opened this issue Aug 22, 2020 · 4 comments · Fixed by #3799
Closed

spirv-opt: CCP pass still changes the module and returns SuccessWithoutChange #3738

Vasniktel opened this issue Aug 22, 2020 · 4 comments · Fixed by #3799
Assignees

Comments

@Vasniktel
Copy link
Collaborator

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
        %154 = OpTypeVector %153 3
        %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
               OpBranch %2209
       %2209 = OpLabel
       %3155 = OpPhi %12 %399 %5 %534 %2272
       %3246 = OpFOrdLessThan %154 %3155 %534
       %3247 = OpAll %153 %3246
               OpLoopMerge %2275 %2272 None
               OpBranchConditional %3247 %2272 %2275
       %2272 = OpLabel
               OpBranch %2209
       %2275 = OpLabel
        %535 = OpExtInst %12 %1 FMix %399 %3155 %534
               OpReturn
               OpFunctionEnd

produces an error

spirv-opt: /home/vasniktel/gsoc/spirv-repo/source/opt/optimizer.cpp:592: 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 <compiled binary> -o out.spv --ccp --validate-after-all.

Can be reproduced on 4dd1223.

The situation is similar to #3636 except that the created constant is not returned and instead used internally by the folder. In this particular example, the folder creates a constant 1 and 1 - %534 (this is required by the OpFMix instruction).

@Vasniktel
Copy link
Collaborator Author

@dnovillo for info.

@dnovillo dnovillo self-assigned this Aug 24, 2020
@dnovillo
Copy link
Contributor

Thanks. I'll try to work on it this week.

@dnovillo
Copy link
Contributor

@Vasniktel apologies for the delay. It is unlikely that I will have time to work on this in the next couple of weeks.

@Vasniktel
Copy link
Collaborator Author

@dnovillo, no worries :)

dnovillo added a commit to dnovillo/SPIRV-Tools that referenced this issue Sep 11, 2020
…constant.

In KhronosGroup#3636, I missed that the instruction folder may create more than a
single constant per call.  Since CCP was only checking whether one
constant had been created after folding, it was wrongly thinking that
the IR had not changed.

Fixes KhronosGroup#3738.
s-perron pushed a commit that referenced this issue Sep 14, 2020
…constant. (#3799)

In #3636, I missed that the instruction folder may create more than a
single constant per call.  Since CCP was only checking whether one
constant had been created after folding, it was wrongly thinking that
the IR had not changed.

Fixes #3738.
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

Successfully merging a pull request may close this issue.

2 participants