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 configure.ac and sync configure. #1594

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pz9115
Copy link
Contributor

@pz9115 pz9115 commented Oct 24, 2024

Use autoupdate updates the configure.ac and sync the configure file with configure.ac with autoconf.

configure Outdated
--enable-multilib build both RV32 and RV64 runtime libraries
[--disable-multilib]
--enable-multilib build both RV32 and RV64 runtime libraries (only
RV64 for musl libc) [--disable-multilib]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is caused by a forgotten change in configure.ac.
Can you update configure.ac?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the new configure file is just generated by $ autoconf configure.ac :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I got that, but with this PR we end up with a wrong message in configure.
So, please fix configure.ac (remove the "only RV64 for musl libc" there).

configure Show resolved Hide resolved
@pz9115 pz9115 closed this Oct 24, 2024
@pz9115 pz9115 reopened this Oct 24, 2024
@pz9115
Copy link
Contributor Author

pz9115 commented Oct 25, 2024

I see the in the configure.ac, we still set rv64imafdc as default input with --with-arch option, Do you think it would be more appropriate to replace it with gc?

And since now gcc supports vector extension, do we need to update the --enable-multilib option to support this commonly used extension?

@cmuellner
Copy link
Collaborator

I see the in the configure.ac, we still set rv64imafdc as default input with --with-arch option, Do you think it would be more appropriate to replace it with gc?

I would leave it as it is now.
For sure it is out of scope of this PR.

And since now gcc supports vector extension, do we need to update the --enable-multilib option to support this commonly used extension?

Yes, but that's also out of this PR's scope (it should be a different PR).

@TommyMurphyTM1234
Copy link
Collaborator

TommyMurphyTM1234 commented Oct 25, 2024

I see the in the configure.ac, we still set rv64imafdc as default input with --with-arch option, Do you think it would be more appropriate to replace it with gc?

Or would rv64gc_zicsr_zifencei be even more appropriate?
Or does rv64gc automatically/silently get expanded to rv64gc_zicsr_zifencei or something similar?

Edit: my comment crossed with @cmuellner's previous one which renders mine moot...

@pz9115
Copy link
Contributor Author

pz9115 commented Oct 25, 2024

I see the in the configure.ac, we still set rv64imafdc as default input with --with-arch option, Do you think it would be more appropriate to replace it with gc?

I would leave it as it is now. For sure it is out of scope of this PR.

And since now gcc supports vector extension, do we need to update the --enable-multilib option to support this commonly used extension?

Yes, but that's also out of this PR's scope (it should be a different PR).

Okay, I will update them in the new PR.

@cmuellner
Copy link
Collaborator

I see the in the configure.ac, we still set rv64imafdc as default input with --with-arch option, Do you think it would be more appropriate to replace it with gc?

Or would rv64gc_zicsr_zifencei be even more appropriate? Or does rv64gc automatically/silently get expanded to rv64gc_zicsr_zifencei or something similar?

I don't mind changing the default.
However, the change should have an observable benefit and not trigger complaints.
And of course such a change needs to come in its own PR.

@pz9115
Copy link
Contributor Author

pz9115 commented Oct 25, 2024

I see the in the configure.ac, we still set rv64imafdc as default input with --with-arch option, Do you think it would be more appropriate to replace it with gc?

Or would rv64gc_zicsr_zifencei be even more appropriate? Or does rv64gc automatically/silently get expanded to rv64gc_zicsr_zifencei or something similar?

Edit: my comment crossed with @cmuellner's previous one which renders mine moot...

@TommyMurphyTM1234 It's up to the ISA spec version, when set 2.2 or 20190608gc equals to imafdc
when use 20191213 gc equals to imafdc_zicsr_zifencei. Currently gcc set 20191213 as the default ISA spec version.

Copy link
Collaborator

@cmuellner cmuellner left a comment

Choose a reason for hiding this comment

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

This still needs the change in configure.ac (removal of the "only RV64 for musl libc").

@pz9115 pz9115 requested a review from cmuellner October 25, 2024 14:50
@pz9115
Copy link
Contributor Author

pz9115 commented Oct 25, 2024

This still needs the change in configure.ac (removal of the "only RV64 for musl libc").

Understood! Update it with the new commit.

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.

3 participants