Skip to content

Use structured order to unroll loops.#3443

Merged
s-perron merged 2 commits intoKhronosGroup:masterfrom
s-perron:i3441
Jun 18, 2020
Merged

Use structured order to unroll loops.#3443
s-perron merged 2 commits intoKhronosGroup:masterfrom
s-perron:i3441

Conversation

@s-perron
Copy link
Copy Markdown
Collaborator

Fixes #3441

@s-perron s-perron requested a review from dnovillo June 17, 2020 20:21
@s-perron s-perron self-assigned this Jun 17, 2020
; CHECK: OpSelectionMerge {{%\w+}} None
; CHECK: OpSelectionMerge [[unrch2:%\w+]] None
; CHECK: [[unrch2]] = OpLabel
; CHECK-NEXT: OpUnreachable
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the CHECKs easier to read if they're intermingled with the code. I'm not sure how they relate to what the code is testing when they're all clustered above like this. Is this way better for some other reason?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases, I see what you mean. Personally, I find the check statements can get lost, even in mediums sized spir-v. If I don't see them all together I miss them or find it harder to see how they relate.

In this case, I don't know a good way to get them to mingle with the code. The first half of the check are for the first iteration of the loop, and the second are for the second iteration after it has been unrolled. They correspond to the same statement in the original, but hey have to appear in this order.

loop_header_, [ordered_loop_blocks, this](BasicBlock* bb) {
if (IsInsideLoop(bb)) ordered_loop_blocks->push_back(bb);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment here describing why the different handling for shaders? I'm not understanding how the shader case is dealing with the unreachable block.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Copy link
Copy Markdown
Contributor

@dnovillo dnovillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The comments around the CHECKs is a good approach as well.

@s-perron s-perron merged commit 545d158 into KhronosGroup:master Jun 18, 2020
@s-perron s-perron deleted the i3441 branch June 18, 2020 20:00
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/glslang/ ebf55a0..8397044 (17 commits)

KhronosGroup/glslang@ebf55a0...8397044

$ git log ebf55a0..8397044 --date=short --no-merges --format='%ad %ae %s'
2020-06-22 gleese Update test expected files with new magic number
2020-06-22 gleese Update SPIR-V generator version
2020-06-05 gleese Update test results to expect OpFUnordNotEqual
2020-06-05 gleese Use OpFUnordNotEqual for floating-point !=
2020-06-22 johnkslang Update README.md
2020-06-19 bclayton Add kokoro configs for android-ndk and cmake
2020-06-19 bclayton Switch ndk_test from gnustl_static to c++_static
2020-06-17 ShabbyX Add -g0 command line argument
2020-06-16 cepheus Build: use better MSVC subfolder names for the previous build changes.
2020-06-16 cepheus Bump version numbers.
2020-06-16 bclayton Move hlsl/ source to glslang/HLSL/
2020-06-16 cepheus Bump version.
2020-06-15 bclayton CMake: Fold HLSL source into glslang
2020-06-15 dj2 Remove unused variable. (KhronosGroup#2273)
2020-06-15 rharrison Remove unused function, BaseTypeName (KhronosGroup#2272)
2020-06-15 cepheus HLSL: Remove support for having GLSL versions of HLSL intrinsics.
2020-06-10 bclayton C Interface: Split SPIR-V C interface to own file

Roll third_party/googletest/ 8567b0929..c6e309b26 (2 commits)

google/googletest@8567b09...c6e309b

$ git log 8567b0929..c6e309b26 --date=short --no-merges --format='%ad %ae %s'
2020-06-17 absl-team Googletest export
2020-06-15 absl-team Googletest export

Roll third_party/re2/ e9d517989..14d319322 (5 commits)

google/re2@e9d5179...14d3193

$ git log e9d517989..14d319322 --date=short --no-merges --format='%ad %ae %s'
2020-06-18 junyer Write tests for the move semantics.
2020-06-18 junyer Improve RE2::Set and FilteredRE2 move semantics.
2020-06-16 junyer Distinguish between missing ')' and unexpected ')'.
2020-06-16 junyer Make RE2::Set and FilteredRE2 movable.
2020-06-15 junyer Herp derp. It's actually constant-time append.

Roll third_party/spirv-cross/ 9e3df69d4..f9ae06512 (12 commits)

KhronosGroup/SPIRV-Cross@9e3df69...f9ae065

$ git log 9e3df69d4..f9ae06512 --date=short --no-merges --format='%ad %ae %s'
2020-06-22 dsinclair Roll deps and update tests.
2020-06-22 post MSL: Remove the old VertexAttr API.
2020-06-19 cwallez Fix placement of SPIRV_CROSS_DEPRECATED.
2020-06-19 post Fix duplicated initialization for loop variables with initializers.
2020-06-18 post MSL: Add test case for constructing struct with non-value-type array.
2020-06-18 post MSL: Deal with loading non-value-type arrays.
2020-06-18 post MSL: Add tests for array copies in and out of buffers.
2020-06-18 post MSL: Improve handling of array types in buffer objects.
2020-06-18 post Clean up some deprecation warnings when building with Makefile.
2020-06-18 post Remove unused member in MSLShaderInput.
2020-06-13 cdavis MSL: Fix up input variables' vector lengths in all stages.
2020-06-16 post HLSL: Fix texProj in legacy HLSL.

Roll third_party/spirv-tools/ 30bf46d..d4b9f57 (12 commits)

KhronosGroup/SPIRV-Tools@30bf46d...d4b9f57

$ git log 30bf46d..d4b9f57 --date=short --no-merges --format='%ad %ae %s'
2020-06-19 jaebaek [spirv-opt] debug info preservation in ssa-rewrite (KhronosGroup#3356)
2020-06-19 ehsannas Updated desc_sroa to support flattening structures (KhronosGroup#3448)
2020-06-19 vasniktel spirv-fuzz: Refactor variable creation (KhronosGroup#3414)
2020-06-19 vasniktel spirv-fuzz: Swap operands in OpBranchConditional (KhronosGroup#3423)
2020-06-18 stevenperron Use structured order to unroll loops. (KhronosGroup#3443)
2020-06-18 jaebaek Debug info preservation in dead branch elimination (KhronosGroup#3425)
2020-06-17 vasniktel Add RemoveParameter method (KhronosGroup#3437)
2020-06-17 vasniktel Fix return type (KhronosGroup#3435)
2020-06-16 ehsannas Eliminate branches with condition of OpConstantNull (KhronosGroup#3438)
2020-06-16 andreperezmaselco.developer spirv-fuzz: Implement vector shuffle fuzzer pass (KhronosGroup#3412)
2020-06-16 andreperezmaselco.developer spirv-fuzz: Add replace linear algebra instruction transformation (KhronosGroup#3402)
2020-06-15 dj2 Update access control lists. (KhronosGroup#3433)
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 this pull request may close these issues.

Block is already a merge block for another header

2 participants