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.

Sign up for GitHub

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

Jump to bottom

printer: ensure compatibility with asmfmt #8

Open
mmcloughlin opened this issue Dec 29, 2018 · 3 comments
Open

printer: ensure compatibility with asmfmt #8

mmcloughlin opened this issue Dec 29, 2018 · 3 comments

Comments

@mmcloughlin
Copy link
Owner

klauspost/asmfmt is the de facto standard for Go assembly formatting. It would be good to produce output that conforms to asmfmt.

I have a preference for avoiding non-Go dependencies (stdlib and sub-repos only). Therefore:

  • If it is possible to produce conforming output without actually depending on asmfmt explicitly, that would be preferred. This may actually be possible since (at the time of writing) many of the asmfmt rules simply wouldn't apply to avo output.
  • If the rules enforced by asmfmt are too complicated, then we can accept the additional dependency.

Either way, it would be good to have a check in CI to confirm that all generated files are formatted correctly. Something like find . -name '*.s' | xargs asmfmt -w and check the git repo is clean.
mmcloughlin added a commit that referenced this issue Jan 11, 2019
@mmcloughlin
printer: use tabwriter to align instructions (#8)
f77a2e3
mmcloughlin added a commit that referenced this issue Jan 11, 2019
@mmcloughlin
lint: ensure conformity with asmfmt
7a22924 
Run asmfmt suring linting and confirm git repository isn't dirty.
This introduces a developer tools dependency on asmfmt, but not a
runtime dependency.

Updates #8
mmcloughlin added a commit that referenced this issue Jan 11, 2019
@mmcloughlin
script: fix asmtfmt install command (#8)
999da51
@mmcloughlin
Copy link
Owner Author

Fixed by 27cea3b

mmcloughlin added a commit that referenced this issue Jan 11, 2019
@mmcloughlin
examples/sum: attempt to avoid asmfmt mismatch
943d5f0 
Updates #41
Updates #8
@mmcloughlin
Copy link
Owner Author

Based on 3ca82be I think we have a mismatch with comments ahead of labels. But there seems to be a special case when there is a comment above and below a label? Needs investigation.

@mmcloughlin mmcloughlin reopened this Jan 11, 2019
@mmcloughlin
Copy link
Owner Author

I think asmfmt sets indentation to 0 after terminal instructions. I'm not quite sure this is correct behavior. Filed klauspost/asmfmt#35 to discuss.

@mmcloughlin mmcloughlin mentioned this issue Jan 13, 2019
Closed
@mmcloughlin mmcloughlin mentioned this issue Jan 28, 2020
Merged
mmcloughlin added a commit that referenced this issue Jan 28, 2020
@mmcloughlin
printer: flush instructions after JMP
ef318f1 
For compatibility with `asmfmt`

Updates #8
mmcloughlin added a commit that referenced this issue Jan 28, 2020
@mmcloughlin
tests/fixedbugs: regression test for issue 100 (#129)
e089a6c 
Adds a regression test based on klauspost/compress#186. This necessitated some related changes:

* Mark "RET" as a terminal instruction
* printer refactor to maintain compatibility with asmfmt
* Tweaks to other regression tests to ensure they are run correctly in CI

Updates #100 #65 #8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@mmcloughlin