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

Add compile-time unrolled for loops #15456

Closed
wants to merge 3 commits into from
Closed

Add compile-time unrolled for loops #15456

wants to merge 3 commits into from

Conversation

juancarlospaco
Copy link
Collaborator

@juancarlospaco juancarlospaco commented Oct 2, 2020

  • Add compile-time unrolled for loops.
  • Documentation, examples, links, runnableExamples, changelog, etc.
  • Tiny diff.

@@ -178,7 +180,7 @@ else: # not defined(nimNewRoof)
inc i


iterator `||`*[S, T](a: S, b: T, annotation: static string = "parallel for"): T {.
iterator `||`*[S, T](a: S, b: T, annotation: static string = "omp parallel for"): T {.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you need a new iterator type, make one, don't abuse the parallel for iterator.

Removing the omp as a prefix of the iterator annotation is a breaking change for anyone using this iterator with a custom annotation.

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 dont understand what you mean, the || keeps working.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now anyone that use || with a custom annotation must prefix their annotation with omp, so this is a breaking change.

@mratsim's laser is a big user of this iterator, and this change will cause it not to compile.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My advise would be to create a brand new loop type. And note that #pragma unroll can unroll any loop, so a solution that allows tagging a generated loop would be much more efficient. Basically implementing the {.unroll.} pragma, if you may.

@juancarlospaco juancarlospaco deleted the compile-time-for branch October 2, 2020 04:06
ringabout added a commit to ringabout/Nim that referenced this pull request Dec 26, 2020
Araq pushed a commit that referenced this pull request Jan 4, 2021
* continue #15456
* follow the advice from juan_carlos
mildred pushed a commit to mildred/Nim that referenced this pull request Jan 11, 2021
@juancarlospaco juancarlospaco mentioned this pull request Jan 11, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
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.

2 participants