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

crt0: make argv 8-byte aligned #374

Merged
merged 1 commit into from
Jun 21, 2021
Merged

Conversation

DavidSTWChen
Copy link
Contributor

No description provided.

@fuchingy
Copy link

@e-puerto and @bsousi5 , could you approve this PR so it can be merged.
This PR will help improve the U84 dhrystone score.

none (5.77) --> .align 8 (5.6) --> .balign 8 (5.77)

Copy link
Contributor

@e-puerto e-puerto left a comment

Choose a reason for hiding this comment

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

LGTM

@paul-walmsley-sifive
Copy link
Contributor

@DavidSTWChen , we can merge this, it's no problem; just one question. Does this patch also need to be applied to the toolchain libgloss as well and sent to the toolchain team? i.e., https://github.com/sifive/riscv-newlib/blob/sifive-newlib-2021.04-release/libgloss/riscv/crt0.S

OIn the FESDK side, we will also have to apply this one to the new release_generator_mongoose branch; we can probably take care of that on our side.

@paul-walmsley-sifive paul-walmsley-sifive merged commit e9065f4 into master Jun 21, 2021
@kito-cheng
Copy link
Member

@paul-walmsley-sifive The assumption for newlib/libgloss is: argv are prepared by the execution environment like simulator, so the execution environment has responsibility to make sure that.

In another word: I think we don't need similar change on newlib/libgloss side :)

lexus0 added a commit that referenced this pull request Jun 25, 2021
The original one is align 8, but there is patch to modify it.
#374
The e76, and s76 will drop with above patch for coremark.
This patch makes balign, and align work together.
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.

5 participants