Skip to content
This repository has been archived by the owner on Jan 29, 2025. It is now read-only.

[hlsl-out] Implement switch statement #1265

Merged
merged 3 commits into from
Aug 23, 2021
Merged

Conversation

Gordon-F
Copy link
Collaborator

Fix #1261

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you!
we need to add switch test to control_flow.wgsl to see how/if it works

if case.fall_through {
writeln!(self.out, "{}/* fallthrough */", &indent_str_2)?;
writeln!(self.out, "{}{{", &indent_str_2)?;
writeln!(self.out, "{}break;", INDENT.repeat(indent + 3))?;
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand. This goes into break and the other branch goes into break too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, misunderstood tint logic in fallthrough case. We should generate it in new block.

@kvark kvark added the can backport PR that can be back-ported to a release branch label Aug 21, 2021
@kvark
Copy link
Member

kvark commented Aug 21, 2021

We'll need to release a patch after this is merged, for iced

@@ -425,7 +425,9 @@ fn convert_wgsl() {
),
(
"control-flow",
Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL,
// TODO: SPIRV https://github.com/gfx-rs/naga/issues/1017
//Targets::SPIRV |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, spv-out panic with the updated test. So, I decided to remove it for a while.

Copy link
Member

Choose a reason for hiding this comment

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

what's the panic exactly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added test case here - #1017

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/back/spv/block.rs:1552:83
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5

@Gordon-F Gordon-F requested a review from kvark August 21, 2021 15:59

// Write `break;` if the block isn't fallthrough
if !case.fall_through {
writeln!(self.out, "{}break;", INDENT.repeat(indent + 2))?;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's not required, since return is present in IR for all cases.

src/back/hlsl/writer.rs Show resolved Hide resolved
@@ -425,7 +425,9 @@ fn convert_wgsl() {
),
(
"control-flow",
Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL,
// TODO: SPIRV https://github.com/gfx-rs/naga/issues/1017
//Targets::SPIRV |
Copy link
Member

Choose a reason for hiding this comment

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

what's the panic exactly?

@kvark kvark merged commit 464788d into gfx-rs:master Aug 23, 2021
@Gordon-F Gordon-F deleted the hlsl-out branch August 23, 2021 07:29
kvark pushed a commit to kvark/naga that referenced this pull request Aug 24, 2021
* [hlsl-out] Implement switch statement

* [hlsl-out] Implement switch statement

* Add switch tests to control-flow snapshot
@kvark kvark mentioned this pull request Aug 24, 2021
@kvark kvark removed the can backport PR that can be back-ported to a release branch label Aug 24, 2021
@kvark
Copy link
Member

kvark commented Aug 24, 2021

Included in v0.6.1

kvark pushed a commit that referenced this pull request Aug 24, 2021
* [hlsl-out] Implement switch statement

* [hlsl-out] Implement switch statement

* Add switch tests to control-flow snapshot
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[hlsl-out] Unimplemented Switch
2 participants