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

Style corrections in Yul Optimizer docs #15053

Merged
merged 1 commit into from May 8, 2024
Merged

Conversation

cameel
Copy link
Member

@cameel cameel commented Apr 25, 2024

This is a bit of cleanup that I did while updating optimizer docs. I separated it from the main PR because it makes actual semantic changes harder to see. It should also be much easier to review.

Main corrections:

  • Add missing backticks.
  • Use consistent style for step naming (capitalized, without spaces, without backticks).
  • Correct minor text errors like missing plurals.

Comment on lines -298 to +302
- SSA Transform
- Common Subexpression Eliminator
- Expression Simplifier
- Unused Assign Eliminator
- Full Inliner
- SSATransform
- CommonSubexpressionEliminator
- ExpressionSimplifier
- UnusedAssignEliminator
- FullInliner
Copy link
Member Author

@cameel cameel Apr 25, 2024

Choose a reason for hiding this comment

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

This list actually seems out of date. Or is e.g. UnusedStoreRemover or LoadResolver really not a major component?

Copy link
Member

@ekpyron ekpyron May 7, 2024

Choose a reason for hiding this comment

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

Well, arguable - the distinction between "main" components and otherwise is blurry anyways and the UnusedStoreRemover and LoadResolver are rather weak in the sense that they only kick in under rather specific circumstances so far. Meaning: I'd not consider too relevant what we list here. The bigger question is whether the list is helpful at all.

@cameel cameel force-pushed the optimizer-docs-style-cleanup branch 2 times, most recently from 125225c to 6702af2 Compare April 27, 2024 19:54
Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Well, nothing against merging this. Not sure we really need to treat these step names as fixed camel case expressions, but anyways: fine with me and at the backticks definitely make sense.

- Add missing backticks.
- Use consistent style for step naming (capitalized, without spaces, without backticks).
- Correct minor text errors like missing plurals.
@cameel cameel force-pushed the optimizer-docs-style-cleanup branch from 6702af2 to 9599d85 Compare May 8, 2024 15:17
@cameel
Copy link
Member Author

cameel commented May 8, 2024

I don't really care if they're camel case or normal text either, just that it's only one and used consistently. Otherwise searching for them is annoying (have to do it twice each time) and also when updating docs I'm unnecessarily wasting time wondering which one I should use.

@cameel cameel enabled auto-merge May 8, 2024 15:24
@cameel cameel merged commit cfff77c into develop May 8, 2024
72 checks passed
@cameel cameel deleted the optimizer-docs-style-cleanup branch May 8, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants