Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Remove fn/closure #[spirv(unroll_loops)] attribute.#946

Merged
oisyn merged 2 commits intoEmbarkStudios:mainfrom
LykenSol:remove-unroll-loops
Nov 28, 2022
Merged

Remove fn/closure #[spirv(unroll_loops)] attribute.#946
oisyn merged 2 commits intoEmbarkStudios:mainfrom
LykenSol:remove-unroll-loops

Conversation

@eddyb
Copy link
Copy Markdown
Contributor

@eddyb eddyb commented Nov 24, 2022

This attribute was pretty hacky, as it affected all loops in the function body, because MIR is a control-flow graph so we can't see "loop expressions" to get attributes from (see below for how we could rectify that!).

To quote the CHANGELOG entry:

  • Removed the fn/closure #[spirv(unroll_loops)] attribute, as it has no users,
    is becoming non-trivial to support, and requires redesign for better ergonomics
    (e.g. #[spirv(unroll)] applied to individual loops, not the whole fn/closure)

For more context, see:

@eddyb eddyb requested a review from oisyn as a code owner November 24, 2022 16:02
@oisyn oisyn enabled auto-merge (rebase) November 28, 2022 10:45
Co-authored-by: Sylvester Hesp <github@oisyn.nl>
@eddyb eddyb requested a review from oisyn November 28, 2022 15:05
@oisyn oisyn merged commit 6eebe6c into EmbarkStudios:main Nov 28, 2022
@eddyb eddyb deleted the remove-unroll-loops branch November 28, 2022 15:45
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.

2 participants