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

Update optimizer docs to include new unchecked loop increment #14650

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

matheusaaguiar
Copy link
Collaborator

Address #14648.

Copy link
Member

@r0qs r0qs left a comment

Choose a reason for hiding this comment

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

Nice, just some suggestions :)

docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
@matheusaaguiar matheusaaguiar force-pushed the uncheckedForLoopIncrementDocs branch from 0102a8e to 3c09b19 Compare October 26, 2023 14:22
@matheusaaguiar matheusaaguiar marked this pull request as ready for review October 26, 2023 14:22
r0qs
r0qs previously approved these changes Oct 26, 2023
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
@matheusaaguiar matheusaaguiar force-pushed the uncheckedForLoopIncrementDocs branch from 0a98268 to 7a1ade0 Compare November 1, 2023 19:14
@ekpyron ekpyron requested a review from mehtavishwa30 November 1, 2023 19:34
r0qs
r0qs previously approved these changes Nov 2, 2023
cameel
cameel previously approved these changes Nov 2, 2023
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

The content seems correct now, so I'm approving already, but there are still some small formatting and wording issues.

To avoid spamming you with multiple comments I pushed some fixups correcting them. Please take a look and if you're fine with them, you can squash and merge.

docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
docs/internals/optimizer.rst Outdated Show resolved Hide resolved
Copy link

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 21, 2023
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Nov 21, 2023
@matheusaaguiar matheusaaguiar force-pushed the uncheckedForLoopIncrementDocs branch from c5abb46 to 15c432c Compare November 23, 2023 13:50
cameel
cameel previously approved these changes Nov 23, 2023
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

I pushed an extra change to clarify that the optimizations provided at the codegen level are available in different code generators. If you're fine with my changes, we can squash and merge this.

@cameel cameel force-pushed the uncheckedForLoopIncrementDocs branch from 820acff to 6dec919 Compare November 23, 2023 18:17
@cameel cameel force-pushed the uncheckedForLoopIncrementDocs branch 3 times, most recently from 2a341f4 to 1dcdda2 Compare November 23, 2023 18:23
@matheusaaguiar matheusaaguiar force-pushed the uncheckedForLoopIncrementDocs branch from 1dcdda2 to 158330b Compare November 23, 2023 19:30
@matheusaaguiar
Copy link
Collaborator Author

Fine by me.
Squashed.

@matheusaaguiar matheusaaguiar merged commit efed3b2 into develop Nov 23, 2023
1 check passed
@matheusaaguiar matheusaaguiar deleted the uncheckedForLoopIncrementDocs branch November 23, 2023 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants