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

[features] Fix minsize feature list order #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sasdf
Copy link
Contributor

@sasdf sasdf commented Dec 17, 2024

Only the last optimization level is effective.

To overwrite the level to Oz with the minsize feature flag, the feature needs to be listed after the compilation modes.

@sasdf sasdf marked this pull request as ready for review December 18, 2024 01:07
Comment on lines +383 to +372
# To overwrite the level to minsize with feature flags, it needs to be
# listed after the compilation modes.
":minsize",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on the flag names in the feature definition (-mllvm), it seems this this may be an LLVM-only feature and this might be an error for GCC-based compilers.

I'd suggest adding this to the feature_set for toolchains where we know the compiler is LLVM. E.g., //platforms/riscv32/features:rv32imcb.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I moved the llvm flags to rv32 features.

Only the last optimization level is effective.
To overwrite the level to minsize with the minsize feature flag,
the feature needs to be listed after the compilation modes.

Change-Id: I63ff18f0b6ae450441708e80abbf7228f031913f
Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com>
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.

2 participants